Bugfix: id_token claims missing when sub is not consistent (#931)

* v2.3.3 hotfix: ensure user_id is not null for in-memory userinfo storage (#924)

* Fix userinfo flow for in-memory storage

Signed-off-by: Emiliano Suñé <emiliano.sune@gmail.com>

* Use in-memory storage for single pod mode

Signed-off-by: Emiliano Suñé <emiliano.sune@gmail.com>

* Bump project version

Signed-off-by: Emiliano Suñé <emiliano.sune@gmail.com>

* Update tests

Signed-off-by: Emiliano Suñé <emiliano.sune@gmail.com>

---------

Signed-off-by: Emiliano Suñé <emiliano.sune@gmail.com>

* bugfix: restore missing claims for non-consistent sub

Signed-off-by: Emiliano Suñé <emiliano.sune@gmail.com>

* Bump app version

Signed-off-by: Emiliano Suñé <emiliano.sune@gmail.com>

* Add test for non-consistent sub scenario

Signed-off-by: Emiliano Suñé <emiliano.sune@gmail.com>

* Fix linting errors

Signed-off-by: Emiliano Suñé <emiliano.sune@gmail.com>

---------

Signed-off-by: Emiliano Suñé <emiliano.sune@gmail.com>

Author: esune

oidc-controller/api/routers/oidc.py

@@ -376,15 +376,21 @@ async def post_token(request: Request, db: Database = Depends(get_db)):
                 ),
             )
 
+        # CRITICAL: For StatelessWrapper (in-memory storage), claims MUST be
+        # stored INSIDE the authorization code in authz_info["user_info"] before
+        # packing, otherwise PyOP will retrieve empty claims when generating the
+        # ID token. This must happen regardless of whether claims contain a "sub"
+        # field. For RedisWrapper, this field is not used (claims are in Redis).
+        authz_info = provider.provider.authz_state.authorization_codes[
+            form_dict["code"]
+        ]
+        authz_info["user_info"] = claims
+
         # Replace auto-generated sub with one coming from proof, if available
         # The Redis storage uses a shared data store, so a new item can be
         # added and the reference in the form needs to be updated with the
         # new key value
         if claims.get("sub"):
-            authz_info = provider.provider.authz_state.authorization_codes[
-                form_dict["code"]
-            ]
-
             # Update the subject with the one from the presentation
             presentation_sub = claims.pop("sub")
 
@@ -408,29 +414,14 @@ async def post_token(request: Request, db: Database = Depends(get_db)):
             # Update our local reference to the new user_id
             user_id = presentation_sub
 
-            # CRITICAL: For StatelessWrapper (in-memory storage), claims are
-            # stored INSIDE the authorization code in authz_info["user_info"].
-            # We must update this with the actual claims before repacking,
-            # otherwise PyOP will retrieve empty claims when generating the ID token.
-            # For RedisWrapper, this field is not used (claims are in Redis).
-            authz_info["user_info"] = claims
-
             # Create subject identifier mapping: subject -> user_id
             # This is needed because PyOP will look up the user_id for this
             # subject. Store the public subject identifier with Redis-aware
             # persistence
             store_subject_identifier(user_id, "public", presentation_sub)
 
-            new_code = provider.provider.authz_state.authorization_codes.pack(
-                authz_info
-            )
-            form_dict["code"] = new_code
-
-            # NOTE: Do NOT add sub back to claims dict. PyOP will get the
-            # sub from authz_info["sub"] when generating the ID token.
-            # If we include sub in claims as well, PyOP will receive it
-            # twice and raise: "TypeError: IdToken() got multiple values
-            # for keyword argument 'sub'"
+            # Need to update user_info again after removing "sub" from claims
+            authz_info["user_info"] = claims
 
             logger.info(
                 "Replaced authorization code with presentation subject",
@@ -442,6 +433,24 @@ async def post_token(request: Request, db: Database = Depends(get_db)):
                 updated_user_info_in_code=True,
             )
 
