Skip to content

Commit 90bb88b

Browse files
committed
Added functional testing for SCIM authentication
A new CI job has been added to test the OIDC authentication for the API endpoints. This CI starts Keycloak in a container, installs coldfront then runs the API functional tests. A few code decisions to note: - The functional tests subclassed the keycloak client provided by `CCI-MOC/onboarding-tools`. Subclassing was needed because the original client class contained code which is out-of-date with the current version of Keycloak - The `is_user_superuser()` function in `utils.py` now also checks if `user.is_superuser`, to account for edge case where a superuser is created but their `is_staff` attribute is not set.
1 parent e9ac8dd commit 90bb88b

13 files changed

+177
-7
lines changed
+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
name: test-py39-functional-scim-auth
2+
3+
on:
4+
push:
5+
branches: [ main ]
6+
pull_request:
7+
branches: [ main ]
8+
9+
jobs:
10+
test-scim-auth:
11+
runs-on: ubuntu-latest
12+
13+
steps:
14+
- name: Check out code
15+
uses: actions/checkout@v3
16+
17+
- name: Set up python
18+
uses: actions/setup-python@v4
19+
with:
20+
python-version: 3.9
21+
22+
- name: Run keycloak container
23+
run: |
24+
./ci/run_keycloak.sh
25+
26+
- name: Install Coldfront and plugin
27+
run: |
28+
./ci/setup.sh
29+
30+
- name: Run SCIM authentication functional test
31+
run: |
32+
./ci/run_functional_test_scim_auth.sh

Diff for: ci/run_functional_test_scim_auth.sh

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# For onbarding-tools
2+
export KEYCLOAK_USERNAME=admin
3+
export KEYCLOAK_PASSWORD=nomoresecret
4+
export KEYCLOAK_URL="http://localhost:8080"
5+
export OIDC_CLIENT_ID=coldfront
6+
export OIDC_CLIENT_SECRET=nomoresecret
7+
export HORIZON_URL="http://foo"
8+
9+
# For coldfront oidc plugin
10+
export DJANGO_SETTINGS_MODULE="local_settings"
11+
export PLUGIN_AUTH_OIDC=True
12+
export OIDC_RP_CLIENT_ID="coldfront"
13+
export OIDC_RP_CLIENT_SECRET='nomoresecret'
14+
export OIDC_OP_AUTHORIZATION_ENDPOINT="http://localhost:8080/realms/master/protocol/openid-connect/auth"
15+
export OIDC_OP_TOKEN_ENDPOINT="http://localhost:8080/realms/master/protocol/openid-connect/token"
16+
export OIDC_OP_USER_ENDPOINT="http://localhost:8080/realms/master/protocol/openid-connect/userinfo"
17+
export OIDC_RP_SIGN_ALGO='RS256'
18+
export OIDC_OP_JWKS_ENDPOINT="http://localhost:8080/realms/master/protocol/openid-connect/certs"
19+
20+
coldfront test coldfront_plugin_api.tests.functional.test_scim_auth

Diff for: ci/run_keycloak.sh

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
set -xe
2+
3+
sudo docker run -d --name keycloak \
4+
-e KEYCLOAK_ADMIN=admin \
5+
-e KEYCLOAK_ADMIN_PASSWORD=nomoresecret \
6+
-p 8080:8080 \
7+
-p 8443:8443 \
8+
quay.io/keycloak/keycloak:latest start-dev

Diff for: src/coldfront_plugin_api/scim_v2/auth_middleware.py

+4-3
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@
1010
class SCIMColdfrontAuthCheckMiddleware(SCIMAuthCheckMiddleware):
1111
def process_request(self, request):
1212
if not request.user or not request.user.is_authenticated:
13-
# PLUGIN_AUTH_OIDC implies DRF OIDC backend is configured
13+
# Check if OIDC authentication is enabled to see if we should authenticate user using OIDC
14+
# PLUGIN_AUTH_OIDC implies OIDC is enabled and DRF OIDC backend is configured
1415
if os.getenv("PLUGIN_AUTH_OIDC") == "True":
15-
oidc_auth_obj = OIDCAuthentication()
16-
request.user = oidc_auth_obj.authenticate(request)
16+
if user_tuple := OIDCAuthentication().authenticate(request):
17+
request.user = user_tuple[0]
1718
return super().process_request(request)
File renamed without changes.

Diff for: src/coldfront_plugin_api/tests/functional/__init__.py

