fix(auth): validate opaque access tokens via userinfo fallback
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
d969526613
commit
0294a99cd4
@@ -75,6 +75,16 @@ class UnifiedTokenVerifier(TokenVerifier):
|
|||||||
self.introspection_uri = settings.introspection_uri
|
self.introspection_uri = settings.introspection_uri
|
||||||
logger.info("Token introspection enabled: %s", self.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)
|
# Build list of valid issuers (internal + public may differ in Docker)
|
||||||
# AS proxy obtains tokens via internal URL (e.g. http://app:80), while
|
# 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)
|
# 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:
|
if access_token is None:
|
||||||
return 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)
|
# Enforce ALLOWED_MGMT_CLIENT allowlist (fail-closed when unset)
|
||||||
token_client_id = access_token.client_id
|
token_client_id = access_token.client_id
|
||||||
if not token_client_id or token_client_id not in self._allowed_mgmt_clients:
|
if not token_client_id or token_client_id not in self._allowed_mgmt_clients:
|
||||||
@@ -344,6 +369,15 @@ class UnifiedTokenVerifier(TokenVerifier):
|
|||||||
payload = await self._introspect_token(token)
|
payload = await self._introspect_token(token)
|
||||||
if payload:
|
if payload:
|
||||||
record_oauth_token_validation("introspect", "valid")
|
record_oauth_token_validation("introspect", "valid")
|
||||||
|
else:
|
||||||
|
# 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:
|
else:
|
||||||
record_oauth_token_validation("introspect", "invalid")
|
record_oauth_token_validation("introspect", "invalid")
|
||||||
return None
|
return None
|
||||||
@@ -545,6 +579,64 @@ class UnifiedTokenVerifier(TokenVerifier):
|
|||||||
logger.error("Unexpected error during token introspection: %s", e)
|
logger.error("Unexpected error during token introspection: %s", e)
|
||||||
return None
|
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(
|
def _create_access_token(
|
||||||
self, token: str, payload: dict[str, Any]
|
self, token: str, payload: dict[str, Any]
|
||||||
) -> AccessToken | None:
|
) -> AccessToken | None:
|
||||||
|
|||||||
@@ -616,3 +616,106 @@ class TestManagementApiAllowlist:
|
|||||||
|
|
||||||
result = await verifier.verify_token_for_management_api(token)
|
result = await verifier.verify_token_for_management_api(token)
|
||||||
assert result is None
|
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()
|
||||||
|
|||||||
Vendored
+1
-1
Submodule third_party/astrolabe updated: 5fc20a78d0...a8bb3c4e03
Reference in New Issue
Block a user