From 4f51575d5929d56f2a4f9c7676c47d86fb1d8a96 Mon Sep 17 00:00:00 2001 From: Andrea Greco Date: Fri, 23 Apr 2021 11:13:39 +0200 Subject: [PATCH 1/4] OpenID: Claims: Add claims inside well-known Some client can't use userinfo, and get propelty from claims. Add claims key inside well-known. --- AUTHORS | 1 + docs/oidc.rst | 19 ++++++++++--------- oauth2_provider/oauth2_validators.py | 25 +++++++++++++++++-------- oauth2_provider/views/oidc.py | 8 ++++++++ tests/test_oidc_views.py | 24 ++++++++++++++++++++---- 5 files changed, 56 insertions(+), 21 deletions(-) diff --git a/AUTHORS b/AUTHORS index a6c6ef1d2..a1591b6da 100644 --- a/AUTHORS +++ b/AUTHORS @@ -66,4 +66,5 @@ pySilver Shaheed Haque Vinay Karanam Eduardo Oliveira +Andrea Greco Dominik George diff --git a/docs/oidc.rst b/docs/oidc.rst index ba69e984f..7eccaad91 100644 --- a/docs/oidc.rst +++ b/docs/oidc.rst @@ -245,16 +245,17 @@ required claims, eg ``iss``, ``aud``, ``exp``, ``iat``, ``auth_time`` etc), and the ``sub`` claim will use the primary key of the user as the value. You'll probably want to customize this and add additional claims or change what is sent for the ``sub`` claim. To do so, you will need to add a method to -our custom validator:: - +our custom validator. +Standard claim ``sub`` is included by default, to remove it override ``get_claim_list``:: class CustomOAuth2Validator(OAuth2Validator): - - def get_additional_claims(self, request): - return { - "sub": request.user.email, - "first_name": request.user.first_name, - "last_name": request.user.last_name, - } + def get_additional_claims(self): + def get_user_email(request): + return request.user.get_full_name() + + # Element name, callback to obtain data + claims_list = [ ("email", get_sub_cod), + ("username", get_user_email) ] + return claims_list .. note:: This ``request`` object is not a ``django.http.Request`` object, but an diff --git a/oauth2_provider/oauth2_validators.py b/oauth2_provider/oauth2_validators.py index f3a24e258..f6517415c 100644 --- a/oauth2_provider/oauth2_validators.py +++ b/oauth2_provider/oauth2_validators.py @@ -728,15 +728,24 @@ def _save_id_token(self, jti, request, expires, *args, **kwargs): def get_jwt_bearer_token(self, token, token_handler, request): return self.get_id_token(token, token_handler, request) - def get_oidc_claims(self, token, token_handler, request): - # Required OIDC claims - claims = { - "sub": str(request.user.id), - } + def get_claim_list(self): + def get_sub_code(request): + return str(request.user.id) + + list = [ ("sub", get_sub_code) ] # https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims - claims.update(**self.get_additional_claims(request)) + add = self.get_additional_claims() + list.extend(add) + + return list + def get_oidc_claims(self, token, token_handler, request): + data = self.get_claim_list() + claims = {} + + for k, call in data: + claims[k] = call(request) return claims def get_id_token_dictionary(self, token, token_handler, request): @@ -889,5 +898,5 @@ def get_userinfo_claims(self, request): """ return self.get_oidc_claims(None, None, request) - def get_additional_claims(self, request): - return {} + def get_additional_claims(self): + return [] diff --git a/oauth2_provider/views/oidc.py b/oauth2_provider/views/oidc.py index b4bb8869b..0cd24fc85 100644 --- a/oauth2_provider/views/oidc.py +++ b/oauth2_provider/views/oidc.py @@ -45,6 +45,13 @@ def get(self, request, *args, **kwargs): signing_algorithms = [Application.HS256_ALGORITHM] if oauth2_settings.OIDC_RSA_PRIVATE_KEY: signing_algorithms = [Application.RS256_ALGORITHM, Application.HS256_ALGORITHM] + + validator_class = oauth2_settings.OAUTH2_VALIDATOR_CLASS + validator = validator_class() + oidc_claims = [] + for el, _ in validator.get_claim_list(): + oidc_claims.append(el) + data = { "issuer": issuer_url, "authorization_endpoint": authorization_endpoint, @@ -57,6 +64,7 @@ def get(self, request, *args, **kwargs): "token_endpoint_auth_methods_supported": ( oauth2_settings.OIDC_TOKEN_ENDPOINT_AUTH_METHODS_SUPPORTED ), + "claims_supported": oidc_claims, } response = JsonResponse(data) response["Access-Control-Allow-Origin"] = "*" diff --git a/tests/test_oidc_views.py b/tests/test_oidc_views.py index 46040f86d..719d10e98 100644 --- a/tests/test_oidc_views.py +++ b/tests/test_oidc_views.py @@ -29,6 +29,7 @@ def test_get_connect_discovery_info(self): "subject_types_supported": ["public"], "id_token_signing_alg_values_supported": ["RS256", "HS256"], "token_endpoint_auth_methods_supported": ["client_secret_post", "client_secret_basic"], + "claims_supported": ["sub"], } response = self.client.get(reverse("oauth2_provider:oidc-connect-discovery-info")) self.assertEqual(response.status_code, 200) @@ -55,6 +56,7 @@ def test_get_connect_discovery_info_without_issuer_url(self): "subject_types_supported": ["public"], "id_token_signing_alg_values_supported": ["RS256", "HS256"], "token_endpoint_auth_methods_supported": ["client_secret_post", "client_secret_basic"], + "claims_supported": ["sub"], } response = self.client.get(reverse("oauth2_provider:oidc-connect-discovery-info")) self.assertEqual(response.status_code, 200) @@ -146,11 +148,21 @@ def test_userinfo_endpoint_bad_token(oidc_tokens, client): assert rsp.status_code == 401 +EXAMPLE_EMAIL = "example.email@example.com" + + +def claim_user_email(request): + return EXAMPLE_EMAIL + + @pytest.mark.django_db def test_userinfo_endpoint_custom_claims(oidc_tokens, client, oauth2_settings): class CustomValidator(OAuth2Validator): - def get_additional_claims(self, request): - return {"state": "very nice"} + def get_additional_claims(self): + return [ + ("username", claim_user_email), + ("email", claim_user_email), + ] oidc_tokens.oauth2_settings.OAUTH2_VALIDATOR_CLASS = CustomValidator auth_header = "Bearer %s" % oidc_tokens.access_token @@ -161,5 +173,9 @@ def get_additional_claims(self, request): data = rsp.json() assert "sub" in data assert data["sub"] == str(oidc_tokens.user.pk) - assert "state" in data - assert data["state"] == "very nice" + + assert "username" in data + assert data["username"] == EXAMPLE_EMAIL + + assert "email" in data + assert data["email"] == EXAMPLE_EMAIL From 74b31276985f236b67b5570c824c63616a3f83b1 Mon Sep 17 00:00:00 2001 From: Dominik George Date: Tue, 4 Jan 2022 23:03:36 +0100 Subject: [PATCH 2/4] OpenID: Fix get_additional_claims API * always propagate request * have get_additional_claims return a dict again * allow get_additional_claims to return plain data instead of callables --- CHANGELOG.md | 1 + docs/oidc.rst | 17 ++++++++----- oauth2_provider/oauth2_validators.py | 24 +++++++++--------- oauth2_provider/views/oidc.py | 4 +-- tests/test_oidc_views.py | 38 +++++++++++++++++++++++----- 5 files changed, 57 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b5a70cd5a..f86c13edc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * #651 Batch expired token deletions in `cleartokens` management command * Added pt-BR translations. * #1070 Add a Celery task for clearing expired tokens, e.g. to be scheduled as a [periodic task](https://docs.celeryproject.org/en/stable/userguide/periodic-tasks.html) +* #1069 OIDC: Re-introduce [additional claims](https://django-oauth-toolkit.readthedocs.io/en/latest/oidc.html#adding-claims-to-the-id-token) beyond `sub` to the id_token. ### Fixed * #1012 Return status for introspecting a nonexistent token from 401 to the correct value of 200 per [RFC 7662](https://datatracker.ietf.org/doc/html/rfc7662#section-2.2). diff --git a/docs/oidc.rst b/docs/oidc.rst index 7eccaad91..27daf3b72 100644 --- a/docs/oidc.rst +++ b/docs/oidc.rst @@ -245,17 +245,22 @@ required claims, eg ``iss``, ``aud``, ``exp``, ``iat``, ``auth_time`` etc), and the ``sub`` claim will use the primary key of the user as the value. You'll probably want to customize this and add additional claims or change what is sent for the ``sub`` claim. To do so, you will need to add a method to -our custom validator. +our custom validator. It should return a dictionary mapping a claim name to +either the claim data, or a callable that will be called with the request to +produce the claim data. Standard claim ``sub`` is included by default, to remove it override ``get_claim_list``:: class CustomOAuth2Validator(OAuth2Validator): - def get_additional_claims(self): + def get_additional_claims(self, request): def get_user_email(request): - return request.user.get_full_name() + return request.user.get_user_email() + claims = {} # Element name, callback to obtain data - claims_list = [ ("email", get_sub_cod), - ("username", get_user_email) ] - return claims_list + claims["email"] = get_user_email + # Element name, plain data returned + claims["username"] = request.user.get_full_name() + + return claims .. note:: This ``request`` object is not a ``django.http.Request`` object, but an diff --git a/oauth2_provider/oauth2_validators.py b/oauth2_provider/oauth2_validators.py index f6517415c..73f27ec15 100644 --- a/oauth2_provider/oauth2_validators.py +++ b/oauth2_provider/oauth2_validators.py @@ -728,24 +728,24 @@ def _save_id_token(self, jti, request, expires, *args, **kwargs): def get_jwt_bearer_token(self, token, token_handler, request): return self.get_id_token(token, token_handler, request) - def get_claim_list(self): - def get_sub_code(request): - return str(request.user.id) + def get_claim_dict(self, request): + def get_sub_code(inner_request): + return str(inner_request.user.id) - list = [ ("sub", get_sub_code) ] + claims = {"sub": get_sub_code} # https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims - add = self.get_additional_claims() - list.extend(add) + add = self.get_additional_claims(request) + claims.update(add) - return list + return claims def get_oidc_claims(self, token, token_handler, request): - data = self.get_claim_list() + data = self.get_claim_dict(request) claims = {} - for k, call in data: - claims[k] = call(request) + for k, v in data.items(): + claims[k] = v(request) if callable(v) else v return claims def get_id_token_dictionary(self, token, token_handler, request): @@ -898,5 +898,5 @@ def get_userinfo_claims(self, request): """ return self.get_oidc_claims(None, None, request) - def get_additional_claims(self): - return [] + def get_additional_claims(self, request): + return {} diff --git a/oauth2_provider/views/oidc.py b/oauth2_provider/views/oidc.py index 0cd24fc85..5e929bd6e 100644 --- a/oauth2_provider/views/oidc.py +++ b/oauth2_provider/views/oidc.py @@ -48,9 +48,7 @@ def get(self, request, *args, **kwargs): validator_class = oauth2_settings.OAUTH2_VALIDATOR_CLASS validator = validator_class() - oidc_claims = [] - for el, _ in validator.get_claim_list(): - oidc_claims.append(el) + oidc_claims = list(validator.get_claim_dict(request).keys()) data = { "issuer": issuer_url, diff --git a/tests/test_oidc_views.py b/tests/test_oidc_views.py index 719d10e98..63d788eac 100644 --- a/tests/test_oidc_views.py +++ b/tests/test_oidc_views.py @@ -156,13 +156,39 @@ def claim_user_email(request): @pytest.mark.django_db -def test_userinfo_endpoint_custom_claims(oidc_tokens, client, oauth2_settings): +def test_userinfo_endpoint_custom_claims_callable(oidc_tokens, client, oauth2_settings): class CustomValidator(OAuth2Validator): - def get_additional_claims(self): - return [ - ("username", claim_user_email), - ("email", claim_user_email), - ] + def get_additional_claims(self, request): + return { + "username": claim_user_email, + "email": claim_user_email, + } + + oidc_tokens.oauth2_settings.OAUTH2_VALIDATOR_CLASS = CustomValidator + auth_header = "Bearer %s" % oidc_tokens.access_token + rsp = client.get( + reverse("oauth2_provider:user-info"), + HTTP_AUTHORIZATION=auth_header, + ) + data = rsp.json() + assert "sub" in data + assert data["sub"] == str(oidc_tokens.user.pk) + + assert "username" in data + assert data["username"] == EXAMPLE_EMAIL + + assert "email" in data + assert data["email"] == EXAMPLE_EMAIL + + +@pytest.mark.django_db +def test_userinfo_endpoint_custom_claims_plain(oidc_tokens, client, oauth2_settings): + class CustomValidator(OAuth2Validator): + def get_additional_claims(self, request): + return { + "username": EXAMPLE_EMAIL, + "email": EXAMPLE_EMAIL, + } oidc_tokens.oauth2_settings.OAUTH2_VALIDATOR_CLASS = CustomValidator auth_header = "Bearer %s" % oidc_tokens.access_token From 726a34453a5ad0608db4f58e159ac53aa98ad9d6 Mon Sep 17 00:00:00 2001 From: Dominik George Date: Tue, 4 Jan 2022 23:45:25 +0100 Subject: [PATCH 3/4] OpenID: Add get_discovery_claims This splits get_additional_claims into two forms. See documentation change for rationale. --- docs/oidc.rst | 37 ++++++++++++++++++++++------ oauth2_provider/oauth2_validators.py | 24 ++++++++++++++---- oauth2_provider/views/oidc.py | 4 ++- tests/test_oidc_views.py | 2 +- 4 files changed, 53 insertions(+), 14 deletions(-) diff --git a/docs/oidc.rst b/docs/oidc.rst index 27daf3b72..143bec5e5 100644 --- a/docs/oidc.rst +++ b/docs/oidc.rst @@ -245,23 +245,46 @@ required claims, eg ``iss``, ``aud``, ``exp``, ``iat``, ``auth_time`` etc), and the ``sub`` claim will use the primary key of the user as the value. You'll probably want to customize this and add additional claims or change what is sent for the ``sub`` claim. To do so, you will need to add a method to -our custom validator. It should return a dictionary mapping a claim name to -either the claim data, or a callable that will be called with the request to -produce the claim data. -Standard claim ``sub`` is included by default, to remove it override ``get_claim_list``:: +our custom validator. It takes one of two forms: + +The first form gets passed a request object, and should return a dictionary +mapping a claim name to claim data:: class CustomOAuth2Validator(OAuth2Validator): def get_additional_claims(self, request): + claims = {} + claims["email"] = request.user.get_user_email() + claims["username"] = request.user.get_full_name() + + return claims + +The second form gets no request object, and should return a dictionary +mapping a claim name to a callable, accepting a request and producing +the claim data:: + class CustomOAuth2Validator(OAuth2Validator): + def get_additional_claims(self): def get_user_email(request): return request.user.get_user_email() claims = {} - # Element name, callback to obtain data claims["email"] = get_user_email - # Element name, plain data returned - claims["username"] = request.user.get_full_name() + claims["username"] = lambda r: r.user.get_full_name() return claims +Standard claim ``sub`` is included by default, to remove it override ``get_claim_dict``. + +In some cases, it might be desirable to not list all claims in discovery info. To customize +which claims are advertised, you can override the ``get_discovery_claims`` method to return +a list of claim names to advertise. If your ``get_additional_claims`` uses the first form +and you still want to advertise claims, you can also override ``get_discovery_claims``. + +In order to help lcients discover claims early, they can be advertised in the discovery +info, under the ``claims_supported`` key. In order for the discovery info view to automatically +add all claims your validator returns, you need to use the second form (producing callables), +because the discovery info views are requested with an unauthenticated request, so directly +producing claim data would fail. If you use the first form, producing claim data directly, +your claims will not be added to discovery info. + .. note:: This ``request`` object is not a ``django.http.Request`` object, but an ``oauthlib.common.Request`` object. This has a number of attributes that diff --git a/oauth2_provider/oauth2_validators.py b/oauth2_provider/oauth2_validators.py index 73f27ec15..4d9480be1 100644 --- a/oauth2_provider/oauth2_validators.py +++ b/oauth2_provider/oauth2_validators.py @@ -1,6 +1,7 @@ import base64 import binascii import http.client +import inspect import json import logging import uuid @@ -725,21 +726,34 @@ def _save_id_token(self, jti, request, expires, *args, **kwargs): ) return id_token + @classmethod + def _get_additional_claims_is_request_agnostic(cls): + return len(inspect.signature(cls.get_additional_claims).parameters) == 1 + def get_jwt_bearer_token(self, token, token_handler, request): return self.get_id_token(token, token_handler, request) def get_claim_dict(self, request): - def get_sub_code(inner_request): - return str(inner_request.user.id) - - claims = {"sub": get_sub_code} + if self._get_additional_claims_is_request_agnostic(): + claims = {"sub": lambda r: str(r.user.id)} + else: + claims = {"sub": str(request.user.id)} # https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims - add = self.get_additional_claims(request) + if self._get_additional_claims_is_request_agnostic(): + add = self.get_additional_claims() + else: + add = self.get_additional_claims(request) claims.update(add) return claims + def get_discovery_claims(self, request): + claims = ["sub"] + if self._get_additional_claims_is_request_agnostic(): + claims += list(self.get_claim_dict(request).keys()) + return claims + def get_oidc_claims(self, token, token_handler, request): data = self.get_claim_dict(request) claims = {} diff --git a/oauth2_provider/views/oidc.py b/oauth2_provider/views/oidc.py index 5e929bd6e..a0206a501 100644 --- a/oauth2_provider/views/oidc.py +++ b/oauth2_provider/views/oidc.py @@ -48,7 +48,9 @@ def get(self, request, *args, **kwargs): validator_class = oauth2_settings.OAUTH2_VALIDATOR_CLASS validator = validator_class() - oidc_claims = list(validator.get_claim_dict(request).keys()) + oidc_claims = validator.get_discovery_claims(request) + if "sub" not in oidc_claims: + oidc_claims.append("sub") data = { "issuer": issuer_url, diff --git a/tests/test_oidc_views.py b/tests/test_oidc_views.py index 63d788eac..fa514ac92 100644 --- a/tests/test_oidc_views.py +++ b/tests/test_oidc_views.py @@ -158,7 +158,7 @@ def claim_user_email(request): @pytest.mark.django_db def test_userinfo_endpoint_custom_claims_callable(oidc_tokens, client, oauth2_settings): class CustomValidator(OAuth2Validator): - def get_additional_claims(self, request): + def get_additional_claims(self): return { "username": claim_user_email, "email": claim_user_email, From 7befb8b934c5dfde7dd97b7d6105525ea131c7b8 Mon Sep 17 00:00:00 2001 From: Dominik George Date: Sun, 23 Jan 2022 00:10:25 +0100 Subject: [PATCH 4/4] OpenID: Ensure claims_supported lists each claim only once --- oauth2_provider/views/oidc.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/oauth2_provider/views/oidc.py b/oauth2_provider/views/oidc.py index a0206a501..e66b30a86 100644 --- a/oauth2_provider/views/oidc.py +++ b/oauth2_provider/views/oidc.py @@ -48,9 +48,7 @@ def get(self, request, *args, **kwargs): validator_class = oauth2_settings.OAUTH2_VALIDATOR_CLASS validator = validator_class() - oidc_claims = validator.get_discovery_claims(request) - if "sub" not in oidc_claims: - oidc_claims.append("sub") + oidc_claims = list(set(validator.get_discovery_claims(request))) data = { "issuer": issuer_url,