Fixed OIDC authentication for SCIM endpoints #43
Conversation
There was a problem hiding this comment.
Since we are in no urgency to merge this now, I think it is important to use this opportunity to make our testing infrastructure more robust by including authentication for testing that OAuth authentication works correctly for the API interface.
Please introduce a new functional CI job that starts up and configures a Keycloak container and checks that authentication proceeds correctly for staff users using an access token and denies correctly unauthenticated users.
Feel free to use this class to help you create a Keycloak client and users as necessary. https://github.com/CCI-MOC/onboarding-tools/blob/master/onboarding_tools/keycloak.py
Previously, the Coldfront SCIM endpoints could not perform OIDC authentication, preventing usage of the client credentials and device authorization flows. A authentication middleware which subclasses from the one provided by `django_scim` has been added, which will perform OIDC authentication given an access token in the "Authorization" HTTP request header. The SCIM endpoint is now available to Coldfront users with "staff" or "superuser" status. It will perform OIDC authentication if the `PLUGIN_AUTH_OIDC` env var is set to True.
| class SCIMColdfrontAuthCheckMiddleware(SCIMAuthCheckMiddleware): | ||
| def process_request(self, request): | ||
| if not request.user or not request.user.is_authenticated: | ||
| # Check if OIDC authentication is enabled to see if we should authenticate user using OIDC |
There was a problem hiding this comment.
You're still explaining the what. A better comment would be
# 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 in case
# of failed authentication, to try again.
There was a problem hiding this comment.
@QuanMPhm I think you should update the comment and then it's good to merge.
| class SCIMColdfrontAuthCheckMiddleware(SCIMAuthCheckMiddleware): | ||
| def process_request(self, request): | ||
| if not request.user or not request.user.is_authenticated: | ||
| # Check if OIDC authentication is enabled to see if we should authenticate user using OIDC |
There was a problem hiding this comment.
@QuanMPhm I think you should update the comment and then it's good to merge.
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.
|
I have updated the comment |
Previously, the Coldfront SCIM endpoints could not perform OIDC authentication, preventing usage of the client credentials and device authorization flows.
A authentication middleware which subclasses from the one provided by
django_scimhas been added, which will perform OIDC authentication given an access token in the "Authorization" HTTP request header.The SCIM endpoint is now available to Coldfront users with "staff" or "superuser" status.