+        # Pack the authorization code with updated authz_info
+        new_code = provider.provider.authz_state.authorization_codes.pack(authz_info)
+        form_dict["code"] = new_code
+
+        # NOTE: Do NOT add sub back to claims dict. PyOP will get the
+        # sub from authz_info["sub"] when generating the ID token.
+        # If we include sub in claims as well, PyOP will receive it
+        # twice and raise: "TypeError: IdToken() got multiple values
+        # for keyword argument 'sub'"
+
+        logger.info(
+            "Updated authorization code with claims for StatelessWrapper",
+            operation="update_auth_code_claims",
+            auth_session_id=str(auth_session.id),
+            user_id=user_id,
+            claims_keys=list(claims.keys()),
+        )
+
         # Store claims in VCUserinfo so PyOP can retrieve them when
         # generating the ID token. This is critical - without this,
         # get_claims_for will return empty dict.

oidc-controller/api/routers/tests/test_oidc_token.py

@@ -676,3 +676,86 @@ async def test_user_info_contains_all_presentation_attributes(
                         assert user_info["acr"] == "vc_authn"
                         assert "given_names" in user_info["vc_presented_attributes"]
                         assert "family_name" in user_info["vc_presented_attributes"]
+
+    @pytest.mark.asyncio
+    async def test_claims_stored_without_sub_field(
+        self, mock_db, mock_auth_session, mock_provider
+    ):
+        """Test claims are stored in authz_info when no sub field exists.
+
+        This covers the scenario where:
+        - generate_consistent_identifier = False (no hash-based sub)
+        - subject_identifier is empty or doesn't match (no attribute-based sub)
+        Result: Token.get_claims() returns claims WITHOUT a "sub" field
+
+        Bug fix: authz_info["user_info"] must be set BEFORE checking if sub exists,
+        otherwise claims are lost in StatelessWrapper mode.
+        """
+        from api.authSessions.crud import AuthSessionCRUD
+        from api.routers.oidc import post_token
+        from api.verificationConfigs.crud import VerificationConfigCRUD
+
+        # Config that won't generate a sub
+        mock_config = MagicMock()
+        mock_config.subject_identifier = ""  # Empty - won't create sub from attribute
+        mock_config.generate_consistent_identifier = False  # Won't create hash sub
+        mock_config.include_v1_attributes = False
+
+        with patch.object(
+            AuthSessionCRUD,
+            "get_by_pyop_auth_code",
+            return_value=mock_auth_session,
+        ):
+            with patch.object(VerificationConfigCRUD, "get", return_value=mock_config):
+                with patch.object(
+                    AuthSessionCRUD,
+                    "update_pyop_user_id",
+                    new_callable=AsyncMock,
+                ) as mock_update:
+                    # Mock jwt.decode - return claims WITHOUT sub field
+                    with patch("jwt.decode") as mock_decode:
+                        mock_decode.return_value = {
+                            "pres_req_conf_id": "showcase-person",
+                            "vc_presented_attributes": {
+                                "given_names": "John",
+                                "family_name": "Doe",
+                            },
+                            "acr": "vc_authn",
+                            # NOTE: NO "sub" field - this is the critical test case
+                        }
+
+                        mock_request = MagicMock()
+                        mock_form = MagicMock()
+                        mock_form._dict = {
+                            "code": "test-auth-code",
+                            "grant_type": "authorization_code",
+                        }
+                        mock_request.form = MagicMock(
+                            return_value=MagicMock(
+                                __aenter__=AsyncMock(return_value=mock_form),
+                                __aexit__=AsyncMock(return_value=None),
+                            )
+                        )
+                        mock_request.headers = {}
+
+                        await post_token(mock_request, mock_db)
+
+                        # Verify authz_info["user_info"] was populated despite no sub
+                        pack_call = (
+                            mock_provider.provider.authz_state.authorization_codes.pack
+                        )
+                        assert pack_call.called
+
+                        authz_info = pack_call.call_args[0][0]
+                        assert "user_info" in authz_info
+
+                        user_info = authz_info["user_info"]
+                        assert user_info["pres_req_conf_id"] == "showcase-person"
+                        assert user_info["acr"] == "vc_authn"
+                        assert "given_names" in user_info["vc_presented_attributes"]
+                        assert "family_name" in user_info["vc_presented_attributes"]
+                        # Verify no sub was added
+                        assert "sub" not in user_info
+
+                        # Verify pyop_user_id was NOT updated (no sub to replace it with)
+                        mock_update.assert_not_called()

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "acapy-vc-authn-oidc"
-version = "2.3.4"
+version = "2.3.5"
 description = "Verifiable Credential Identity Provider for OpenID Connect."
 authors = [ { name = "OpenWallet Foundation" } ]
 license = "Apache-2.0"