From bc6595b139dcf4b1c02552b3b3c46d201f5c502a Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Wed, 17 Jun 2026 20:29:20 +0200 Subject: [PATCH] test(auth): make sync userinfo tests def; note defensive userinfo guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address claude-review round 6 (LGTM) nits on #919: - test_userinfo_token_cached_with_short_ttl and test_userinfo_token_with_exp_uses_real_expiry call only the sync _create_access_token_with_cache_key — declare them as plain def (no await). - Comment the userinfo_uri guard in _validate_via_userinfo as defensive / direct-call support (the management caller already gates on userinfo_uri). Left as-is: the hasattr(settings, "userinfo_uri") guard — kept to mirror the adjacent introspection_uri block (consistency requested in round 2). The _verify_mcp_audience metric-when-unconfigured note is a pre-existing, out-of- scope item for a follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/auth/unified_verifier.py | 3 +++ tests/unit/test_unified_verifier.py | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/nextcloud_mcp_server/auth/unified_verifier.py b/nextcloud_mcp_server/auth/unified_verifier.py index 520ad375..b9a6b873 100644 --- a/nextcloud_mcp_server/auth/unified_verifier.py +++ b/nextcloud_mcp_server/auth/unified_verifier.py @@ -651,6 +651,9 @@ class UnifiedTokenVerifier(TokenVerifier): Returns: Userinfo claims if valid, else None. """ + # Defensive: the management-API caller already gates on + # self.userinfo_uri before invoking this, but the guard keeps the method + # safe to call directly (e.g. in unit tests). if not self.userinfo_uri: logger.debug("No userinfo endpoint configured") return None diff --git a/tests/unit/test_unified_verifier.py b/tests/unit/test_unified_verifier.py index 04ac13cd..e34c3b7b 100644 --- a/tests/unit/test_unified_verifier.py +++ b/tests/unit/test_unified_verifier.py @@ -833,7 +833,7 @@ class TestUserinfoFallback: # Second call served from cache — userinfo probed only once. userinfo_mock.assert_awaited_once() - async def test_userinfo_token_cached_with_short_ttl(self, userinfo_settings): + def test_userinfo_token_cached_with_short_ttl(self, userinfo_settings): """userinfo tokens (no exp) get the short userinfo TTL, not the 1h default. The short TTL is keyed off the explicit via_userinfo argument, not a @@ -850,7 +850,7 @@ class TestUserinfoFallback: assert access_token.expires_at <= int(before + 300) + 2 assert access_token.expires_at < int(before + verifier.cache_ttl) - async def test_userinfo_token_with_exp_uses_real_expiry(self, userinfo_settings): + def test_userinfo_token_with_exp_uses_real_expiry(self, userinfo_settings): """When userinfo (unusually) returns an exp, the real token expiry wins over the short userinfo TTL.""" verifier = UnifiedTokenVerifier(userinfo_settings)