django.utils.oidc_user_from_decoded_token: Relax fields constraint
ClosedPublic
Actions

Authored by ardumont on Mar 18 2021, 6:01 PM.

Details

Reviewers

anlambert

Group Reviewers

Reviewers

Maniphest Tasks

T2858: Use keycloak authentication for the deposit

Commits

rDAUTH57583d2bcc3f: django.utils.oidc_user_from_decoded_token: Relax fields constraint

Summary

In some application (e.g. deposit), those user fields might not be filled in. As it's not enforced by
keycloak, relax such constraint.

Without this diff, user connection on the deposit fails with the following server side:

swh-deposit_1                   | Internal Server Error: /deposit/1/servicedocument/
swh-deposit_1                   | Traceback (most recent call last):
swh-deposit_1                   |   File "/srv/softwareheritage/venv/lib/python3.7/site-packages/django/core/handlers/exception.py", line 34, in inner
swh-deposit_1                   |     response = get_response(request)
swh-deposit_1                   |   File "/srv/softwareheritage/venv/lib/python3.7/site-packages/django/core/handlers/base.py", line 115, in _get_response
swh-deposit_1                   |     response = self.process_exception_by_middleware(e, request)
swh-deposit_1                   |   File "/srv/softwareheritage/venv/lib/python3.7/site-packages/django/core/handlers/base.py", line 113, in _get_response
swh-deposit_1                   |     response = wrapped_callback(request, *callback_args, **callback_kwargs)
swh-deposit_1                   |   File "/srv/softwareheritage/venv/lib/python3.7/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
swh-deposit_1                   |     return view_func(*args, **kwargs)
swh-deposit_1                   |   File "/srv/softwareheritage/venv/lib/python3.7/site-packages/django/views/generic/base.py", line 71, in view
swh-deposit_1                   |     return self.dispatch(request, *args, **kwargs)
swh-deposit_1                   |   File "/srv/softwareheritage/venv/lib/python3.7/site-packages/rest_framework/views.py", line 509, in dispatch
swh-deposit_1                   |     response = self.handle_exception(exc)
swh-deposit_1                   |   File "/srv/softwareheritage/venv/lib/python3.7/site-packages/rest_framework/views.py", line 469, in handle_exception
swh-deposit_1                   |     self.raise_uncaught_exception(exc)
swh-deposit_1                   |   File "/srv/softwareheritage/venv/lib/python3.7/site-packages/rest_framework/views.py", line 480, in raise_uncaught_exception
swh-deposit_1                   |     raise exc
swh-deposit_1                   |   File "/srv/softwareheritage/venv/lib/python3.7/site-packages/rest_framework/views.py", line 497, in dispatch
swh-deposit_1                   |     self.initial(request, *args, **kwargs)
swh-deposit_1                   |   File "/srv/softwareheritage/venv/lib/python3.7/site-packages/rest_framework/views.py", line 414, in initial
swh-deposit_1                   |     self.perform_authentication(request)
swh-deposit_1                   |   File "/srv/softwareheritage/venv/lib/python3.7/site-packages/rest_framework/views.py", line 324, in perform_authentication
swh-deposit_1                   |     request.user
swh-deposit_1                   |   File "/srv/softwareheritage/venv/lib/python3.7/site-packages/rest_framework/request.py", line 227, in user
swh-deposit_1                   |     self._authenticate()
swh-deposit_1                   |   File "/srv/softwareheritage/venv/lib/python3.7/site-packages/rest_framework/request.py", line 380, in _authenticate
swh-deposit_1                   |     user_auth_tuple = authenticator.authenticate(self)
swh-deposit_1                   |   File "/srv/softwareheritage/venv/lib/python3.7/site-packages/rest_framework/authentication.py", line 87, in authenticate
swh-deposit_1                   |     return self.authenticate_credentials(userid, password, request)
swh-deposit_1                   |   File "/srv/softwareheritage/venv/lib/python3.7/site-packages/swh/deposit/auth.py", line 151, in authenticate_credentials
swh-deposit_1                   |     oidc_user = oidc_user_from_profile(self.client, oidc_profile)
swh-deposit_1                   |   File "/srv/softwareheritage/venv/lib/python3.7/site-packages/swh/auth/django/utils.py", line 80, in oidc_user_from_profile
swh-deposit_1                   |     user = oidc_user_from_decoded_token(decoded_token, client_id=oidc_client.client_id)
swh-deposit_1                   |   File "/srv/softwareheritage/venv/lib/python3.7/site-packages/swh/auth/django/utils.py", line 39, in oidc_user_from_decoded_token
swh-deposit_1                   |     email=decoded_token["email"],
swh-deposit_1                   | KeyError: 'email'

Related to T2858

Test Plan

Testing through docker with the diff D5279 (to create users) and D5137 to actually use the deposit with said user [1]

[1] configuring the deposit within docker:

swh deposit admin user create --username hal-test --provider-url https://hal-test.archives-ouvertes.fr/ --domain archives-ouvertes.fr

Diff Detail

Repository

rDAUTH Common authentication libraries

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Mar 18 2021, 6:01 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptMar 18 2021, 6:01 PM

Build is green

Patch application report for D5281 (id=18918)

Rebasing onto 679c0a5e60...

Current branch diff-target is up to date.

Changes applied before test

commit 7c87b9568bb97521a2f3a21dab3566ab36bb5f3c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Mar 18 18:00:20 2021 +0100

    django.utils.oidc_user_from_decoded_token: Relax fields constraint
    
    In some application, those user fields might not be filled in. As it's not enforced by
    keycloak, relax such constraint.
    
    Related to T2858

See https://jenkins.softwareheritage.org/job/DAUTH/job/tests-on-diff/34/ for more details.

Harbormaster completed remote builds in B19998: Diff 18918.Mar 18 2021, 6:02 PM

ardumont requested review of this revision.Mar 18 2021, 6:02 PM

ardumont edited the test plan for this revision. (Show Details)Mar 18 2021, 6:04 PM

Looks good. Maybe add a simple test checking a user can be created from the swh.auth.tests.sample_data.DECODED_TOKEN without the user info fields ?

This revision is now accepted and ready to land.Mar 18 2021, 6:05 PM

ardumont edited the summary of this revision. (Show Details)Mar 18 2021, 6:06 PM

ardumont edited the test plan for this revision. (Show Details)

Add a test

Build is green

Patch application report for D5281 (id=18919)

Rebasing onto 679c0a5e60...

Current branch diff-target is up to date.

Changes applied before test

commit 57583d2bcc3ff821591b9640d4141d7bfb32222e
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Mar 18 18:00:20 2021 +0100

    django.utils.oidc_user_from_decoded_token: Relax fields constraint
    
    In some application, those user fields might not be filled in. As it's not enforced by
    keycloak, relax such constraint.
    
    Related to T2858

See https://jenkins.softwareheritage.org/job/DAUTH/job/tests-on-diff/35/ for more details.

Harbormaster completed remote builds in B19999: Diff 18919.Mar 18 2021, 6:18 PM

Closed by commit rDAUTH57583d2bcc3f: django.utils.oidc_user_from_decoded_token: Relax fields constraint (authored by ardumont). · Explain WhyMar 18 2021, 6:20 PM

This revision was automatically updated to reflect the committed changes.

ardumont added a commit: rDAUTH57583d2bcc3f: django.utils.oidc_user_from_decoded_token: Relax fields constraint.