Commit Graph

8 Commits

Author SHA1 Message Date
Chris Coutinho 47fb562326 fix: replace assert with proper guard and invalidate scope cache after provisioning
Replace `assert entry.code_challenge` with a proper if-guard returning a
500 JSON error in the token endpoint, since Python's -O flag strips
asserts and would silently disable PKCE enforcement.

Invalidate the scope cache immediately after Login Flow v2 provisioning
completes, so users no longer hit ProvisioningRequiredError for up to
5 minutes after successfully authenticating.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-03 09:31:36 +01:00
Chris Coutinho f43343356e fix: address review feedback — security, caching, CI 429 retry
- Add 429 retry with exponential backoff to register_client() (fixes CI
  oauth matrix failures from parallel DCR requests)
- Make client_id, redirect_uri, and PKCE mandatory at token endpoint
- Add null-checks for discovery_url and OAuth credentials in proxy flows
- Add OIDC discovery document caching with 5-min TTL
- Add per-IP rate limiting on /oauth/register DCR proxy
- Discover DCR endpoint from OIDC discovery instead of hardcoding
- Extract extract_user_id_from_token to auth/token_utils.py (breaks
  circular imports between server/ and auth/ layers)
- Add TTL scope cache in scope_authorization.py (avoids DB hit per tool)
- Add defense-in-depth scope validation in storage layer
- Broaden elicitation exception handling with graceful fallback
- Add idempotentHint to nc_auth_check_status, return "pending" status
  after accepted elicitation, add polling interval to description
- Change ALL_SUPPORTED_SCOPES from tuple to frozenset for O(1) lookups
- Replace Optional[str] with str | None throughout config.py
- Use default_factory for ProxyCodeEntry/ASProxySession dataclasses
- Add proxy code/session cleanup to background loop
- Fix OIDC verification CI step to only run for oauth/login-flow modes
- Add unit tests for access.py REST endpoints (10 tests)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-02 17:22:23 +01:00
Chris Coutinho 0d14c75eb1 fix: address remaining PR #589 review findings
- Consolidate MCP session + login flow cleanup into _mcp_session_with_login_flow() helper,
  replacing 4 duplicated AsyncExitStack sites in app.py
- Fix get_shared_storage() race condition by using module-level anyio.Lock() init
  (reverts regression from ba59763)
- Collapse cosmetic if/else branching in scope_authorization.py
- Consolidate dual password storage paths into single store_app_password_with_scopes() call
- Mark unused request param as _ in list_supported_scopes
- Make ALL_SUPPORTED_SCOPES an immutable tuple; use list() instead of .copy()
- Add hasattr(ctx, "elicit") guard in elicitation.py, narrow except to NotImplementedError
- Add YAML comment explaining --oauth flag for mcp-login-flow service

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-02 09:59:56 +01:00
Chris Coutinho ba597634bd fix: address PR #589 review findings
- Fix anyio.Lock() created at module import time; use lazy init in
  get_shared_storage() to avoid instantiation before event loop exists
- Stop get_login_flow_session from silently swallowing DB exceptions;
  re-raise and handle in caller with proper error response
- Update ProvisionAccessResponse and UpdateScopesResponse status field
  docs to include all actual values (declined, cancelled, unchanged)
- Narrow except clause in present_login_url to (AttributeError,
  NotImplementedError) instead of bare Exception
- Add KeyError handling in LoginFlowV2Client.initiate() and poll() for
  clear errors on malformed Nextcloud responses
- Simplify redundant env-var bypass branches in scope_authorization.py
- Extract _maybe_login_flow_cleanup() context manager to replace 4
  inline cleanup loop registrations in app.py; move sleep to end of
  loop body so cleanup runs once at startup
- Replace fragile string replacement in _rewrite_login_flow_url with
  proper urllib.parse URL handling

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-02 09:10:57 +01:00
Chris Coutinho 1a6ce0fa7d fix: address PR review issues for Login Flow v2
- Fix circular dependency in scope_authorization: auth tools requiring
  only identity scopes (openid/profile/email) now bypass the login flow
  provisioning check, so unprovisioned users can call provisioning tools
- Fix no-op detection in nc_auth_update_scopes: NULL scopes (legacy "all")
  now correctly map to ALL_SUPPORTED_SCOPES instead of empty set
- Fix get_app_password_with_scopes swallowing exceptions: re-raise instead
  of returning None, matching sibling methods
- Add missing audit logging to update_app_password_scopes,
  delete_login_flow_session, and delete_expired_login_flow_sessions
- Pin setup-uv to v7.3.1 in CI unit-test job (was v7.3.0)
- Add FastMCP type annotation to register_auth_tools parameter
- Log warning when user accepts elicitation without checking acknowledged box

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-01 19:02:30 +01:00
Chris Coutinho db1e0606ad fix: address PR #589 review feedback (round 2)
Consolidate three independent RefreshTokenStorage lazy singletons into a
single lock-protected get_shared_storage() function, eliminating race
conditions on concurrent first-access. Remove blanket try/except in
_get_stored_scopes so storage errors propagate as proper MCP errors
instead of silently triggering "please provision" messages. Handle
declined/cancelled elicitation results in Login Flow tools by cleaning up
sessions and returning clear status. Add update_app_password_scopes() to
avoid unnecessary decrypt/re-encrypt when only scopes change. Add
unprovisioned-user early exit and no-op detection to nc_auth_update_scopes.
Remove four dead config fields and misleading NEXTCLOUD_PASSWORD deprecation
warning. Add periodic login flow session cleanup task. Generate separate
Fernet keys per service. Add board cleanup in deck integration test. Gate
CI unit tests on linting and skip Astrolabe build for single-user profile.
Fix test markers from oauth to multi_user_basic for astrolabe integration
tests. Update login_flow.py docstrings to document outbound HTTP calls.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-01 16:35:31 +01:00
Chris Coutinho e28af5453b fix: address PR #589 review feedback for Login Flow v2
- Fix data loss in nc_auth_update_scopes: remove premature
  delete_app_password call; old password stays valid until upsert
  replaces it on successful re-provisioning
- Replace assert with proper error return in nc_auth_check_status
- Add lazy singleton for RefreshTokenStorage in auth_tools,
  scope_authorization, and context to avoid per-call re-initialization
- Centralize _is_login_flow_mode() to get_settings().enable_login_flow
  and remove duplicate definitions and per-call os.getenv reads
- Add dev-only comment to TOKEN_ENCRYPTION_KEY in docker-compose.yml
- Gate OIDC build steps in CI behind matrix.needs-playwright
- Add diagnostic step reporting Playwright skip count in CI

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-28 10:08:55 +01:00
Chris Coutinho 8b5c2395b5 feat: add Docker Compose profiles and Login Flow v2 service
Add selective service startup via Docker Compose profiles so each MCP
deployment mode runs independently. Also add the new mcp-login-flow
service (port 8004) for Login Flow v2 authentication (ADR-022).

Profile assignments:
- single-user: mcp (port 8000)
- multi-user-basic: mcp-multi-user-basic (port 8003)
- oauth: mcp-oauth (port 8001)
- keycloak: keycloak + mcp-keycloak (port 8002)
- login-flow: mcp-login-flow (port 8004)

Infrastructure services (db, redis, app, recipes) always start.

Integration tests cover the full Login Flow v2 provisioning flow:
OAuth → browser login → app password → Nextcloud API access for
notes, calendar, contacts, files, deck, and cookbook operations.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-27 20:33:54 +01:00