diff --git a/nextcloud_mcp_server/auth/unified_verifier.py b/nextcloud_mcp_server/auth/unified_verifier.py index 3aafe2d0..915e4ff3 100644 --- a/nextcloud_mcp_server/auth/unified_verifier.py +++ b/nextcloud_mcp_server/auth/unified_verifier.py @@ -231,6 +231,10 @@ class UnifiedTokenVerifier(TokenVerifier): # tokens are stamped with ``_auth_via_userinfo`` in the cache; for them # we rely on the per-user authorization every management endpoint # enforces (token sub == requested resource owner). + # Recover the via-userinfo flag from the cache entry. On a cache miss + # this is the entry _verify_without_audience_check just wrote (no await + # between that write and this read, so it is always present); on a cache + # hit it was written by an earlier call. cached_entry = self._token_cache.get(cache_key) via_userinfo = bool(cached_entry and cached_entry[0].get("_auth_via_userinfo")) if via_userinfo: @@ -393,10 +397,16 @@ class UnifiedTokenVerifier(TokenVerifier): else: record_oauth_token_validation("introspect", "invalid") - if payload is None: - # Introspection is unconfigured, or it reported the token - # inactive. Set validation_method first so a userinfo - # exception caught by the outer handler is attributed right. + # Fall through to userinfo when introspection is unconfigured or + # returned None. NOTE: _introspect_token returns None for BOTH an + # active=false response (the nx101294 cross-client case we must + # handle) AND a network/timeout error — both reach userinfo here. + # That is safe: userinfo is itself an authoritative live check (a + # revoked/invalid token gets a 401), so a flapping introspection + # endpoint cannot cause an invalid token to be accepted. + if payload is None and self.userinfo_uri: + # Set validation_method first so a userinfo exception caught + # by the outer handler is attributed correctly. validation_method = "userinfo" payload = await self._validate_via_userinfo(token) if payload: @@ -405,6 +415,11 @@ class UnifiedTokenVerifier(TokenVerifier): record_oauth_token_validation("userinfo", "invalid") return None + if payload is None: + # No validator was configured, or none succeeded. Don't record + # a userinfo failure metric when userinfo was never attempted. + return None + # Check payload is valid if not payload: return None diff --git a/tests/unit/test_unified_verifier.py b/tests/unit/test_unified_verifier.py index 7d00292a..c124d1bc 100644 --- a/tests/unit/test_unified_verifier.py +++ b/tests/unit/test_unified_verifier.py @@ -741,6 +741,20 @@ class TestUserinfoFallback: assert result.resource == "testuser" introspect_mock.assert_not_called() # skipped when unconfigured + 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.""" + base_settings.introspection_uri = None + base_settings.userinfo_uri = None + verifier = UnifiedTokenVerifier(base_settings) + assert verifier.introspection_uri is None + assert verifier.userinfo_uri is None + + result = await verifier._verify_without_audience_check( + "opaque-no-validator", "mgmt:none" + ) + assert result is None + async def test_mgmt_userinfo_not_called_when_introspection_succeeds( self, monkeypatch, userinfo_settings ):