Skip to content

Commit d3bee1c

Browse files
authored
Ensure user_id is not null for in-memory userinfo storage (#925)
* 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> * Fix linting errors Signed-off-by: Emiliano Suñé <emiliano.sune@gmail.com> --------- Signed-off-by: Emiliano Suñé <emiliano.sune@gmail.com>
1 parent d932aa3 commit d3bee1c

File tree

6 files changed

+268
-11
lines changed

6 files changed

+268
-11
lines changed

docker/manage

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,7 @@ single-pod)
518518

519519
configureEnvironment $@
520520
export CONTROLLER_REPLICAS=1
521+
export USE_REDIS_ADAPTER=false
521522
echo "Running single pod setup..."
522523
docker-compose up -d ${DEFAULT_CONTAINERS} ${ACAPY_CONTAINERS} ${PROD_CONTAINERS}
523524
docker-compose logs -f

oidc-controller/api/core/models.py

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -143,20 +143,33 @@ def get_claims_for(self, user_id, requested_claims, userinfo=None):
143143
Return stored claims for the given user_id.
144144
PyOP calls this method when generating ID tokens.
145145
146+
For StatelessWrapper (in-memory): PyOP passes user_id=None and
147+
userinfo contains the claims from the authorization code.
148+
149+
For RedisWrapper (multi-pod): PyOP passes user_id and userinfo=None,
150+
and we look up claims from Redis storage.
151+
146152
Args:
147-
user_id: The user identifier to look up
153+
user_id: The user identifier to look up (None for StatelessWrapper)
148154
requested_claims: Claims requested by the client (ignored)
149-
userinfo: Additional userinfo (ignored)
155+
userinfo: Claims dict from authz code (for StatelessWrapper only)
150156
151157
Returns:
152158
Dictionary of claims for this user, including custom claims
153159
from VC presentation
154160
"""
155161
try:
162+
# StatelessWrapper case: claims are passed in userinfo parameter
156163
if user_id is None:
157-
raise ValueError("user_id cannot be None when retrieving claims")
164+
claims = userinfo or {}
165+
logger.debug(
166+
"VCUserinfo.get_claims_for called with user_id=None (StatelessWrapper mode)",
167+
storage_type="stateless",
168+
claims_keys=list(claims.keys()),
169+
)
170+
return claims
158171

159-
# RedisWrapper doesn't support .get(), use [] with KeyError
172+
# RedisWrapper case: look up claims from storage
160173
try:
161174
claims = self._claims_storage[user_id]
162175
except KeyError:
@@ -171,8 +184,6 @@ def get_claims_for(self, user_id, requested_claims, userinfo=None):
171184
logger.debug(f" Returning claims keys: {list(claims.keys())}")
172185

173186
return claims
174-
except ValueError:
175-
raise
176187
except Exception as e:
177188
logger.error(f"VCUserinfo.get_claims_for ERROR: {e}")
178189
raise

oidc-controller/api/core/tests/test_models.py

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -152,11 +152,23 @@ def test_set_claims_for_user_with_none_user_id_raises_error(self, dict_storage):
152152
with pytest.raises(ValueError, match="user_id cannot be None"):
153153
userinfo.set_claims_for_user(None, {"claim": "value"})
154154

155-
def test_get_claims_for_with_none_user_id_raises_error(self, dict_storage):
156-
"""Test that get_claims_for raises ValueError for None user_id."""
155+
def test_get_claims_for_with_none_user_id_stateless_mode(self, dict_storage):
156+
"""Test that get_claims_for handles None user_id for StatelessWrapper."""
157157
userinfo = VCUserinfo({}, claims_storage=dict_storage)
158-
with pytest.raises(ValueError, match="user_id cannot be None"):
159-
userinfo.get_claims_for(None, {}, None)
158+
# StatelessWrapper mode: user_id=None, claims passed in userinfo param
159+
userinfo_claims = {
160+
"pres_req_conf_id": "test_config",
161+
"vc_presented_attributes": '{"email": "test@example.com"}',
162+
}
163+
result = userinfo.get_claims_for(None, {}, userinfo=userinfo_claims)
164+
assert result == userinfo_claims
165+
assert result["pres_req_conf_id"] == "test_config"
166+
167+
def test_get_claims_for_stateless_mode_with_empty_userinfo(self, dict_storage):
168+
"""Test StatelessWrapper mode returns empty dict when userinfo is None."""
169+
userinfo = VCUserinfo({}, claims_storage=dict_storage)
170+
result = userinfo.get_claims_for(None, {}, userinfo=None)
171+
assert result == {}
160172

161173
def test_getitem_with_none_user_id_raises_error(self, dict_storage):
162174
"""Test that __getitem__ raises ValueError for None user_id."""
@@ -282,3 +294,42 @@ def test_get_claims_for_storage_exception_handling(self):
282294

283295
with pytest.raises(RuntimeError, match="Storage read failed"):
284296
userinfo.get_claims_for("user_id", {}, None)
297+
298+
def test_stateless_mode_with_custom_vc_claims(self, dict_storage):
299+
"""Test StatelessWrapper mode properly returns all VC claims."""
300+
userinfo = VCUserinfo({}, claims_storage=dict_storage)
301+
vc_claims = {
302+
"pres_req_conf_id": "showcase-person",
303+
"vc_presented_attributes": '{"given_names": "John", "family_name": "Doe"}',
304+
"acr": "vc_authn",
305+
"nonce": "test_nonce_123",
306+
}
307+
308+
# Simulate StatelessWrapper calling get_claims_for
309+
result = userinfo.get_claims_for(None, {}, userinfo=vc_claims)
310+
311+
assert result == vc_claims
312+
assert result["pres_req_conf_id"] == "showcase-person"
313+
assert "vc_presented_attributes" in result
314+
assert result["acr"] == "vc_authn"
315+
assert result["nonce"] == "test_nonce_123"
316+
317+
def test_redis_mode_vs_stateless_mode(self, dict_storage):
318+
"""Test that Redis mode and StatelessWrapper mode work correctly."""
319+
userinfo = VCUserinfo({}, claims_storage=dict_storage)
320+
321+
# Redis mode: store claims with user_id
322+
redis_claims = {"redis_claim": "redis_value"}
323+
userinfo.set_claims_for_user("redis_user_id", redis_claims)
324+
325+
# Redis mode: retrieve with user_id, userinfo=None
326+
result_redis = userinfo.get_claims_for("redis_user_id", {}, None)
327+
assert result_redis == redis_claims
328+
329+
# StatelessWrapper mode: retrieve with user_id=None, claims in userinfo
330+
stateless_claims = {"stateless_claim": "stateless_value"}
331+
result_stateless = userinfo.get_claims_for(None, {}, userinfo=stateless_claims)
332+
assert result_stateless == stateless_claims
333+
334+
# Verify they're independent
335+
assert result_redis != result_stateless

oidc-controller/api/routers/oidc.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,13 @@ async def post_token(request: Request, db: Database = Depends(get_db)):
408408
# Update our local reference to the new user_id
409409
user_id = presentation_sub
410410

411+
# CRITICAL: For StatelessWrapper (in-memory storage), claims are
412+
# stored INSIDE the authorization code in authz_info["user_info"].
413+
# We must update this with the actual claims before repacking,
414+
# otherwise PyOP will retrieve empty claims when generating the ID token.
415+
# For RedisWrapper, this field is not used (claims are in Redis).
416+
authz_info["user_info"] = claims
417+
411418
# Create subject identifier mapping: subject -> user_id
412419
# This is needed because PyOP will look up the user_id for this
413420
# subject. Store the public subject identifier with Redis-aware
@@ -432,6 +439,7 @@ async def post_token(request: Request, db: Database = Depends(get_db)):
432439
original_user_id=auth_session.pyop_user_id,
433440
new_user_id=presentation_sub,
434441
updated_auth_session=True,
442+
updated_user_info_in_code=True,
435443
)
436444

437445
# Store claims in VCUserinfo so PyOP can retrieve them when

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

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,14 @@ async def test_claims_stored_with_presentation_sub_as_user_id(
225225
# Critical: no duplicate sub
226226
assert "sub" not in stored_claims
227227

228+
# Verify authz_info["user_info"] was updated for StatelessWrapper
229+
authz_codes = (
230+
mock_provider.provider.authz_state.authorization_codes
231+
)
232+
pack_call_args = authz_codes.pack.call_args[0][0]
233+
assert "user_info" in pack_call_args
234+
assert pack_call_args["user_info"] == stored_claims
235+
228236
@pytest.mark.asyncio
229237
async def test_sub_not_included_in_userinfo_claims(
230238
self, mock_db, mock_auth_session, mock_ver_config, mock_provider
@@ -490,3 +498,181 @@ async def test_claims_storage_exception_raises_http_exception(
490498
assert exc_info.value.status_code == 500
491499
assert "Failed to store claims" in exc_info.value.detail
492500
assert "Redis connection failed" in exc_info.value.detail
501+
502+
503+
class TestPostTokenStatelessWrapper:
504+
"""Test StatelessWrapper-specific behavior in post_token endpoint."""
505+
506+
@pytest.mark.asyncio
507+
async def test_authz_info_user_info_updated_before_pack(
508+
self, mock_db, mock_auth_session, mock_ver_config, mock_provider
509+
):
510+
"""Test that authz_info['user_info'] is updated with claims before packing."""
511+
from api.authSessions.crud import AuthSessionCRUD
512+
from api.routers.oidc import post_token
513+
from api.verificationConfigs.crud import VerificationConfigCRUD
514+
515+
with patch.object(
516+
AuthSessionCRUD,
517+
"get_by_pyop_auth_code",
518+
return_value=mock_auth_session,
519+
):
520+
with patch.object(
521+
VerificationConfigCRUD, "get", return_value=mock_ver_config
522+
):
523+
with patch.object(
524+
AuthSessionCRUD,
525+
"update_pyop_user_id",
526+
new_callable=AsyncMock,
527+
):
528+
with patch("jwt.decode") as mock_decode:
529+
mock_decode.return_value = {"sub": "John@showcase-person"}
530+
531+
mock_request = MagicMock()
532+
mock_form = MagicMock()
533+
mock_form._dict = {
534+
"code": "test-auth-code",
535+
"grant_type": "authorization_code",
536+
}
537+
mock_request.form = MagicMock(
538+
return_value=MagicMock(
539+
__aenter__=AsyncMock(return_value=mock_form),
540+
__aexit__=AsyncMock(return_value=None),
541+
)
542+
)
543+
mock_request.headers = {}
544+
545+
await post_token(mock_request, mock_db)
546+
547+
# Verify authz_info was packed with user_info field
548+
authz_codes = (
549+
mock_provider.provider.authz_state.authorization_codes
550+
)
551+
authz_codes.pack.assert_called_once()
552+
553+
packed_authz_info = authz_codes.pack.call_args[0][0]
554+
555+
# Critical: user_info field must be present and contain claims
556+
assert "user_info" in packed_authz_info
557+
user_info = packed_authz_info["user_info"]
558+
559+
# Verify user_info contains the presentation claims
560+
assert "pres_req_conf_id" in user_info
561+
assert user_info["pres_req_conf_id"] == "showcase-person"
562+
assert "vc_presented_attributes" in user_info
563+
assert "acr" in user_info
564+
565+
# Verify sub is NOT in user_info (it goes in authz_info["sub"])
566+
assert "sub" not in user_info
567+
568+
@pytest.mark.asyncio
569+
async def test_authz_info_sub_updated_with_presentation_sub(
570+
self, mock_db, mock_auth_session, mock_ver_config, mock_provider
571+
):
572+
"""Test that authz_info['sub'] is updated with presentation subject."""
573+
from api.authSessions.crud import AuthSessionCRUD
574+
from api.routers.oidc import post_token
575+
from api.verificationConfigs.crud import VerificationConfigCRUD
576+
577+
with patch.object(
578+
AuthSessionCRUD,
579+
"get_by_pyop_auth_code",
580+
return_value=mock_auth_session,
581+
):
582+
with patch.object(
583+
VerificationConfigCRUD, "get", return_value=mock_ver_config
584+
):
585+
with patch.object(
586+
AuthSessionCRUD,
587+
"update_pyop_user_id",
588+
new_callable=AsyncMock,
589+
):
590+
with patch("jwt.decode") as mock_decode:
591+
mock_decode.return_value = {"sub": "John@showcase-person"}
592+
593+
mock_request = MagicMock()
594+
mock_form = MagicMock()
595+
mock_form._dict = {
596+
"code": "test-auth-code",
597+
"grant_type": "authorization_code",
598+
}
599+
mock_request.form = MagicMock(
600+
return_value=MagicMock(
601+
__aenter__=AsyncMock(return_value=mock_form),
602+
__aexit__=AsyncMock(return_value=None),
603+
)
604+
)
605+
mock_request.headers = {}
606+
607+
await post_token(mock_request, mock_db)
608+
609+
# Verify authz_info["sub"] was updated before packing
610+
authz_codes = (
611+
mock_provider.provider.authz_state.authorization_codes
612+
)
613+
packed_authz_info = authz_codes.pack.call_args[0][0]
614+
615+
assert packed_authz_info["sub"] == "John@showcase-person"
616+
617+
@pytest.mark.asyncio
618+
async def test_user_info_contains_all_presentation_attributes(
619+
self, mock_db, mock_auth_session, mock_ver_config, mock_provider
620+
):
621+
"""Test that user_info in authz_info contains all presentation attributes."""
622+
from api.authSessions.crud import AuthSessionCRUD
623+
from api.routers.oidc import post_token
624+
from api.verificationConfigs.crud import VerificationConfigCRUD
625+
626+
with patch.object(
627+
AuthSessionCRUD,
628+
"get_by_pyop_auth_code",
629+
return_value=mock_auth_session,
630+
):
631+
with patch.object(
632+
VerificationConfigCRUD, "get", return_value=mock_ver_config
633+
):
634+
with patch.object(
635+
AuthSessionCRUD,
636+
"update_pyop_user_id",
637+
new_callable=AsyncMock,
638+
):
639+
with patch("jwt.decode") as mock_decode:
640+
mock_decode.return_value = {"sub": "John@showcase-person"}
641+
642+
mock_request = MagicMock()
643+
mock_form = MagicMock()
644+
mock_form._dict = {
645+
"code": "test-auth-code",
646+
"grant_type": "authorization_code",
647+
}
648+
mock_request.form = MagicMock(
649+
return_value=MagicMock(
650+
__aenter__=AsyncMock(return_value=mock_form),
651+
__aexit__=AsyncMock(return_value=None),
652+
)
653+
)
654+
mock_request.headers = {}
655+
656+
await post_token(mock_request, mock_db)
657+
658+
# Get the user_info that was packed into authz_info
659+
authz_codes = (
660+
mock_provider.provider.authz_state.authorization_codes
661+
)
662+
packed_authz_info = authz_codes.pack.call_args[0][0]
663+
user_info = packed_authz_info["user_info"]
664+
665+
# Verify all expected attributes are present
666+
expected_keys = [
667+
"pres_req_conf_id",
668+
"vc_presented_attributes",
669+
"acr",
670+
]
671+
for key in expected_keys:
672+
assert key in user_info, f"Missing expected key: {key}"
673+
674+
# Verify values
675+
assert user_info["pres_req_conf_id"] == "showcase-person"
676+
assert user_info["acr"] == "vc_authn"
677+
assert "given_names" in user_info["vc_presented_attributes"]
678+
assert "family_name" in user_info["vc_presented_attributes"]

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[project]
22
name = "acapy-vc-authn-oidc"
3-
version = "2.3.3"
3+
version = "2.3.4"
44
description = "Verifiable Credential Identity Provider for OpenID Connect."
55
authors = [ { name = "OpenWallet Foundation" } ]
66
license = "Apache-2.0"

0 commit comments

Comments
 (0)