Files
nextcloud-mcp-server/third_party/astroglobe/lib/Settings/Admin.php
T
Chris Coutinho 44391d3d1d fix: address critical code review issues (4 fixes)
This commit addresses 4 critical issues identified in code review:

1. **Token Rotation Race Condition** (token_broker.py)
   - Added per-user locking mechanism to prevent concurrent refresh token corruption
   - Implemented double-check pattern for cache after acquiring lock
   - Users can now safely refresh concurrently without token desync

2. **Hardcoded OAuth Client ID** (PHP files)
   - Made client ID configurable via `astroglobe_client_id` in system config
   - Updated McpServerClient to provide getClientId() method
   - Injected McpServerClient into IdpTokenRefresher and OAuthController
   - Updated admin settings UI to display client ID configuration status
   - App gracefully handles missing client ID with warnings in admin UI

3. **Missing Cache Invalidation** (management.py:revoke_user_access)
   - Added cache.invalidate() call when revoking user access
   - Ensures both storage AND cache are cleared atomically
   - Prevents stale cached tokens from being used after revocation

4. **Error Message Exposure** (management.py)
   - Created _sanitize_error_for_client() helper function
   - Updated all error handlers to log detailed errors internally
   - Returns generic messages to clients to prevent information leakage
   - Protects against exposing database paths, API URLs, tokens, etc.

All changes are backward compatible and preserve existing functionality.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
2025-12-18 00:01:54 +01:00

145 lines
4.0 KiB
PHP

<?php
declare(strict_types=1);
namespace OCA\Astroglobe\Settings;
use OCA\Astroglobe\AppInfo\Application;
use OCA\Astroglobe\Service\McpServerClient;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\AppFramework\Services\IInitialState;
use OCP\IConfig;
use OCP\Settings\ISettings;
/**
* Admin settings panel for Astroglobe.
*
* Displays semantic search service status, indexing metrics,
* configuration, and provides administrative controls.
*/
class Admin implements ISettings {
// Search settings keys and defaults
public const SETTING_SEARCH_ALGORITHM = 'search_algorithm';
public const SETTING_SEARCH_FUSION = 'search_fusion';
public const SETTING_SEARCH_SCORE_THRESHOLD = 'search_score_threshold';
public const SETTING_SEARCH_LIMIT = 'search_limit';
public const DEFAULT_SEARCH_ALGORITHM = 'hybrid';
public const DEFAULT_SEARCH_FUSION = 'rrf';
public const DEFAULT_SEARCH_SCORE_THRESHOLD = 0;
public const DEFAULT_SEARCH_LIMIT = 20;
private $client;
private $config;
private $initialState;
public function __construct(
McpServerClient $client,
IConfig $config,
IInitialState $initialState,
) {
$this->client = $client;
$this->config = $config;
$this->initialState = $initialState;
}
/**
* @return TemplateResponse
*/
public function getForm(): TemplateResponse {
// Fetch data from MCP server
$serverStatus = $this->client->getStatus();
$vectorSyncStatus = $this->client->getVectorSyncStatus();
// Get configuration from config.php
$serverUrl = $this->config->getSystemValue('mcp_server_url', '');
$apiKeyConfigured = !empty($this->config->getSystemValue('mcp_server_api_key', ''));
$clientId = $this->config->getSystemValue('astroglobe_client_id', '');
$clientIdConfigured = !empty($clientId);
$clientSecret = $this->config->getSystemValue('astroglobe_client_secret', '');
$clientSecretConfigured = !empty($clientSecret);
// Check for server connection error
if (isset($serverStatus['error'])) {
return new TemplateResponse(
Application::APP_ID,
'settings/error',
[
'error' => 'Cannot connect to MCP server',
'details' => $serverStatus['error'],
'server_url' => $serverUrl,
'help_text' => 'Ensure MCP server is running and accessible. Check config.php for correct mcp_server_url.',
],
TemplateResponse::RENDER_AS_BLANK
);
}
// Load search settings from app config
$searchSettings = [
'algorithm' => $this->config->getAppValue(
Application::APP_ID,
self::SETTING_SEARCH_ALGORITHM,
self::DEFAULT_SEARCH_ALGORITHM
),
'fusion' => $this->config->getAppValue(
Application::APP_ID,
self::SETTING_SEARCH_FUSION,
self::DEFAULT_SEARCH_FUSION
),
'scoreThreshold' => (int)$this->config->getAppValue(
Application::APP_ID,
self::SETTING_SEARCH_SCORE_THRESHOLD,
(string)self::DEFAULT_SEARCH_SCORE_THRESHOLD
),
'limit' => (int)$this->config->getAppValue(
Application::APP_ID,
self::SETTING_SEARCH_LIMIT,
(string)self::DEFAULT_SEARCH_LIMIT
),
];
// Provide initial state for Vue.js frontend (if needed)
$this->initialState->provideInitialState('server-data', [
'serverStatus' => $serverStatus,
'vectorSyncStatus' => $vectorSyncStatus,
'config' => [
'serverUrl' => $serverUrl,
'apiKeyConfigured' => $apiKeyConfigured,
],
'searchSettings' => $searchSettings,
]);
$parameters = [
'serverStatus' => $serverStatus,
'vectorSyncStatus' => $vectorSyncStatus,
'serverUrl' => $serverUrl,
'apiKeyConfigured' => $apiKeyConfigured,
'clientIdConfigured' => $clientIdConfigured,
'clientSecretConfigured' => $clientSecretConfigured,
'vectorSyncEnabled' => $serverStatus['vector_sync_enabled'] ?? false,
'searchSettings' => $searchSettings,
];
return new TemplateResponse(
Application::APP_ID,
'settings/admin',
$parameters,
TemplateResponse::RENDER_AS_BLANK
);
}
/**
* @return string The section ID
*/
public function getSection(): string {
return 'astroglobe';
}
/**
* @return int Priority (lower = higher up)
*/
public function getPriority(): int {
return 10;
}
}