diff --git a/.github/workflows/test-py39-functional-scim-auth.yaml b/.github/workflows/test-py39-functional-scim-auth.yaml new file mode 100644 index 0000000..97fa6ee --- /dev/null +++ b/.github/workflows/test-py39-functional-scim-auth.yaml @@ -0,0 +1,32 @@ +name: test-py39-functional-scim-auth + +on: + push: + branches: [ main ] + pull_request: + branches: [ main ] + +jobs: + test-scim-auth: + runs-on: ubuntu-latest + + steps: + - name: Check out code + uses: actions/checkout@v3 + + - name: Set up python + uses: actions/setup-python@v4 + with: + python-version: 3.9 + + - name: Run keycloak container + run: | + ./ci/run_keycloak.sh + + - name: Install Coldfront and plugin + run: | + ./ci/setup.sh + + - name: Run SCIM authentication functional test + run: | + ./ci/run_functional_test_scim_auth.sh diff --git a/ci/run_functional_test_scim_auth.sh b/ci/run_functional_test_scim_auth.sh new file mode 100755 index 0000000..b1325fc --- /dev/null +++ b/ci/run_functional_test_scim_auth.sh @@ -0,0 +1,20 @@ +# For onbarding-tools +export KEYCLOAK_USERNAME=admin +export KEYCLOAK_PASSWORD=nomoresecret +export KEYCLOAK_URL="http://localhost:8080" +export OIDC_CLIENT_ID=coldfront +export OIDC_CLIENT_SECRET=nomoresecret +export HORIZON_URL="http://foo" + +# For coldfront oidc plugin +export DJANGO_SETTINGS_MODULE="local_settings" +export PLUGIN_AUTH_OIDC=True +export OIDC_RP_CLIENT_ID="coldfront" +export OIDC_RP_CLIENT_SECRET='nomoresecret' +export OIDC_OP_AUTHORIZATION_ENDPOINT="http://localhost:8080/realms/master/protocol/openid-connect/auth" +export OIDC_OP_TOKEN_ENDPOINT="http://localhost:8080/realms/master/protocol/openid-connect/token" +export OIDC_OP_USER_ENDPOINT="http://localhost:8080/realms/master/protocol/openid-connect/userinfo" +export OIDC_RP_SIGN_ALGO='RS256' +export OIDC_OP_JWKS_ENDPOINT="http://localhost:8080/realms/master/protocol/openid-connect/certs" + +coldfront test coldfront_plugin_api.tests.functional.test_scim_auth diff --git a/ci/run_keycloak.sh b/ci/run_keycloak.sh new file mode 100755 index 0000000..17e45b1 --- /dev/null +++ b/ci/run_keycloak.sh @@ -0,0 +1,8 @@ +set -xe + +sudo docker run -d --name keycloak \ + -e KEYCLOAK_ADMIN=admin \ + -e KEYCLOAK_ADMIN_PASSWORD=nomoresecret \ + -p 8080:8080 \ + -p 8443:8443 \ + quay.io/keycloak/keycloak:latest start-dev diff --git a/src/coldfront_plugin_api/config.py b/src/coldfront_plugin_api/config.py index 9764e28..3486c31 100644 --- a/src/coldfront_plugin_api/config.py +++ b/src/coldfront_plugin_api/config.py @@ -14,4 +14,5 @@ "GROUP_ADAPTER": "coldfront_plugin_api.scim_v2.adapter_group.SCIMColdfrontGroup", "GROUP_FILTER_PARSER": "coldfront_plugin_api.scim_v2.filters.ColdfrontGroupFilterQuery", "GET_IS_AUTHENTICATED_PREDICATE": "coldfront_plugin_api.utils.is_user_superuser", + "AUTH_CHECK_MIDDLEWARE": "coldfront_plugin_api.scim_v2.auth_middleware.SCIMColdfrontAuthCheckMiddleware", } diff --git a/src/coldfront_plugin_api/scim_v2/auth_middleware.py b/src/coldfront_plugin_api/scim_v2/auth_middleware.py new file mode 100644 index 0000000..2363691 --- /dev/null +++ b/src/coldfront_plugin_api/scim_v2/auth_middleware.py @@ -0,0 +1,20 @@ +import os +import logging + +from django_scim.middleware import SCIMAuthCheckMiddleware +from mozilla_django_oidc.contrib.drf import OIDCAuthentication + +logger = logging.getLogger(__name__) + + +class SCIMColdfrontAuthCheckMiddleware(SCIMAuthCheckMiddleware): + def process_request(self, request): + if not request.user or not request.user.is_authenticated: + # django-scim2 does not use by default the DRF backend of mozilla-django-oidc, + # and therefore does not support authentication with bearer tokens, only + # session cookies. We manually call `authenticate()` on the DRF backend if + # the user is not already authenticated, and if OIDC authentication is enabled. + if os.getenv("PLUGIN_AUTH_OIDC") == "True": + if user_tuple := OIDCAuthentication().authenticate(request): + request.user = user_tuple[0] + return super().process_request(request) diff --git a/src/coldfront_plugin_api/tests/unit/base.py b/src/coldfront_plugin_api/tests/base.py similarity index 100% rename from src/coldfront_plugin_api/tests/unit/base.py rename to src/coldfront_plugin_api/tests/base.py diff --git a/src/coldfront_plugin_api/tests/functional/__init__.py b/src/coldfront_plugin_api/tests/functional/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/coldfront_plugin_api/tests/functional/test_scim_auth.py b/src/coldfront_plugin_api/tests/functional/test_scim_auth.py new file mode 100644 index 0000000..e2e13da --- /dev/null +++ b/src/coldfront_plugin_api/tests/functional/test_scim_auth.py @@ -0,0 +1,65 @@ +import os + +from django.test import Client + + +from coldfront_plugin_api.tests.functional import utils +from coldfront_plugin_api.tests.base import TestBase + + +class TestAuthOIDC(TestBase): + @classmethod + def setUpClass(self): + super().setUpClass() + self.kc_client = utils.UpdatedKeycloakClient() + # Only initialize Coldfront client if running in Github action + if os.getenv("CI") == "true": + self.kc_client.create_client( + "master", + "coldfront", + "nomoresecret", + ["http://foo/signup/oidc_redirect_uri"], + ) + self.kc_client.create_user("master", "staff@email.com", "staff", "staff") + self.kc_client.create_user("master", "user@email.com", "user", "user") + + def setUp(self): + super().setUp() + self.staff_user = self.new_user("staff@email.com") + self.normal_user = self.new_user("user@email.com") + + self.staff_user.is_staff = True + self.staff_user.save() + + def test_oidc_authenticated(self): + # Test for both staff and normal authenticated users + def impersonate_and_get_endpoint( + user_to_impersonate, endpoint_url, expected_status_code + ): + user_token = self.kc_client.impersonate_access_token(user_to_impersonate) + + cf_client = Client() + r = cf_client.get( + endpoint_url, + HTTP_AUTHORIZATION=f"Bearer {user_token}", + ) + self.assertEqual(r.status_code, expected_status_code) + + for endpoint_url in [ + "/api/scim/v2/Users", + "/api/scim/v2/Groups", + "/api/allocations", + ]: + impersonate_and_get_endpoint(self.staff_user.username, endpoint_url, 200) + impersonate_and_get_endpoint(self.normal_user.username, endpoint_url, 403) + + def test_oidc_unauthenticated(self): + # Test for unauthenticated user case + cf_client = Client() + for endpoint_url in [ + "/api/scim/v2/Users", + "/api/scim/v2/Groups", + "/api/allocations", + ]: + r = cf_client.get(endpoint_url) + self.assertEqual(r.status_code, 401) diff --git a/src/coldfront_plugin_api/tests/functional/utils.py b/src/coldfront_plugin_api/tests/functional/utils.py new file mode 100644 index 0000000..6dd29ee --- /dev/null +++ b/src/coldfront_plugin_api/tests/functional/utils.py @@ -0,0 +1,41 @@ +import urllib +import requests + +from onboarding_tools.keycloak import KeycloakClient +from onboarding_tools import settings + + +class UpdatedKeycloakClient(KeycloakClient): + @staticmethod + def construct_url(realm, path): + return f"{settings.KEYCLOAK_URL}/admin/realms/{realm}/{path}" + + @property + def url_base(self): + return f"{settings.KEYCLOAK_URL}/admin/realms" + + @staticmethod + def auth_endpoint(realm): + return f"{settings.KEYCLOAK_URL}/realms/{realm}/protocol/openid-connect/auth" + + @staticmethod + def token_endpoint(realm): + return f"{settings.KEYCLOAK_URL}/realms/{realm}/protocol/openid-connect/token" + + def impersonate_access_token(self, user): + user_session = requests.session() + user_session.cookies.update(self.impersonate(user).cookies) + params = { + "response_mode": "fragment", + "response_type": "token", + "client_id": settings.OIDC_CLIENT_ID, + "client_secret": settings.OIDC_CLIENT_SECRET, + "redirect_uri": f"{settings.HORIZON_URL}/signup/oidc_redirect_uri", + "scope": "openid profile email", + } + response = user_session.get( + self.auth_endpoint("master"), params=params, allow_redirects=False + ) + redirect = response.headers["Location"] + token = urllib.parse.parse_qs(redirect)["access_token"][0] + return token diff --git a/src/coldfront_plugin_api/tests/unit/test_fix_allocations.py b/src/coldfront_plugin_api/tests/unit/test_fix_allocations.py index d5d6ed5..f31ecc2 100644 --- a/src/coldfront_plugin_api/tests/unit/test_fix_allocations.py +++ b/src/coldfront_plugin_api/tests/unit/test_fix_allocations.py @@ -6,7 +6,7 @@ AllocationUserStatusChoice, ) from coldfront.core.project.models import ProjectUser -from coldfront_plugin_api.tests.unit import base +from coldfront_plugin_api.tests import base from unittest.mock import patch diff --git a/src/coldfront_plugin_api/tests/unit/test_groups.py b/src/coldfront_plugin_api/tests/unit/test_groups.py index aeb88bb..d75eadd 100644 --- a/src/coldfront_plugin_api/tests/unit/test_groups.py +++ b/src/coldfront_plugin_api/tests/unit/test_groups.py @@ -2,9 +2,10 @@ import uuid from coldfront.core.resource import models as resource_models +from coldfront_plugin_api.tests import base from rest_framework.test import APIClient -from coldfront_plugin_api.tests.unit import base, fakes +from coldfront_plugin_api.tests.unit import fakes def get_payload_for_single_operation(operation, username): diff --git a/src/coldfront_plugin_api/tests/unit/test_users.py b/src/coldfront_plugin_api/tests/unit/test_users.py index 8aa8457..1ad9bfe 100644 --- a/src/coldfront_plugin_api/tests/unit/test_users.py +++ b/src/coldfront_plugin_api/tests/unit/test_users.py @@ -3,9 +3,10 @@ from coldfront.core.resource import models as resource_models from coldfront.core.user.models import User +from coldfront_plugin_api.tests import base from rest_framework.test import APIClient -from coldfront_plugin_api.tests.unit import base, fakes +from coldfront_plugin_api.tests.unit import fakes class TestUsers(base.TestBase): diff --git a/src/coldfront_plugin_api/utils.py b/src/coldfront_plugin_api/utils.py index 5c87148..e7a7ebb 100644 --- a/src/coldfront_plugin_api/utils.py +++ b/src/coldfront_plugin_api/utils.py @@ -59,7 +59,7 @@ def is_user_superuser(user: User): As a temporary hack, this function will handle raising the appropriate 403 error if user is authenticated, but not superuser """ - if user.is_authenticated and not user.is_superuser: + if user.is_authenticated and not (user.is_staff or user.is_superuser): raise PermissionDenied else: return user.is_authenticated diff --git a/test-requirements.txt b/test-requirements.txt index 30f933e..ad83e35 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1 +1,2 @@ git+https://github.com/nerc-project/coldfront-plugin-cloud@main#egg=coldfront_plugin_cloud +git+https://github.com/CCI-MOC/onboarding-tools@master#egg=onboarding_tools