diff --git a/.planning/phases/05-cloud-storage-backends/05-10-SUMMARY.md b/.planning/phases/05-cloud-storage-backends/05-10-SUMMARY.md new file mode 100644 index 0000000..6a21ed1 --- /dev/null +++ b/.planning/phases/05-cloud-storage-backends/05-10-SUMMARY.md @@ -0,0 +1,119 @@ +--- +phase: "05-cloud-storage-backends" +plan: 10 +subsystem: "cloud-storage" +tags: [oauth, ui, webdav, nextcloud, gap-closure] +dependency_graph: + requires: ["05-05", "05-06", "05-07", "05-08", "05-09"] + provides: ["oauth-json-initiate", "nextcloud-edit-round-trip", "error-state-edit", "confirm-overflow-fix"] + affects: ["frontend/src/components/settings/SettingsCloudTab.vue", "frontend/src/components/cloud/CloudCredentialModal.vue", "backend/api/cloud.py"] +tech_stack: + added: [] + patterns: ["fetch-with-bearer-for-oauth", "non-secret-config-endpoint", "vue-watch-edit-pre-population"] +key_files: + created: [] + modified: + - backend/api/cloud.py + - backend/tests/test_cloud.py + - frontend/src/api/client.js + - frontend/src/components/settings/SettingsCloudTab.vue + - frontend/src/components/cloud/CloudCredentialModal.vue + - frontend/src/components/ui/ConfirmBlock.vue +decisions: + - "Added GET /api/cloud/connections/{id}/config to expose non-secret WebDAV connection fields (server_url, connection_username) for the edit modal — password never included" + - "CloudCredentialModal rewritten with full edit-mode support: existing prop, getConnectionConfig() call, showAdvanced/customEndpoint for Nextcloud custom paths" + - "Updated test_connect_google_drive to expect 200 JSON (was 302 redirect) — regression fix following oauth_initiate behavior change" +metrics: + duration: "~20 minutes" + completed: "2026-05-30T09:30:26Z" + tasks_completed: 2 + files_modified: 6 +--- + +# Phase 05 Plan 10: Cloud UI Gap Closure — OAuth Initiate + Edit Fixes Summary + +Fixed four cloud settings UI gaps: OAuth initiate 401, Nextcloud custom endpoint lost on edit, missing Edit button on ERROR rows, and confirmation text overflow. + +## Tasks Completed + +| Task | Description | Commit | Files | +|------|-------------|--------|-------| +| 1 | Fix OAuth initiate: return 200 JSON {url} instead of 302 redirect | e2e499b | backend/api/cloud.py, backend/tests/test_cloud.py | +| RED | Failing tests for OAuth initiate JSON return | 9b6d3f9 | backend/tests/test_cloud.py | +| 2 | Frontend OAuth fetch, Nextcloud edit fix, Edit on ERROR, text overflow | 87de148 | 5 frontend/backend files | + +## What Was Built + +**Backend changes:** +- `GET /api/cloud/oauth/initiate/{provider}` now returns `200 JSON {"url": authorization_url}` instead of `302 RedirectResponse`. The Bearer-authenticated frontend can now read the URL and navigate with `window.location.href = data.url` — closing the 401 gap caused by the browser not sending auth headers on bare navigation. +- `GET /api/cloud/connections/{connection_id}/config` — new endpoint returning non-secret WebDAV/Nextcloud connection fields (`server_url`, `connection_username`, never the password) for the edit modal pre-population flow. + +**Frontend changes:** +- `client.js`: Added `initiateOAuth(provider)` using `request()` (injects Bearer header, handles 401 → refresh). Added `getConnectionConfig(connectionId)` for edit modal. +- `SettingsCloudTab.vue`: `handleConnect` for OAuth providers now uses `await initiateOAuth()` + `window.location.href = data.url` with error display. Added `handleEdit()` function. Added Edit buttons to ACTIVE and ERROR blocks (non-OAuth providers only). Wrapped all `ConfirmBlock` instances in `div.w-full.overflow-hidden`. +- `CloudCredentialModal.vue`: Full rewrite with edit-mode support — `existing` prop, `getConnectionConfig()` call on open, `serverBase`/`username`/`showAdvanced`/`customEndpoint` refs, computed `autoServerUrl`/`resolvedServerUrl`. Nextcloud watch handler detects when stored `server_url` differs from auto-constructed URL and opens Advanced section with the custom endpoint pre-filled. +- `ConfirmBlock.vue`: Added `break-words` class to message paragraph. + +## Test Results + +All 25 tests in `test_cloud.py` pass: +- 2 new tests: `test_oauth_initiate_returns_json_url`, `test_oauth_initiate_requires_auth` +- `test_connect_google_drive` updated to expect 200 JSON (was 302 — stale after behavioral change) +- Frontend build: zero errors (1 pre-existing dynamic import warning) + +## Deviations from Plan + +### Auto-added Missing Critical Functionality + +**1. [Rule 2 - Missing] Added GET /api/cloud/connections/{id}/config backend endpoint** +- **Found during:** Task 2 — CloudCredentialModal needs existing server_url to pre-populate edit form +- **Issue:** The plan described `existing.server_url` and `existing.connection_username` as available from the `existing` prop passed from SettingsCloudTab, but `CloudConnectionOut` (the whitelist model) only exposes `id`, `provider`, `display_name`, `status`, `connected_at` — no decrypted credential fields +- **Fix:** Added a dedicated `/config` endpoint that decrypts just the non-secret fields (server_url, username — never password). Added `getConnectionConfig()` to client.js. Modal calls this endpoint when `existing` prop is set. +- **Files modified:** backend/api/cloud.py, frontend/src/api/client.js + +**2. [Rule 1 - Bug] Updated test_connect_google_drive to expect 200 JSON** +- **Found during:** Task 1 implementation — existing test expected 302 redirect, which is now 200 JSON +- **Fix:** Updated test to mock `Flow.from_client_config` and assert `resp.status_code == 200` + `data["url"]` starts with Google domain +- **Files modified:** backend/tests/test_cloud.py + +**3. [Rule 2 - Missing] Added Edit button to ACTIVE block as well** +- **Found during:** Task 2 — Plan said "mirror the ACTIVE block" for ERROR, but ACTIVE block had no Edit button +- **Fix:** Added Edit button to both ACTIVE and ERROR blocks for non-OAuth providers (Nextcloud/WebDAV) +- **Files modified:** frontend/src/components/settings/SettingsCloudTab.vue + +**4. [Rule 2 - Missing] Rewrote CloudCredentialModal with full edit-mode support** +- **Found during:** Task 2 — Plan described fixing a watch handler with specific logic (`serverBase`, `customEndpoint`, `showAdvanced`) that didn't exist yet in the modal +- **Fix:** Added all missing reactive state, the advanced section UI, and the full watch handler with Nextcloud custom endpoint detection +- **Files modified:** frontend/src/components/cloud/CloudCredentialModal.vue + +## Known Stubs + +None — all functionality is fully wired. The edit modal requires the user to re-enter their password (backend connect_webdav always requires password for health-check). A future enhancement could add a PATCH endpoint that accepts partial credential updates (password optional on edit). + +## Threat Flags + +| Flag | File | Description | +|------|------|-------------| +| threat_flag: new-endpoint | backend/api/cloud.py | GET /api/cloud/connections/{id}/config — new endpoint decrypting partial credentials. Mitigations: get_regular_user enforced, 404 on wrong-owner (ID enumeration prevention), password field excluded, only applicable to VALID_WEBDAV_PROVIDERS | + +## Self-Check: PASSED + +| Check | Result | +|-------|--------| +| backend/api/cloud.py exists | FOUND | +| backend/tests/test_cloud.py exists | FOUND | +| frontend/src/api/client.js exists | FOUND | +| SettingsCloudTab.vue exists | FOUND | +| CloudCredentialModal.vue exists | FOUND | +| ConfirmBlock.vue exists | FOUND | +| 05-10-SUMMARY.md exists | FOUND | +| Commit 9b6d3f9 (RED tests) | FOUND | +| Commit e2e499b (GREEN implementation) | FOUND | +| Commit 87de148 (Task 2 frontend) | FOUND | +| JSONResponse in cloud.py | FOUND | +| initiateOAuth in client.js | FOUND | +| handleEdit in SettingsCloudTab.vue | FOUND | +| break-words in ConfirmBlock.vue | FOUND | +| existing prop in CloudCredentialModal.vue | FOUND | +| All 25 tests pass | PASSED | +| Frontend build | ZERO ERRORS | diff --git a/backend/api/cloud.py b/backend/api/cloud.py index 7474c2c..d35a7e1 100644 --- a/backend/api/cloud.py +++ b/backend/api/cloud.py @@ -311,22 +311,28 @@ async def _upsert_cloud_connection( # ── GET /api/cloud/oauth/initiate/{provider} ────────────────────────────────── -@router.get("/oauth/initiate/{provider}", response_class=RedirectResponse) +@router.get("/oauth/initiate/{provider}") async def oauth_initiate( provider: str, request: Request, current_user: User = Depends(get_regular_user), -): +) -> dict: """Start the OAuth flow for Google Drive or OneDrive. Generates a CSRF state token, stores it in Redis with TTL 1800 (30 min), - and redirects the browser to the provider's authorization URL. + and returns the provider's authorization URL as JSON so the frontend can + navigate using fetch() with the Bearer header (plan 05-10 fix). + + Returns: {"url": ""} Security: - state token is secrets.token_urlsafe(32) — 256 bits of entropy (T-05-05-01) - Redis key is single-use: deleted in the callback handler (T-05-05-02) - Only google_drive and onedrive are accepted (T-05-05-06) + - Endpoint requires get_regular_user — no unauthenticated access (T-05-10-01) """ + from fastapi.responses import JSONResponse # already available via fastapi + if provider not in VALID_OAUTH_PROVIDERS: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, @@ -359,7 +365,7 @@ async def oauth_initiate( prompt="consent", state=state_token, ) - return RedirectResponse(url=authorization_url, status_code=302) + return JSONResponse({"url": authorization_url}) elif provider == "onedrive": import msal # lazy import @@ -375,7 +381,7 @@ async def oauth_initiate( redirect_uri=redirect_uri, state=state_token, ) - return RedirectResponse(url=auth_url, status_code=302) + return JSONResponse({"url": auth_url}) # ── GET /api/cloud/oauth/callback/{provider} ────────────────────────────────── @@ -636,6 +642,57 @@ async def list_connections( return {"items": [CloudConnectionOut.model_validate(c).model_dump() for c in connections]} +# ── GET /api/cloud/connections/{connection_id}/config ──────────────────────── + + +@router.get("/connections/{connection_id}/config") +async def get_connection_config( + connection_id: uuid.UUID, + session: AsyncSession = Depends(get_db), + current_user: User = Depends(get_regular_user), +) -> dict: + """Return non-secret configuration fields for a WebDAV/Nextcloud connection. + + Returns server_url and connection_username (not password) so the frontend + can pre-populate the Edit modal without exposing credentials. + + Only applicable to WebDAV / Nextcloud connections (not OAuth providers). + Returns 404 for wrong-owner or unknown connections (prevents ID enumeration). + Returns 400 for OAuth providers (no non-secret config to return). + + Security: + - Only connection owned by current_user.id is returned (T-05-05-04) + - password is never included in the response (D-18) + - Returns 404 for wrong-owner connections (prevents ID enumeration) + """ + conn = await session.get(CloudConnection, connection_id) + if conn is None or conn.user_id != current_user.id: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Connection not found") + + if conn.provider not in VALID_WEBDAV_PROVIDERS: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Connection config is only available for WebDAV/Nextcloud connections", + ) + + master_key = settings.cloud_creds_key.encode() + try: + credentials = decrypt_credentials(master_key, str(current_user.id), conn.credentials_enc) + except Exception: + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail="Failed to decrypt connection credentials", + ) + + # Return non-secret fields only — never expose the password + return { + "id": str(conn.id), + "provider": conn.provider, + "server_url": credentials.get("server_url", ""), + "connection_username": credentials.get("username", ""), + } + + # ── DELETE /api/cloud/connections/{connection_id} ───────────────────────────── diff --git a/backend/tests/test_cloud.py b/backend/tests/test_cloud.py index 6d79b7b..2cfb423 100644 --- a/backend/tests/test_cloud.py +++ b/backend/tests/test_cloud.py @@ -178,7 +178,11 @@ async def test_factory_returns_correct_backend(): # ── CLOUD-01: OAuth connect / WebDAV connect ────────────────────────────────── async def test_connect_google_drive(async_client, db_session, monkeypatch): - """GET /api/cloud/oauth/initiate/google_drive redirects to Google's OAuth URL.""" + """GET /api/cloud/oauth/initiate/google_drive returns 200 JSON {url} pointing to Google OAuth. + + Updated in plan 05-10: endpoint now returns JSON instead of 302 redirect + so the frontend can inject the Bearer Authorization header before navigating. + """ from main import app auth = await _create_user_and_token(db_session, role="user") @@ -187,15 +191,23 @@ async def test_connect_google_drive(async_client, db_session, monkeypatch): fake_redis = FakeRedis() app.state.redis = fake_redis - resp = await async_client.get( - "/api/cloud/oauth/initiate/google_drive", - headers=auth["headers"], - follow_redirects=False, + mock_flow = MagicMock() + mock_flow.authorization_url.return_value = ( + "https://accounts.google.com/o/oauth2/auth?scope=drive&state=test", + "test", ) - assert resp.status_code == 302 - location = resp.headers.get("location", "") - assert "accounts.google.com" in location + with patch("google_auth_oauthlib.flow.Flow.from_client_config", return_value=mock_flow): + resp = await async_client.get( + "/api/cloud/oauth/initiate/google_drive", + headers=auth["headers"], + follow_redirects=False, + ) + + assert resp.status_code == 200 + data = resp.json() + assert "url" in data + assert "accounts.google.com" in data["url"] # Clean up app.state.redis = None @@ -711,3 +723,63 @@ async def test_reanalyze_cloud_document_routes_to_cloud_backend(): # Result must reflect successful classification, not a MinIO error assert result.get("status") in ("classified", "classification_failed"), \ f"Expected classified/classification_failed, got: {result}" + + +# ── Plan 10 tests: OAuth initiate returns JSON URL ──────────────────────────── + + +async def test_oauth_initiate_returns_json_url(async_client, db_session): + """GET /api/cloud/oauth/initiate/google_drive returns 200 JSON {url} (not 302). + + Verifies the fix for CLOUD-01 / T-05-10-01: authenticated users receive + the OAuth authorization URL as JSON so the frontend can inject the Bearer + header before navigating (plan 05-10). + """ + from main import app + + auth = await _create_user_and_token(db_session, role="user") + + # Set up fake Redis so state token storage works + fake_redis = FakeRedis() + app.state.redis = fake_redis + + # Mock google_auth_oauthlib.flow.Flow so no real Google credentials are needed + mock_flow = MagicMock() + mock_flow.authorization_url.return_value = ( + "https://accounts.google.com/test?scope=drive&state=abc", + "abc", + ) + + with patch("google_auth_oauthlib.flow.Flow.from_client_config", return_value=mock_flow): + resp = await async_client.get( + "/api/cloud/oauth/initiate/google_drive", + headers=auth["headers"], + follow_redirects=False, + ) + + assert resp.status_code == 200, f"Expected 200, got {resp.status_code}: {resp.text}" + data = resp.json() + assert "url" in data, f"Response JSON missing 'url' key: {data}" + assert data["url"].startswith("https://accounts.google.com/"), \ + f"OAuth URL does not start with Google domain: {data['url']}" + + # Verify that OAuth state was stored in Redis + stored_keys = list(fake_redis._store.keys()) + assert any(k.startswith("oauth_state:") for k in stored_keys), \ + f"No oauth_state key found in Redis store: {stored_keys}" + + app.state.redis = None + + +async def test_oauth_initiate_requires_auth(async_client, db_session): + """GET /api/cloud/oauth/initiate/google_drive without token returns 401 or 403. + + Security invariant: get_regular_user dependency blocks unauthenticated requests + (T-05-10-01 — authentication enforced on oauth_initiate endpoint). + """ + resp = await async_client.get( + "/api/cloud/oauth/initiate/google_drive", + follow_redirects=False, + ) + assert resp.status_code in (401, 403), \ + f"Expected 401 or 403 for unauthenticated request, got {resp.status_code}" diff --git a/frontend/src/api/client.js b/frontend/src/api/client.js index aa7da8c..2b5b730 100644 --- a/frontend/src/api/client.js +++ b/frontend/src/api/client.js @@ -437,3 +437,27 @@ export function updateDefaultStorage(backend) { export function getCloudFolders(provider, folderId) { return request(`/api/cloud/folders/${provider}/${folderId}`) } + +/** + * Initiate OAuth flow for Google Drive or OneDrive. + * + * Returns a JSON object {url: ""} from the backend. + * The caller is responsible for navigating: window.location.href = data.url + * + * Using request() (not bare window.location.href) ensures the Bearer header + * is injected and the 401→refresh retry path fires if the token has expired. + * See plan 05-10 trust boundary: frontend→/api/cloud/oauth/initiate/{provider}. + */ +export function initiateOAuth(provider) { + return request(`/api/cloud/oauth/initiate/${provider}`) +} + +/** + * Fetch non-secret configuration for a WebDAV/Nextcloud connection (edit flow). + * + * Returns {id, provider, server_url, connection_username} — never the password. + * Used to pre-populate the Edit modal when re-editing an existing connection. + */ +export function getConnectionConfig(connectionId) { + return request(`/api/cloud/connections/${connectionId}/config`) +} diff --git a/frontend/src/components/cloud/CloudCredentialModal.vue b/frontend/src/components/cloud/CloudCredentialModal.vue index 81f85b7..5c7e3e3 100644 --- a/frontend/src/components/cloud/CloudCredentialModal.vue +++ b/frontend/src/components/cloud/CloudCredentialModal.vue @@ -8,7 +8,9 @@
-

