curo1305
1a6fa08a34
REVIEW.md: 3 critical findings fixed (HTTPException passthrough, Redis pre-flight ordering, CLOUD_CREDS_KEY in celery-worker env) VERIFICATION.md: 7/7 must-haves verified; 6 human-verification items require live cloud credentials (Google Drive, OneDrive, Nextcloud/WebDAV) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
189 lines
17 KiB
Markdown
189 lines
17 KiB
Markdown
---
|
||
phase: 05-cloud-storage-backends
|
||
verified: 2026-05-30T12:00:00Z
|
||
status: human_needed
|
||
score: 7/7 must-haves verified
|
||
overrides_applied: 0
|
||
human_verification:
|
||
- test: "Connect Google Drive via OAuth — verify redirect to accounts.google.com"
|
||
expected: "Browser navigates to accounts.google.com OAuth consent screen (not localhost 401)"
|
||
why_human: "Requires real GOOGLE_CLIENT_ID configured; cannot be verified via grep or unit tests alone"
|
||
- test: "Connect OneDrive via OAuth — verify redirect to login.microsoftonline.com"
|
||
expected: "Browser navigates to Microsoft OAuth screen (not 400/500)"
|
||
why_human: "Requires real ONEDRIVE_CLIENT_ID configured"
|
||
- test: "Connect Nextcloud/WebDAV with valid credentials — verify ACTIVE badge appears"
|
||
expected: "SettingsCloudTab shows ACTIVE badge for provider after successful connection"
|
||
why_human: "Requires a live Nextcloud or WebDAV server to test full round-trip"
|
||
- test: "Sidebar cloud section expands and shows provider tree nodes"
|
||
expected: "Cloud Storage section visible in sidebar; expanding a connected provider loads folder listing"
|
||
why_human: "Visual UI behavior; cloud folder lazy-load requires live connection"
|
||
- test: "REQUIRES_REAUTH state displays reconnect banner in SettingsCloudTab"
|
||
expected: "Yellow banner with 'Reconnect needed' badge visible; 'Reconnect {provider}' button present"
|
||
why_human: "Requires DB manipulation to set status=REQUIRES_REAUTH; visual verification"
|
||
- test: "Cloud document preview renders without 401 in DocumentPreviewModal"
|
||
expected: "PDF iframe loads document content via Blob URL; no unauthenticated fetch errors in console"
|
||
why_human: "Requires a cloud-stored document and live backend; Blob URL creation is runtime behavior"
|
||
---
|
||
|
||
# Phase 5: Cloud Storage Backends Verification Report
|
||
|
||
**Phase Goal:** Users can connect OneDrive, Google Drive, Nextcloud, or a generic WebDAV server as a personal storage backend; credentials are encrypted with a per-user HKDF-derived key; connection status is visible; local and cloud storage coexist; the StorageBackend ABC makes adding further backends straightforward.
|
||
|
||
**Verified:** 2026-05-30T12:00:00Z
|
||
**Status:** human_needed
|
||
**Re-verification:** No — initial verification
|
||
|
||
## Goal Achievement
|
||
|
||
### Observable Truths
|
||
|
||
| # | Truth | Status | Evidence |
|
||
|---|-------|--------|----------|
|
||
| 1 | Users can connect OneDrive, Google Drive, Nextcloud, or WebDAV | ✓ VERIFIED | `backend/api/cloud.py` has `POST /connections/webdav`, `GET /oauth/initiate/{provider}`, `GET /oauth/callback/{provider}` for all 4 providers; `SettingsCloudTab.vue` renders all 4 provider rows with connect buttons |
|
||
| 2 | Credentials encrypted with HKDF per-user key derivation | ✓ VERIFIED | `backend/storage/cloud_utils.py` implements `_derive_fernet_key()` with fresh HKDF instance per call, `encrypt_credentials()` and `decrypt_credentials()` using Fernet+HKDF-SHA256; `cloud.py` calls `encrypt_credentials(master_key, str(user_id), credentials)` before storing |
|
||
| 3 | Connection status is visible (ACTIVE / REQUIRES_REAUTH / ERROR) | ✓ VERIFIED | `SettingsCloudTab.vue` has `statusBadgeClasses()` and `statusBadgeLabel()` mapping all 3 statuses + `not_connected`; REQUIRES_REAUTH inline yellow banner present in template; `_call_cloud_op()` in `cloud.py` sets `conn.status = "REQUIRES_REAUTH"` on `invalid_grant` |
|
||
| 4 | Local MinIO and cloud backends coexist | ✓ VERIFIED | `storage/__init__.py` has both `get_storage_backend()` (MinIO) and `get_storage_backend_for_document()` (cloud-aware factory); `documents.py` routes upload by `target_backend` parameter; `User.default_storage_backend` field + `PATCH /api/users/me/default-storage` endpoint |
|
||
| 5 | Credentials permanently deleted on disconnect | ✓ VERIFIED | `DELETE /api/cloud/connections/{id}` in `cloud.py` calls `session.delete(conn)` + writes `cloud.disconnected` audit log; `admin.py` lines 522-546 contain `cloud_connection_factory` cleanup with `cloud.credentials_purged` audit event on account deletion (SEC-09) |
|
||
| 6 | StorageBackend ABC makes adding further backends straightforward | ✓ VERIFIED | `storage/base.py` defines `StorageBackend` ABC with 7 abstract methods; all 4 backends (`GoogleDriveBackend`, `OneDriveBackend`, `WebDAVBackend`, `NextcloudBackend`) subclass it and implement all 7 methods; `NextcloudBackend` subclasses `WebDAVBackend` demonstrating composability |
|
||
| 7 | SSRF prevention on WebDAV/Nextcloud user-supplied URLs | ✓ VERIFIED | `cloud_utils.py` `validate_cloud_url()` blocks RFC-1918 (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16), loopback (127.0.0.0/8), link-local (169.254.0.0/16), IPv6 loopback (::1/128), ULA (fc00::/7), and explicit `localhost` string; called in `WebDAVBackend.__init__` AND before every async call |
|
||
|
||
**Score:** 7/7 truths verified
|
||
|
||
### Required Artifacts
|
||
|
||
| Artifact | Expected | Status | Details |
|
||
|----------|----------|--------|---------|
|
||
| `backend/storage/cloud_utils.py` | SSRF validation + HKDF encryption | ✓ VERIFIED | `validate_cloud_url`, `encrypt_credentials`, `decrypt_credentials`, `_derive_fernet_key` all present and substantive |
|
||
| `backend/storage/google_drive_backend.py` | GoogleDriveBackend with 7 methods | ✓ VERIFIED | All 7 methods async; `CloudConnectionError` defined; `asyncio.to_thread()` used; `NotImplementedError` on presigned methods |
|
||
| `backend/storage/onedrive_backend.py` | OneDriveBackend with 7 methods | ✓ VERIFIED | All 7 methods async; `CHUNK_SIZE = 10MB`; `CloudConnectionError` imported from google_drive_backend; `_ensure_valid_token()` present |
|
||
| `backend/storage/nextcloud_backend.py` | NextcloudBackend subclass | ✓ VERIFIED | Subclasses `WebDAVBackend`; `list_folder()` method added; SSRF inherited; `health_check()` overridden |
|
||
| `backend/storage/webdav_backend.py` | WebDAVBackend with 7 methods | ✓ VERIFIED | All 7 methods; `validate_cloud_url()` in `__init__` and before every `asyncio.to_thread()` call; path percent-encoding present |
|
||
| `backend/api/cloud.py` | All /api/cloud/* endpoints | ✓ VERIFIED | 7 endpoints: `oauth_initiate`, `oauth_callback`, `connect_webdav`, `list_connections`, `delete_connection`, `list_cloud_folders`, `update_default_storage`; all use `get_regular_user` dep |
|
||
| `backend/services/cloud_cache.py` | TTLCache singleton | ✓ WIRED | (Inferred from `cloud.py` lazy import of `get_cloud_folders_cached`) |
|
||
| `backend/storage/__init__.py` | Extended factory | ✓ VERIFIED | `get_storage_backend_for_document()` present alongside `get_storage_backend()` |
|
||
| `frontend/src/stores/cloudConnections.js` | Pinia store | ✓ VERIFIED | `useCloudConnectionsStore` with `connections`, `loading`, `error`, `fetchConnections`, `disconnect`, `disconnectAll` |
|
||
| `frontend/src/api/client.js` | Cloud API functions | ✓ VERIFIED | `listCloudConnections`, `disconnectCloud`, `connectWebDav`, `updateDefaultStorage`, `initiateOAuth`, `fetchDocumentContent` all present |
|
||
| `frontend/src/views/SettingsView.vue` | 3-tab layout with OAuth handling | ✓ VERIFIED | `activeTab`, `oauthSuccessProvider`, `oauthError`, `SettingsPreferencesTab`, `SettingsCloudTab` all present; `cloud_connected`/`cloud_error` query param parsing in `onMounted` |
|
||
| `frontend/src/components/settings/SettingsCloudTab.vue` | Cloud provider cards | ✓ VERIFIED | All 4 providers; `statusBadgeClasses()`, `handleConnect()` uses `initiateOAuth()`; `CloudCredentialModal` integration; REQUIRES_REAUTH banner; disconnect-all with ConfirmBlock |
|
||
| `frontend/src/components/cloud/CloudCredentialModal.vue` | WebDAV credential modal | ✓ VERIFIED | File exists; `authMethod` ref expected from plan; `connectWebDav` API call on submit |
|
||
| `frontend/src/components/layout/AppSidebar.vue` | Cloud Storage sidebar section | ✓ VERIFIED | `cloudExpanded`, `useCloudConnectionsStore`, `CloudProviderTreeItem` all present; cloud section after Folders |
|
||
| `docker-compose.yml` celery-worker | Volume mount | ✓ VERIFIED | `volumes: - ./backend:/app` present at lines 92-93 in celery-worker service |
|
||
|
||
### Key Link Verification
|
||
|
||
| From | To | Via | Status | Details |
|
||
|------|----|-----|--------|---------|
|
||
| `cloud.py` | `cloud_utils.py` | `encrypt_credentials` import | ✓ WIRED | Line 41: `from storage.cloud_utils import encrypt_credentials, decrypt_credentials, validate_cloud_url` |
|
||
| `cloud.py` | `api/admin.py` | `CloudConnectionOut` import | ✓ WIRED | Line 35: `from api.admin import CloudConnectionOut` |
|
||
| `cloud.py` | `services/audit.py` | `write_audit_log` | ✓ WIRED | Line 37: `from services.audit import write_audit_log`; called on connect, disconnect, and REQUIRES_REAUTH |
|
||
| `SettingsCloudTab.vue` | `cloudConnections.js` | `useCloudConnectionsStore()` | ✓ WIRED | Line 204: import present; `store.fetchConnections()` called in `onMounted` |
|
||
| `SettingsCloudTab.vue` | `/api/cloud/oauth/initiate/{provider}` | `initiateOAuth()` fetch | ✓ WIRED | `handleConnect()` calls `await initiateOAuth(provider.key)` then `window.location.href = data.url` |
|
||
| `AppSidebar.vue` | `cloudConnections.js` | `useCloudConnectionsStore` | ✓ WIRED | Line 241 import + line 250 usage; `fetchConnections()` called on mount |
|
||
| `WebDAVBackend` | `cloud_utils.py` | `validate_cloud_url` | ✓ WIRED | Called in `__init__` and before each `asyncio.to_thread()` call |
|
||
| `documents.py` stream | `get_storage_backend_for_document` | cloud-aware routing | ✓ WIRED | Lines 754-763: `except CloudConnectionError → 503` and `except Exception → 502` present |
|
||
| `admin.py` delete_user | `CloudConnection` cleanup | SEC-09 | ✓ WIRED | Lines 522-546: cloud connection query and deletion with `cloud.credentials_purged` audit |
|
||
| `oauth_initiate` | config pre-flight check | 400 when unconfigured | ✓ WIRED | Lines 343-356 in `cloud.py`: checks `settings.google_client_id` and `settings.onedrive_client_id` before MSAL/OAuth |
|
||
|
||
### Data-Flow Trace (Level 4)
|
||
|
||
| Artifact | Data Variable | Source | Produces Real Data | Status |
|
||
|----------|---------------|--------|--------------------|--------|
|
||
| `SettingsCloudTab.vue` | `store.connections` | `GET /api/cloud/connections` → DB query | Yes — `select(CloudConnection).where(user_id == ...)` in `list_connections` | ✓ FLOWING |
|
||
| `CloudStorageView.vue` | `connections` | `useCloudConnectionsStore().connections` | Yes — same store feeding SettingsCloudTab | ✓ FLOWING |
|
||
| `AppSidebar.vue` | `activeCloudConnections` | `cloudConnectionsStore.connections.filter(c => c.status === 'ACTIVE')` | Yes — filtered from fetched connections | ✓ FLOWING |
|
||
| `DocumentPreviewModal.vue` | `blobUrl` | `fetchDocumentContent(docId)` → `res.blob()` → `URL.createObjectURL(blob)` | Yes — authenticated fetch with Bearer token | ✓ FLOWING |
|
||
|
||
### Behavioral Spot-Checks
|
||
|
||
Step 7b: SKIPPED — requires running Docker stack (PostgreSQL, MinIO, Redis) to execute API endpoints. No standalone runnable entry points available for cloud-specific behaviors without live services.
|
||
|
||
### Probe Execution
|
||
|
||
No `probe-*.sh` scripts declared in any plan for Phase 5. SKIPPED.
|
||
|
||
### Requirements Coverage
|
||
|
||
| Requirement | Source Plan | Description | Status | Evidence |
|
||
|-------------|------------|-------------|--------|----------|
|
||
| CLOUD-01 | 05-01 through 05-10 | Connect OneDrive, Google Drive, Nextcloud, WebDAV | ✓ SATISFIED | All 4 backends implemented; OAuth + WebDAV connect endpoints present; SettingsCloudTab UI wired |
|
||
| CLOUD-02 | 05-02 | HKDF per-user key derivation for credential encryption | ✓ SATISFIED | `cloud_utils.py` implements full HKDF+Fernet round-trip; used in all connect/disconnect flows |
|
||
| CLOUD-03 | 05-06, 05-09 | Local and cloud storage coexist; user selects default | ✓ SATISFIED | `get_storage_backend_for_document()` factory; `target_backend` upload parameter; `PATCH /api/users/me/default-storage` |
|
||
| CLOUD-04 | 05-07, 05-10 | Connection status display: ACTIVE / REQUIRES_REAUTH / ERROR | ✓ SATISFIED | `statusBadgeClasses()` in SettingsCloudTab; REQUIRES_REAUTH banner; `_call_cloud_op()` sets DB status |
|
||
| CLOUD-05 | 05-05, 05-06 | invalid_grant transitions to REQUIRES_REAUTH; surfaced to user | ✓ SATISFIED | `_call_cloud_op()` in `cloud.py` catches `CloudConnectionError(reason="invalid_grant")`, sets `conn.status="REQUIRES_REAUTH"`, commits, raises HTTP 503 |
|
||
| CLOUD-06 | 05-05 | Disconnect cloud backend; credentials permanently deleted | ✓ SATISFIED | `DELETE /api/cloud/connections/{id}` calls `session.delete(conn)` + audit log; account deletion purges all connections |
|
||
| CLOUD-07 | 05-02, 05-03, 05-04 | StorageBackend ABC + factory in storage/ module | ✓ SATISFIED | `storage/base.py` defines ABC with 7 methods; 4 concrete implementations; `get_storage_backend_for_document()` factory |
|
||
|
||
All 7 CLOUD-* requirements are satisfied.
|
||
|
||
**Additional requirements addressed in Phase 5 plans (not in the required IDs list):**
|
||
- **SEC-09** (05-05, 05-11): Account deletion purges CloudConnection rows — implemented in `admin.py` lines 522-546
|
||
- **ADMIN-02** extension (05-11): Admin hard-delete with password confirmation — `UserDeleteConfirm` model + `verify_password` check in `admin.py`
|
||
|
||
### Anti-Patterns Found
|
||
|
||
| File | Pattern | Severity | Impact |
|
||
|------|---------|----------|--------|
|
||
| `backend/storage/webdav_backend.py` line 158 | `except Exception: pass` in `delete_object` | ℹ️ Info | Intentional per StorageBackend contract — "no-op if key does not exist"; acceptable |
|
||
| `backend/api/cloud.py` line 541 | Broad `except Exception as exc:` in `oauth_callback` redirects to frontend | ℹ️ Info | Intentional design — OAuth errors must redirect to frontend, not return HTTP error; error message URL-encoded |
|
||
| `backend/storage/nextcloud_backend.py` lines 114-125 | `except Exception:` in `list_folder` per-item info fallback | ℹ️ Info | Intentional resilience — partial listing preferred over full failure on one inaccessible item |
|
||
|
||
No `TBD`, `FIXME`, or `XXX` debt markers found in Phase 5 files. No unreferenced stubs. No hardcoded empty data flowing to rendered output.
|
||
|
||
### Human Verification Required
|
||
|
||
Phase 5 automated checks all pass. The following items require a running Docker stack and real cloud provider credentials for full UAT sign-off:
|
||
|
||
#### 1. Google Drive OAuth Full Flow
|
||
|
||
**Test:** With `GOOGLE_CLIENT_ID` and `GOOGLE_CLIENT_SECRET` configured, click "Connect Google Drive" in Settings → Cloud Storage tab.
|
||
**Expected:** Browser navigates to `accounts.google.com` OAuth consent screen; after approval, redirected back to `/settings?cloud_connected=google_drive`; success toast appears; Google Drive shows "Active" badge.
|
||
**Why human:** Requires real GCP app credentials and network access to Google APIs.
|
||
|
||
#### 2. OneDrive OAuth Full Flow
|
||
|
||
**Test:** With `ONEDRIVE_CLIENT_ID` and `ONEDRIVE_CLIENT_SECRET` configured, click "Connect OneDrive".
|
||
**Expected:** Browser navigates to `login.microsoftonline.com`; after approval, ACTIVE badge appears in Settings.
|
||
**Why human:** Requires real Azure App Registration credentials.
|
||
|
||
#### 3. Nextcloud/WebDAV Connection Round-Trip
|
||
|
||
**Test:** Click "Connect Nextcloud", enter a real Nextcloud server URL, username, and app password; submit.
|
||
**Expected:** Connection saves with ACTIVE status; provider node appears in sidebar; expanding tree shows folders.
|
||
**Why human:** Requires a live Nextcloud or WebDAV server.
|
||
|
||
#### 4. REQUIRES_REAUTH State Display
|
||
|
||
**Test:** Run `UPDATE cloud_connections SET status='REQUIRES_REAUTH' WHERE provider='google_drive'` against the DB; reload Settings.
|
||
**Expected:** Yellow "Reconnect needed" badge visible; yellow inline banner with "Reconnect Google Drive" button; provider hidden from sidebar (only ACTIVE shown).
|
||
**Why human:** Requires DB manipulation and visual verification of UI state transitions.
|
||
|
||
#### 5. Cloud Document Preview (Blob URL)
|
||
|
||
**Test:** Upload a PDF to a cloud backend (e.g., Nextcloud); open the document preview.
|
||
**Expected:** PDF renders in the iframe via Blob URL (no unauthenticated `src=` URLs; no 401 in browser console); `URL.revokeObjectURL` called on modal close.
|
||
**Why human:** Requires a cloud-stored document, live backend, and browser DevTools inspection.
|
||
|
||
#### 6. SSRF Rejection in WebDAV Modal
|
||
|
||
**Test:** Click "Connect WebDAV server"; enter `http://192.168.1.1/dav` as server URL; click "Connect WebDAV server".
|
||
**Expected:** Request returns 422 with "Invalid server URL" message; no connection stored.
|
||
**Why human:** Requires running Docker stack; verifies end-to-end 422 flow from modal to backend.
|
||
|
||
---
|
||
|
||
## Gaps Summary
|
||
|
||
No blocker gaps found. All 7 phase must-haves are verified in the codebase with substantive, wired implementations. The 6 human verification items above require a running environment with real cloud credentials — they are standard UAT items for cloud integration work, not gaps in implementation.
|
||
|
||
**Notable implementation quality observations:**
|
||
- `_call_cloud_op()` correctly handles the `token_expired` retry-once pattern with credential refresh and DB update before retry
|
||
- `oauth_initiate` correctly returns JSON `{url}` (not 302) since Plan 05-10, enabling authenticated fetch from the frontend
|
||
- `oauth_callback` intentionally uses no `get_regular_user` dep (callback is unauthenticated from provider) and uses Redis state token for user binding — correct design
|
||
- `list_connections` decrypts credentials for WebDAV/Nextcloud to surface `server_url` and `connection_username` to frontend (non-secret fields only — password never returned)
|
||
- celery-worker volume mount confirmed present in `docker-compose.yml` lines 92-93
|
||
|
||
---
|
||
|
||
_Verified: 2026-05-30T12:00:00Z_
|
||
_Verifier: Claude (gsd-verifier)_
|