From 92f2d74637fa1bace78f7e712a1b7e7733567890 Mon Sep 17 00:00:00 2001 From: Brandon Date: Sun, 29 Mar 2026 12:56:50 -0600 Subject: [PATCH] feat: auto-derive oidc.discovery_url from NEXTCLOUD_HOST When OIDC_DISCOVERY_URL is not explicitly set, the status endpoint now auto-derives the discovery URL from NEXTCLOUD_HOST using the standard well-known path. This allows Astrolabe to discover OIDC endpoints without requiring explicit OIDC configuration. The oidc block is now included in the status response regardless of auth mode when a discovery URL is available (explicit or derived), enabling smoother auth mode transitions. Closes #1 --- nextcloud_mcp_server/api/management.py | 32 +++-- tests/unit/test_management_status_endpoint.py | 130 +++++++++++++++++- 2 files changed, 146 insertions(+), 16 deletions(-) diff --git a/nextcloud_mcp_server/api/management.py b/nextcloud_mcp_server/api/management.py index e389427..21e0864 100644 --- a/nextcloud_mcp_server/api/management.py +++ b/nextcloud_mcp_server/api/management.py @@ -235,24 +235,26 @@ async def get_server_status(request: Request) -> JSONResponse: if mode == AuthMode.MULTI_USER_BASIC: response_data["supports_app_passwords"] = settings.enable_offline_access - # Include OIDC configuration if OAuth is available - # This includes OAuth mode AND hybrid mode (multi_user_basic + offline_access) - # Astrolabe needs OIDC config to discover IdP for OAuth flow in hybrid mode - oauth_provisioning_available = auth_mode == "oauth" or ( - mode == AuthMode.MULTI_USER_BASIC and settings.enable_offline_access - ) - if oauth_provisioning_available: - # Provide IdP discovery information for NC PHP app - oidc_config = {} + # Include OIDC configuration for client discovery (e.g. Astrolabe PHP app). + # Always attempt to provide oidc.discovery_url so clients can discover the + # IdP regardless of the current auth mode. This enables smoother transitions + # between auth modes and lets Astrolabe pre-discover OIDC endpoints. + oidc_config: dict[str, str] = {} - if settings.oidc_discovery_url: - oidc_config["discovery_url"] = settings.oidc_discovery_url + if settings.oidc_discovery_url: + # Explicit OIDC_DISCOVERY_URL takes precedence + oidc_config["discovery_url"] = settings.oidc_discovery_url + elif settings.nextcloud_host: + # Auto-derive from NEXTCLOUD_HOST — Nextcloud exposes OIDC discovery + # at the standard well-known path when user_oidc is enabled + host = settings.nextcloud_host.rstrip("/") + oidc_config["discovery_url"] = f"{host}/.well-known/openid-configuration" - if settings.oidc_issuer: - oidc_config["issuer"] = settings.oidc_issuer + if settings.oidc_issuer: + oidc_config["issuer"] = settings.oidc_issuer - if oidc_config: - response_data["oidc"] = oidc_config + if oidc_config: + response_data["oidc"] = oidc_config return JSONResponse(response_data) diff --git a/tests/unit/test_management_status_endpoint.py b/tests/unit/test_management_status_endpoint.py index ac6ad1a..02debeb 100644 --- a/tests/unit/test_management_status_endpoint.py +++ b/tests/unit/test_management_status_endpoint.py @@ -37,6 +37,7 @@ def create_mock_settings( oidc_issuer: str | None = None, vector_sync_enabled: bool = False, nextcloud_url: str = "http://localhost", + nextcloud_host: str | None = "http://localhost", enable_token_exchange: bool = False, mcp_client_id: str | None = None, mcp_client_secret: str | None = None, @@ -49,6 +50,7 @@ def create_mock_settings( settings.oidc_issuer = oidc_issuer settings.vector_sync_enabled = vector_sync_enabled settings.nextcloud_url = nextcloud_url + settings.nextcloud_host = nextcloud_host settings.enable_token_exchange = enable_token_exchange settings.mcp_client_id = mcp_client_id settings.mcp_client_secret = mcp_client_secret @@ -133,6 +135,7 @@ class TestStatusEndpointOidcConfig: enable_offline_access=False, # Key difference: no offline access oidc_discovery_url="http://keycloak/.well-known/openid-configuration", oidc_issuer="http://keycloak/realms/test", + nextcloud_host=None, ) with ( @@ -196,12 +199,13 @@ class TestStatusEndpointOidcConfig: ) def test_single_user_basic_no_oidc(self): - """Test that single-user BasicAuth mode doesn't return OIDC config.""" + """Test that single-user BasicAuth mode doesn't return OIDC config when no host.""" mock_settings = create_mock_settings( enable_multi_user_basic=False, enable_offline_access=False, oidc_discovery_url="http://keycloak/.well-known/openid-configuration", oidc_issuer="http://keycloak/realms/test", + nextcloud_host=None, ) with ( @@ -344,3 +348,127 @@ class TestStatusEndpointBasicResponse: data = response.json() assert data["vector_sync_enabled"] is True + + +class TestStatusEndpointOidcAutoDerivation: + """Tests for OIDC discovery_url auto-derivation from NEXTCLOUD_HOST.""" + + def test_derives_discovery_url_from_nextcloud_host(self): + """Test that discovery_url is auto-derived from nextcloud_url when not explicit.""" + mock_settings = create_mock_settings( + oidc_discovery_url=None, + oidc_issuer=None, + ) + mock_settings.nextcloud_host = "https://cloud.example.com" + + with ( + patch( + "nextcloud_mcp_server.api.management.get_settings", + return_value=mock_settings, + ), + patch( + "nextcloud_mcp_server.api.management.detect_auth_mode", + return_value=AuthMode.SINGLE_USER_BASIC, + ), + ): + app = create_test_app() + client = TestClient(app) + response = client.get("/api/v1/status") + + assert response.status_code == 200 + data = response.json() + + assert "oidc" in data + assert ( + data["oidc"]["discovery_url"] + == "https://cloud.example.com/.well-known/openid-configuration" + ) + + def test_derives_discovery_url_strips_trailing_slash(self): + """Test that trailing slash on nextcloud_host is stripped.""" + mock_settings = create_mock_settings( + oidc_discovery_url=None, + oidc_issuer=None, + ) + mock_settings.nextcloud_host = "https://cloud.example.com/" + + with ( + patch( + "nextcloud_mcp_server.api.management.get_settings", + return_value=mock_settings, + ), + patch( + "nextcloud_mcp_server.api.management.detect_auth_mode", + return_value=AuthMode.SINGLE_USER_BASIC, + ), + ): + app = create_test_app() + client = TestClient(app) + response = client.get("/api/v1/status") + + assert response.status_code == 200 + data = response.json() + + assert "oidc" in data + assert ( + data["oidc"]["discovery_url"] + == "https://cloud.example.com/.well-known/openid-configuration" + ) + + def test_explicit_discovery_url_takes_precedence(self): + """Test that explicit OIDC_DISCOVERY_URL overrides auto-derivation.""" + mock_settings = create_mock_settings( + oidc_discovery_url="https://keycloak.example.com/.well-known/openid-configuration", + oidc_issuer=None, + ) + mock_settings.nextcloud_host = "https://cloud.example.com" + + with ( + patch( + "nextcloud_mcp_server.api.management.get_settings", + return_value=mock_settings, + ), + patch( + "nextcloud_mcp_server.api.management.detect_auth_mode", + return_value=AuthMode.SINGLE_USER_BASIC, + ), + ): + app = create_test_app() + client = TestClient(app) + response = client.get("/api/v1/status") + + assert response.status_code == 200 + data = response.json() + + assert "oidc" in data + assert ( + data["oidc"]["discovery_url"] + == "https://keycloak.example.com/.well-known/openid-configuration" + ) + + def test_no_oidc_when_no_host_and_no_discovery_url(self): + """Test that oidc block is absent when neither host nor discovery_url is set.""" + mock_settings = create_mock_settings( + oidc_discovery_url=None, + oidc_issuer=None, + ) + mock_settings.nextcloud_host = None + + with ( + patch( + "nextcloud_mcp_server.api.management.get_settings", + return_value=mock_settings, + ), + patch( + "nextcloud_mcp_server.api.management.detect_auth_mode", + return_value=AuthMode.SINGLE_USER_BASIC, + ), + ): + app = create_test_app() + client = TestClient(app) + response = client.get("/api/v1/status") + + assert response.status_code == 200 + data = response.json() + + assert "oidc" not in data