fix(oauth): follow redirects when fetching OIDC discovery
Nextcloud installs without pretty URLs return a 301 from `/.well-known/openid-configuration` to `/index.php/.well-known/openid-configuration` (e.g. Hetzner StorageShare). `_get_cached_discovery` did not enable follow_redirects, so httpx raised HTTPStatusError on the 301 and the AS-proxy authorize handler returned 500, breaking client connections (e.g. claude.ai). Pass `follow_redirects=True` to the httpx client used for the discovery fetch only — downstream OIDC endpoints (token, userinfo, etc.) are absolute URLs read from the discovery doc and are unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
02ea8f5fca
commit
d4760f64dc
@@ -136,13 +136,18 @@ def _transform_scopes_for_idp(scopes: str, resource_server_id: str) -> str:
|
|||||||
|
|
||||||
|
|
||||||
async def _get_cached_discovery(url: str) -> dict[str, Any]:
|
async def _get_cached_discovery(url: str) -> dict[str, Any]:
|
||||||
"""Fetch OIDC discovery document with caching (5-minute TTL)."""
|
"""Fetch OIDC discovery document with caching (5-minute TTL).
|
||||||
|
|
||||||
|
Follows redirects so the configured discovery URL works against Nextcloud
|
||||||
|
instances without pretty URLs enabled, where ``/.well-known/openid-configuration``
|
||||||
|
issues a 301 to ``/index.php/.well-known/openid-configuration``.
|
||||||
|
"""
|
||||||
now = time.time()
|
now = time.time()
|
||||||
if url in _discovery_cache:
|
if url in _discovery_cache:
|
||||||
expires_at, data = _discovery_cache[url]
|
expires_at, data = _discovery_cache[url]
|
||||||
if now < expires_at:
|
if now < expires_at:
|
||||||
return data
|
return data
|
||||||
async with nextcloud_httpx_client() as http_client:
|
async with nextcloud_httpx_client(follow_redirects=True) as http_client:
|
||||||
response = await http_client.get(url)
|
response = await http_client.get(url)
|
||||||
response.raise_for_status()
|
response.raise_for_status()
|
||||||
data = response.json()
|
data = response.json()
|
||||||
|
|||||||
@@ -0,0 +1,61 @@
|
|||||||
|
"""Unit tests for OIDC discovery fetch in oauth_routes."""
|
||||||
|
|
||||||
|
from unittest.mock import patch
|
||||||
|
|
||||||
|
import httpx
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from nextcloud_mcp_server.auth import oauth_routes
|
||||||
|
from nextcloud_mcp_server.auth.oauth_routes import _get_cached_discovery
|
||||||
|
|
||||||
|
pytestmark = pytest.mark.unit
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture(autouse=True)
|
||||||
|
def _clear_discovery_cache():
|
||||||
|
"""Reset the in-memory discovery cache between tests."""
|
||||||
|
oauth_routes._discovery_cache.clear()
|
||||||
|
yield
|
||||||
|
oauth_routes._discovery_cache.clear()
|
||||||
|
|
||||||
|
|
||||||
|
async def test_discovery_follows_redirect_to_index_php():
|
||||||
|
"""Discovery fetch must follow 301s.
|
||||||
|
|
||||||
|
Hetzner StorageShare and other Nextcloud installs without pretty URLs
|
||||||
|
redirect ``/.well-known/openid-configuration`` to
|
||||||
|
``/index.php/.well-known/openid-configuration``. Without follow_redirects
|
||||||
|
the OAuth authorize handler raises HTTPStatusError and returns 500
|
||||||
|
(see oauth_routes._get_cached_discovery).
|
||||||
|
"""
|
||||||
|
|
||||||
|
pretty_url = "https://nx.example.com/.well-known/openid-configuration"
|
||||||
|
rewritten_url = "https://nx.example.com/index.php/.well-known/openid-configuration"
|
||||||
|
discovery_doc = {
|
||||||
|
"issuer": "https://nx.example.com",
|
||||||
|
"authorization_endpoint": "https://nx.example.com/index.php/apps/oidc/authorize",
|
||||||
|
"token_endpoint": "https://nx.example.com/index.php/apps/oidc/token",
|
||||||
|
}
|
||||||
|
|
||||||
|
def handler(request: httpx.Request) -> httpx.Response:
|
||||||
|
if str(request.url) == pretty_url:
|
||||||
|
return httpx.Response(301, headers={"location": rewritten_url})
|
||||||
|
if str(request.url) == rewritten_url:
|
||||||
|
return httpx.Response(200, json=discovery_doc)
|
||||||
|
return httpx.Response(404)
|
||||||
|
|
||||||
|
transport = httpx.MockTransport(handler)
|
||||||
|
|
||||||
|
def fake_client(**kwargs):
|
||||||
|
kwargs["transport"] = transport
|
||||||
|
return httpx.AsyncClient(**kwargs)
|
||||||
|
|
||||||
|
with patch(
|
||||||
|
"nextcloud_mcp_server.auth.oauth_routes.nextcloud_httpx_client",
|
||||||
|
side_effect=fake_client,
|
||||||
|
) as factory:
|
||||||
|
result = await _get_cached_discovery(pretty_url)
|
||||||
|
|
||||||
|
assert result == discovery_doc
|
||||||
|
factory.assert_called_once()
|
||||||
|
assert factory.call_args.kwargs.get("follow_redirects") is True
|
||||||
Reference in New Issue
Block a user