fix(oauth): Remove the option to force_register new clients
This commit is contained in:
@@ -135,16 +135,15 @@ Each Nextcloud app has a corresponding server module that:
|
|||||||
OAuth integration tests support both **automated** (Playwright) and **interactive** authentication flows:
|
OAuth integration tests support both **automated** (Playwright) and **interactive** authentication flows:
|
||||||
|
|
||||||
**Automated Testing (Default - Recommended for CI/CD):**
|
**Automated Testing (Default - Recommended for CI/CD):**
|
||||||
- **Default fixtures**: `nc_oauth_client`, `nc_mcp_oauth_client` now use Playwright automation by default
|
- **Default fixtures**: `nc_oauth_client`, `nc_mcp_oauth_client` use Playwright automation
|
||||||
- Uses Playwright headless browser automation to complete OAuth flow programmatically
|
- Uses Playwright headless browser automation to complete OAuth flow programmatically
|
||||||
- **Shared OAuth Client**: All test users authenticate using a single OAuth client (matching MCP server behavior)
|
- **Shared OAuth Client**: All test users authenticate using a single OAuth client
|
||||||
- Single `client_id`/`client_secret` pair is registered and reused for all test users
|
- Stored in `.nextcloud_oauth_shared_test_client.json`
|
||||||
- Stored in `.nextcloud_oauth_shared_test_client.json` with `force_register=False` for reuse
|
- Matches production MCP server behavior
|
||||||
- Reduces OAuth client registrations and matches production MCP server architecture
|
- Each user gets their own unique access token
|
||||||
|
- Implementation: `shared_oauth_client_credentials` fixture in `tests/conftest.py:812`
|
||||||
- All Playwright fixtures: `playwright_oauth_token`, `nc_oauth_client`, `nc_mcp_oauth_client`, `nc_oauth_client_playwright`, `nc_mcp_oauth_client_playwright`
|
- All Playwright fixtures: `playwright_oauth_token`, `nc_oauth_client`, `nc_mcp_oauth_client`, `nc_oauth_client_playwright`, `nc_mcp_oauth_client_playwright`
|
||||||
- Multi-user fixtures: `alice_oauth_token`, `bob_oauth_token`, `charlie_oauth_token`, `diana_oauth_token`
|
- Multi-user fixtures: `alice_oauth_token`, `bob_oauth_token`, `charlie_oauth_token`, `diana_oauth_token`
|
||||||
- All use `shared_oauth_client_credentials` fixture for consistent client credentials
|
|
||||||
- Each user gets unique access tokens via same OAuth client (like multiple users using the MCP server)
|
|
||||||
- Requires: `NEXTCLOUD_HOST`, `NEXTCLOUD_USERNAME`, `NEXTCLOUD_PASSWORD` environment variables
|
- Requires: `NEXTCLOUD_HOST`, `NEXTCLOUD_USERNAME`, `NEXTCLOUD_PASSWORD` environment variables
|
||||||
- Uses `pytest-playwright-asyncio` for async Playwright fixtures
|
- Uses `pytest-playwright-asyncio` for async Playwright fixtures
|
||||||
- Playwright configuration: Use pytest CLI args like `--browser firefox --headed` to customize
|
- Playwright configuration: Use pytest CLI args like `--browser firefox --headed` to customize
|
||||||
@@ -179,14 +178,12 @@ OAuth integration tests support both **automated** (Playwright) and **interactiv
|
|||||||
- `mcp-oauth` (port 8001): Uses OAuth authentication - for OAuth-specific testing
|
- `mcp-oauth` (port 8001): Uses OAuth authentication - for OAuth-specific testing
|
||||||
- Start OAuth MCP server: `docker-compose up --build -d mcp-oauth`
|
- Start OAuth MCP server: `docker-compose up --build -d mcp-oauth`
|
||||||
- **Important**: When working on OAuth functionality, always rebuild `mcp-oauth` container, not `mcp`
|
- **Important**: When working on OAuth functionality, always rebuild `mcp-oauth` container, not `mcp`
|
||||||
- Shared OAuth client is registered once and reused across test runs
|
- OAuth client credentials cached in `.nextcloud_oauth_shared_test_client.json`
|
||||||
- Client credentials cached in `.nextcloud_oauth_shared_test_client.json`
|
|
||||||
|
|
||||||
**CI/CD Considerations:**
|
**CI/CD Considerations:**
|
||||||
- Interactive OAuth tests are automatically skipped when `GITHUB_ACTIONS` environment variable is set
|
- Interactive OAuth tests are automatically skipped when `GITHUB_ACTIONS` environment variable is set
|
||||||
- Automated Playwright tests will run in CI/CD environments
|
- Automated Playwright tests will run in CI/CD environments
|
||||||
- Use Firefox browser in CI: `--browser firefox` (Chromium may have issues with localhost redirects)
|
- Use Firefox browser in CI: `--browser firefox` (Chromium may have issues with localhost redirects)
|
||||||
- Shared client approach reduces test time and API calls to Nextcloud
|
|
||||||
|
|
||||||
### Configuration Files
|
### Configuration Files
|
||||||
|
|
||||||
|
|||||||
@@ -217,11 +217,12 @@ NEXTCLOUD_HOST=https://nextcloud.example.com
|
|||||||
|
|
||||||
**How it works**:
|
**How it works**:
|
||||||
1. Server checks `/.well-known/openid-configuration` for `registration_endpoint`
|
1. Server checks `/.well-known/openid-configuration` for `registration_endpoint`
|
||||||
2. Calls `/apps/oidc/register` to register new client
|
2. Calls `/apps/oidc/register` to register a client on first startup
|
||||||
3. Saves credentials to `.nextcloud_oauth_client.json`
|
3. Saves credentials to `.nextcloud_oauth_client.json`
|
||||||
4. Re-registers if credentials expire
|
4. Reuses these credentials on subsequent startups
|
||||||
|
5. Re-registers only if credentials are missing or expired
|
||||||
|
|
||||||
**Best for**: Development, testing, short-lived deployments
|
**Best for**: Development, testing, quick deployments
|
||||||
|
|
||||||
### Pre-configured Client
|
### Pre-configured Client
|
||||||
|
|
||||||
|
|||||||
+7
-7
@@ -165,23 +165,23 @@ You have two options for managing OAuth clients:
|
|||||||
|
|
||||||
### Mode A: Automatic Registration (Dynamic Client Registration)
|
### Mode A: Automatic Registration (Dynamic Client Registration)
|
||||||
|
|
||||||
**Best for**: Development, testing, short-lived deployments
|
**Best for**: Development, testing, quick deployments
|
||||||
|
|
||||||
**How it works**:
|
**How it works**:
|
||||||
- MCP server automatically registers OAuth client at startup
|
- MCP server automatically registers an OAuth client on first startup
|
||||||
- Uses Nextcloud's dynamic client registration endpoint
|
- Uses Nextcloud's dynamic client registration endpoint
|
||||||
- Saves credentials to `.nextcloud_oauth_client.json`
|
- Saves credentials to `.nextcloud_oauth_client.json`
|
||||||
|
- Reuses stored credentials on subsequent restarts
|
||||||
- Re-registers automatically if credentials expire
|
- Re-registers automatically if credentials expire
|
||||||
|
|
||||||
**Pros**:
|
**Pros**:
|
||||||
- Zero configuration required
|
- Zero configuration required
|
||||||
- Quick setup
|
- Quick setup
|
||||||
- No manual client management
|
- Automatic credential management
|
||||||
|
|
||||||
**Cons**:
|
**Cons**:
|
||||||
- Clients expire (default: 1 hour, configurable)
|
- Clients expire (default: 1 hour, configurable)
|
||||||
- Must re-register on restart if expired
|
- Must have dynamic client registration enabled on Nextcloud
|
||||||
- Not ideal for long-running production
|
|
||||||
|
|
||||||
**Configuration**: Skip to [Step 4](#step-4-configure-mcp-server) with minimal config.
|
**Configuration**: Skip to [Step 4](#step-4-configure-mcp-server) with minimal config.
|
||||||
|
|
||||||
@@ -192,8 +192,8 @@ You have two options for managing OAuth clients:
|
|||||||
**Best for**: Production, long-running deployments, stable environments
|
**Best for**: Production, long-running deployments, stable environments
|
||||||
|
|
||||||
**How it works**:
|
**How it works**:
|
||||||
- You manually register OAuth client via Nextcloud CLI
|
- You manually register an OAuth client via Nextcloud CLI
|
||||||
- Provide client credentials to MCP server
|
- Provide client credentials to MCP server via environment variables
|
||||||
- Credentials don't expire
|
- Credentials don't expire
|
||||||
|
|
||||||
**Pros**:
|
**Pros**:
|
||||||
|
|||||||
@@ -151,11 +151,11 @@ curl https://your.nextcloud.instance.com/.well-known/openid-configuration
|
|||||||
This quick start uses **automatic client registration** which is perfect for:
|
This quick start uses **automatic client registration** which is perfect for:
|
||||||
- Development
|
- Development
|
||||||
- Testing
|
- Testing
|
||||||
- Short-lived deployments
|
- Quick deployments
|
||||||
|
|
||||||
For **production deployments**, you should:
|
For **production deployments**, consider:
|
||||||
1. Pre-register OAuth clients manually
|
1. Pre-registering OAuth client manually
|
||||||
2. Use dedicated client credentials
|
2. Using dedicated client credentials that don't expire
|
||||||
3. See [OAuth Setup Guide](oauth-setup.md) for production configuration
|
3. See [OAuth Setup Guide](oauth-setup.md) for production configuration
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|||||||
@@ -211,7 +211,6 @@ async def load_or_register_client(
|
|||||||
storage_path: str | Path,
|
storage_path: str | Path,
|
||||||
client_name: str = "Nextcloud MCP Server",
|
client_name: str = "Nextcloud MCP Server",
|
||||||
redirect_uris: list[str] | None = None,
|
redirect_uris: list[str] | None = None,
|
||||||
force_register: bool = True,
|
|
||||||
) -> ClientInfo:
|
) -> ClientInfo:
|
||||||
"""
|
"""
|
||||||
Load client from storage or register a new one if not found/expired.
|
Load client from storage or register a new one if not found/expired.
|
||||||
@@ -219,7 +218,7 @@ async def load_or_register_client(
|
|||||||
This function:
|
This function:
|
||||||
1. Checks for existing client credentials in storage
|
1. Checks for existing client credentials in storage
|
||||||
2. Validates the credentials are not expired
|
2. Validates the credentials are not expired
|
||||||
3. Registers a new client if needed
|
3. Registers a new client if needed (no stored credentials or expired)
|
||||||
4. Saves the new client credentials
|
4. Saves the new client credentials
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
@@ -228,7 +227,6 @@ async def load_or_register_client(
|
|||||||
storage_path: Path to store client credentials
|
storage_path: Path to store client credentials
|
||||||
client_name: Name of the client application
|
client_name: Name of the client application
|
||||||
redirect_uris: List of redirect URIs
|
redirect_uris: List of redirect URIs
|
||||||
force_register: Force registration even if valid credentials exist
|
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
ClientInfo with valid credentials
|
ClientInfo with valid credentials
|
||||||
@@ -239,11 +237,10 @@ async def load_or_register_client(
|
|||||||
"""
|
"""
|
||||||
storage_path = Path(storage_path)
|
storage_path = Path(storage_path)
|
||||||
|
|
||||||
# Try to load existing client unless forced to register
|
# Try to load existing client
|
||||||
if not force_register:
|
client_info = load_client_from_file(storage_path)
|
||||||
client_info = load_client_from_file(storage_path)
|
if client_info:
|
||||||
if client_info:
|
return client_info
|
||||||
return client_info
|
|
||||||
|
|
||||||
# Register new client
|
# Register new client
|
||||||
logger.info("Registering new OAuth client...")
|
logger.info("Registering new OAuth client...")
|
||||||
|
|||||||
+2
-2
@@ -22,8 +22,8 @@ asyncio_mode = "auto"
|
|||||||
asyncio_default_test_loop_scope = "session"
|
asyncio_default_test_loop_scope = "session"
|
||||||
asyncio_default_fixture_loop_scope = "session"
|
asyncio_default_fixture_loop_scope = "session"
|
||||||
log_cli = 1
|
log_cli = 1
|
||||||
log_cli_level = "INFO"
|
log_cli_level = "WARN"
|
||||||
log_level = "INFO"
|
log_level = "WARN"
|
||||||
markers = [
|
markers = [
|
||||||
"integration: marks tests as slow (deselect with '-m \"not slow\"')",
|
"integration: marks tests as slow (deselect with '-m \"not slow\"')",
|
||||||
"oauth: marks tests as oauth (deselect with '-m \"not oauth\"')"
|
"oauth: marks tests as oauth (deselect with '-m \"not oauth\"')"
|
||||||
|
|||||||
@@ -762,7 +762,6 @@ async def interactive_oauth_token(oauth_callback_server) -> str:
|
|||||||
registration_endpoint=registration_endpoint,
|
registration_endpoint=registration_endpoint,
|
||||||
storage_path=".nextcloud_oauth_shared_test_client.json",
|
storage_path=".nextcloud_oauth_shared_test_client.json",
|
||||||
redirect_uris=[callback_url],
|
redirect_uris=[callback_url],
|
||||||
force_register=False, # Reuse existing credentials if valid
|
|
||||||
)
|
)
|
||||||
|
|
||||||
# First, open Nextcloud login page to establish session
|
# First, open Nextcloud login page to establish session
|
||||||
@@ -852,7 +851,6 @@ async def shared_oauth_client_credentials():
|
|||||||
storage_path=".nextcloud_oauth_shared_test_client.json",
|
storage_path=".nextcloud_oauth_shared_test_client.json",
|
||||||
client_name="Nextcloud MCP Server - Shared Test Client",
|
client_name="Nextcloud MCP Server - Shared Test Client",
|
||||||
redirect_uris=[callback_url],
|
redirect_uris=[callback_url],
|
||||||
force_register=False, # Reuse existing credentials if valid
|
|
||||||
)
|
)
|
||||||
|
|
||||||
logger.info(f"Shared OAuth client ready: {client_info.client_id[:16]}...")
|
logger.info(f"Shared OAuth client ready: {client_info.client_id[:16]}...")
|
||||||
|
|||||||
Reference in New Issue
Block a user