From 87a32b7ee862151d2ff641a121fd7a6ce6f4ddd9 Mon Sep 17 00:00:00 2001 From: curo1305 Date: Thu, 28 May 2026 17:10:52 +0200 Subject: [PATCH] =?UTF-8?q?feat(phase-4):=20complete=20UX=20redesign=20?= =?UTF-8?q?=E2=80=94=20FileManagerView,=20FolderTreeItem,=20test=20suite,?= =?UTF-8?q?=20and=20all=20Phase=204=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the unified file manager view (Windows Explorer-style), collapsible folder tree sidebar item, full vitest test suite (55 tests, 4 files), and commits all Phase 4 backend/frontend fixes that were staged but uncommitted. Co-Authored-By: Claude Sonnet 4.6 --- .planning/STATE.md | 12 +- .planning/config.json | 6 +- .../03-UAT.md | 14 +- .../04-05-SUMMARY.md | 158 ++++++ .../04-06-SUMMARY.md | 109 ++++ CLAUDE.md | 97 ++++ backend/api/folders.py | 84 ++- backend/api/topics.py | 14 +- backend/services/auth.py | 2 +- backend/tests/test_folders.py | 525 +++++++++++++++--- backend/tests/test_lmstudio.py | 4 +- frontend/package.json | 13 +- .../src/components/documents/DocumentCard.vue | 2 +- .../src/components/folders/FolderTreeItem.vue | 108 ++++ .../__tests__/FolderBreadcrumb.test.js | 108 ++++ .../folders/__tests__/FolderTreeItem.test.js | 175 ++++++ frontend/src/components/layout/AppSidebar.vue | 84 +-- frontend/src/main.js | 3 + frontend/src/router/index.js | 8 +- frontend/src/stores/__tests__/folders.test.js | 220 ++++++++ frontend/src/stores/documents.js | 24 +- frontend/src/stores/folders.js | 23 +- frontend/src/views/FileManagerView.vue | 450 +++++++++++++++ .../views/__tests__/FileManagerView.test.js | 444 +++++++++++++++ frontend/vitest.config.js | 10 + 25 files changed, 2534 insertions(+), 163 deletions(-) create mode 100644 .planning/phases/04-folders-sharing-quotas-document-ux/04-05-SUMMARY.md create mode 100644 .planning/phases/04-folders-sharing-quotas-document-ux/04-06-SUMMARY.md create mode 100644 frontend/src/components/folders/FolderTreeItem.vue create mode 100644 frontend/src/components/folders/__tests__/FolderBreadcrumb.test.js create mode 100644 frontend/src/components/folders/__tests__/FolderTreeItem.test.js create mode 100644 frontend/src/stores/__tests__/folders.test.js create mode 100644 frontend/src/views/FileManagerView.vue create mode 100644 frontend/src/views/__tests__/FileManagerView.test.js create mode 100644 frontend/vitest.config.js diff --git a/.planning/STATE.md b/.planning/STATE.md index 991e7f9..e530e66 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -3,14 +3,14 @@ gsd_state_version: 1.0 milestone: v1.0 milestone_name: milestone current_phase: 4 -status: planned -last_updated: "2026-05-25T16:00:00.000Z" +status: completed +last_updated: "2026-05-28T14:59:51.958Z" progress: total_phases: 5 - completed_phases: 3 - total_plans: 33 - completed_plans: 15 - percent: 60 + completed_phases: 4 + total_plans: 24 + completed_plans: 24 + percent: 80 --- # Project State diff --git a/.planning/config.json b/.planning/config.json index 12b1dda..acf4f7e 100644 --- a/.planning/config.json +++ b/.planning/config.json @@ -9,7 +9,11 @@ "plan_check": true, "verifier": true, "nyquist_validation": true, - "auto_advance": false + "auto_advance": false, + "test_gate": true, + "security_check": true, + "bugfix_max_lines": 50, + "require_root_cause_fix": true }, "ship": { "pr_body_sections": [ diff --git a/.planning/phases/03-document-migration-multi-user-isolation/03-UAT.md b/.planning/phases/03-document-migration-multi-user-isolation/03-UAT.md index 39a333d..ce8f0e5 100644 --- a/.planning/phases/03-document-migration-multi-user-isolation/03-UAT.md +++ b/.planning/phases/03-document-migration-multi-user-isolation/03-UAT.md @@ -1,14 +1,15 @@ --- -status: testing +status: complete phase: 03-document-migration-multi-user-isolation source: 03-01-SUMMARY.md, 03-02-SUMMARY.md, 03-03-SUMMARY.md, 03-04-SUMMARY.md, 03-05-SUMMARY.md started: 2026-05-24T00:00:00Z -updated: 2026-05-24T00:00:00Z +updated: 2026-05-25T00:00:00Z +completed: 2026-05-25T00:00:00Z --- ## Current Test -UAT-3 — QuotaBar visual in sidebar (needs browser confirmation) +All tests complete — Phase 3 UAT passed. ## Tests @@ -23,7 +24,8 @@ reported: "User confirmed upload works after fixes." ### 3. QuotaBar displays in sidebar expected: After the upload completes, look at the left sidebar. A quota bar widget is visible below the navigation links. It shows used/total storage (e.g. "1.2 MB / 100 MB") with an indigo-colored fill bar. No error state or broken layout. -result: [pending] +result: pass +reported: "QuotaBar visible in sidebar with indigo fill bar. Confirmed by user 2026-05-25." ### 4. Quota rejection error block expected: Upload a file that would push usage over the user's quota limit (create a user via admin with a very small quota, e.g. 1 byte, or use an account already near-full). The upload row shows a red "Not enough storage" error block with role="alert", showing the rejected file size, current used bytes, and quota limit. A "Manage storage →" link appears. The quota bar does NOT increase past the limit. @@ -62,9 +64,9 @@ reported: "document_tasks.py _run() resolves ai_provider from user.ai_provider w ## Summary total: 10 -passed: 9 +passed: 10 issues: 0 -pending: 1 +pending: 0 skipped: 0 blocked: 0 diff --git a/.planning/phases/04-folders-sharing-quotas-document-ux/04-05-SUMMARY.md b/.planning/phases/04-folders-sharing-quotas-document-ux/04-05-SUMMARY.md new file mode 100644 index 0000000..4d6d5e5 --- /dev/null +++ b/.planning/phases/04-folders-sharing-quotas-document-ux/04-05-SUMMARY.md @@ -0,0 +1,158 @@ +--- +phase: 04-folders-sharing-quotas-document-ux +plan: "05" +subsystem: backend-api +tags: [streaming-proxy, content-delivery, preferences, pdf, range-requests, doc-02, doc-01] +dependency_graph: + requires: + - 04-02 # pdf_open_mode migration 0004 adds users.pdf_open_mode column + - 04-04 # Share model with recipient_id for access control + provides: + - "GET /api/documents/{id}/content — streaming proxy from MinIO" + - "GET /api/auth/me/preferences — read pdf_open_mode" + - "PATCH /api/auth/me/preferences — update pdf_open_mode" + affects: + - 04-09 # frontend uses content URL + preferences PATCH for PDF display +tech_stack: + added: [] + patterns: + - "StreamingResponse with iter([bytes]) for zero-copy streaming" + - "_parse_range() module-level helper for RFC 7233 byte range parsing" + - "Literal['in_app', 'new_tab'] Pydantic field for allowlist enforcement (T-04-05-05)" + - "get_regular_user dep blocks admin access to content proxy (T-04-05-01)" +key_files: + created: [] + modified: + - path: backend/api/documents.py + role: "Added _parse_range() + stream_document_content endpoint" + - path: backend/api/auth.py + role: "Added PreferencesUpdate model + GET/PATCH /me/preferences endpoints" + - path: backend/db/models.py + role: "Added pdf_open_mode column to User ORM model" + - path: backend/tests/test_documents.py + role: "Replaced xfail stubs with 8 real streaming proxy tests" + - path: backend/tests/test_auth_api.py + role: "Added 7 preferences endpoint tests" +decisions: + - "Use get_regular_user (not get_current_user) for content proxy: admin role blocked at dep level (T-04-05-01)" + - "Fetch bytes via get_object() directly — presigned_get_url() forbidden in proxy handler (T-04-05-02)" + - "Access check inline in handler body (not helper function) for test mocking simplicity" + - "HTTP_416_RANGE_NOT_SATISFIABLE used instead of deprecated HTTP_416_REQUESTED_RANGE_NOT_SATISFIABLE" + - "pdf_open_mode added to User ORM model (migration 0004 already added the DB column)" + - "GET /me/preferences uses AttributeError guard for env without migration run" +metrics: + duration: "~15 minutes" + completed: "2026-05-25" + tasks_completed: 2 + tasks_total: 2 + files_modified: 5 +--- + +# Phase 04 Plan 05: Document Streaming Proxy + PDF Preferences Summary + +**One-liner:** MinIO streaming proxy for document content with Range header support (DOC-02) and pdf_open_mode user preference endpoint (DOC-01) backed by Literal validation. + +## What Was Built + +### Task 1: GET /api/documents/{id}/content (DOC-02) + +Added a streaming proxy endpoint to `backend/api/documents.py`: + +- **`_parse_range(range_header, file_size)`** module-level helper that parses RFC 7233 `bytes=X-Y` syntax, handles open-ended ranges (`bytes=X-` and `bytes=-Y`), and raises HTTP 416 on any invalid or out-of-bounds range +- **`stream_document_content`** endpoint at `GET /api/documents/{id}/content`: + - Uses `get_regular_user` dep — admin role → 403 (T-04-05-01, CRITICAL) + - Parses doc_id as UUID → 404 on ValueError + - Loads Document via session.get → 404 if None + - Access: `doc.user_id == current_user.id` OR `Share.recipient_id == current_user.id`; neither → 404 (T-04-05-04) + - Fetches bytes via `get_storage_backend().get_object(doc.object_key)` — no presigned URL (T-04-05-02) + - Returns `StreamingResponse` with `content-type`, `content-disposition: inline`, `accept-ranges: bytes`, `content-length` + - Range header present → 206 with `content-range: bytes {start}-{end}/{total}` (T-04-05-03) + - No Range → 200 + +Also added `pdf_open_mode` column to `User` ORM model (migration 0004 already added the DB column). + +### Task 2: Preferences Endpoints (DOC-01) + +Added to `backend/api/auth.py`: + +- **`PreferencesUpdate`** Pydantic model with `pdf_open_mode: Literal["in_app", "new_tab"]` +- **`GET /api/auth/me/preferences`** — returns `{"pdf_open_mode": ...}` using `get_current_user` (both roles) +- **`PATCH /api/auth/me/preferences`** — validates via Literal, updates `current_user.pdf_open_mode`, commits, returns updated value + +## Commits + +| Hash | Message | +|------|---------| +| `8e6cb6e` | test(phase-4-05): add failing tests for document streaming proxy (DOC-02) — RED phase | +| `f868a4e` | feat(phase-4-05): document streaming proxy GET /api/documents/{id}/content (DOC-02) — GREEN phase | +| `2a0df32` | feat(phase-4-05): PATCH /api/auth/me/preferences for pdf_open_mode (DOC-01) | + +## Test Results + +**Before this plan:** 85 passed, 10 xfailed (document tests only had xfail stubs) +**After this plan:** 137 passed, 35 xfailed — all new tests green + +| Test | Result | +|------|--------| +| test_content_stream_200 | PASSED | +| test_content_stream_206_range | PASSED | +| test_content_stream_admin_403 | PASSED | +| test_content_stream_no_presigned_url | PASSED | +| test_content_stream_share_recipient_200 | PASSED | +| test_content_stream_not_found | PASSED | +| test_content_stream_invalid_id | PASSED | +| test_parse_range_416 | PASSED | +| test_get_preferences_default | PASSED | +| test_patch_preferences_in_app | PASSED | +| test_patch_preferences_new_tab | PASSED | +| test_patch_preferences_invalid_value | PASSED | +| test_patch_preferences_persists | PASSED | +| test_preferences_requires_auth | PASSED | +| test_patch_preferences_requires_auth | PASSED | + +Pre-existing failure: `test_extract_docx` (missing `python-docx` module in local env — not introduced by this plan). + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] Deprecated HTTP_416 status constant** +- **Found during:** Task 1 implementation +- **Issue:** `HTTP_416_REQUESTED_RANGE_NOT_SATISFIABLE` is deprecated in the installed FastAPI version; `HTTP_416_RANGE_NOT_SATISFIABLE` is the current constant +- **Fix:** Used `HTTP_416_RANGE_NOT_SATISFIABLE` throughout +- **Files modified:** `backend/api/documents.py` +- **Commit:** f868a4e + +**2. [Rule 2 - Missing functionality] pdf_open_mode absent from User ORM model** +- **Found during:** Task 2 — plan noted "migration 0004 adds column" but the ORM `User` class in `models.py` did not declare `pdf_open_mode` +- **Fix:** Added `pdf_open_mode: Mapped[str]` with `server_default="in_app"` to the User class +- **Files modified:** `backend/db/models.py` +- **Commit:** f868a4e + +## Security Invariants Verified + +| Threat ID | Status | +|-----------|--------| +| T-04-05-01: Admin blocked at content proxy | VERIFIED — `get_regular_user` dep; `test_content_stream_admin_403` passes | +| T-04-05-02: No presigned URL in proxy | VERIFIED — `presigned_mock.assert_not_called()` in `test_content_stream_no_presigned_url` | +| T-04-05-03: Range validation bounds | VERIFIED — `test_parse_range_416` confirms 416 on out-of-bounds | +| T-04-05-04: Non-recipient 404 | VERIFIED — unshared doc returns 404; share recipient gets 200 | +| T-04-05-05: pdf_open_mode Literal | VERIFIED — `test_patch_preferences_invalid_value` confirms 422 on invalid value | + +## Known Stubs + +None — all behavior is fully implemented and wired. + +## Threat Flags + +None — no new network endpoints, auth paths, or file access patterns beyond what was planned in the threat model. + +## Self-Check: PASSED + +- [x] `backend/api/documents.py` — stream_document_content endpoint exists +- [x] `backend/api/auth.py` — /me/preferences routes registered (GET + PATCH) +- [x] `backend/db/models.py` — pdf_open_mode column on User model +- [x] `backend/tests/test_documents.py` — 8 streaming proxy tests pass +- [x] `backend/tests/test_auth_api.py` — 7 preferences tests pass +- [x] Commits: 8e6cb6e (RED), f868a4e (GREEN Task 1), 2a0df32 (Task 2) +- [x] Full suite: 137 passed, 1 pre-existing failure (test_extract_docx — missing docx module) diff --git a/.planning/phases/04-folders-sharing-quotas-document-ux/04-06-SUMMARY.md b/.planning/phases/04-folders-sharing-quotas-document-ux/04-06-SUMMARY.md new file mode 100644 index 0000000..1d7b149 --- /dev/null +++ b/.planning/phases/04-folders-sharing-quotas-document-ux/04-06-SUMMARY.md @@ -0,0 +1,109 @@ +--- +phase: 04-folders-sharing-quotas-document-ux +plan: "06" +subsystem: admin-audit +tags: [audit-log, admin-api, celery, csv-export, minio, security] +dependency_graph: + requires: ["04-03", "04-04"] + provides: ["ADMIN-06", "D-17"] + affects: ["backend/api/audit.py", "backend/tasks/audit_tasks.py", "backend/celery_app.py", "backend/main.py"] +tech_stack: + added: [] + patterns: + - "Admin-only audit log viewer with paginated, filtered SQLAlchemy query" + - "Streaming CSV export via FastAPI StreamingResponse + csv.DictWriter" + - "Celery beat crontab schedule at midnight UTC for daily MinIO export" + - "Deferred imports inside async task body to prevent circular imports" + - "_audit_to_dict() safe whitelist serializer pattern (mirrors _user_to_dict)" +key_files: + created: + - backend/api/audit.py + - backend/tasks/audit_tasks.py + modified: + - backend/celery_app.py + - backend/main.py +decisions: + - "CSV export reuses _audit_to_dict() whitelist helper — single source of truth for safe field set" + - "audit_tasks.* routed to documents queue — reuses existing documents worker (no new queue needed)" + - "crontab alias uses _crontab (underscore prefix) consistent with existing _timedelta alias" +metrics: + duration_seconds: 262 + completed_date: "2026-05-25" + tasks_completed: 2 + files_created: 2 + files_modified: 2 +--- + +# Phase 4 Plan 06: Admin Audit Log API + Celery Daily Export Summary + +**One-liner:** Admin-only paginated/filtered audit log viewer with CSV streaming export (ADMIN-06) and midnight-UTC Celery beat task uploading daily CSVs to MinIO audit-logs bucket (D-17). + +## Tasks Completed + +| Task | Name | Commit | Files | +|------|------|--------|-------| +| 1 | Admin audit log viewer + CSV export | 364447d | backend/api/audit.py, backend/main.py | +| 2 | Celery daily export task + beat schedule | f89f787 | backend/tasks/audit_tasks.py, backend/celery_app.py | + +## What Was Built + +### Task 1: backend/api/audit.py + +Two admin-only endpoints protected by `Depends(get_current_admin)`: + +- `GET /api/admin/audit-log` — paginated (page/per_page), filtered (start, end, user_id, event_type). Returns `{items, total, page, per_page}`. Runs a separate COUNT query for total using the same filters. +- `GET /api/admin/audit-log/export` — same filter params, no pagination; streams CSV with `Content-Disposition: attachment; filename=audit-export.csv`. + +The `_audit_to_dict()` helper is the single source of truth for the safe field set: `id, event_type, user_id, actor_id, resource_id, ip_address, metadata_, created_at`. The dict literal contains no `filename`, `extracted_text`, `password_hash`, or `credentials_enc` keys. Both the JSON and CSV paths use this same helper. + +### Task 2: backend/tasks/audit_tasks.py + celery_app.py + +- `audit_log_daily_export` Celery task: sync entry point → `asyncio.run(_run_daily_export())`. +- `_run_daily_export()`: queries yesterday's `AuditLog` rows (UTC midnight to midnight), writes CSV via `csv.DictWriter`, uploads to MinIO via `put_object_raw(bucket="audit-logs", key="audit-logs/YYYY-MM-DD.csv", ...)`. Wraps everything in try/except — returns `{"exported": 0, "error": str(e)}` on failure. +- All imports deferred inside `_run_daily_export()` body (same circular-import-prevention pattern as `document_tasks._run`). +- `celery_app.py`: `_crontab` aliased import, beat entry `"audit-log-daily-export"` at `_crontab(hour=0, minute=0)`, task route `"tasks.audit_tasks.*": {"queue": "documents"}`. + +## Deviations from Plan + +None — plan executed exactly as written. + +## Security Invariants Verified + +| Threat ID | Check | Result | +|-----------|-------|--------| +| T-04-06-01 | `Depends(get_current_admin)` on both endpoints (grep: 2 occurrences at lines 94, 129) | PASS | +| T-04-06-02 | `_audit_to_dict()` dict literal contains no forbidden keys (grep: filename/extracted_text only in comments) | PASS | +| T-04-06-03 | CSV export uses same `_audit_to_dict()` helper as JSON viewer | PASS | +| T-04-06-04 | `put_object_raw` uses `bucket="audit-logs"` (not documents bucket) | PASS | + +## Test Results + +``` +tests/test_audit.py: 4 xfailed (stub tests from Wave 0 — plan 04-06 implements the API, + detailed integration tests will be written in the full TDD pass) +Full suite: 1 failed (test_extractor.py::test_extract_docx — pre-existing missing module, + out of scope), 130 passed, 7 skipped, 35 xfailed +``` + +Pre-existing failures (not caused by this plan): +- `test_extractor.py::test_extract_docx` — missing python-docx module in local env +- `test_documents.py::test_content_stream_200` — intentional TDD RED from plan 04-05 (commit 8e6cb6e) + +## Known Stubs + +None — both endpoints are fully implemented and wired. + +## Threat Flags + +None — no new network endpoints or trust boundaries beyond those documented in the plan's threat model. + +## Self-Check: PASSED + +- [x] `backend/api/audit.py` exists: FOUND +- [x] `backend/tasks/audit_tasks.py` exists: FOUND +- [x] Task 1 commit 364447d: FOUND +- [x] Task 2 commit f89f787: FOUND +- [x] `python3 -c "from api.audit import router"` exits 0: PASS +- [x] `python3 -c "from tasks.audit_tasks import audit_log_daily_export"` exits 0: PASS +- [x] `beat_schedule` contains `audit-log-daily-export`: PASS +- [x] `task_routes` contains `tasks.audit_tasks.*`: PASS diff --git a/CLAUDE.md b/CLAUDE.md index d286aa4..ab3a3c8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -67,6 +67,103 @@ cd frontend && npm run dev cd backend && pytest -v ``` +## Testing Protocol (Non-Negotiable) + +Every feature, function, and bug fix requires tests. No phase or plan may advance until all tests pass. + +### Rules + +- **Coverage**: Every new function, endpoint, and UI component must have at least one test — unit for isolated logic, integration for DB/service boundaries, E2E for critical user flows +- **Gate**: `pytest -v` (backend) and frontend test suite must pass with zero failures before marking a plan complete or advancing to the next phase +- **Bug fixes**: Must fix the root cause, not work around it. Maximum 50 lines of changed code per fix. If a fix requires more, it is scope-creep and must be broken into a separate plan +- **No workarounds**: `# type: ignore`, `noqa`, skipping a test, or adding a `try/except` that silently swallows an error are prohibited as bug fixes +- **Regression**: Any time a bug is fixed, a test must be added that would have caught it + +### Test types per layer + +| Layer | Required test type | +|---|---| +| Service / business logic | Unit tests with mocked dependencies | +| DB queries / ORM | Integration tests against real PostgreSQL (not SQLite for quota/UUID tests) | +| API endpoints | `httpx.AsyncClient` integration tests with real DB fixtures | +| Auth flows | Full round-trip tests (register → login → TOTP → refresh → revoke) | +| Security invariants | Dedicated negative tests (wrong owner → 403/404, admin → 403, replay → 401) | +| Frontend | Vitest unit tests for stores/composables; Playwright or Cypress for critical flows | + +--- + +## Security Protocol (Non-Negotiable) + +A dedicated **security agent** runs after every plan execution and before any phase is marked complete. This agent has full read/write/edit access to the entire codebase and is the final gate before advancement. + +### Security agent mandate + +The security agent must check — and fix — every class of vulnerability listed below. It may not flag and defer; it must resolve or escalate blocking issues. + +#### OWASP Top 10 + auth-specific + +| Threat | Required mitigation | +|---|---| +| SQL injection | All queries via ORM or parameterized statements — zero raw string interpolation | +| XSS | CSP headers, `httpOnly` cookies, no `innerHTML` with user data, Vue template auto-escaping never bypassed | +| CSRF | `SameSite=Strict` cookie + `Origin`/`Referer` header validation on all state-changing endpoints | +| Broken auth | Short-lived JWT (≤15 min), refresh rotation, family revocation on reuse, constant-time comparison | +| IDOR / broken access control | Every resource endpoint asserts `resource.user_id == current_user.id`; admin blocked from document content | +| Security misconfiguration | No debug mode in production, no stack traces in API responses, no default credentials | +| Sensitive data exposure | Passwords hashed Argon2id, PII fields encrypted at rest, `credentials_enc` never in API responses | +| Insecure deserialization | No `pickle`, no `eval`, no dynamic `__import__`; all user-supplied data validated via Pydantic | +| Vulnerable dependencies | `pip audit` / `npm audit` run; critical/high CVEs blocked | +| Insufficient logging | All auth events, quota violations, and admin actions written to audit log without document content | + +#### Advanced threats + +- **Path traversal**: All file path construction uses `os.path.basename` / `pathlib` — never joins user-supplied strings directly +- **SSRF**: All outbound HTTP (HIBP, cloud OAuth) via an allowlisted client; user-supplied URLs for WebDAV/Nextcloud must pass hostname allowlist +- **Timing attacks**: `hmac.compare_digest` / `secrets.compare_digest` for all token, TOTP, and backup-code comparison — no `==` +- **Race conditions / TOCTOU**: Quota enforcement via single atomic `UPDATE … RETURNING` — never read-then-write in Python +- **Mass assignment**: Pydantic models explicitly declare every accepted field; no `**kwargs` passthrough from request body to ORM +- **Privilege escalation**: `get_regular_user` and `get_current_admin` deps checked on every endpoint; no role elevation path exists +- **Token replay**: JTI stored in DB; used TOTP codes invalidated within the 90 s window; refresh token family revocation on reuse + +#### Zero-day / defense-in-depth + +- **Minimal attack surface**: Every endpoint that is not needed is absent — no commented-out code, no `TODO: remove` endpoints left alive +- **Principle of least privilege**: `docuvault_app` DB role has DML only; `docuvault_migrate` has DDL; MinIO bucket policy denies public access +- **Secrets in env only**: No credentials, API keys, or signing secrets in code, commits, or `.env` files checked in; `.gitignore` enforces this +- **Dependency pinning**: `requirements.txt` and `package-lock.json` pin exact versions; no floating `>=` for security-critical packages (PyJWT, pwdlib, cryptography) +- **Container hardening**: Non-root user in Dockerfile, read-only filesystem where possible, no `--privileged` containers +- **Header hardening**: `X-Content-Type-Options: nosniff`, `X-Frame-Options: DENY`, `Referrer-Policy: strict-origin-when-cross-origin` on every response + +### Database user table encryption + +Sensitive user PII (email, display name) must be encrypted at the application layer before storage: + +- Encryption: AES-256-GCM via `cryptography` library, per-row nonce, master key from env var +- Key derivation: HKDF-SHA256 with `purpose=b"user-pii"` salt — same pattern as cloud credentials +- Admin queries: never return plaintext PII for users other than the requesting user +- Indexing: email lookup uses a deterministic HMAC-SHA256 index (`email_hmac` column) — the encrypted column is never used for WHERE clauses + +### Login token hardening (state of the art) + +- **Algorithm**: ES256 (ECDSA P-256) — asymmetric; the private key signs, the public key verifies; a leaked public key cannot forge tokens +- **Access token TTL**: 15 minutes maximum +- **Refresh token**: 30-day httpOnly Strict cookie; rotated on every use; reuse of a rotated token revokes entire family and fires a security alert email +- **JTI claim**: Every token has a unique `jti`; revoked JTIs stored in Redis with TTL matching the token lifetime +- **Token binding**: Access token embeds a `fgp` (fingerprint) claim = HMAC of `User-Agent + Accept-Language`; backend validates on every request +- **Rotation on privilege change**: Password change, TOTP enroll/revoke, and account deactivation immediately revoke all active sessions + +### Security gate checklist (must all pass before phase advances) + +- [ ] `bandit -r backend/` — zero HIGH severity findings +- [ ] `pip audit` — zero critical/high CVEs +- [ ] `npm audit --audit-level=high` — zero high/critical vulnerabilities +- [ ] All security-invariant tests pass (wrong owner, admin block, token replay, CSRF) +- [ ] No new `# noqa: S` suppressions without a documented justification comment +- [ ] Admin endpoints verified to never return `password_hash`, `credentials_enc`, or document content +- [ ] No hardcoded secrets detected by `git secrets` / `trufflehog` + +--- + ## Security Requirements (Non-Negotiable) - Rate limiting on all auth endpoints (login, register, password reset, TOTP) diff --git a/backend/api/folders.py b/backend/api/folders.py index be500b3..eae2d37 100644 --- a/backend/api/folders.py +++ b/backend/api/folders.py @@ -112,6 +112,21 @@ async def create_folder( if parent is None or parent.user_id != current_user.id: raise HTTPException(status_code=404, detail="Parent folder not found") + # Explicit duplicate check — UniqueConstraint won't fire when parent_id IS NULL + # because SQL treats NULL as distinct from NULL in unique indexes. + dup = await session.execute( + select(Folder).where( + Folder.user_id == current_user.id, + Folder.name == body.name, + Folder.parent_id == parent_uuid, + ) + ) + if dup.scalar_one_or_none() is not None: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail="A folder with that name already exists here", + ) + folder = Folder( user_id=current_user.id, name=body.name, @@ -144,23 +159,57 @@ async def create_folder( @router.get("") async def list_folders( + parent_id: Optional[str] = None, session: AsyncSession = Depends(get_db), current_user: User = Depends(get_regular_user), ): - """List the current user's top-level folders (parent_id IS NULL). + """List the current user's folders at a given level. - FOLD-02: returns only folders belonging to current_user with no parent. + FOLD-02: when parent_id is omitted, returns root folders (parent_id IS NULL). + When parent_id is supplied, returns that folder's direct children (asserts ownership). + Each folder includes has_children so the frontend can hide expand arrows on leaf nodes. """ + parent_uuid: Optional[uuid.UUID] = None + if parent_id is not None: + try: + parent_uuid = uuid.UUID(parent_id) + except ValueError: + raise HTTPException(status_code=404, detail="Parent folder not found") + parent_folder = await session.get(Folder, parent_uuid) + if parent_folder is None or parent_folder.user_id != current_user.id: + raise HTTPException(status_code=404, detail="Parent folder not found") + + if parent_uuid is None: + where_clause = Folder.parent_id.is_(None) + else: + where_clause = Folder.parent_id == parent_uuid + result = await session.execute( select(Folder) - .where( - Folder.user_id == current_user.id, - Folder.parent_id.is_(None), - ) + .where(Folder.user_id == current_user.id, where_clause) .order_by(Folder.name) ) folders = result.scalars().all() - return {"items": [_folder_to_dict(f) for f in folders]} + + # One extra query to know which of these folders have sub-folders. + # Allows the frontend to hide expand arrows on leaf nodes without extra round-trips. + folder_ids = [f.id for f in folders] + folders_with_children: set = set() + if folder_ids: + children_result = await session.execute( + select(Folder.parent_id.distinct()).where( + Folder.user_id == current_user.id, + Folder.parent_id.in_(folder_ids), + ) + ) + folders_with_children = {row[0] for row in children_result} + + return { + "items": [ + {**_folder_to_dict(f), "has_children": f.id in folders_with_children} + for f in folders + ] + } # ── GET /api/folders/{folder_id} ────────────────────────────────────────────── @@ -235,6 +284,22 @@ async def rename_folder( raise HTTPException(status_code=404, detail="Folder not found") old_name = folder.name + # Explicit duplicate check — same NULL parent_id issue as create_folder. + if body.name != folder.name: + dup = await session.execute( + select(Folder).where( + Folder.user_id == current_user.id, + Folder.name == body.name, + Folder.parent_id == folder.parent_id, + Folder.id != folder.id, + ) + ) + if dup.scalar_one_or_none() is not None: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail="A folder with that name already exists here", + ) + folder.name = body.name try: await session.commit() @@ -303,7 +368,8 @@ async def delete_folder( " WHERE f.user_id = :uid" ") SELECT id FROM subtree" ), - {"root_id": str(folder.id), "uid": str(current_user.id)}, + # Use .hex (no dashes) — SQLite stores UUID as 32-char hex; PostgreSQL accepts both. + {"root_id": folder.id.hex, "uid": current_user.id.hex}, ) subtree_folder_ids = [str(row[0]) for row in cte_result.fetchall()] except OperationalError: @@ -344,7 +410,7 @@ async def delete_folder( "CASE WHEN used_bytes > :delta THEN used_bytes - :delta ELSE 0 END " "WHERE user_id = :uid" ), - {"delta": total_bytes, "uid": str(current_user.id)}, + {"delta": total_bytes, "uid": current_user.id.hex}, ) # Delete MinIO objects best-effort (per-object, never abort on failure) diff --git a/backend/api/topics.py b/backend/api/topics.py index 75a9e3f..fb99a55 100644 --- a/backend/api/topics.py +++ b/backend/api/topics.py @@ -7,7 +7,7 @@ from fastapi import APIRouter, Depends, HTTPException from pydantic import BaseModel from sqlalchemy.ext.asyncio import AsyncSession -from db.models import Topic, User +from db.models import Document, Topic, User from deps.auth import get_current_user from deps.db import get_db from services import classifier, storage @@ -137,10 +137,18 @@ async def suggest_topics( """Suggest topics for a document using AI. D-11: classifier uses the user's namespace (system + user topics) for suggestions. + D-16 / SEC-IDOR: asserts document ownership — cross-user access returns 404 + to prevent document ID enumeration (same pattern as documents router). """ - meta = await storage.get_metadata(session, body.document_id) - if meta is None: + try: + uid = uuid.UUID(body.document_id) + except ValueError: raise HTTPException(404, "Document not found") + + doc = await session.get(Document, uid) + if doc is None or doc.user_id != current_user.id: + raise HTTPException(404, "Document not found") + try: suggestions = await classifier.suggest_topics_for_document(session, body.document_id) except Exception as e: diff --git a/backend/services/auth.py b/backend/services/auth.py index 790ede9..508a096 100644 --- a/backend/services/auth.py +++ b/backend/services/auth.py @@ -356,7 +356,7 @@ async def check_hibp(password: str) -> bool: Returns True if the password has been breached, False otherwise. On network error: logs a warning and returns False (fail-open, T-02-06). """ - sha1 = hashlib.sha1(password.encode("utf-8")).hexdigest().upper() + sha1 = hashlib.sha1(password.encode("utf-8"), usedforsecurity=False).hexdigest().upper() # noqa: S324 — HIBP k-anonymity protocol mandates SHA-1; not used as a security primitive prefix, suffix = sha1[:5], sha1[5:] try: diff --git a/backend/tests/test_folders.py b/backend/tests/test_folders.py index 044679d..73ef7d6 100644 --- a/backend/tests/test_folders.py +++ b/backend/tests/test_folders.py @@ -1,123 +1,494 @@ """ -Folder API tests — Wave 0 xfail stubs for Phase 4. +Folder API tests — FOLD-01 through FOLD-05. -All tests in this file are xfail stubs. They will be implemented in Plans 04-02 -through 04-04. The stubs ensure pytest collects them and keeps CI green before -implementation code exists. +Covers: + POST /api/folders — create (FOLD-01) + GET /api/folders — list root / children (FOLD-02) + GET /api/folders/{id} — get + breadcrumb (FOLD-02, FOLD-05) + PATCH /api/folders/{id} — rename (FOLD-03) + DELETE /api/folders/{id} — delete cascade (FOLD-03) + PATCH /api/documents/{id}/folder — move document (FOLD-04) + +Security invariants (T-04-03-xx) tested throughout. """ from __future__ import annotations -import os +import uuid as _uuid import pytest +import pytest_asyncio +from sqlalchemy.ext.asyncio import AsyncSession + +from db.models import Document, Folder, Quota, User -# --------------------------------------------------------------------------- -# FOLD-01: Create folder -# --------------------------------------------------------------------------- +# ── Helpers ─────────────────────────────────────────────────────────────────── -@pytest.mark.xfail(strict=False) -async def test_create_folder(async_client, auth_user): - """POST /api/folders creates a folder, returns 201.""" - pytest.xfail("not implemented yet") +async def _create_folder(db: AsyncSession, user: User, name: str, parent=None) -> Folder: + """Create a Folder row directly via ORM.""" + f = Folder( + user_id=user.id, + name=name, + parent_id=parent.id if parent else None, + ) + db.add(f) + await db.commit() + await db.refresh(f) + return f -@pytest.mark.xfail(strict=False) -async def test_create_folder_duplicate_name(async_client, auth_user): +async def _create_document(db: AsyncSession, user: User, *, folder=None) -> Document: + """Create a minimal Document row via ORM (no MinIO object needed).""" + doc_id = _uuid.uuid4() + doc = Document( + id=doc_id, + user_id=user.id, + filename="test.pdf", + content_type="application/pdf", + size_bytes=1024, + storage_backend="minio", + status="uploaded", + object_key=f"{user.id}/{doc_id}/{_uuid.uuid4()}.pdf", + folder_id=folder.id if folder else None, + ) + db.add(doc) + await db.commit() + await db.refresh(doc) + return doc + + +# ── FOLD-01: Create folder ──────────────────────────────────────────────────── + + +async def test_create_root_folder(async_client, auth_user): + """POST /api/folders creates a root folder, returns 201 with id/name.""" + resp = await async_client.post( + "/api/folders", + json={"name": "MyFolder"}, + headers=auth_user["headers"], + ) + assert resp.status_code == 201 + data = resp.json() + assert data["name"] == "MyFolder" + assert data["parent_id"] is None + assert "id" in data + + +async def test_create_subfolder(async_client, auth_user, db_session): + """POST /api/folders with parent_id creates a child folder.""" + parent = await _create_folder(db_session, auth_user["user"], "Parent") + + resp = await async_client.post( + "/api/folders", + json={"name": "Child", "parent_id": str(parent.id)}, + headers=auth_user["headers"], + ) + assert resp.status_code == 201 + data = resp.json() + assert data["name"] == "Child" + assert data["parent_id"] == str(parent.id) + + +async def test_create_folder_duplicate_name_409(async_client, auth_user, db_session): """POST /api/folders with same name under same parent returns 409.""" - pytest.xfail("not implemented yet") + await _create_folder(db_session, auth_user["user"], "DupFolder") + + resp = await async_client.post( + "/api/folders", + json={"name": "DupFolder"}, + headers=auth_user["headers"], + ) + assert resp.status_code == 409 -# --------------------------------------------------------------------------- -# FOLD-02: Rename folder -# --------------------------------------------------------------------------- +async def test_create_folder_invalid_parent_404(async_client, auth_user): + """POST /api/folders with non-existent parent_id returns 404.""" + resp = await async_client.post( + "/api/folders", + json={"name": "Orphan", "parent_id": str(_uuid.uuid4())}, + headers=auth_user["headers"], + ) + assert resp.status_code == 404 -@pytest.mark.xfail(strict=False) -async def test_rename_folder(async_client, auth_user): - """PATCH /api/folders/{id} changes name, returns 200.""" - pytest.xfail("not implemented yet") +async def test_create_folder_other_users_parent_404(async_client, auth_user, admin_user, db_session): + """POST /api/folders with parent owned by another user returns 404 (IDOR).""" + other_parent = await _create_folder(db_session, admin_user["user"], "AdminParent") + + resp = await async_client.post( + "/api/folders", + json={"name": "Steal", "parent_id": str(other_parent.id)}, + headers=auth_user["headers"], + ) + assert resp.status_code == 404 -@pytest.mark.xfail(strict=False) -async def test_rename_folder_wrong_owner(async_client, auth_user): - """PATCH /api/folders/{id} by non-owner returns 404.""" - pytest.xfail("not implemented yet") +async def test_create_folder_requires_auth(async_client): + """POST /api/folders without auth returns 401 or 403.""" + resp = await async_client.post("/api/folders", json={"name": "Unauth"}) + assert resp.status_code in (401, 403) -# --------------------------------------------------------------------------- -# FOLD-03: Delete folder -# --------------------------------------------------------------------------- +# ── FOLD-02: List folders ───────────────────────────────────────────────────── -@pytest.mark.xfail(strict=False) -async def test_delete_empty_folder(async_client, auth_user): +async def test_list_root_folders(async_client, auth_user, db_session): + """GET /api/folders returns only root folders for current user.""" + await _create_folder(db_session, auth_user["user"], "RootA") + await _create_folder(db_session, auth_user["user"], "RootB") + + resp = await async_client.get("/api/folders", headers=auth_user["headers"]) + assert resp.status_code == 200 + items = resp.json()["items"] + names = {f["name"] for f in items} + assert "RootA" in names + assert "RootB" in names + + +async def test_list_root_folders_excludes_other_users(async_client, auth_user, admin_user, db_session): + """GET /api/folders does not return other users' folders.""" + await _create_folder(db_session, admin_user["user"], "AdminRoot") + + resp = await async_client.get("/api/folders", headers=auth_user["headers"]) + names = {f["name"] for f in resp.json()["items"]} + assert "AdminRoot" not in names + + +async def test_list_root_folders_excludes_subfolders(async_client, auth_user, db_session): + """GET /api/folders (no parent_id) does not return nested folders.""" + root = await _create_folder(db_session, auth_user["user"], "Root") + await _create_folder(db_session, auth_user["user"], "Child", parent=root) + + resp = await async_client.get("/api/folders", headers=auth_user["headers"]) + items = resp.json()["items"] + names = [f["name"] for f in items] + assert "Root" in names + assert "Child" not in names + + +async def test_list_children(async_client, auth_user, db_session): + """GET /api/folders?parent_id=X returns direct children of X.""" + root = await _create_folder(db_session, auth_user["user"], "Root") + child = await _create_folder(db_session, auth_user["user"], "Child", parent=root) + + resp = await async_client.get( + f"/api/folders?parent_id={root.id}", headers=auth_user["headers"] + ) + assert resp.status_code == 200 + items = resp.json()["items"] + assert len(items) == 1 + assert items[0]["id"] == str(child.id) + + +async def test_list_children_other_user_parent_404(async_client, auth_user, admin_user, db_session): + """GET /api/folders?parent_id= with other user's folder returns 404 (T-04-03-04).""" + other_folder = await _create_folder(db_session, admin_user["user"], "OtherFolder") + + resp = await async_client.get( + f"/api/folders?parent_id={other_folder.id}", headers=auth_user["headers"] + ) + assert resp.status_code == 404 + + +async def test_list_folders_has_children_field(async_client, auth_user, db_session): + """GET /api/folders includes has_children: true for folders with sub-folders.""" + root = await _create_folder(db_session, auth_user["user"], "WithChild") + await _create_folder(db_session, auth_user["user"], "Leaf", parent=root) + await _create_folder(db_session, auth_user["user"], "NoChild") + + resp = await async_client.get("/api/folders", headers=auth_user["headers"]) + items = {f["name"]: f for f in resp.json()["items"]} + + assert items["WithChild"]["has_children"] is True + assert items["NoChild"]["has_children"] is False + + +# ── FOLD-02 / FOLD-05: Get folder + breadcrumb ──────────────────────────────── + + +async def test_get_folder(async_client, auth_user, db_session): + """GET /api/folders/{id} returns folder metadata.""" + folder = await _create_folder(db_session, auth_user["user"], "GetMe") + + resp = await async_client.get(f"/api/folders/{folder.id}", headers=auth_user["headers"]) + assert resp.status_code == 200 + data = resp.json() + assert data["name"] == "GetMe" + assert data["id"] == str(folder.id) + + +async def test_get_folder_breadcrumb_single(async_client, auth_user, db_session): + """GET /api/folders/{id} breadcrumb for root folder = [{id, name}].""" + folder = await _create_folder(db_session, auth_user["user"], "RootFolder") + + resp = await async_client.get(f"/api/folders/{folder.id}", headers=auth_user["headers"]) + crumbs = resp.json()["breadcrumb"] + assert len(crumbs) == 1 + assert crumbs[0]["name"] == "RootFolder" + + +async def test_get_folder_breadcrumb_deep(async_client, auth_user, db_session): + """GET /api/folders/{id} breadcrumb is root-first for 3-level hierarchy.""" + root = await _create_folder(db_session, auth_user["user"], "Root") + mid = await _create_folder(db_session, auth_user["user"], "Mid", parent=root) + leaf = await _create_folder(db_session, auth_user["user"], "Leaf", parent=mid) + + resp = await async_client.get(f"/api/folders/{leaf.id}", headers=auth_user["headers"]) + assert resp.status_code == 200 + crumbs = resp.json()["breadcrumb"] + assert len(crumbs) == 3 + assert crumbs[0]["name"] == "Root" + assert crumbs[1]["name"] == "Mid" + assert crumbs[2]["name"] == "Leaf" + + +async def test_get_folder_wrong_owner_404(async_client, auth_user, admin_user, db_session): + """GET /api/folders/{id} for another user's folder returns 404 (T-04-03-04).""" + other = await _create_folder(db_session, admin_user["user"], "OtherFolder") + + resp = await async_client.get(f"/api/folders/{other.id}", headers=auth_user["headers"]) + assert resp.status_code == 404 + + +async def test_get_folder_not_found_404(async_client, auth_user): + """GET /api/folders/{id} with non-existent id returns 404.""" + resp = await async_client.get(f"/api/folders/{_uuid.uuid4()}", headers=auth_user["headers"]) + assert resp.status_code == 404 + + +# ── FOLD-03: Rename folder ──────────────────────────────────────────────────── + + +async def test_rename_folder(async_client, auth_user, db_session): + """PATCH /api/folders/{id} changes folder name, returns 200.""" + folder = await _create_folder(db_session, auth_user["user"], "OldName") + + resp = await async_client.patch( + f"/api/folders/{folder.id}", + json={"name": "NewName"}, + headers=auth_user["headers"], + ) + assert resp.status_code == 200 + assert resp.json()["name"] == "NewName" + + +async def test_rename_folder_duplicate_409(async_client, auth_user, db_session): + """PATCH /api/folders/{id} to a name that already exists at same level → 409.""" + await _create_folder(db_session, auth_user["user"], "Existing") + to_rename = await _create_folder(db_session, auth_user["user"], "ToRename") + + resp = await async_client.patch( + f"/api/folders/{to_rename.id}", + json={"name": "Existing"}, + headers=auth_user["headers"], + ) + assert resp.status_code == 409 + + +async def test_rename_folder_wrong_owner_404(async_client, auth_user, admin_user, db_session): + """PATCH /api/folders/{id} for another user's folder returns 404 (T-04-03-04).""" + other = await _create_folder(db_session, admin_user["user"], "AdminFolder") + + resp = await async_client.patch( + f"/api/folders/{other.id}", + json={"name": "Hijacked"}, + headers=auth_user["headers"], + ) + assert resp.status_code == 404 + + +# ── FOLD-03: Delete folder ──────────────────────────────────────────────────── + + +async def test_delete_empty_folder(async_client, auth_user, db_session): """DELETE /api/folders/{id} on empty folder returns 204.""" - pytest.xfail("not implemented yet") + folder = await _create_folder(db_session, auth_user["user"], "Empty") + + resp = await async_client.delete(f"/api/folders/{folder.id}", headers=auth_user["headers"]) + assert resp.status_code == 204 + + # Verify it's gone + resp2 = await async_client.get(f"/api/folders/{folder.id}", headers=auth_user["headers"]) + assert resp2.status_code == 404 -@pytest.mark.xfail(strict=False) -async def test_delete_folder_cascade(async_client, auth_user): - """DELETE /api/folders/{id} on non-empty folder deletes all docs + decrements quota.""" - pytest.xfail("not implemented yet") +async def test_delete_folder_cascade_documents(async_client, auth_user, db_session): + """DELETE /api/folders/{id} cascades to documents inside it.""" + folder = await _create_folder(db_session, auth_user["user"], "WithDocs") + doc = await _create_document(db_session, auth_user["user"], folder=folder) + + resp = await async_client.delete(f"/api/folders/{folder.id}", headers=auth_user["headers"]) + assert resp.status_code == 204 + + # Document should be deleted + doc_resp = await async_client.get(f"/api/documents/{doc.id}", headers=auth_user["headers"]) + assert doc_resp.status_code == 404 -@pytest.mark.xfail(strict=False) -async def test_delete_folder_wrong_owner(async_client, auth_user): - """DELETE /api/folders/{id} by non-owner returns 404.""" - pytest.xfail("not implemented yet") +async def test_delete_folder_cascade_quota(async_client, auth_user, db_session): + """DELETE /api/folders/{id} decrements quota by sum of deleted doc sizes.""" + folder = await _create_folder(db_session, auth_user["user"], "ForQuota") + + # Manually set quota used_bytes to 2048 (2 x 1024) + quota = await db_session.get(Quota, auth_user["user"].id) + quota.used_bytes = 2048 + await db_session.commit() + + await _create_document(db_session, auth_user["user"], folder=folder) # 1024 bytes + + resp = await async_client.delete(f"/api/folders/{folder.id}", headers=auth_user["headers"]) + assert resp.status_code == 204 + + await db_session.refresh(quota) + assert quota.used_bytes == 1024 # decremented by 1024 -# --------------------------------------------------------------------------- -# FOLD-04: Move document -# --------------------------------------------------------------------------- +async def test_delete_subfolder_not_in_parent_list(async_client, auth_user, db_session): + """DELETE /api/folders/{id} on subfolder — parent folder still exists after.""" + root = await _create_folder(db_session, auth_user["user"], "Root") + sub = await _create_folder(db_session, auth_user["user"], "Sub", parent=root) + + resp = await async_client.delete(f"/api/folders/{sub.id}", headers=auth_user["headers"]) + assert resp.status_code == 204 + + # Root still exists + root_resp = await async_client.get(f"/api/folders/{root.id}", headers=auth_user["headers"]) + assert root_resp.status_code == 200 -@pytest.mark.xfail(strict=False) -async def test_move_document(async_client, auth_user): +async def test_delete_folder_wrong_owner_404(async_client, auth_user, admin_user, db_session): + """DELETE /api/folders/{id} for another user's folder returns 404 (T-04-03-04).""" + other = await _create_folder(db_session, admin_user["user"], "AdminFolder") + + resp = await async_client.delete(f"/api/folders/{other.id}", headers=auth_user["headers"]) + assert resp.status_code == 404 + + +async def test_delete_folder_not_found_404(async_client, auth_user): + """DELETE /api/folders/{id} with non-existent id returns 404.""" + resp = await async_client.delete( + f"/api/folders/{_uuid.uuid4()}", headers=auth_user["headers"] + ) + assert resp.status_code == 404 + + +# ── FOLD-04: Move document ──────────────────────────────────────────────────── + + +async def test_move_document_to_folder(async_client, auth_user, db_session): """PATCH /api/documents/{id}/folder moves doc to target folder, returns 200.""" - pytest.xfail("not implemented yet") + folder = await _create_folder(db_session, auth_user["user"], "Destination") + doc = await _create_document(db_session, auth_user["user"]) + + resp = await async_client.patch( + f"/api/documents/{doc.id}/folder", + json={"folder_id": str(folder.id)}, + headers=auth_user["headers"], + ) + assert resp.status_code == 200 + assert resp.json()["folder_id"] == str(folder.id) -@pytest.mark.xfail(strict=False) -async def test_move_wrong_owner_404(async_client, auth_user): - """PATCH /api/documents/{id}/folder where doc or target folder belongs to other user returns 404.""" - pytest.xfail("not implemented yet") +async def test_move_document_to_root(async_client, auth_user, db_session): + """PATCH /api/documents/{id}/folder with folder_id: null moves doc to root.""" + folder = await _create_folder(db_session, auth_user["user"], "Source") + doc = await _create_document(db_session, auth_user["user"], folder=folder) + + resp = await async_client.patch( + f"/api/documents/{doc.id}/folder", + json={"folder_id": None}, + headers=auth_user["headers"], + ) + assert resp.status_code == 200 + assert resp.json()["folder_id"] is None -# --------------------------------------------------------------------------- -# FOLD-05: Breadcrumb, sort, FTS -# --------------------------------------------------------------------------- +async def test_move_document_wrong_owner_404(async_client, auth_user, admin_user, db_session): + """PATCH /api/documents/{id}/folder for another user's doc returns 404 (IDOR).""" + other_doc = await _create_document(db_session, admin_user["user"]) + folder = await _create_folder(db_session, auth_user["user"], "MyFolder") + + resp = await async_client.patch( + f"/api/documents/{other_doc.id}/folder", + json={"folder_id": str(folder.id)}, + headers=auth_user["headers"], + ) + assert resp.status_code == 404 -@pytest.mark.xfail(strict=False) -async def test_breadcrumb_path(async_client, auth_user): - """GET /api/folders/{id} returns breadcrumb array of {id, name} from root to current.""" - pytest.xfail("not implemented yet") +async def test_move_document_to_other_users_folder_404(async_client, auth_user, admin_user, db_session): + """PATCH /api/documents/{id}/folder with target folder owned by another user → 404 (T-04-03-05).""" + doc = await _create_document(db_session, auth_user["user"]) + other_folder = await _create_folder(db_session, admin_user["user"], "AdminFolder") + + resp = await async_client.patch( + f"/api/documents/{doc.id}/folder", + json={"folder_id": str(other_folder.id)}, + headers=auth_user["headers"], + ) + assert resp.status_code == 404 -@pytest.mark.xfail(strict=False) -async def test_document_sort(async_client, auth_user): - """GET /api/documents?sort=name|date|size returns correctly ordered results.""" - pytest.xfail("not implemented yet") +async def test_move_document_invalid_folder_404(async_client, auth_user, db_session): + """PATCH /api/documents/{id}/folder with non-existent folder_id returns 404.""" + doc = await _create_document(db_session, auth_user["user"]) + + resp = await async_client.patch( + f"/api/documents/{doc.id}/folder", + json={"folder_id": str(_uuid.uuid4())}, + headers=auth_user["headers"], + ) + assert resp.status_code == 404 -@pytest.mark.xfail(strict=False) -@pytest.mark.skipif( - not os.environ.get("INTEGRATION"), - reason="requires PostgreSQL", -) -async def test_fts_search(async_client, auth_user): - """GET /api/documents?q=term returns matching docs only; requires PostgreSQL FTS.""" - pytest.xfail("not implemented yet") +# ── FOLD-05: Sort and breadcrumb edge cases ─────────────────────────────────── -@pytest.mark.xfail(strict=False) -@pytest.mark.skipif( - not os.environ.get("INTEGRATION"), - reason="requires PostgreSQL", -) -async def test_fts_search_scoped_to_owner(async_client, auth_user): - """GET /api/documents?q=term does not return other user's matching docs.""" - pytest.xfail("not implemented yet") +async def test_list_folders_sorted_by_name(async_client, auth_user, db_session): + """GET /api/folders returns folders ordered alphabetically by name.""" + for name in ("Zebra", "Apple", "Mango"): + await _create_folder(db_session, auth_user["user"], name) + + resp = await async_client.get("/api/folders", headers=auth_user["headers"]) + names = [f["name"] for f in resp.json()["items"]] + assert names == sorted(names) + + +async def test_breadcrumb_two_levels(async_client, auth_user, db_session): + """GET /api/folders/{id} breadcrumb for two-level tree is [root, child].""" + root = await _create_folder(db_session, auth_user["user"], "Root") + child = await _create_folder(db_session, auth_user["user"], "Child", parent=root) + + resp = await async_client.get(f"/api/folders/{child.id}", headers=auth_user["headers"]) + crumbs = resp.json()["breadcrumb"] + assert len(crumbs) == 2 + assert crumbs[0]["id"] == str(root.id) + assert crumbs[1]["id"] == str(child.id) + + +# ── Security: admin blocked from folder endpoints ──────────────────────────── + + +async def test_admin_cannot_access_folder_endpoints(async_client, admin_user, db_session): + """Admin role is blocked from folder endpoints (T-04-03-01: get_regular_user only).""" + resp = await async_client.post( + "/api/folders", json={"name": "AdminFolder"}, headers=admin_user["headers"] + ) + assert resp.status_code == 403 + + +async def test_list_documents_scoped_to_folder(async_client, auth_user, db_session): + """GET /api/documents?folder_id=X returns only docs in that folder.""" + folder_a = await _create_folder(db_session, auth_user["user"], "FolderA") + folder_b = await _create_folder(db_session, auth_user["user"], "FolderB") + doc_a = await _create_document(db_session, auth_user["user"], folder=folder_a) + await _create_document(db_session, auth_user["user"], folder=folder_b) + + resp = await async_client.get( + f"/api/documents?folder_id={folder_a.id}", headers=auth_user["headers"] + ) + assert resp.status_code == 200 + data = resp.json() + assert data["total"] == 1 + assert data["items"][0]["id"] == str(doc_a.id) diff --git a/backend/tests/test_lmstudio.py b/backend/tests/test_lmstudio.py index 223f02f..41bdbf5 100644 --- a/backend/tests/test_lmstudio.py +++ b/backend/tests/test_lmstudio.py @@ -30,7 +30,7 @@ async def test_lmstudio_health_check(): @pytest.mark.asyncio async def test_lmstudio_classify(): from ai.lmstudio_provider import LMStudioProvider - from config import DEFAULT_SYSTEM_PROMPT + from services.classifier import _DEFAULT_SYSTEM_PROMPT provider = LMStudioProvider( base_url="http://host.docker.internal:1234", @@ -39,7 +39,7 @@ async def test_lmstudio_classify(): result = await provider.classify( document_text="This document is an invoice for software development services.", existing_topics=["Finance", "Legal", "HR"], - system_prompt=DEFAULT_SYSTEM_PROMPT, + system_prompt=_DEFAULT_SYSTEM_PROMPT, ) # Result should have some topics assigned or suggested assert isinstance(result.topics, list) diff --git a/frontend/package.json b/frontend/package.json index 5bb67df..6ce0a35 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -8,15 +8,18 @@ "preview": "vite preview" }, "dependencies": { + "pinia": "^2.1.0", "vue": "^3.4.0", - "vue-router": "^4.3.0", - "pinia": "^2.1.0" + "vue-router": "^4.3.0" }, "devDependencies": { "@vitejs/plugin-vue": "^5.0.0", - "vite": "^5.2.0", - "tailwindcss": "^3.4.0", + "@vue/test-utils": "^2.4.10", + "autoprefixer": "^10.4.0", + "happy-dom": "^20.9.0", "postcss": "^8.4.0", - "autoprefixer": "^10.4.0" + "tailwindcss": "^3.4.0", + "vite": "^5.2.0", + "vitest": "^4.1.7" } } diff --git a/frontend/src/components/documents/DocumentCard.vue b/frontend/src/components/documents/DocumentCard.vue index 2d8608d..9679789 100644 --- a/frontend/src/components/documents/DocumentCard.vue +++ b/frontend/src/components/documents/DocumentCard.vue @@ -106,7 +106,7 @@ const foldersStore = useFoldersStore() const showShareModal = ref(false) const showFolderPicker = ref(false) -const allFolders = computed(() => foldersStore.folders) +const allFolders = computed(() => foldersStore.rootFolders) function openShareModal() { showShareModal.value = true diff --git a/frontend/src/components/folders/FolderTreeItem.vue b/frontend/src/components/folders/FolderTreeItem.vue new file mode 100644 index 0000000..00e1aee --- /dev/null +++ b/frontend/src/components/folders/FolderTreeItem.vue @@ -0,0 +1,108 @@ + + + diff --git a/frontend/src/components/folders/__tests__/FolderBreadcrumb.test.js b/frontend/src/components/folders/__tests__/FolderBreadcrumb.test.js new file mode 100644 index 0000000..57b97ae --- /dev/null +++ b/frontend/src/components/folders/__tests__/FolderBreadcrumb.test.js @@ -0,0 +1,108 @@ +import { describe, it, expect } from 'vitest' +import { mount } from '@vue/test-utils' +import FolderBreadcrumb from '../FolderBreadcrumb.vue' + +function seg(id, name) { return { id, name } } + +describe('FolderBreadcrumb', () => { + it('always renders a "Home" / "Folders" root button', () => { + const w = mount(FolderBreadcrumb, { props: { segments: [] } }) + expect(w.find('button').exists()).toBe(true) + }) + + it('clicking root button emits navigate(null)', async () => { + const w = mount(FolderBreadcrumb, { props: { segments: [] } }) + await w.find('button').trigger('click') + expect(w.emitted('navigate')).toBeTruthy() + expect(w.emitted('navigate')[0]).toEqual([null]) + }) + + it('renders intermediate segments as clickable buttons', () => { + const w = mount(FolderBreadcrumb, { + props: { segments: [seg('r1', 'Root'), seg('f1', 'Test')] }, + }) + // "Root" is intermediate (not last), "Test" is last (plain text) + const buttons = w.findAll('button') + // first button is "Home/Folders", second is "Root" + expect(buttons.length).toBe(2) + expect(buttons[1].text()).toBe('Root') + }) + + it('clicking intermediate segment emits navigate(id)', async () => { + const w = mount(FolderBreadcrumb, { + props: { segments: [seg('r1', 'Root'), seg('f1', 'Test')] }, + }) + const buttons = w.findAll('button') + await buttons[1].trigger('click') // "Root" button + expect(w.emitted('navigate')).toBeTruthy() + expect(w.emitted('navigate')[0]).toEqual(['r1']) + }) + + it('renders last segment as plain non-interactive text', () => { + const w = mount(FolderBreadcrumb, { + props: { segments: [seg('r1', 'Root'), seg('f1', 'Test')] }, + }) + // Last segment "Test" should be a , not a button + const spans = w.findAll('span') + const lastSpan = spans.find(s => s.text() === 'Test') + expect(lastSpan).toBeTruthy() + }) + + it('last segment is NOT clickable (no navigate event)', async () => { + const w = mount(FolderBreadcrumb, { + props: { segments: [seg('r1', 'Root'), seg('f1', 'Test')] }, + }) + const spans = w.findAll('span') + const lastSpan = spans.find(s => s.text() === 'Test') + if (lastSpan) await lastSpan.trigger('click') + // navigate should NOT have been emitted by clicking the last segment + const navigateEvents = (w.emitted('navigate') || []).filter(e => e[0] === 'f1') + expect(navigateEvents.length).toBe(0) + }) + + it('single segment: just root button + last segment as text', () => { + const w = mount(FolderBreadcrumb, { + props: { segments: [seg('f1', 'OnlyFolder')] }, + }) + // Only the "Home" button and "OnlyFolder" as plain text + const buttons = w.findAll('button') + expect(buttons.length).toBe(1) // just "Home" + expect(w.text()).toContain('OnlyFolder') + }) + + it('collapses >4 segments with ellipsis, preserving first and last two', () => { + const segments = [ + seg('a', 'A'), seg('b', 'B'), seg('c', 'C'), + seg('d', 'D'), seg('e', 'E'), + ] + const w = mount(FolderBreadcrumb, { props: { segments } }) + const text = w.text() + expect(text).toContain('A') // first preserved + expect(text).toContain('…') // ellipsis present + expect(text).toContain('D') // second-to-last preserved + expect(text).toContain('E') // last preserved + expect(text).not.toContain('B') // middle segments collapsed + expect(text).not.toContain('C') + }) + + it('3 segments: all rendered without ellipsis', () => { + const segments = [seg('a', 'A'), seg('b', 'B'), seg('c', 'C')] + const w = mount(FolderBreadcrumb, { props: { segments } }) + const text = w.text() + expect(text).toContain('A') + expect(text).toContain('B') + expect(text).toContain('C') + expect(text).not.toContain('…') + }) + + it('deep 3-level path: clicking middle segment navigates correctly', async () => { + const segments = [seg('root', 'Root'), seg('mid', 'Mid'), seg('cur', 'Current')] + const w = mount(FolderBreadcrumb, { props: { segments } }) + const buttons = w.findAll('button') + // buttons[0] = Home, buttons[1] = Root, buttons[2] = Mid + await buttons[2].trigger('click') + const events = w.emitted('navigate') || [] + const midClicks = events.filter(e => e[0] === 'mid') + expect(midClicks.length).toBe(1) + }) +}) diff --git a/frontend/src/components/folders/__tests__/FolderTreeItem.test.js b/frontend/src/components/folders/__tests__/FolderTreeItem.test.js new file mode 100644 index 0000000..0b3d38a --- /dev/null +++ b/frontend/src/components/folders/__tests__/FolderTreeItem.test.js @@ -0,0 +1,175 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { mount } from '@vue/test-utils' +import { setActivePinia, createPinia } from 'pinia' +import { createRouter, createMemoryHistory } from 'vue-router' +import FolderTreeItem from '../FolderTreeItem.vue' + +const mockListFolders = vi.fn() + +vi.mock('../../../api/client.js', () => ({ + listFolders: (...a) => mockListFolders(...a), +})) + +// Minimal router so router-link renders correctly +function makeRouter(currentPath = '/') { + return createRouter({ + history: createMemoryHistory(), + routes: [ + { path: '/', component: { template: '
' } }, + { path: '/folders/:folderId', component: { template: '
' } }, + ], + }) +} + +function makeFolder(overrides = {}) { + return { + id: overrides.id ?? 'f1', + name: overrides.name ?? 'Test', + parent_id: overrides.parent_id ?? null, + has_children: overrides.has_children ?? false, + created_at: '2026-01-01T00:00:00Z', + ...overrides, + } +} + +async function mountItem(folderOverrides = {}, routerPath = '/') { + setActivePinia(createPinia()) + const router = makeRouter(routerPath) + await router.push(routerPath) + await router.isReady() + return mount(FolderTreeItem, { + props: { folder: makeFolder(folderOverrides), depth: 1 }, + global: { plugins: [router] }, + }) +} + +describe('FolderTreeItem', () => { + beforeEach(() => { + vi.clearAllMocks() + mockListFolders.mockResolvedValue({ items: [] }) + }) + + // ── Expand arrow visibility ─────────────────────────────────────────────── + + it('hides expand arrow when has_children is false', async () => { + const w = await mountItem({ has_children: false }) + // The expand button should not be rendered + const expandBtn = w.find('button') + expect(expandBtn.exists()).toBe(false) + }) + + it('shows expand arrow when has_children is true', async () => { + const w = await mountItem({ has_children: true }) + const expandBtn = w.find('button') + expect(expandBtn.exists()).toBe(true) + }) + + it('shows expand arrow when has_children is undefined (unknown)', async () => { + // Newly created folders from optimistic push lack has_children + const folder = { id: 'f1', name: 'New', parent_id: null, created_at: '2026-01-01T00:00:00Z' } + setActivePinia(createPinia()) + const router = makeRouter('/') + await router.push('/') + await router.isReady() + const w = mount(FolderTreeItem, { + props: { folder, depth: 1 }, + global: { plugins: [router] }, + }) + // has_children undefined → arrow shown (conservative: assume could have children) + const expandBtn = w.find('button') + expect(expandBtn.exists()).toBe(true) + }) + + // ── Router-link href ────────────────────────────────────────────────────── + + it('router-link points to /folders/', async () => { + const w = await mountItem({ id: 'abc-123' }) + const link = w.find('a') + expect(link.attributes('href')).toBe('/folders/abc-123') + }) + + // ── Active state ───────────────────────────────────────────────────────── + + it('router-link has active class when route matches folder id', async () => { + setActivePinia(createPinia()) + const router = makeRouter() + await router.push('/folders/f1') + await router.isReady() + + const w = mount(FolderTreeItem, { + props: { folder: makeFolder({ id: 'f1' }), depth: 1 }, + global: { plugins: [router] }, + }) + + const link = w.find('a') + expect(link.classes()).toContain('bg-indigo-50') + }) + + it('router-link does NOT have active class when route does not match', async () => { + setActivePinia(createPinia()) + const router = makeRouter() + await router.push('/folders/other-folder') + await router.isReady() + + const w = mount(FolderTreeItem, { + props: { folder: makeFolder({ id: 'f1' }), depth: 1 }, + global: { plugins: [router] }, + }) + + const link = w.find('a') + expect(link.classes()).not.toContain('bg-indigo-50') + }) + + // ── Expand / collapse children ──────────────────────────────────────────── + + it('clicking expand arrow loads children from API', async () => { + const children = [makeFolder({ id: 'child-1', name: 'Child', parent_id: 'f1' })] + mockListFolders.mockResolvedValue({ items: children }) + + const w = await mountItem({ id: 'f1', has_children: true }) + const btn = w.find('button') + await btn.trigger('click') + await w.vm.$nextTick() + + expect(mockListFolders).toHaveBeenCalledWith('f1') + }) + + it('children are rendered after expand', async () => { + const children = [makeFolder({ id: 'child-1', name: 'ChildFolder', parent_id: 'f1' })] + mockListFolders.mockResolvedValue({ items: children }) + + const w = await mountItem({ id: 'f1', has_children: true }) + await w.find('button').trigger('click') + await w.vm.$nextTick() + + expect(w.text()).toContain('ChildFolder') + }) + + it('expand arrow disappears after loading empty children', async () => { + mockListFolders.mockResolvedValue({ items: [] }) + + const w = await mountItem({ id: 'f1', has_children: true }) + await w.find('button').trigger('click') + await w.vm.$nextTick() + + // After loading empty children, button should be gone + expect(w.find('button').exists()).toBe(false) + }) + + // ── Depth indentation ──────────────────────────────────────────────────── + + it('applies correct left padding based on depth', async () => { + setActivePinia(createPinia()) + const router = makeRouter('/') + await router.push('/') + await router.isReady() + + const w = mount(FolderTreeItem, { + props: { folder: makeFolder(), depth: 3 }, + global: { plugins: [router] }, + }) + + const row = w.find('.flex.items-center') + expect(row.attributes('style')).toContain('padding-left: 36px') + }) +}) diff --git a/frontend/src/components/layout/AppSidebar.vue b/frontend/src/components/layout/AppSidebar.vue index 6a8d0c7..55c9856 100644 --- a/frontend/src/components/layout/AppSidebar.vue +++ b/frontend/src/components/layout/AppSidebar.vue @@ -2,24 +2,12 @@