docs(auth): document userinfo path in security model; drop dead guard; pin MCP asymmetry
Address claude-review round 8 on #919: - Security-model docstring: note that opaque cross-client tokens authenticate via the userinfo liveness check (not JWKS/expiry) and bypass the client allowlist, with per-user authz as the gate. - Remove the redundant `if not payload: return None` after the JWT/opaque branches (both already return None on failure) — replace with a comment. - Add test_mcp_path_does_not_use_userinfo_for_opaque_token to pin that the userinfo fallback is management-path-only (MCP path still 401s). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
a53e6e7721
commit
7ef0e9d83b
@@ -175,6 +175,12 @@ class UnifiedTokenVerifier(TokenVerifier):
|
||||
- Verifies token signature against Nextcloud's JWKS (cryptographic proof)
|
||||
- Verifies token is not expired
|
||||
- Extracts user identity from validated token claims
|
||||
- NOTE: for opaque cross-client tokens (e.g. Astrolabe) that
|
||||
introspection reports inactive, authentication falls back to the
|
||||
userinfo endpoint — a live IdP liveness check (200 + ``sub``)
|
||||
rather than local JWKS/expiry verification. Such tokens are stamped
|
||||
``_auth_via_userinfo`` and bypass the client allowlist (step 4);
|
||||
per-user authorization (step 2) remains the security gate.
|
||||
|
||||
2. **Authorization layer** (management API endpoints):
|
||||
- EVERY endpoint verifies: token.sub == requested_resource_owner
|
||||
@@ -420,9 +426,8 @@ class UnifiedTokenVerifier(TokenVerifier):
|
||||
# a userinfo failure metric when userinfo was never attempted.
|
||||
return None
|
||||
|
||||
# Check payload is valid
|
||||
if not payload:
|
||||
return None
|
||||
# Both branches above either set a populated payload or have already
|
||||
# returned None, so payload is guaranteed truthy here.
|
||||
|
||||
# Skip audience validation - any valid Nextcloud token is accepted
|
||||
logger.debug(
|
||||
|
||||
@@ -780,6 +780,21 @@ class TestUserinfoFallback:
|
||||
assert result is not None
|
||||
assert result.resource == "testuser"
|
||||
|
||||
async def test_mcp_path_does_not_use_userinfo_for_opaque_token(
|
||||
self, userinfo_settings
|
||||
):
|
||||
"""The userinfo fallback applies only to the management API path, never
|
||||
the MCP-audience path — an opaque token there is still rejected."""
|
||||
verifier = UnifiedTokenVerifier(userinfo_settings)
|
||||
userinfo_mock = AsyncMock(return_value={"sub": "testuser"})
|
||||
with (
|
||||
patch.object(verifier, "_introspect_token", AsyncMock(return_value=None)),
|
||||
patch.object(verifier, "_validate_via_userinfo", userinfo_mock),
|
||||
):
|
||||
result = await verifier.verify_token("opaque-astrolabe-token")
|
||||
assert result is None
|
||||
userinfo_mock.assert_not_called()
|
||||
|
||||
async def test_opaque_rejected_when_no_validators_configured(self, base_settings):
|
||||
"""With neither introspection nor userinfo configured, an opaque token is
|
||||
rejected without recording a misleading userinfo-failure metric."""
|
||||
|
||||
Reference in New Issue
Block a user