From 0294a99cd4c195b8f321eb9ef2ad33ede4709d0d Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Wed, 17 Jun 2026 18:53:47 +0200 Subject: [PATCH] fix(auth): validate opaque access tokens via userinfo fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The management API (used by the Astrolabe PHP app for /api/v1/apps and /api/v1/webhooks) only accepted JWT access tokens. Opaque tokens were sent to Nextcloud's oidc introspection endpoint, which returns `active: false` for tokens minted for a *different* OIDC client (e.g. Astrolabe) even when they are live — so every call 401'd. This surfaced on the nx101294 tenant: webhook setup failed and the webhook-preset UI (including the Files preset) showed empty, because getWebhookPresets errors out before its `files`-always-available filter runs. Add a userinfo-endpoint fallback in UnifiedTokenVerifier: when introspection reports an opaque token inactive, validate it against the discovered userinfo_endpoint (a 200 with a `sub` proves a live bearer regardless of issuing client). userinfo returns no client_id/scope, so such tokens are stamped `_auth_via_userinfo` and the ALLOWED_MGMT_CLIENT allowlist is relaxed for that path only — authorization is still enforced per-user (token sub == requested resource owner) by every management endpoint. JWT and introspection paths are unchanged and still enforce the allowlist. Also bumps the astrolabe submodule to 0.29.0 (the deployed version that exhibits the issue). Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/auth/unified_verifier.py | 96 +++++++++++++++- tests/unit/test_unified_verifier.py | 103 ++++++++++++++++++ third_party/astrolabe | 2 +- 3 files changed, 198 insertions(+), 3 deletions(-) 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