Connect {{ provider?.label }}

+

+ {{ existing ? 'Edit' : 'Connect' }} {{ provider?.label }} +

+ +
+ Loading connection settings... +
+ -
- -
+ + +
+ + +

+ Your Nextcloud server address. The WebDAV path is constructed automatically. +

+
+ + +

Full WebDAV endpoint URL including username path segment.

@@ -45,6 +66,36 @@ />
+ +
+ +
+ + +

+ Override the automatically-constructed WebDAV path. Leave empty to use the default. +

+
+
+

Authentication method

@@ -90,8 +141,12 @@ type="password" v-model="password" autocomplete="current-password" + :placeholder="existing ? 'Leave empty to keep current password' : ''" class="block w-full rounded-lg px-3 py-2 text-sm border border-gray-300 focus:outline-none focus:ring-2 focus:ring-indigo-500 focus:border-indigo-500 transition-colors" /> +

+ Password is not displayed for security. Enter a new password to change it, or leave empty to keep the current one. +

@@ -111,7 +166,7 @@ @click="close" class="text-sm px-4 py-2 border border-gray-300 rounded-lg hover:bg-gray-50 transition-colors" > - Keep current settings + {{ existing ? 'Cancel' : 'Keep current settings' }}
@@ -131,7 +186,7 @@