Whitespace-only changes.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import os
2+
3+
from django.test import Client
4+
5+
6+
from coldfront_plugin_api.tests.functional import utils
7+
from coldfront_plugin_api.tests.base import TestBase
8+
9+
10+
class TestAuthOIDC(TestBase):
11+
@classmethod
12+
def setUpClass(self):
13+
super().setUpClass()
14+
self.kc_client = utils.UpdatedKeycloakClient()
15+
# Only initialize Coldfront client if running in Github action
16+
if os.getenv("CI") == "true":
17+
self.kc_client.create_client(
18+
"master",
19+
"coldfront",
20+
"nomoresecret",
21+
["http://foo/signup/oidc_redirect_uri"],
22+
)
23+
self.kc_client.create_user("master", "[email protected]", "staff", "staff")
24+
self.kc_client.create_user("master", "[email protected]", "user", "user")
25+
26+
def setUp(self):
27+
super().setUp()
28+
self.staff_user = self.new_user("[email protected]")
29+
self.normal_user = self.new_user("[email protected]")
30+
31+
self.staff_user.is_staff = True
32+
self.staff_user.save()
33+
34+
def test_oidc_authenticated(self):
35+
# Test for both staff and normal authenticated users
36+
def impersonate_and_get_endpoint(
37+
user_to_impersonate, endpoint_url, expected_status_code
38+
):
39+
user_token = self.kc_client.impersonate_access_token(user_to_impersonate)
40+
41+
cf_client = Client()
42+
r = cf_client.get(
43+
endpoint_url,
44+
HTTP_AUTHORIZATION=f"Bearer {user_token}",
45+
)
46+
self.assertEqual(r.status_code, expected_status_code)
47+
48+
for endpoint_url in [
49+
"/api/scim/v2/Users",
50+
"/api/scim/v2/Groups",
51+
"/api/allocations",
52+
]:
53+
impersonate_and_get_endpoint(self.staff_user.username, endpoint_url, 200)
54+
impersonate_and_get_endpoint(self.normal_user.username, endpoint_url, 403)
55+
56+
def test_oidc_unauthenticated(self):
57+
# Test for unauthenticated user case
58+
cf_client = Client()
59+
for endpoint_url in [
60+
"/api/scim/v2/Users",
61+
"/api/scim/v2/Groups",
62+
"/api/allocations",
63+
]:
64+
r = cf_client.get(endpoint_url)
65+
self.assertEqual(r.status_code, 401)

Diff for: src/coldfront_plugin_api/tests/functional/utils.py

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import urllib
2+
import requests
3+
4+
from onboarding_tools.keycloak import KeycloakClient
5+
from onboarding_tools import settings
6+
7+
8+
class UpdatedKeycloakClient(KeycloakClient):
9+
@staticmethod
10+
def construct_url(realm, path):
11+
return f"{settings.KEYCLOAK_URL}/admin/realms/{realm}/{path}"
12+
13+
@property
14+
def url_base(self):
15+
return f"{settings.KEYCLOAK_URL}/admin/realms"
16+
17+
@staticmethod
18+
def auth_endpoint(realm):
19+
return f"{settings.KEYCLOAK_URL}/realms/{realm}/protocol/openid-connect/auth"
20+
21+
@staticmethod
22+
def token_endpoint(realm):
23+
return f"{settings.KEYCLOAK_URL}/realms/{realm}/protocol/openid-connect/token"
24+
25+
def impersonate_access_token(self, user):
26+
user_session = requests.session()
27+
user_session.cookies.update(self.impersonate(user).cookies)
28+
params = {
29+
"response_mode": "fragment",
30+
"response_type": "token",
31+
"client_id": settings.OIDC_CLIENT_ID,
32+
"client_secret": settings.OIDC_CLIENT_SECRET,
33+
"redirect_uri": f"{settings.HORIZON_URL}/signup/oidc_redirect_uri",
34+
"scope": "openid profile email",
35+
}
36+
response = user_session.get(
37+
self.auth_endpoint("master"), params=params, allow_redirects=False
38+
)
39+
redirect = response.headers["Location"]
40+
token = urllib.parse.parse_qs(redirect)["access_token"][0]
41+
return token

Diff for: src/coldfront_plugin_api/tests/unit/test_fix_allocations.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
AllocationUserStatusChoice,
77
)
88
from coldfront.core.project.models import ProjectUser
9-
from coldfront_plugin_api.tests.unit import base
9+
from coldfront_plugin_api.tests import base
1010

1111
from unittest.mock import patch
1212

Diff for: src/coldfront_plugin_api/tests/unit/test_groups.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@
22
import uuid
33

44
from coldfront.core.resource import models as resource_models
5+
from coldfront_plugin_api.tests import base
56
from rest_framework.test import APIClient
67

7-
from coldfront_plugin_api.tests.unit import base, fakes
8+
from coldfront_plugin_api.tests.unit import fakes
89

910

1011
def get_payload_for_single_operation(operation, username):

Diff for: src/coldfront_plugin_api/tests/unit/test_users.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@
33

44
from coldfront.core.resource import models as resource_models
55
from coldfront.core.user.models import User
6+
from coldfront_plugin_api.tests import base
67
from rest_framework.test import APIClient
78

8-
from coldfront_plugin_api.tests.unit import base, fakes
9+
from coldfront_plugin_api.tests.unit import fakes
910

1011

1112
class TestUsers(base.TestBase):

Diff for: src/coldfront_plugin_api/utils.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ def is_user_superuser(user: User):
5959
As a temporary hack, this function will handle raising the appropriate 403 error if
6060
user is authenticated, but not superuser
6161
"""
62-
if user.is_authenticated and not user.is_staff:
62+
if user.is_authenticated and not (user.is_staff or user.is_superuser):
6363
raise PermissionDenied
6464
else:
6565
return user.is_authenticated

Diff for: test-requirements.txt

+1
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
git+https://github.com/nerc-project/coldfront-plugin-cloud@main#egg=coldfront_plugin_cloud
2+
git+https://github.com/CCI-MOC/onboarding-tools@master#egg=onboarding_tools

0 commit comments

Comments
 (0)