From a53e6e77219701f2a753be0c1657f6859dbab052 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Wed, 17 Jun 2026 20:34:31 +0200 Subject: [PATCH] test(auth): cover userinfo SSRF scheme guard; note empty-scope caveat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address claude-review round 7 nits on #919: - Add test_validate_via_userinfo_rejects_non_http_scheme — a non-http(s) userinfo_uri is refused before any request (covers the SSRF scheme guard). - Docstring caution on _validate_via_userinfo: userinfo-validated tokens carry empty scopes, so management endpoints must not gate on scopes for this path. Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/auth/unified_verifier.py | 5 +++++ tests/unit/test_unified_verifier.py | 12 ++++++++++++ 2 files changed, 17 insertions(+) diff --git a/nextcloud_mcp_server/auth/unified_verifier.py b/nextcloud_mcp_server/auth/unified_verifier.py index b9a6b873..50341040 100644 --- a/nextcloud_mcp_server/auth/unified_verifier.py +++ b/nextcloud_mcp_server/auth/unified_verifier.py @@ -640,6 +640,11 @@ class UnifiedTokenVerifier(TokenVerifier): management-API allowlist is relaxed for it (authorization is still enforced per-user by every management endpoint). + Caution: userinfo-validated tokens carry **empty scopes**. Callers must + not gate management endpoints on scopes for this path (e.g. a future + ``@require_scopes``) or they would silently reject valid cross-client + tokens; the per-user ``sub`` check is the authorization gate. + Security note — bounded staleness: userinfo carries no token ``exp``, so a validated token is cached for ``userinfo_cache_ttl`` (5 min) rather than the 1-hour default. A revoked/expired opaque token may therefore be diff --git a/tests/unit/test_unified_verifier.py b/tests/unit/test_unified_verifier.py index e34c3b7b..95d4dd52 100644 --- a/tests/unit/test_unified_verifier.py +++ b/tests/unit/test_unified_verifier.py @@ -665,6 +665,18 @@ class TestUserinfoFallback: result = await verifier._validate_via_userinfo("opaque-token") assert result is None + async def test_validate_via_userinfo_rejects_non_http_scheme( + self, userinfo_settings + ): + """A non-http(s) userinfo_uri is refused before any request (SSRF guard).""" + verifier = UnifiedTokenVerifier(userinfo_settings) + verifier.userinfo_uri = "ftp://evil/userinfo" + get_mock = AsyncMock() + with patch.object(verifier.http_client, "get", get_mock): + result = await verifier._validate_via_userinfo("opaque-token") + assert result is None + get_mock.assert_not_called() + async def test_validate_via_userinfo_not_configured(self, base_settings): base_settings.userinfo_uri = None verifier = UnifiedTokenVerifier(base_settings)