fix: resolve stale credentials causing astrolabe background sync test failures
The revoke test failed because it only completed Step 2 (app password) but not Step 1 (OAuth authorization). In hybrid mode, Astrolabe requires both steps for $isFullyConfigured=true, which gates the "Revoke Access" button. Changes: - Use complete_astrolabe_authorization() in revoke test for full two-step flow - Add stale state cleanup (app passwords, bruteforce entries, Astrolabe prefs) to both enablement and revoke tests - Add startup cleanup of invalid app passwords in BasicAuth mode - Pre-validate credentials before entering scanner loop to fail fast - Handle 401/403/429 in scanner with proper backoff and circuit breaking - Clean up app passwords in test_users_setup fixture teardown Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -1826,6 +1826,19 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None =
|
||||
f"Cannot start vector sync - Qdrant initialization failed: {e}"
|
||||
) from e
|
||||
|
||||
# Clean up stale app passwords at startup (BasicAuth mode only)
|
||||
if not oauth_enabled:
|
||||
try:
|
||||
removed = await token_storage.cleanup_invalid_app_passwords(
|
||||
nextcloud_host=nextcloud_host_for_sync
|
||||
)
|
||||
if removed:
|
||||
logger.info(
|
||||
f"Cleaned up {len(removed)} stale app password(s): {removed}"
|
||||
)
|
||||
except Exception as e:
|
||||
logger.warning(f"App password cleanup failed (non-fatal): {e}")
|
||||
|
||||
# Initialize shared state
|
||||
send_stream, receive_stream = anyio.create_memory_object_stream(
|
||||
max_buffer_size=settings.vector_sync_queue_max_size
|
||||
|
||||
@@ -1414,6 +1414,67 @@ class RefreshTokenStorage:
|
||||
logger.debug(f"Found {len(user_ids)} users with app passwords")
|
||||
return user_ids
|
||||
|
||||
async def cleanup_invalid_app_passwords(self, nextcloud_host: str) -> list[str]:
|
||||
"""
|
||||
Validate stored app passwords against Nextcloud and remove invalid ones.
|
||||
|
||||
Makes a lightweight OCS request for each stored user to check if credentials
|
||||
are still valid. Removes entries that return 401/403.
|
||||
|
||||
Args:
|
||||
nextcloud_host: Nextcloud base URL
|
||||
|
||||
Returns:
|
||||
List of user IDs whose app passwords were removed
|
||||
"""
|
||||
import httpx
|
||||
|
||||
if not self._initialized:
|
||||
await self.initialize()
|
||||
|
||||
user_ids = await self.get_all_app_password_user_ids()
|
||||
if not user_ids:
|
||||
return []
|
||||
|
||||
removed: list[str] = []
|
||||
|
||||
for user_id in user_ids:
|
||||
app_password = await self.get_app_password(user_id)
|
||||
if not app_password:
|
||||
continue
|
||||
|
||||
try:
|
||||
async with httpx.AsyncClient(
|
||||
base_url=nextcloud_host,
|
||||
auth=httpx.BasicAuth(user_id, app_password),
|
||||
timeout=10.0,
|
||||
) as client:
|
||||
response = await client.get(
|
||||
"/ocs/v2.php/cloud/user",
|
||||
headers={
|
||||
"OCS-APIRequest": "true",
|
||||
"Accept": "application/json",
|
||||
},
|
||||
)
|
||||
|
||||
if response.status_code in (401, 403):
|
||||
logger.info(
|
||||
f"App password for {user_id} is invalid "
|
||||
f"(HTTP {response.status_code}), removing"
|
||||
)
|
||||
await self.delete_app_password(user_id)
|
||||
removed.append(user_id)
|
||||
else:
|
||||
logger.debug(
|
||||
f"App password for {user_id} validated "
|
||||
f"(HTTP {response.status_code})"
|
||||
)
|
||||
|
||||
except Exception as e:
|
||||
logger.warning(f"Could not validate app password for {user_id}: {e}")
|
||||
|
||||
return removed
|
||||
|
||||
|
||||
async def generate_encryption_key() -> str:
|
||||
"""
|
||||
|
||||
@@ -31,7 +31,7 @@ from anyio.streams.memory import (
|
||||
MemoryObjectReceiveStream,
|
||||
MemoryObjectSendStream,
|
||||
)
|
||||
from httpx import BasicAuth
|
||||
from httpx import BasicAuth, HTTPStatusError
|
||||
|
||||
from nextcloud_mcp_server.client import NextcloudClient
|
||||
from nextcloud_mcp_server.config import get_settings
|
||||
@@ -210,9 +210,36 @@ async def user_scanner_task(
|
||||
mode_label = "BasicAuth" if use_basic_auth else "OAuth"
|
||||
logger.info(f"[{mode_label}] Scanner started for user: {user_id}")
|
||||
settings = get_settings()
|
||||
max_consecutive_errors = 5
|
||||
|
||||
task_status.started()
|
||||
|
||||
# Pre-validate credentials before entering scan loop
|
||||
try:
|
||||
nc_client = await get_user_client(
|
||||
user_id, token_broker, nextcloud_host, use_basic_auth=use_basic_auth
|
||||
)
|
||||
try:
|
||||
await nc_client.capabilities() # Lightweight OCS call to validate creds
|
||||
logger.info(f"[{mode_label}] Credentials validated for {user_id}")
|
||||
except HTTPStatusError as e:
|
||||
if e.response.status_code in (401, 403):
|
||||
logger.warning(
|
||||
f"[{mode_label}] Credential validation failed for {user_id} "
|
||||
f"(HTTP {e.response.status_code}), not starting scan loop"
|
||||
)
|
||||
return
|
||||
raise
|
||||
finally:
|
||||
await nc_client.close()
|
||||
except NotProvisionedError:
|
||||
logger.warning(
|
||||
f"[{mode_label}] User {user_id} not provisioned, not starting scan loop"
|
||||
)
|
||||
return
|
||||
|
||||
consecutive_errors = 0
|
||||
|
||||
while not shutdown_event.is_set():
|
||||
nc_client = None
|
||||
try:
|
||||
@@ -228,21 +255,61 @@ async def user_scanner_task(
|
||||
nc_client=nc_client,
|
||||
)
|
||||
|
||||
consecutive_errors = 0 # Reset on success
|
||||
|
||||
except NotProvisionedError:
|
||||
logger.warning(
|
||||
f"[{mode_label}] User {user_id} no longer provisioned, stopping scanner"
|
||||
)
|
||||
break
|
||||
|
||||
except HTTPStatusError as e:
|
||||
status_code = e.response.status_code
|
||||
if status_code in (401, 403):
|
||||
logger.warning(
|
||||
f"[{mode_label}] Scanner auth failed for {user_id} "
|
||||
f"(HTTP {status_code}), stopping scanner. "
|
||||
f"User may need to re-provision credentials."
|
||||
)
|
||||
break
|
||||
elif status_code == 429:
|
||||
logger.warning(
|
||||
f"[{mode_label}] Scanner rate-limited for {user_id}, "
|
||||
f"backing off 60s"
|
||||
)
|
||||
try:
|
||||
with anyio.move_on_after(60):
|
||||
await shutdown_event.wait()
|
||||
except anyio.get_cancelled_exc_class():
|
||||
break
|
||||
continue
|
||||
else:
|
||||
consecutive_errors += 1
|
||||
logger.error(
|
||||
f"[{mode_label}] Scanner HTTP error for {user_id}: {e} "
|
||||
f"({consecutive_errors}/{max_consecutive_errors})",
|
||||
exc_info=True,
|
||||
)
|
||||
|
||||
except Exception as e:
|
||||
consecutive_errors += 1
|
||||
logger.error(
|
||||
f"[{mode_label}] Scanner error for {user_id}: {e}", exc_info=True
|
||||
f"[{mode_label}] Scanner error for {user_id}: {e} "
|
||||
f"({consecutive_errors}/{max_consecutive_errors})",
|
||||
exc_info=True,
|
||||
)
|
||||
|
||||
finally:
|
||||
if nc_client:
|
||||
await nc_client.close()
|
||||
|
||||
if consecutive_errors >= max_consecutive_errors:
|
||||
logger.error(
|
||||
f"[{mode_label}] Scanner for {user_id} hit {max_consecutive_errors} "
|
||||
f"consecutive errors, stopping scanner"
|
||||
)
|
||||
break
|
||||
|
||||
# Sleep until next interval or wake event
|
||||
try:
|
||||
with anyio.move_on_after(settings.vector_sync_scan_interval):
|
||||
|
||||
@@ -2400,6 +2400,30 @@ async def test_users_setup(anyio_backend, nc_client: NextcloudClient):
|
||||
except Exception as e:
|
||||
logger.warning(f"Error deleting test user {username}: {e}")
|
||||
|
||||
# Clean up app passwords from MCP server to prevent stale scanners
|
||||
for username in created_users:
|
||||
try:
|
||||
import subprocess
|
||||
|
||||
subprocess.run(
|
||||
[
|
||||
"docker",
|
||||
"compose",
|
||||
"exec",
|
||||
"-T",
|
||||
"mcp-multi-user-basic",
|
||||
"sqlite3",
|
||||
"/app/data/tokens.db",
|
||||
f"DELETE FROM app_passwords WHERE user_id = '{username}';",
|
||||
],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=10,
|
||||
)
|
||||
logger.info(f"Cleaned up app password for {username}")
|
||||
except Exception as e:
|
||||
logger.debug(f"App password cleanup for {username}: {e}")
|
||||
|
||||
|
||||
async def _get_oauth_token_for_user(
|
||||
browser,
|
||||
|
||||
@@ -861,6 +861,43 @@ async def test_multi_user_astrolabe_background_sync_enablement(
|
||||
This tests ADR-002 Tier 2 authentication: User-specific app passwords for background operations
|
||||
in multi-user BasicAuth deployments.
|
||||
"""
|
||||
# Clear stale state from previous test runs
|
||||
logger.info("Clearing stale app passwords and bruteforce entries...")
|
||||
subprocess.run(
|
||||
[
|
||||
"docker",
|
||||
"compose",
|
||||
"exec",
|
||||
"-T",
|
||||
"mcp-multi-user-basic",
|
||||
"sqlite3",
|
||||
"/app/data/tokens.db",
|
||||
"DELETE FROM app_passwords;",
|
||||
],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=10,
|
||||
)
|
||||
subprocess.run(
|
||||
[
|
||||
"docker",
|
||||
"compose",
|
||||
"exec",
|
||||
"-T",
|
||||
"db",
|
||||
"mariadb",
|
||||
"-u",
|
||||
"root",
|
||||
"-ppassword",
|
||||
"nextcloud",
|
||||
"-e",
|
||||
"DELETE FROM oc_bruteforce_attempts;",
|
||||
],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=10,
|
||||
)
|
||||
|
||||
# Configure Astrolabe to point to the mcp-multi-user-basic server
|
||||
logger.info("Configuring Astrolabe for mcp-multi-user-basic server...")
|
||||
await configure_astrolabe_for_mcp_server(
|
||||
@@ -1198,6 +1235,64 @@ async def test_revoke_background_sync_access(
|
||||
This tests the fix for the issue where POST requests to the revoke endpoint
|
||||
were returning errors due to HTTP method mismatch (was DELETE, now POST).
|
||||
"""
|
||||
# Clear stale state from previous test runs
|
||||
logger.info(
|
||||
"Clearing stale app passwords, bruteforce entries, and Astrolabe preferences..."
|
||||
)
|
||||
subprocess.run(
|
||||
[
|
||||
"docker",
|
||||
"compose",
|
||||
"exec",
|
||||
"-T",
|
||||
"mcp-multi-user-basic",
|
||||
"sqlite3",
|
||||
"/app/data/tokens.db",
|
||||
"DELETE FROM app_passwords;",
|
||||
],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=10,
|
||||
)
|
||||
subprocess.run(
|
||||
[
|
||||
"docker",
|
||||
"compose",
|
||||
"exec",
|
||||
"-T",
|
||||
"db",
|
||||
"mariadb",
|
||||
"-u",
|
||||
"root",
|
||||
"-ppassword",
|
||||
"nextcloud",
|
||||
"-e",
|
||||
"DELETE FROM oc_bruteforce_attempts;",
|
||||
],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=10,
|
||||
)
|
||||
subprocess.run(
|
||||
[
|
||||
"docker",
|
||||
"compose",
|
||||
"exec",
|
||||
"-T",
|
||||
"db",
|
||||
"mariadb",
|
||||
"-u",
|
||||
"root",
|
||||
"-ppassword",
|
||||
"nextcloud",
|
||||
"-e",
|
||||
"DELETE FROM oc_preferences WHERE appid = 'astrolabe';",
|
||||
],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=10,
|
||||
)
|
||||
|
||||
# Configure Astrolabe to point to the mcp-multi-user-basic server
|
||||
logger.info("Configuring Astrolabe for mcp-multi-user-basic server...")
|
||||
await configure_astrolabe_for_mcp_server(
|
||||
@@ -1218,9 +1313,14 @@ async def test_revoke_background_sync_access(
|
||||
# Step 1: Login to Nextcloud
|
||||
await login_to_nextcloud(page, username, password)
|
||||
|
||||
# Step 2: Generate app password and enable background sync
|
||||
app_password = await generate_app_password(page, username)
|
||||
await enable_background_sync_via_app_password(page, username, app_password)
|
||||
# Step 2: Complete full authorization (OAuth Step 1 + App Password Step 2)
|
||||
auth_result = await complete_astrolabe_authorization(page, username, password)
|
||||
assert auth_result["step1"], (
|
||||
f"OAuth authorization (Step 1) failed for {username}"
|
||||
)
|
||||
assert auth_result["step2"], (
|
||||
f"App password setup (Step 2) failed for {username}"
|
||||
)
|
||||
|
||||
# Step 3: Verify background sync is enabled
|
||||
assert await verify_app_password_created(username), (
|
||||
|
||||
Reference in New Issue
Block a user