providers/oauth2: fix inconsistent sub value when setting via mappi…

…ng (#8677) Signed-off-by: Jens Langhammer <jens@goauthentik.io>
goauthentik · Feb 25, 2024 · 646276b · 646276b
1 parent 58f9d86
commit 646276b
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 5 deletions.
diff --git a/authentik/providers/oauth2/tests/test_authorize.py b/authentik/providers/oauth2/tests/test_authorize.py
@@ -344,7 +344,12 @@ def test_full_implicit(self):
                 ]
             )
         )
-        Application.objects.create(name="app", slug="app", provider=provider)
+        provider.property_mappings.add(
+            ScopeMapping.objects.create(
+                name=generate_id(), scope_name="test", expression="""return {"sub": "foo"}"""
+            )
+        )
+        Application.objects.create(name=generate_id(), slug=generate_id(), provider=provider)
         state = generate_id()
         user = create_test_admin_user()
         self.client.force_login(user)
@@ -365,7 +370,7 @@ def test_full_implicit(self):
                     "response_type": "id_token",
                     "client_id": "test",
                     "state": state,
-                    "scope": "openid",
+                    "scope": "openid test",
                     "redirect_uri": "http://localhost",
                     "nonce": generate_id(),
                 },
@@ -390,6 +395,7 @@ def test_full_implicit(self):
             )
             jwt = self.validate_jwt(token, provider)
             self.assertEqual(jwt["amr"], ["pwd"])
+            self.assertEqual(jwt["sub"], "foo")
             self.assertAlmostEqual(
                 jwt["exp"] - now().timestamp(),
                 expires,

diff --git a/authentik/providers/oauth2/views/userinfo.py b/authentik/providers/oauth2/views/userinfo.py
@@ -101,8 +101,8 @@ def get_claims(self, provider: OAuth2Provider, token: BaseGrantModel) -> dict[st
                     value=value,
                 )
                 continue
-            LOGGER.debug("updated scope", scope=scope)
             always_merger.merge(final_claims, value)
+            LOGGER.debug("updated scope", scope=scope)
         return final_claims
 
     def dispatch(self, request: HttpRequest, *args: Any, **kwargs: Any) -> HttpResponse:
@@ -121,8 +121,9 @@ def get(self, request: HttpRequest, **kwargs) -> HttpResponse:
         """Handle GET Requests for UserInfo"""
         if not self.token:
             return HttpResponseBadRequest()
-        claims = self.get_claims(self.token.provider, self.token)
-        claims["sub"] = self.token.id_token.sub
+        claims = {}
+        claims.setdefault("sub", self.token.id_token.sub)
+        claims.update(self.get_claims(self.token.provider, self.token))
         if self.token.id_token.nonce:
             claims["nonce"] = self.token.id_token.nonce
         response = TokenResponse(claims)