From 7d775d2a5242737cf8a27246005333494c75ac1d Mon Sep 17 00:00:00 2001
From: Chris Coutinho <chris@coutinho.io>
Date: Sun, 5 Apr 2026 15:06:56 +0200
Subject: [PATCH] refactor: remove ALLOWED_MCP_CLOUD_CLIENTS and add keycloak
 CI profile
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Remove the unused ALLOWED_MCP_CLOUD_CLIENTS env var — all clients are
defined via ALLOWED_MCP_CLIENTS or the static well-known defaults.
Add keycloak as an integration test profile in CI now that login-flow
replaces the old bearer token approach for external IdPs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 .github/workflows/test.yml                   | 37 +++++++++++++++++++-
 nextcloud_mcp_server/auth/client_registry.py |  8 -----
 tests/unit/test_client_registry.py           | 10 ------
 3 files changed, 36 insertions(+), 19 deletions(-)

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 98f03490..a1183929 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -43,6 +43,7 @@ jobs:
           - "single-user"
           - "multi-user-basic"
           - "login-flow"
+          - "keycloak"
         include:
           # Version-specific image pins — Renovate updates these via customManagers in renovate.json
           # Each entry is pinned to its major version (e.g., NC 31 only gets 31.x updates)
@@ -81,6 +82,14 @@ jobs:
             needs-playwright: true
             extra-args: ""
 
+          - mode: keycloak
+            profile: keycloak
+            markers: "keycloak"
+            wait-port: 8002
+            mcp-internal-url: "http://mcp-keycloak:8002"
+            needs-playwright: true
+            extra-args: ""
+
     name: integration (${{ matrix.mode }} / nc${{ matrix.nextcloud_version }})
 
     steps:
@@ -174,14 +183,40 @@ jobs:
           done
           echo "MCP service is ready on port ${{ matrix.wait-port }}."
 
+      - name: Wait for Keycloak
+        if: matrix.mode == 'keycloak'
+        run: |
+          echo "Waiting for Keycloak realm at http://localhost:8888..."
+          max_attempts=30
+          attempt=0
+          until curl -sf http://localhost:8888/realms/nextcloud-mcp > /dev/null 2>&1; do
+            attempt=$((attempt + 1))
+            if [ $attempt -ge $max_attempts ]; then
+              echo "Keycloak did not become ready in time."
+              docker compose --profile keycloak logs keycloak
+              exit 1
+            fi
+            echo "Attempt $attempt/$max_attempts: Not ready, sleeping 5s..."
+            sleep 5
+          done
+          echo "Keycloak is ready."
+
       - name: Verify OIDC configuration
-        if: matrix.mode == 'login-flow'
+        if: matrix.mode == 'login-flow' || matrix.mode == 'keycloak'
         run: |
           echo "=== OIDC Discovery ==="
           curl -s http://localhost:8080/.well-known/openid-configuration | jq .
           echo "=== OIDC App Status ==="
           docker compose exec -T app php occ app:list --output=json 2>/dev/null | jq '.enabled.oidc // "NOT INSTALLED"'
 
+      - name: Verify Keycloak realm
+        if: matrix.mode == 'keycloak'
+        run: |
+          echo "=== Keycloak Realm Discovery ==="
+          curl -s http://localhost:8888/realms/nextcloud-mcp/.well-known/openid-configuration | jq .
+          echo "=== Keycloak Provider in Nextcloud ==="
+          docker compose exec -T app php occ user_oidc:provider keycloak 2>/dev/null || echo "Provider not yet configured"
+
       - name: Run tests (${{ matrix.mode }})
         env:
           NEXTCLOUD_HOST: "http://localhost:8080"
diff --git a/nextcloud_mcp_server/auth/client_registry.py b/nextcloud_mcp_server/auth/client_registry.py
index b0d2183f..91958e7b 100644
--- a/nextcloud_mcp_server/auth/client_registry.py
+++ b/nextcloud_mcp_server/auth/client_registry.py
@@ -61,14 +61,6 @@ class ClientRegistry:
         - https:// redirect URIs are allowed (cloud clients)
         - http:// non-localhost redirect URIs are rejected with a warning
         """
-        # Deprecation warning for old env var
-        if os.getenv("ALLOWED_MCP_CLOUD_CLIENTS"):
-            logger.warning(
-                "ALLOWED_MCP_CLOUD_CLIENTS is deprecated. "
-                "Merge entries into ALLOWED_MCP_CLIENTS using the format: "
-                "client_id|https://redirect-uri"
-            )
-
         allowed_clients = os.getenv("ALLOWED_MCP_CLIENTS", "").strip()
 
         if allowed_clients:
diff --git a/tests/unit/test_client_registry.py b/tests/unit/test_client_registry.py
index d67ce81e..f8836805 100644
--- a/tests/unit/test_client_registry.py
+++ b/tests/unit/test_client_registry.py
@@ -23,7 +23,6 @@ def _get_registry(monkeypatch, value: str | None = None):
         monkeypatch.setenv("ALLOWED_MCP_CLIENTS", value)
     else:
         monkeypatch.delenv("ALLOWED_MCP_CLIENTS", raising=False)
-    monkeypatch.delenv("ALLOWED_MCP_CLOUD_CLIENTS", raising=False)
     return registry_mod.get_client_registry()
 
 
@@ -162,15 +161,6 @@ def test_well_known_clients_wildcard_scopes(monkeypatch):
         )
 
 
-def test_deprecated_cloud_clients_warning(monkeypatch, caplog):
-    monkeypatch.setenv("ALLOWED_MCP_CLOUD_CLIENTS", "old|https://old.com/cb")
-    monkeypatch.setenv("ALLOWED_MCP_CLIENTS", "new-client")
-    with caplog.at_level(logging.WARNING):
-        registry_mod.get_client_registry()
-
-    assert "ALLOWED_MCP_CLOUD_CLIENTS is deprecated" in caplog.text
-
-
 def test_client_name_resolution(monkeypatch):
     registry = _get_registry(monkeypatch, "claude-desktop, custom-tool")
     assert registry.get_client("claude-desktop").name == "Claude Desktop"