From e1e9c9b918fd766b800038161f332a73f64ff23f Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Wed, 17 Jun 2026 20:23:16 +0200 Subject: [PATCH] fix(auth): quiet per-validation userinfo TTL log; test introspection-timeout fall-through MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address claude-review round 5 on #919: - The "userinfo has no exp; caching for Ns only" log fired on every fresh userinfo validation (userinfo never returns exp) — downgrade WARNING → DEBUG; the bounded-staleness window is already documented on _validate_via_userinfo. - Add test_introspection_timeout_falls_through_to_userinfo: drives a real introspection timeout (httpx.TimeoutException on the POST, caught inside _introspect_token → None) through to a successful userinfo validation, pinning the documented error fall-through end to end. Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/auth/unified_verifier.py | 5 +++- tests/unit/test_unified_verifier.py | 27 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/nextcloud_mcp_server/auth/unified_verifier.py b/nextcloud_mcp_server/auth/unified_verifier.py index 915e4ff3..520ad375 100644 --- a/nextcloud_mcp_server/auth/unified_verifier.py +++ b/nextcloud_mcp_server/auth/unified_verifier.py @@ -758,7 +758,10 @@ class UnifiedTokenVerifier(TokenVerifier): # opaque token can't be honored for the full hour-long default TTL. if via_userinfo: ttl = self.userinfo_cache_ttl - logger.warning( + # userinfo never returns exp, so this fires on every fresh + # userinfo validation — keep it at DEBUG (the bounded-staleness + # window is documented on _validate_via_userinfo). + logger.debug( "Token validated via userinfo has no 'exp'; caching for %ss only", ttl, ) diff --git a/tests/unit/test_unified_verifier.py b/tests/unit/test_unified_verifier.py index c124d1bc..04ac13cd 100644 --- a/tests/unit/test_unified_verifier.py +++ b/tests/unit/test_unified_verifier.py @@ -741,6 +741,33 @@ class TestUserinfoFallback: assert result.resource == "testuser" introspect_mock.assert_not_called() # skipped when unconfigured + async def test_introspection_timeout_falls_through_to_userinfo( + self, monkeypatch, userinfo_settings + ): + """A real introspection timeout (caught inside _introspect_token, which + returns None) falls through to userinfo — the authoritative live check — + exercising the whole chain, not just a mocked _introspect_token.""" + monkeypatch.setenv("ALLOWED_MGMT_CLIENT", "astrolabe") + verifier = UnifiedTokenVerifier(userinfo_settings) + + userinfo_resp = MagicMock() + userinfo_resp.status_code = 200 + userinfo_resp.json.return_value = {"sub": "testuser"} + with ( + patch.object( + verifier.http_client, + "post", + AsyncMock(side_effect=httpx.TimeoutException("introspect down")), + ), + patch.object( + verifier.http_client, "get", AsyncMock(return_value=userinfo_resp) + ), + ): + result = await verifier.verify_token_for_management_api("opaque-timeout") + + assert result is not None + assert result.resource == "testuser" + 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."""