Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 30 additions & 21 deletions oidc-controller/api/routers/oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand All @@ -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",
Expand All @@ -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.
Expand Down
83 changes: 83 additions & 0 deletions oidc-controller/api/routers/tests/test_oidc_token.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -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"
Expand Down