Merge remote-tracking branch 'origin/master' into docs/adr-024-dynaconf-config-management
This commit is contained in:
Chris Coutinho
@@ -33,7 +33,7 @@ jobs:
|
||||
|
||||
- name: Run Claude Code Review
|
||||
id: claude-review
|
||||
uses: anthropics/claude-code-action@58dbe8ed6879f0d3b02ac295b20d5fdfe7733e0c # v1.0.85
|
||||
uses: anthropics/claude-code-action@6e2bd52842c65e914eba5c8badd17560bd26b5de # v1.0.89
|
||||
with:
|
||||
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
|
||||
allowed_bots: "renovate-bot-cbcoutinho"
|
||||
|
||||
@@ -32,7 +32,7 @@ jobs:
|
||||
|
||||
- name: Run Claude Code
|
||||
id: claude
|
||||
uses: anthropics/claude-code-action@58dbe8ed6879f0d3b02ac295b20d5fdfe7733e0c # v1.0.85
|
||||
uses: anthropics/claude-code-action@6e2bd52842c65e914eba5c8badd17560bd26b5de # v1.0.89
|
||||
with:
|
||||
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
|
||||
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -5,6 +5,18 @@ All notable changes to the Nextcloud MCP Server will be documented in this file.
|
||||
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
|
||||
and this project adheres to [PEP 440](https://peps.python.org/pep-0440/).
|
||||
|
||||
## v0.68.3 (2026-04-05)
|
||||
|
||||
### Fix
|
||||
|
||||
- address PR review feedback for client registry and DCR proxy
|
||||
- support cloud OAuth clients and graceful DCR fallback
|
||||
|
||||
### Refactor
|
||||
|
||||
- remove ALLOWED_MCP_CLOUD_CLIENTS and add keycloak CI profile
|
||||
- consolidate ALLOWED_MCP_CLIENTS and add redirect URI validation
|
||||
|
||||
## v0.68.2 (2026-04-04)
|
||||
|
||||
### Fix
|
||||
|
||||
@@ -1,69 +0,0 @@
|
||||
From deab2dac3d73d25f20a95c18103f327ab48f837a Mon Sep 17 00:00:00 2001
|
||||
From: Chris Coutinho <chris@coutinho.io>
|
||||
Date: Sun, 12 Oct 2025 21:09:29 +0200
|
||||
Subject: [PATCH 1/1] Fix Bearer token authentication causing session logout
|
||||
|
||||
When using Bearer token authentication with OIDC, API requests to
|
||||
endpoints with @CORS annotations (like Notes API) were failing with
|
||||
401 Unauthorized errors. This occurred because:
|
||||
|
||||
1. Bearer token validation successfully authenticated the user
|
||||
2. A session was created for the authenticated user
|
||||
3. Nextcloud's CORSMiddleware detected the logged-in session but no
|
||||
CSRF token, causing it to call session->logout()
|
||||
4. The logout invalidated the session, breaking the API request
|
||||
|
||||
This fix sets the 'app_api' session flag during Bearer token
|
||||
authentication, which instructs CORSMiddleware to skip the CSRF check
|
||||
and logout logic. This is the same mechanism used by Nextcloud's
|
||||
AppAPI framework for external application authentication.
|
||||
|
||||
The flag is set at all successful Bearer token authentication points:
|
||||
- Line 243: After OIDC Identity Provider validation
|
||||
- Line 310: After auto-provisioning with bearer provisioning
|
||||
- Line 315: After existing user authentication
|
||||
- Line 337: After LDAP user sync
|
||||
|
||||
Fixes: Bearer token authentication for all Nextcloud APIs
|
||||
Tested-with: nextcloud-mcp-server integration tests
|
||||
Signed-off-by: Chris Coutinho <chris@coutinho.io>
|
||||
---
|
||||
lib/User/Backend.php | 4 ++++
|
||||
1 file changed, 4 insertions(+)
|
||||
|
||||
diff --git a/lib/User/Backend.php b/lib/User/Backend.php
|
||||
index 23cfb18..65665cc 100644
|
||||
--- a/lib/User/Backend.php
|
||||
+++ b/lib/User/Backend.php
|
||||
@@ -240,6 +240,7 @@ class Backend extends ABackend implements IPasswordConfirmationBackend, IGetDisp
|
||||
$this->eventDispatcher->dispatchTyped($validationEvent);
|
||||
$oidcProviderUserId = $validationEvent->getUserId();
|
||||
if ($oidcProviderUserId !== null) {
|
||||
+ $this->session->set('app_api', true);
|
||||
return $oidcProviderUserId;
|
||||
} else {
|
||||
$this->logger->debug('[NextcloudOidcProviderValidator] The bearer token validation has failed');
|
||||
@@ -306,10 +307,12 @@ class Backend extends ABackend implements IPasswordConfirmationBackend, IGetDisp
|
||||
}
|
||||
|
||||
$this->session->set('last-password-confirm', strtotime('+4 year', time()));
|
||||
+ $this->session->set('app_api', true);
|
||||
return $userId;
|
||||
} elseif ($this->userExists($tokenUserId)) {
|
||||
$this->checkFirstLogin($tokenUserId);
|
||||
$this->session->set('last-password-confirm', strtotime('+4 year', time()));
|
||||
+ $this->session->set('app_api', true);
|
||||
return $tokenUserId;
|
||||
} else {
|
||||
// check if the user exists locally
|
||||
@@ -331,6 +334,7 @@ class Backend extends ABackend implements IPasswordConfirmationBackend, IGetDisp
|
||||
}
|
||||
$this->checkFirstLogin($tokenUserId);
|
||||
$this->session->set('last-password-confirm', strtotime('+4 year', time()));
|
||||
+ $this->session->set('app_api', true);
|
||||
return $tokenUserId;
|
||||
}
|
||||
}
|
||||
--
|
||||
2.51.0
|
||||
|
||||
@@ -1,18 +0,0 @@
|
||||
diff --git a/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php b/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php
|
||||
index 4453f5a7d4b..f1ca9b48d21 100644
|
||||
--- a/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php
|
||||
+++ b/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php
|
||||
@@ -73,6 +73,13 @@ class CORSMiddleware extends Middleware {
|
||||
$user = array_key_exists('PHP_AUTH_USER', $this->request->server) ? $this->request->server['PHP_AUTH_USER'] : null;
|
||||
$pass = array_key_exists('PHP_AUTH_PW', $this->request->server) ? $this->request->server['PHP_AUTH_PW'] : null;
|
||||
|
||||
+ // Allow Bearer token authentication for CORS requests
|
||||
+ // Bearer tokens are stateless and don't require CSRF protection
|
||||
+ $authorizationHeader = $this->request->getHeader('Authorization');
|
||||
+ if (!empty($authorizationHeader) && str_starts_with($authorizationHeader, 'Bearer ')) {
|
||||
+ return;
|
||||
+ }
|
||||
+
|
||||
// Allow to use the current session if a CSRF token is provided
|
||||
if ($this->request->passesCSRFCheck()) {
|
||||
return;
|
||||
@@ -1,64 +0,0 @@
|
||||
#!/bin/bash
|
||||
#
|
||||
# Apply upstream CORSMiddleware Bearer token authentication patch
|
||||
#
|
||||
# This patch allows Bearer tokens to bypass CORS/CSRF checks, fixing
|
||||
# authentication issues with app-specific APIs (Notes, Calendar, etc.)
|
||||
# when using OAuth/OIDC Bearer tokens.
|
||||
#
|
||||
# Upstream PR: https://github.com/nextcloud/server/pull/55878
|
||||
# Commit: 8fb5e77db82 (fix(cors): Allow Bearer token authentication)
|
||||
#
|
||||
|
||||
set -e
|
||||
|
||||
PATCH_FILE="/docker-entrypoint-hooks.d/patches/cors-bearer-token.patch"
|
||||
TARGET_FILE="/var/www/html/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php"
|
||||
|
||||
echo "===================================================================="
|
||||
echo "Applying CORSMiddleware Bearer token authentication patch..."
|
||||
echo "===================================================================="
|
||||
|
||||
# Check if patch file exists
|
||||
if [ ! -f "$PATCH_FILE" ]; then
|
||||
echo "⚠ Warning: Patch file not found: $PATCH_FILE"
|
||||
echo " Skipping CORS Bearer token patch"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# Check if target file exists
|
||||
if [ ! -f "$TARGET_FILE" ]; then
|
||||
echo "⚠ Warning: Target file not found: $TARGET_FILE"
|
||||
echo " Skipping CORS Bearer token patch"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# Check if already patched
|
||||
if grep -q "Allow Bearer token authentication for CORS requests" "$TARGET_FILE"; then
|
||||
echo "✓ CORSMiddleware already patched for Bearer token support"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
echo "Applying patch to CORSMiddleware.php..."
|
||||
|
||||
# Apply the patch
|
||||
cd /var/www/html
|
||||
if patch -p1 --dry-run < "$PATCH_FILE" > /dev/null 2>&1; then
|
||||
patch -p1 < "$PATCH_FILE"
|
||||
echo "✓ Patch applied successfully"
|
||||
else
|
||||
echo "⚠ Warning: Patch failed to apply (may already be applied or file changed)"
|
||||
echo " This is expected if using a Nextcloud version that already includes the fix"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
echo ""
|
||||
echo "===================================================================="
|
||||
echo "✓ CORSMiddleware Bearer token patch applied"
|
||||
echo "===================================================================="
|
||||
echo ""
|
||||
echo "Benefits:"
|
||||
echo " • Bearer tokens now work with app-specific APIs (Notes, Calendar, etc.)"
|
||||
echo " • OAuth/OIDC authentication works without CORS errors"
|
||||
echo " • Stateless API authentication is properly supported"
|
||||
echo ""
|
||||
@@ -1,6 +1,6 @@
|
||||
[tool.commitizen]
|
||||
name = "cz_conventional_commits"
|
||||
version = "0.58.26"
|
||||
version = "0.58.28"
|
||||
tag_format = "nextcloud-mcp-server-$version"
|
||||
version_scheme = "semver"
|
||||
update_changelog_on_bump = true
|
||||
|
||||
@@ -14,6 +14,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
- Configurable resource limits
|
||||
- Grafana dashboard annotations
|
||||
|
||||
## nextcloud-mcp-server-0.58.28 (2026-04-05)
|
||||
|
||||
### Fix
|
||||
|
||||
- address PR review feedback for client registry and DCR proxy
|
||||
- support cloud OAuth clients and graceful DCR fallback
|
||||
|
||||
### Refactor
|
||||
|
||||
- remove ALLOWED_MCP_CLOUD_CLIENTS and add keycloak CI profile
|
||||
- consolidate ALLOWED_MCP_CLIENTS and add redirect URI validation
|
||||
|
||||
## nextcloud-mcp-server-0.58.27 (2026-04-05)
|
||||
|
||||
## nextcloud-mcp-server-0.58.26 (2026-04-04)
|
||||
|
||||
### Fix
|
||||
|
||||
@@ -2,8 +2,8 @@ apiVersion: v2
|
||||
name: nextcloud-mcp-server
|
||||
description: A Helm chart for Nextcloud MCP Server - enables AI assistants to interact with Nextcloud
|
||||
type: application
|
||||
version: 0.58.26
|
||||
appVersion: "0.68.2"
|
||||
version: 0.58.28
|
||||
appVersion: "0.68.3"
|
||||
keywords:
|
||||
- nextcloud
|
||||
- mcp
|
||||
|
||||
@@ -230,6 +230,10 @@ services:
|
||||
- ENABLE_TOKEN_EXCHANGE=true
|
||||
- TOKEN_EXCHANGE_CACHE_TTL=300 # Cache exchanged tokens for 5 minutes (default)
|
||||
|
||||
# Login Flow v2 (ADR-022) with external IdP
|
||||
- ENABLE_LOGIN_FLOW=true
|
||||
- ENABLE_DCR=true
|
||||
|
||||
# OAuth scopes (optional - uses defaults if not specified)
|
||||
- NEXTCLOUD_OIDC_SCOPES=openid profile email offline_access notes:read notes:write calendar:read calendar:write contacts:read contacts:write cookbook:read cookbook:write deck:read deck:write tables:read tables:write files:read files:write sharing:read sharing:write todo:read todo:write
|
||||
|
||||
|
||||
@@ -36,6 +36,12 @@ class ClientRegistry:
|
||||
2. Integrate with IdP client registry
|
||||
3. Store client metadata in database
|
||||
4. Support client updates and revocation
|
||||
|
||||
Scope Policy:
|
||||
All clients are registered with allowed_scopes=["*"] (wildcard).
|
||||
The MCP server acts as an OAuth AS proxy — it validates client
|
||||
identity and redirect URIs locally, but delegates scope enforcement
|
||||
to the upstream IdP (Nextcloud or Keycloak).
|
||||
"""
|
||||
|
||||
def __init__(self, allow_dynamic_registration: bool = False):
|
||||
@@ -50,25 +56,72 @@ class ClientRegistry:
|
||||
self._load_static_clients()
|
||||
|
||||
def _load_static_clients(self):
|
||||
"""Load statically configured clients from environment."""
|
||||
# Load from ALLOWED_MCP_CLIENTS environment variable
|
||||
"""Load statically configured clients from environment.
|
||||
|
||||
Format: comma-separated entries, each either:
|
||||
- Simple client ID: gets localhost redirect URIs
|
||||
- client_id|redirect_uri: gets the specified redirect URI
|
||||
|
||||
Redirect URI rules:
|
||||
- http://localhost:* and http://127.0.0.1:* are allowed (native clients)
|
||||
- https:// redirect URIs are allowed (cloud clients)
|
||||
- http:// non-localhost redirect URIs are rejected with a warning
|
||||
"""
|
||||
allowed_clients = os.getenv("ALLOWED_MCP_CLIENTS", "").strip()
|
||||
|
||||
if allowed_clients:
|
||||
# Parse comma-separated list
|
||||
for client_id in allowed_clients.split(","):
|
||||
client_id = client_id.strip()
|
||||
if client_id:
|
||||
# Create basic client info
|
||||
# In production, would load full metadata from database
|
||||
self._clients[client_id] = MCPClientInfo(
|
||||
client_id=client_id,
|
||||
name=self._get_client_name(client_id),
|
||||
redirect_uris=["http://localhost:*", "http://127.0.0.1:*"],
|
||||
allowed_scopes=["openid", "profile", "email", "mcp-server:api"],
|
||||
for entry in allowed_clients.split(","):
|
||||
entry = entry.strip()
|
||||
if not entry:
|
||||
continue
|
||||
|
||||
if "|" in entry:
|
||||
cid, redirect = entry.split("|", 1)
|
||||
cid, redirect = cid.strip(), redirect.strip()
|
||||
|
||||
if not cid or not redirect:
|
||||
logger.warning(
|
||||
f"Skipping malformed ALLOWED_MCP_CLIENTS entry: {entry!r}"
|
||||
)
|
||||
continue
|
||||
|
||||
parsed = urlparse(redirect)
|
||||
hostname = parsed.hostname
|
||||
if hostname is None:
|
||||
logger.warning(
|
||||
f"Skipping client {cid!r}: cannot parse hostname "
|
||||
f"from {redirect!r}"
|
||||
)
|
||||
continue
|
||||
is_loopback = hostname in ("localhost", "127.0.0.1", "::1")
|
||||
|
||||
if not (
|
||||
parsed.scheme == "https"
|
||||
or (parsed.scheme == "http" and is_loopback)
|
||||
):
|
||||
logger.warning(
|
||||
f"Rejecting client {cid!r}: HTTP redirect URIs are only "
|
||||
f"allowed for localhost, got {redirect!r}"
|
||||
)
|
||||
continue
|
||||
|
||||
self._clients[cid] = MCPClientInfo(
|
||||
client_id=cid,
|
||||
name=self._get_client_name(cid),
|
||||
redirect_uris=[redirect],
|
||||
allowed_scopes=["*"],
|
||||
is_public=True,
|
||||
)
|
||||
logger.info(f"Registered static client: {client_id}")
|
||||
logger.info(f"Registered static client: {cid}")
|
||||
else:
|
||||
self._clients[entry] = MCPClientInfo(
|
||||
client_id=entry,
|
||||
name=self._get_client_name(entry),
|
||||
redirect_uris=["http://localhost:*", "http://127.0.0.1:*"],
|
||||
allowed_scopes=["*"],
|
||||
is_public=True,
|
||||
)
|
||||
logger.info(f"Registered static client: {entry}")
|
||||
|
||||
# Add well-known clients if not explicitly configured
|
||||
if not self._clients:
|
||||
@@ -78,6 +131,7 @@ class ClientRegistry:
|
||||
"""Get human-readable name for client_id."""
|
||||
known_names = {
|
||||
"claude-desktop": "Claude Desktop",
|
||||
"claude-ai": "Claude AI",
|
||||
"continue-dev": "Continue IDE Extension",
|
||||
"zed-editor": "Zed Editor",
|
||||
"vscode-mcp": "VS Code MCP Extension",
|
||||
@@ -92,7 +146,7 @@ class ClientRegistry:
|
||||
client_id="claude-desktop",
|
||||
name="Claude Desktop",
|
||||
redirect_uris=["http://localhost:*", "http://127.0.0.1:*"],
|
||||
allowed_scopes=["openid", "profile", "email", "mcp-server:api"],
|
||||
allowed_scopes=["*"],
|
||||
is_public=True,
|
||||
metadata={"vendor": "Anthropic"},
|
||||
),
|
||||
@@ -100,7 +154,7 @@ class ClientRegistry:
|
||||
client_id="test-mcp-client",
|
||||
name="Test MCP Client",
|
||||
redirect_uris=["http://localhost:*", "http://127.0.0.1:*"],
|
||||
allowed_scopes=["openid", "profile", "email", "mcp-server:api"],
|
||||
allowed_scopes=["*"],
|
||||
is_public=True,
|
||||
metadata={"purpose": "testing"},
|
||||
),
|
||||
@@ -163,6 +217,8 @@ class ClientRegistry:
|
||||
"""
|
||||
# Parse the redirect URI
|
||||
parsed = urlparse(redirect_uri)
|
||||
if not parsed.hostname:
|
||||
return False
|
||||
|
||||
# Check against registered patterns
|
||||
for pattern in client.redirect_uris:
|
||||
@@ -171,7 +227,7 @@ class ClientRegistry:
|
||||
pattern_base = pattern.replace(":*", "")
|
||||
if redirect_uri.startswith(pattern_base + ":"):
|
||||
# Validate it's localhost with a port
|
||||
if parsed.hostname in ["localhost", "127.0.0.1"]:
|
||||
if parsed.hostname in ("localhost", "127.0.0.1", "::1"):
|
||||
return True
|
||||
elif redirect_uri == pattern:
|
||||
return True
|
||||
|
||||
@@ -1228,7 +1228,6 @@ async def oauth_register_proxy(request: Request) -> JSONResponse:
|
||||
)
|
||||
|
||||
oauth_config = oauth_ctx["config"]
|
||||
nextcloud_host = oauth_config["nextcloud_host"]
|
||||
|
||||
# Rate limit DCR requests per client IP
|
||||
client_ip = request.client.host if request.client else "unknown"
|
||||
@@ -1249,21 +1248,29 @@ async def oauth_register_proxy(request: Request) -> JSONResponse:
|
||||
timestamps.append(now)
|
||||
_dcr_rate_limit[client_ip] = timestamps
|
||||
|
||||
# Discover registration endpoint from OIDC discovery (prefer over hardcoded path)
|
||||
# Discover registration endpoint from OIDC discovery
|
||||
discovery_url = oauth_config.get("discovery_url")
|
||||
registration_endpoint = None
|
||||
if discovery_url:
|
||||
try:
|
||||
discovery = await _get_cached_discovery(discovery_url)
|
||||
registration_endpoint = discovery.get(
|
||||
"registration_endpoint", f"{nextcloud_host}/apps/oidc/register"
|
||||
)
|
||||
registration_endpoint = discovery.get("registration_endpoint")
|
||||
except Exception:
|
||||
logger.warning(
|
||||
"Failed to fetch OIDC discovery for DCR endpoint, using fallback"
|
||||
)
|
||||
registration_endpoint = f"{nextcloud_host}/apps/oidc/register"
|
||||
else:
|
||||
registration_endpoint = f"{nextcloud_host}/apps/oidc/register"
|
||||
logger.warning("Failed to fetch OIDC discovery for DCR endpoint")
|
||||
|
||||
if not registration_endpoint:
|
||||
logger.warning(
|
||||
"DCR proxy: Upstream IdP does not support dynamic client registration"
|
||||
)
|
||||
return JSONResponse(
|
||||
{
|
||||
"error": "registration_not_supported",
|
||||
"error_description": "The upstream identity provider does not support "
|
||||
"dynamic client registration. Configure the client statically using "
|
||||
"ALLOWED_MCP_CLIENTS.",
|
||||
},
|
||||
status_code=400,
|
||||
)
|
||||
|
||||
logger.info(f"DCR proxy: Forwarding registration to {registration_endpoint}")
|
||||
|
||||
@@ -1276,7 +1283,7 @@ async def oauth_register_proxy(request: Request) -> JSONResponse:
|
||||
|
||||
if response.status_code not in (200, 201):
|
||||
logger.error(
|
||||
f"DCR proxy: Nextcloud registration failed: {response.status_code} {response.text}"
|
||||
f"DCR proxy: Upstream registration failed: {response.status_code} {response.text}"
|
||||
)
|
||||
return JSONResponse(
|
||||
response.json()
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
[project]
|
||||
name = "nextcloud-mcp-server"
|
||||
version = "0.68.2"
|
||||
version = "0.68.3"
|
||||
description = "Model Context Protocol (MCP) server for Nextcloud integration - enables AI assistants to interact with Nextcloud data"
|
||||
authors = [
|
||||
{name = "Chris Coutinho", email = "chris@coutinho.io"}
|
||||
|
||||
@@ -0,0 +1,218 @@
|
||||
"""
|
||||
Integration tests for DCR and static OIDC clients against the mcp-keycloak service.
|
||||
|
||||
The mcp-keycloak service uses Keycloak as an external IdP. Keycloak supports
|
||||
Dynamic Client Registration (RFC 7591/7592), so the DCR proxy forwards
|
||||
registrations to Keycloak and registers the resulting client locally.
|
||||
|
||||
Static clients configured via ALLOWED_MCP_CLIENTS should work for the
|
||||
authorization flow without DCR.
|
||||
|
||||
Requires: docker compose --profile keycloak up --build -d
|
||||
"""
|
||||
|
||||
import base64
|
||||
import hashlib
|
||||
import logging
|
||||
import secrets
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
pytestmark = [pytest.mark.integration, pytest.mark.keycloak]
|
||||
|
||||
MCP_KEYCLOAK_BASE_URL = "http://localhost:8002"
|
||||
KEYCLOAK_BASE_URL = "http://localhost:8888"
|
||||
KEYCLOAK_REALM = "nextcloud-mcp"
|
||||
|
||||
|
||||
@pytest.fixture(scope="module")
|
||||
async def keycloak_mcp_available():
|
||||
"""Check that the mcp-keycloak service is reachable and return AS metadata."""
|
||||
async with httpx.AsyncClient(timeout=10.0) as client:
|
||||
try:
|
||||
resp = await client.get(
|
||||
f"{MCP_KEYCLOAK_BASE_URL}/.well-known/oauth-authorization-server"
|
||||
)
|
||||
resp.raise_for_status()
|
||||
return resp.json()
|
||||
except (httpx.ConnectError, httpx.HTTPStatusError) as e:
|
||||
pytest.skip(f"mcp-keycloak service not available: {e}")
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
async def dcr_client(keycloak_mcp_available):
|
||||
"""Register a client via DCR proxy and clean up after test."""
|
||||
async with httpx.AsyncClient(timeout=30.0) as http:
|
||||
# Register via MCP proxy
|
||||
resp = await http.post(
|
||||
f"{MCP_KEYCLOAK_BASE_URL}/oauth/register",
|
||||
json={
|
||||
"client_name": "test-dcr-keycloak",
|
||||
"redirect_uris": ["http://localhost:9999/callback"],
|
||||
"grant_types": ["authorization_code"],
|
||||
"response_types": ["code"],
|
||||
"token_endpoint_auth_method": "client_secret_basic",
|
||||
"scope": "openid profile email",
|
||||
},
|
||||
)
|
||||
resp.raise_for_status()
|
||||
data = resp.json()
|
||||
|
||||
yield data
|
||||
|
||||
# Cleanup: delete via Keycloak directly (proxy URI uses internal hostname)
|
||||
client_id = data.get("client_id")
|
||||
rat = data.get("registration_access_token")
|
||||
if client_id and rat:
|
||||
async with httpx.AsyncClient(timeout=10.0) as http:
|
||||
await http.delete(
|
||||
f"{KEYCLOAK_BASE_URL}/realms/{KEYCLOAK_REALM}"
|
||||
f"/clients-registrations/openid-connect/{client_id}",
|
||||
headers={"Authorization": f"Bearer {rat}"},
|
||||
)
|
||||
|
||||
|
||||
# --- DCR tests ---
|
||||
|
||||
|
||||
async def test_dcr_proxy_registers_client(dcr_client):
|
||||
"""DCR proxy should forward registration to Keycloak and return
|
||||
RFC 7591 response with client credentials."""
|
||||
assert "client_id" in dcr_client
|
||||
assert "client_secret" in dcr_client
|
||||
assert dcr_client["client_name"] == "test-dcr-keycloak"
|
||||
|
||||
|
||||
async def test_dcr_proxy_returns_rfc7592_fields(dcr_client):
|
||||
"""DCR response should include RFC 7592 management fields for
|
||||
client lifecycle management."""
|
||||
assert "registration_access_token" in dcr_client
|
||||
assert dcr_client["registration_access_token"]
|
||||
assert "registration_client_uri" in dcr_client
|
||||
assert dcr_client["registration_client_uri"]
|
||||
|
||||
|
||||
async def test_dcr_client_accepted_by_authorize(keycloak_mcp_available, dcr_client):
|
||||
"""A DCR-registered client should be accepted by the authorization endpoint
|
||||
(redirects to IdP login rather than returning an error)."""
|
||||
# Generate PKCE challenge (required by the server)
|
||||
verifier = secrets.token_urlsafe(64)
|
||||
challenge = hashlib.sha256(verifier.encode()).digest()
|
||||
code_challenge = base64.urlsafe_b64encode(challenge).rstrip(b"=").decode()
|
||||
|
||||
async with httpx.AsyncClient(timeout=30.0, follow_redirects=False) as http:
|
||||
resp = await http.get(
|
||||
f"{MCP_KEYCLOAK_BASE_URL}/oauth/authorize",
|
||||
params={
|
||||
"response_type": "code",
|
||||
"client_id": dcr_client["client_id"],
|
||||
"redirect_uri": "http://localhost:9999/callback",
|
||||
"state": "test-state",
|
||||
"scope": "openid",
|
||||
"code_challenge": code_challenge,
|
||||
"code_challenge_method": "S256",
|
||||
},
|
||||
)
|
||||
|
||||
# Should redirect to IdP (302) not error (400)
|
||||
assert resp.status_code == 302, (
|
||||
f"Expected redirect to IdP, got {resp.status_code}: {resp.text}"
|
||||
)
|
||||
|
||||
|
||||
async def test_dcr_client_deletion_via_keycloak(keycloak_mcp_available):
|
||||
"""Full DCR lifecycle: register via proxy, verify, delete via RFC 7592."""
|
||||
async with httpx.AsyncClient(timeout=30.0) as http:
|
||||
# Register
|
||||
resp = await http.post(
|
||||
f"{MCP_KEYCLOAK_BASE_URL}/oauth/register",
|
||||
json={
|
||||
"client_name": "test-dcr-lifecycle",
|
||||
"redirect_uris": ["http://localhost:9999/callback"],
|
||||
"grant_types": ["authorization_code"],
|
||||
"response_types": ["code"],
|
||||
},
|
||||
)
|
||||
assert resp.status_code in (200, 201)
|
||||
data = resp.json()
|
||||
client_id = data["client_id"]
|
||||
rat = data["registration_access_token"]
|
||||
|
||||
try:
|
||||
# Delete via Keycloak (external URL)
|
||||
del_resp = await http.delete(
|
||||
f"{KEYCLOAK_BASE_URL}/realms/{KEYCLOAK_REALM}"
|
||||
f"/clients-registrations/openid-connect/{client_id}",
|
||||
headers={"Authorization": f"Bearer {rat}"},
|
||||
)
|
||||
assert del_resp.status_code == 204
|
||||
except Exception:
|
||||
# Best-effort cleanup if assertion failed
|
||||
try:
|
||||
await http.delete(
|
||||
f"{KEYCLOAK_BASE_URL}/realms/{KEYCLOAK_REALM}"
|
||||
f"/clients-registrations/openid-connect/{client_id}",
|
||||
headers={"Authorization": f"Bearer {rat}"},
|
||||
)
|
||||
except Exception:
|
||||
pass
|
||||
raise
|
||||
|
||||
|
||||
# --- AS metadata tests ---
|
||||
|
||||
|
||||
async def test_as_metadata_advertises_registration_endpoint(keycloak_mcp_available):
|
||||
"""AS metadata should advertise /oauth/register for DCR discovery."""
|
||||
metadata = keycloak_mcp_available
|
||||
assert "registration_endpoint" in metadata
|
||||
assert metadata["registration_endpoint"].endswith("/oauth/register")
|
||||
|
||||
|
||||
async def test_as_metadata_has_required_fields(keycloak_mcp_available):
|
||||
"""Verify the AS metadata contains all RFC 8414 required fields."""
|
||||
metadata = keycloak_mcp_available
|
||||
required_fields = [
|
||||
"issuer",
|
||||
"authorization_endpoint",
|
||||
"token_endpoint",
|
||||
"response_types_supported",
|
||||
"grant_types_supported",
|
||||
"code_challenge_methods_supported",
|
||||
]
|
||||
for field in required_fields:
|
||||
assert field in metadata, f"Missing required field: {field}"
|
||||
|
||||
|
||||
# --- Static client / unknown client tests ---
|
||||
|
||||
|
||||
async def test_authorize_rejects_unknown_client(keycloak_mcp_available):
|
||||
"""Authorization endpoint should reject client_ids that are neither
|
||||
statically configured nor dynamically registered."""
|
||||
async with httpx.AsyncClient(timeout=30.0, follow_redirects=False) as http:
|
||||
resp = await http.get(
|
||||
f"{MCP_KEYCLOAK_BASE_URL}/oauth/authorize",
|
||||
params={
|
||||
"response_type": "code",
|
||||
"client_id": "nonexistent-client-id",
|
||||
"redirect_uri": "http://localhost:9999/callback",
|
||||
"state": "test-state",
|
||||
"scope": "openid",
|
||||
},
|
||||
)
|
||||
|
||||
# Should return an error (400 or redirect with error)
|
||||
assert resp.status_code in (400, 302)
|
||||
if resp.status_code == 400:
|
||||
data = resp.json()
|
||||
assert "error" in data
|
||||
else:
|
||||
# 302 redirect must carry an error parameter
|
||||
location = resp.headers.get("location", "")
|
||||
assert "error=" in location, (
|
||||
f"302 redirect should contain error= in Location, got: {location}"
|
||||
)
|
||||
@@ -0,0 +1,200 @@
|
||||
"""Unit tests for ClientRegistry ALLOWED_MCP_CLIENTS parsing and validation."""
|
||||
|
||||
import logging
|
||||
|
||||
import pytest
|
||||
|
||||
import nextcloud_mcp_server.auth.client_registry as registry_mod
|
||||
|
||||
pytestmark = pytest.mark.unit
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _reset_registry():
|
||||
"""Reset the singleton registry before each test."""
|
||||
registry_mod._registry = None
|
||||
yield
|
||||
registry_mod._registry = None
|
||||
|
||||
|
||||
def _get_registry(monkeypatch, value: str | None = None):
|
||||
"""Helper to create a registry with the given ALLOWED_MCP_CLIENTS value."""
|
||||
if value is not None:
|
||||
monkeypatch.setenv("ALLOWED_MCP_CLIENTS", value)
|
||||
else:
|
||||
monkeypatch.delenv("ALLOWED_MCP_CLIENTS", raising=False)
|
||||
return registry_mod.get_client_registry()
|
||||
|
||||
|
||||
def test_simple_client_ids(monkeypatch):
|
||||
registry = _get_registry(monkeypatch, "claude-desktop, zed-editor")
|
||||
clients = registry.list_clients()
|
||||
assert len(clients) == 2
|
||||
|
||||
claude = registry.get_client("claude-desktop")
|
||||
assert claude is not None
|
||||
assert claude.redirect_uris == ["http://localhost:*", "http://127.0.0.1:*"]
|
||||
assert claude.allowed_scopes == ["*"]
|
||||
|
||||
zed = registry.get_client("zed-editor")
|
||||
assert zed is not None
|
||||
assert zed.redirect_uris == ["http://localhost:*", "http://127.0.0.1:*"]
|
||||
|
||||
|
||||
def test_pipe_separated_https(monkeypatch):
|
||||
registry = _get_registry(monkeypatch, "myapp|https://app.example.com/callback")
|
||||
client = registry.get_client("myapp")
|
||||
assert client is not None
|
||||
assert client.redirect_uris == ["https://app.example.com/callback"]
|
||||
assert client.allowed_scopes == ["*"]
|
||||
|
||||
|
||||
def test_pipe_separated_localhost(monkeypatch):
|
||||
registry = _get_registry(monkeypatch, "dev-tool|http://localhost:3000/cb")
|
||||
client = registry.get_client("dev-tool")
|
||||
assert client is not None
|
||||
assert client.redirect_uris == ["http://localhost:3000/cb"]
|
||||
|
||||
|
||||
def test_pipe_separated_loopback_ip(monkeypatch):
|
||||
registry = _get_registry(monkeypatch, "dev|http://127.0.0.1:9090/cb")
|
||||
client = registry.get_client("dev")
|
||||
assert client is not None
|
||||
assert client.redirect_uris == ["http://127.0.0.1:9090/cb"]
|
||||
|
||||
|
||||
def test_mixed_entries(monkeypatch):
|
||||
registry = _get_registry(
|
||||
monkeypatch, "claude-desktop, cloud-app|https://cloud.example.com/cb"
|
||||
)
|
||||
clients = registry.list_clients()
|
||||
assert len(clients) == 2
|
||||
|
||||
claude = registry.get_client("claude-desktop")
|
||||
assert claude is not None
|
||||
assert claude.redirect_uris == ["http://localhost:*", "http://127.0.0.1:*"]
|
||||
|
||||
cloud = registry.get_client("cloud-app")
|
||||
assert cloud is not None
|
||||
assert cloud.redirect_uris == ["https://cloud.example.com/cb"]
|
||||
|
||||
|
||||
def test_http_non_localhost_rejected(monkeypatch, caplog):
|
||||
with caplog.at_level(logging.WARNING):
|
||||
registry = _get_registry(monkeypatch, "bad-client|http://evil.com/cb")
|
||||
|
||||
assert registry.get_client("bad-client") is None
|
||||
assert "Rejecting client" in caplog.text
|
||||
assert "evil.com" in caplog.text
|
||||
|
||||
|
||||
def test_empty_string_uses_well_known(monkeypatch):
|
||||
registry = _get_registry(monkeypatch, "")
|
||||
clients = registry.list_clients()
|
||||
client_ids = {c.client_id for c in clients}
|
||||
assert "claude-desktop" in client_ids
|
||||
assert "test-mcp-client" in client_ids
|
||||
|
||||
|
||||
def test_unset_env_uses_well_known(monkeypatch):
|
||||
registry = _get_registry(monkeypatch, None)
|
||||
clients = registry.list_clients()
|
||||
client_ids = {c.client_id for c in clients}
|
||||
assert "claude-desktop" in client_ids
|
||||
assert "test-mcp-client" in client_ids
|
||||
|
||||
|
||||
def test_malformed_entries_skipped_with_warning(monkeypatch, caplog):
|
||||
with caplog.at_level(logging.WARNING):
|
||||
registry = _get_registry(monkeypatch, "good, |, , bad|")
|
||||
|
||||
# Only "good" should be registered
|
||||
assert registry.get_client("good") is not None
|
||||
assert len(registry.list_clients()) == 1
|
||||
assert "malformed" in caplog.text.lower()
|
||||
|
||||
|
||||
def test_all_scopes_wildcard(monkeypatch):
|
||||
registry = _get_registry(monkeypatch, "test-client")
|
||||
client = registry.get_client("test-client")
|
||||
assert client is not None
|
||||
assert client.allowed_scopes == ["*"]
|
||||
|
||||
|
||||
def test_validate_client_wildcard_scopes(monkeypatch):
|
||||
registry = _get_registry(monkeypatch, "test-client")
|
||||
valid, err = registry.validate_client(
|
||||
"test-client", scopes=["anything", "goes", "here"]
|
||||
)
|
||||
assert valid is True
|
||||
assert err is None
|
||||
|
||||
|
||||
def test_validate_redirect_uri_https_match(monkeypatch):
|
||||
registry = _get_registry(monkeypatch, "cloud|https://x.com/cb")
|
||||
valid, err = registry.validate_client("cloud", redirect_uri="https://x.com/cb")
|
||||
assert valid is True
|
||||
assert err is None
|
||||
|
||||
|
||||
def test_validate_redirect_uri_https_mismatch(monkeypatch):
|
||||
registry = _get_registry(monkeypatch, "cloud|https://x.com/cb")
|
||||
valid, err = registry.validate_client("cloud", redirect_uri="https://other.com/cb")
|
||||
assert valid is False
|
||||
assert "redirect_uri" in err.lower()
|
||||
|
||||
|
||||
def test_validate_redirect_uri_localhost_wildcard(monkeypatch):
|
||||
registry = _get_registry(monkeypatch, "native-client")
|
||||
valid, err = registry.validate_client(
|
||||
"native-client", redirect_uri="http://localhost:12345/callback"
|
||||
)
|
||||
assert valid is True
|
||||
assert err is None
|
||||
|
||||
|
||||
def test_well_known_clients_wildcard_scopes(monkeypatch):
|
||||
registry = _get_registry(monkeypatch, None)
|
||||
for client in registry.list_clients():
|
||||
assert client.allowed_scopes == ["*"], (
|
||||
f"Well-known client {client.client_id} should have wildcard scopes"
|
||||
)
|
||||
|
||||
|
||||
def test_client_name_resolution(monkeypatch):
|
||||
registry = _get_registry(monkeypatch, "claude-desktop, custom-tool")
|
||||
assert registry.get_client("claude-desktop").name == "Claude Desktop"
|
||||
assert registry.get_client("custom-tool").name == "Custom Tool"
|
||||
|
||||
|
||||
def test_ipv6_loopback_allowed(monkeypatch):
|
||||
registry = _get_registry(monkeypatch, "ipv6-app|http://[::1]:3000/cb")
|
||||
client = registry.get_client("ipv6-app")
|
||||
assert client is not None
|
||||
assert client.redirect_uris == ["http://[::1]:3000/cb"]
|
||||
|
||||
|
||||
def test_malformed_uri_no_hostname_skipped(monkeypatch, caplog):
|
||||
with caplog.at_level(logging.WARNING):
|
||||
registry = _get_registry(monkeypatch, "bad|http:///no-host")
|
||||
|
||||
assert registry.get_client("bad") is None
|
||||
assert "cannot parse hostname" in caplog.text
|
||||
|
||||
|
||||
def test_validate_redirect_uri_ipv6_loopback(monkeypatch):
|
||||
"""IPv6 loopback redirect URIs should match wildcard localhost patterns."""
|
||||
registry = _get_registry(monkeypatch, "ipv6-app|http://[::1]:3000/cb")
|
||||
valid, err = registry.validate_client(
|
||||
"ipv6-app", redirect_uri="http://[::1]:3000/cb"
|
||||
)
|
||||
assert valid is True
|
||||
assert err is None
|
||||
|
||||
|
||||
def test_validate_redirect_uri_no_hostname(monkeypatch):
|
||||
"""Redirect URIs with no parseable hostname should be rejected."""
|
||||
registry = _get_registry(monkeypatch, "test-client")
|
||||
valid, err = registry.validate_client("test-client", redirect_uri="not-a-uri")
|
||||
assert valid is False
|
||||
assert "redirect_uri" in err.lower()
|
||||
@@ -0,0 +1,65 @@
|
||||
"""Unit tests for DCR proxy registration_not_supported path."""
|
||||
|
||||
import json
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from nextcloud_mcp_server.auth.oauth_routes import oauth_register_proxy
|
||||
|
||||
pytestmark = pytest.mark.unit
|
||||
|
||||
|
||||
def _make_request(body: dict, oauth_config: dict) -> MagicMock:
|
||||
"""Create a mock Starlette Request."""
|
||||
request = AsyncMock()
|
||||
request.json = AsyncMock(return_value=body)
|
||||
request.client = MagicMock()
|
||||
request.client.host = "127.0.0.1"
|
||||
request.app = MagicMock()
|
||||
request.app.state.oauth_context = {"config": oauth_config}
|
||||
return request
|
||||
|
||||
|
||||
_DCR_BODY = {
|
||||
"client_name": "test",
|
||||
"redirect_uris": ["http://localhost:9999/cb"],
|
||||
}
|
||||
|
||||
|
||||
async def test_registration_not_supported_when_no_endpoint():
|
||||
"""When discovery doc lacks registration_endpoint, return 400."""
|
||||
request = _make_request(
|
||||
body=_DCR_BODY,
|
||||
oauth_config={
|
||||
"discovery_url": "https://idp.example.com/.well-known/openid-configuration"
|
||||
},
|
||||
)
|
||||
|
||||
discovery_doc = {
|
||||
"issuer": "https://idp.example.com",
|
||||
"authorization_endpoint": "https://idp.example.com/auth",
|
||||
}
|
||||
|
||||
with patch(
|
||||
"nextcloud_mcp_server.auth.oauth_routes._get_cached_discovery",
|
||||
new_callable=AsyncMock,
|
||||
return_value=discovery_doc,
|
||||
):
|
||||
response = await oauth_register_proxy(request)
|
||||
|
||||
assert response.status_code == 400
|
||||
body = json.loads(response.body)
|
||||
assert body["error"] == "registration_not_supported"
|
||||
assert "ALLOWED_MCP_CLIENTS" in body["error_description"]
|
||||
|
||||
|
||||
async def test_registration_not_supported_when_no_discovery_url():
|
||||
"""When no discovery_url is configured, return 400."""
|
||||
request = _make_request(body=_DCR_BODY, oauth_config={})
|
||||
|
||||
response = await oauth_register_proxy(request)
|
||||
|
||||
assert response.status_code == 400
|
||||
body = json.loads(response.body)
|
||||
assert body["error"] == "registration_not_supported"
|
||||
Reference in New Issue
Block a user