refactor: Extract storage helper and improve PHP error handling
Management API: - Extract _get_app_password_storage() helper function - Reduces code duplication across 3 endpoints - Adds TYPE_CHECKING import for type hints PHP CredentialsController: - Add partial_success field to distinguish full vs partial success - Add local_storage and mcp_sync boolean fields for clarity - Rename 'warning' to 'mcp_error' for consistency - Improves UI feedback when MCP server sync fails Response structure now clearly indicates: - Full success: partial_success=false, local_storage=true, mcp_sync=true - Partial success: partial_success=true, local_storage=true, mcp_sync=false - Full failure: success=false (unchanged) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -13,9 +13,12 @@ The PHP app obtains tokens through PKCE flow and uses them to access these endpo
|
||||
import logging
|
||||
import time
|
||||
from importlib.metadata import version
|
||||
from typing import Any
|
||||
from typing import TYPE_CHECKING, Any
|
||||
|
||||
import httpx
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from nextcloud_mcp_server.auth.storage import RefreshTokenStorage
|
||||
from starlette.requests import Request
|
||||
from starlette.responses import JSONResponse
|
||||
|
||||
@@ -181,6 +184,31 @@ def _validate_query_string(query: str, max_length: int = 10000) -> None:
|
||||
raise ValueError(f"Query too long: maximum {max_length} characters")
|
||||
|
||||
|
||||
async def _get_app_password_storage(request: Request) -> "RefreshTokenStorage":
|
||||
"""Get or initialize RefreshTokenStorage for app password operations.
|
||||
|
||||
Checks app.state.storage first, then falls back to creating from environment.
|
||||
This helper avoids repeated storage initialization logic across endpoints.
|
||||
|
||||
Args:
|
||||
request: Starlette request with app state
|
||||
|
||||
Returns:
|
||||
Initialized RefreshTokenStorage instance
|
||||
"""
|
||||
from nextcloud_mcp_server.auth.storage import RefreshTokenStorage
|
||||
|
||||
storage = getattr(request.app.state, "storage", None)
|
||||
|
||||
if not storage:
|
||||
# Multi-user BasicAuth mode may not have oauth_context
|
||||
# Initialize storage from environment
|
||||
storage = RefreshTokenStorage.from_env()
|
||||
await storage.initialize()
|
||||
|
||||
return storage
|
||||
|
||||
|
||||
async def get_server_status(request: Request) -> JSONResponse:
|
||||
"""GET /api/v1/status - Server status and version.
|
||||
|
||||
@@ -636,17 +664,7 @@ async def provision_app_password(request: Request) -> JSONResponse:
|
||||
|
||||
# Store the validated app password
|
||||
try:
|
||||
# Get storage from app state or create from env
|
||||
storage = getattr(request.app.state, "storage", None)
|
||||
|
||||
if not storage:
|
||||
# Multi-user BasicAuth mode may not have oauth_context
|
||||
# Initialize storage from environment
|
||||
from nextcloud_mcp_server.auth.storage import RefreshTokenStorage
|
||||
|
||||
storage = RefreshTokenStorage.from_env()
|
||||
await storage.initialize()
|
||||
|
||||
storage = await _get_app_password_storage(request)
|
||||
await storage.store_app_password(username, app_password)
|
||||
|
||||
logger.info(f"Provisioned app password for user: {username}")
|
||||
@@ -710,16 +728,7 @@ async def get_app_password_status(request: Request) -> JSONResponse:
|
||||
)
|
||||
|
||||
try:
|
||||
# Get storage
|
||||
storage = getattr(request.app.state, "storage", None)
|
||||
|
||||
if not storage:
|
||||
from nextcloud_mcp_server.auth.storage import RefreshTokenStorage
|
||||
|
||||
storage = RefreshTokenStorage.from_env()
|
||||
await storage.initialize()
|
||||
|
||||
# Check if app password exists
|
||||
storage = await _get_app_password_storage(request)
|
||||
app_password = await storage.get_app_password(username)
|
||||
|
||||
return JSONResponse(
|
||||
@@ -810,16 +819,7 @@ async def delete_app_password(request: Request) -> JSONResponse:
|
||||
)
|
||||
|
||||
try:
|
||||
# Get storage
|
||||
storage = getattr(request.app.state, "storage", None)
|
||||
|
||||
if not storage:
|
||||
from nextcloud_mcp_server.auth.storage import RefreshTokenStorage
|
||||
|
||||
storage = RefreshTokenStorage.from_env()
|
||||
await storage.initialize()
|
||||
|
||||
# Delete app password
|
||||
storage = await _get_app_password_storage(request)
|
||||
deleted = await storage.delete_app_password(username)
|
||||
|
||||
if deleted:
|
||||
|
||||
@@ -115,6 +115,9 @@ class CredentialsController extends Controller {
|
||||
$this->logger->warning("MCP server URL not configured, app password stored locally only");
|
||||
return new JSONResponse([
|
||||
'success' => true,
|
||||
'partial_success' => true,
|
||||
'local_storage' => true,
|
||||
'mcp_sync' => false,
|
||||
'message' => 'App password saved locally (MCP server not configured)'
|
||||
], Http::STATUS_OK);
|
||||
}
|
||||
@@ -143,27 +146,36 @@ class CredentialsController extends Controller {
|
||||
$this->logger->info("Successfully provisioned app password to MCP server for user: $userId");
|
||||
return new JSONResponse([
|
||||
'success' => true,
|
||||
'partial_success' => false,
|
||||
'local_storage' => true,
|
||||
'mcp_sync' => true,
|
||||
'message' => 'App password saved successfully'
|
||||
], Http::STATUS_OK);
|
||||
} else {
|
||||
$error = $body['error'] ?? 'Unknown error';
|
||||
$this->logger->error("MCP server rejected app password for user $userId: $error");
|
||||
// Still return success since it was stored locally
|
||||
// Return partial success since it was stored locally but MCP sync failed
|
||||
return new JSONResponse([
|
||||
'success' => true,
|
||||
'partial_success' => true,
|
||||
'local_storage' => true,
|
||||
'mcp_sync' => false,
|
||||
'message' => 'App password saved locally (MCP server sync failed)',
|
||||
'warning' => $error
|
||||
'mcp_error' => $error
|
||||
], Http::STATUS_OK);
|
||||
}
|
||||
} catch (\Exception $e) {
|
||||
$this->logger->error("Failed to send app password to MCP server for user $userId", [
|
||||
'error' => $e->getMessage()
|
||||
]);
|
||||
// Still return success since it was stored locally
|
||||
// Return partial success since it was stored locally but MCP was unreachable
|
||||
return new JSONResponse([
|
||||
'success' => true,
|
||||
'partial_success' => true,
|
||||
'local_storage' => true,
|
||||
'mcp_sync' => false,
|
||||
'message' => 'App password saved locally (MCP server unreachable)',
|
||||
'warning' => $e->getMessage()
|
||||
'mcp_error' => $e->getMessage()
|
||||
], Http::STATUS_OK);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user