refactor: remove ALLOWED_MCP_CLOUD_CLIENTS and add keycloak CI profile
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>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
91e7665f41
commit
7d775d2a52
@@ -43,6 +43,7 @@ jobs:
|
|||||||
- "single-user"
|
- "single-user"
|
||||||
- "multi-user-basic"
|
- "multi-user-basic"
|
||||||
- "login-flow"
|
- "login-flow"
|
||||||
|
- "keycloak"
|
||||||
include:
|
include:
|
||||||
# Version-specific image pins — Renovate updates these via customManagers in renovate.json
|
# 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)
|
# 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
|
needs-playwright: true
|
||||||
extra-args: ""
|
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 }})
|
name: integration (${{ matrix.mode }} / nc${{ matrix.nextcloud_version }})
|
||||||
|
|
||||||
steps:
|
steps:
|
||||||
@@ -174,14 +183,40 @@ jobs:
|
|||||||
done
|
done
|
||||||
echo "MCP service is ready on port ${{ matrix.wait-port }}."
|
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
|
- name: Verify OIDC configuration
|
||||||
if: matrix.mode == 'login-flow'
|
if: matrix.mode == 'login-flow' || matrix.mode == 'keycloak'
|
||||||
run: |
|
run: |
|
||||||
echo "=== OIDC Discovery ==="
|
echo "=== OIDC Discovery ==="
|
||||||
curl -s http://localhost:8080/.well-known/openid-configuration | jq .
|
curl -s http://localhost:8080/.well-known/openid-configuration | jq .
|
||||||
echo "=== OIDC App Status ==="
|
echo "=== OIDC App Status ==="
|
||||||
docker compose exec -T app php occ app:list --output=json 2>/dev/null | jq '.enabled.oidc // "NOT INSTALLED"'
|
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 }})
|
- name: Run tests (${{ matrix.mode }})
|
||||||
env:
|
env:
|
||||||
NEXTCLOUD_HOST: "http://localhost:8080"
|
NEXTCLOUD_HOST: "http://localhost:8080"
|
||||||
|
|||||||
@@ -61,14 +61,6 @@ class ClientRegistry:
|
|||||||
- https:// redirect URIs are allowed (cloud clients)
|
- https:// redirect URIs are allowed (cloud clients)
|
||||||
- http:// non-localhost redirect URIs are rejected with a warning
|
- 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()
|
allowed_clients = os.getenv("ALLOWED_MCP_CLIENTS", "").strip()
|
||||||
|
|
||||||
if allowed_clients:
|
if allowed_clients:
|
||||||
|
|||||||
@@ -23,7 +23,6 @@ def _get_registry(monkeypatch, value: str | None = None):
|
|||||||
monkeypatch.setenv("ALLOWED_MCP_CLIENTS", value)
|
monkeypatch.setenv("ALLOWED_MCP_CLIENTS", value)
|
||||||
else:
|
else:
|
||||||
monkeypatch.delenv("ALLOWED_MCP_CLIENTS", raising=False)
|
monkeypatch.delenv("ALLOWED_MCP_CLIENTS", raising=False)
|
||||||
monkeypatch.delenv("ALLOWED_MCP_CLOUD_CLIENTS", raising=False)
|
|
||||||
return registry_mod.get_client_registry()
|
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):
|
def test_client_name_resolution(monkeypatch):
|
||||||
registry = _get_registry(monkeypatch, "claude-desktop, custom-tool")
|
registry = _get_registry(monkeypatch, "claude-desktop, custom-tool")
|
||||||
assert registry.get_client("claude-desktop").name == "Claude Desktop"
|
assert registry.get_client("claude-desktop").name == "Claude Desktop"
|
||||||
|
|||||||
Reference in New Issue
Block a user