diff --git a/nextcloud_mcp_server/auth/unified_verifier.py b/nextcloud_mcp_server/auth/unified_verifier.py index 8dc83b2a..1afd5106 100644 --- a/nextcloud_mcp_server/auth/unified_verifier.py +++ b/nextcloud_mcp_server/auth/unified_verifier.py @@ -75,6 +75,16 @@ class UnifiedTokenVerifier(TokenVerifier): self.introspection_uri = settings.introspection_uri logger.info("Token introspection enabled: %s", self.introspection_uri) + # Userinfo fallback (for opaque tokens minted for a *different* OIDC + # client, e.g. Astrolabe, which the Nextcloud oidc app's introspection + # endpoint reports inactive cross-client). A 200 from userinfo proves + # the bearer is a live token for its user regardless of issuing client. + self.userinfo_uri: str | None = getattr(settings, "userinfo_uri", None) + if self.userinfo_uri: + logger.info( + "Userinfo token validation fallback enabled: %s", self.userinfo_uri + ) + # Build list of valid issuers (internal + public may differ in Docker) # AS proxy obtains tokens via internal URL (e.g. http://app:80), while # NEXTCLOUD_PUBLIC_ISSUER_URL is the browser-facing URL (e.g. http://localhost:8080) @@ -210,6 +220,21 @@ class UnifiedTokenVerifier(TokenVerifier): if access_token is None: return None + # Opaque tokens validated via the userinfo fallback carry no verifiable + # client_id, so the ALLOWED_MGMT_CLIENT allowlist cannot apply. Such + # 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). + cached_entry = self._token_cache.get(cache_key) + via_userinfo = bool(cached_entry and cached_entry[0].get("_auth_via_userinfo")) + if via_userinfo: + logger.warning( + "Opaque token validated via userinfo endpoint; ALLOWED_MGMT_CLIENT " + "allowlist not enforced for user %s (per-user authorization applies)", + access_token.resource, + ) + return access_token + # Enforce ALLOWED_MGMT_CLIENT allowlist (fail-closed when unset) token_client_id = access_token.client_id if not token_client_id or token_client_id not in self._allowed_mgmt_clients: @@ -345,8 +370,17 @@ class UnifiedTokenVerifier(TokenVerifier): if payload: record_oauth_token_validation("introspect", "valid") else: - record_oauth_token_validation("introspect", "invalid") - return None + # Introspection can report opaque tokens minted for a + # *different* OIDC client (e.g. Astrolabe) as inactive even + # when they are live. Fall back to the userinfo endpoint, + # which validates any live bearer regardless of client. + payload = await self._validate_via_userinfo(token) + if payload: + validation_method = "userinfo" + record_oauth_token_validation("userinfo", "valid") + else: + record_oauth_token_validation("introspect", "invalid") + return None # Check payload is valid if not payload: @@ -545,6 +579,64 @@ class UnifiedTokenVerifier(TokenVerifier): logger.error("Unexpected error during token introspection: %s", e) return None + async def _validate_via_userinfo(self, token: str) -> dict[str, Any] | None: + """Validate an opaque token by calling the OIDC userinfo endpoint. + + Fallback for opaque access tokens that the Nextcloud ``oidc`` app's + introspection endpoint reports ``active=false`` cross-client (i.e. + tokens minted for a *different* client such as Astrolabe). A 200 from + userinfo with a ``sub`` claim proves the bearer is a valid, unexpired + token for that user. + + Unlike introspection, userinfo returns neither ``client_id`` nor + ``scope``, so the returned payload is stamped with ``_auth_via_userinfo`` + and the management-API allowlist is relaxed for this path (authorization + is still enforced per-user by every management endpoint). + + Args: + token: Bearer token to validate. + + Returns: + Userinfo claims (with ``_auth_via_userinfo`` set) if valid, else None. + """ + if not self.userinfo_uri: + logger.debug("No userinfo endpoint configured") + return None + + try: + response = await self.http_client.get( + self.userinfo_uri, + headers={"Authorization": f"Bearer {token}"}, + ) + except httpx.TimeoutException: + logger.error("Timeout while validating token via userinfo") + return None + except httpx.RequestError as e: + logger.error("Network error while validating token via userinfo: %s", e) + return None + + if response.status_code != 200: + logger.warning( + "Userinfo token validation failed: HTTP %s", response.status_code + ) + return None + + try: + data = response.json() + except Exception as e: + logger.error("Failed to parse userinfo response: %s", e) + return None + + if not data.get("sub"): + logger.warning("Userinfo response missing 'sub' claim") + return None + + logger.debug("Token validated via userinfo for user: %s", data.get("sub")) + # Stamp so verify_token_for_management_api can relax the client allowlist + # for tokens that legitimately lack a verifiable client_id. + data["_auth_via_userinfo"] = True + return data + def _create_access_token( self, token: str, payload: dict[str, Any] ) -> AccessToken | None: diff --git a/tests/unit/test_unified_verifier.py b/tests/unit/test_unified_verifier.py index 70ca88c5..6f35e2c2 100644 --- a/tests/unit/test_unified_verifier.py +++ b/tests/unit/test_unified_verifier.py @@ -616,3 +616,106 @@ class TestManagementApiAllowlist: result = await verifier.verify_token_for_management_api(token) assert result is None + + +class TestUserinfoFallback: + """Opaque tokens that introspection reports inactive fall back to userinfo. + + Covers the nx101294 case: the Astrolabe OIDC client issues opaque access + tokens that Nextcloud's oidc app introspection reports active=false + cross-client. The userinfo endpoint validates them regardless of client. + """ + + @pytest.fixture + def userinfo_settings(self, base_settings): + base_settings.userinfo_uri = "https://idp.example.com/userinfo" + return base_settings + + async def test_validate_via_userinfo_success(self, userinfo_settings): + verifier = UnifiedTokenVerifier(userinfo_settings) + mock_resp = MagicMock() + mock_resp.status_code = 200 + mock_resp.json.return_value = {"sub": "testuser"} + with patch.object( + verifier.http_client, "get", AsyncMock(return_value=mock_resp) + ): + result = await verifier._validate_via_userinfo("opaque-token") + assert result is not None + assert result["sub"] == "testuser" + assert result["_auth_via_userinfo"] is True + + async def test_validate_via_userinfo_non_200(self, userinfo_settings): + verifier = UnifiedTokenVerifier(userinfo_settings) + mock_resp = MagicMock() + mock_resp.status_code = 401 + with patch.object( + verifier.http_client, "get", AsyncMock(return_value=mock_resp) + ): + result = await verifier._validate_via_userinfo("opaque-token") + assert result is None + + async def test_validate_via_userinfo_missing_sub(self, userinfo_settings): + verifier = UnifiedTokenVerifier(userinfo_settings) + mock_resp = MagicMock() + mock_resp.status_code = 200 + mock_resp.json.return_value = {"name": "no sub claim"} + with patch.object( + verifier.http_client, "get", AsyncMock(return_value=mock_resp) + ): + result = await verifier._validate_via_userinfo("opaque-token") + assert result is None + + async def test_validate_via_userinfo_not_configured(self, base_settings): + base_settings.userinfo_uri = None + verifier = UnifiedTokenVerifier(base_settings) + result = await verifier._validate_via_userinfo("opaque-token") + assert result is None + + async def test_mgmt_opaque_userinfo_fallback_accepted_despite_allowlist( + self, monkeypatch, userinfo_settings + ): + """Introspection inactive -> userinfo validates -> accepted even though + no client_id matches the allowlist (per-user authorization applies).""" + monkeypatch.setenv("ALLOWED_MGMT_CLIENT", "astrolabe") + verifier = UnifiedTokenVerifier(userinfo_settings) + + with ( + patch.object(verifier, "_introspect_token", AsyncMock(return_value=None)), + patch.object( + verifier, + "_validate_via_userinfo", + AsyncMock(return_value={"sub": "testuser", "_auth_via_userinfo": True}), + ), + ): + result = await verifier.verify_token_for_management_api("opaque-token-123") + + assert result is not None + assert result.resource == "testuser" + assert result.client_id == "" # userinfo provides no client_id + + async def test_mgmt_userinfo_not_called_when_introspection_succeeds( + self, monkeypatch, userinfo_settings + ): + monkeypatch.setenv("ALLOWED_MGMT_CLIENT", "astrolabe") + verifier = UnifiedTokenVerifier(userinfo_settings) + + introspection_payload = { + "sub": "testuser", + "client_id": "astrolabe", + "scope": "openid", + "exp": int(time.time() + 3600), + } + userinfo_mock = AsyncMock(return_value={"sub": "x", "_auth_via_userinfo": True}) + with ( + patch.object( + verifier, + "_introspect_token", + AsyncMock(return_value=introspection_payload), + ), + patch.object(verifier, "_validate_via_userinfo", userinfo_mock), + ): + result = await verifier.verify_token_for_management_api("opaque-token-123") + + assert result is not None + assert result.client_id == "astrolabe" + userinfo_mock.assert_not_called() diff --git a/third_party/astrolabe b/third_party/astrolabe index 5fc20a78..a8bb3c4e 160000 --- a/third_party/astrolabe +++ b/third_party/astrolabe @@ -1 +1 @@ -Subproject commit 5fc20a78d049670b7e298cf5487b27c7d63b028a +Subproject commit a8bb3c4e03f230fabe35f513f2bc6ca2231cddbf