4adc77d8cc
Wave 0: 11 xfail stubs across test_shares/test_documents/test_audit Wave 1 (parallel): SHARE-05 badge + SHARE-03 permission control; cloud-delete propagation Wave 2: audit handle enrichment, user_handle filter, CSV fetch+Blob, daily-export UI Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
263 lines
16 KiB
Markdown
263 lines
16 KiB
Markdown
---
|
|
phase: "06.2"
|
|
plan: "02"
|
|
type: execute
|
|
wave: 1
|
|
depends_on:
|
|
- "06.2-01"
|
|
files_modified:
|
|
- backend/api/shares.py
|
|
- frontend/src/components/documents/DocumentCard.vue
|
|
- frontend/src/components/sharing/ShareModal.vue
|
|
- frontend/src/stores/documents.js
|
|
- backend/tests/test_shares.py
|
|
autonomous: true
|
|
requirements:
|
|
- SHARE-03
|
|
- SHARE-05
|
|
must_haves:
|
|
truths:
|
|
- "Documents shared with others display a 'Shared' pill in DocumentCard (reads doc.is_shared, not doc.share_count)"
|
|
- "Owner can set permission to 'view' or 'edit' when creating a share"
|
|
- "Owner can toggle permission per share row after creation"
|
|
- "PATCH /api/shares/{id} by the wrong owner returns 404 (IDOR protection)"
|
|
- "POST /api/shares respects the permission field from the request body"
|
|
artifacts:
|
|
- path: "backend/api/shares.py"
|
|
provides: "ShareCreate model with permission field; PATCH /{share_id} endpoint"
|
|
contains: "class SharePermissionPatch"
|
|
- path: "frontend/src/components/documents/DocumentCard.vue"
|
|
provides: "Corrected is_shared guard on Shared pill"
|
|
contains: "v-if=\"doc.is_shared\""
|
|
- path: "frontend/src/components/sharing/ShareModal.vue"
|
|
provides: "Permission dropdown in creation row; View/Edit toggle per share row"
|
|
contains: "Permission level"
|
|
key_links:
|
|
- from: "frontend/src/components/sharing/ShareModal.vue"
|
|
to: "PATCH /api/shares/{id}"
|
|
via: "docsStore.updateSharePermission(shareId, permission)"
|
|
pattern: "updateSharePermission"
|
|
- from: "backend/api/shares.py PATCH"
|
|
to: "Share.owner_id"
|
|
via: "IDOR check — 404 on mismatch"
|
|
pattern: "share.owner_id != current_user.id"
|
|
---
|
|
|
|
<objective>
|
|
Close SHARE-05 (badge uses wrong field) and SHARE-03 (no permission control) in a single vertical slice. Delivers: corrected is_shared badge, permission dropdown at share creation, View/Edit toggle per share row, and the backing PATCH endpoint with IDOR protection.
|
|
|
|
Purpose: Users can now see which documents they've shared (correct badge), set the permission level when sharing, and change it afterward. Closes two open v1 requirements.
|
|
|
|
Output: Modified shares.py (new ShareCreate.permission field + PATCH endpoint), modified DocumentCard.vue (badge fix), modified ShareModal.vue (dropdown + toggle UI), modified documents store (updateSharePermission action), three promoted test stubs.
|
|
</objective>
|
|
|
|
<execution_context>
|
|
@/Users/nik/Documents/Progamming/document_scanner/.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-RESEARCH.md
|
|
@/Users/nik/Documents/Progamming/document_scanner/.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-UI-SPEC.md
|
|
</execution_context>
|
|
|
|
<context>
|
|
@/Users/nik/Documents/Progamming/document_scanner/.planning/ROADMAP.md
|
|
@/Users/nik/Documents/Progamming/document_scanner/.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-CONTEXT.md
|
|
|
|
<interfaces>
|
|
<!-- Key types and contracts the executor needs. Extracted from codebase. -->
|
|
|
|
From backend/api/shares.py (current state):
|
|
class ShareCreate(BaseModel):
|
|
document_id: str
|
|
recipient_handle: str
|
|
# permission="view" hardcoded at line 97 in grant_share()
|
|
|
|
@router.delete("/{share_id}", status_code=204)
|
|
async def revoke_share(share_id: str, ...) -> None:
|
|
sid = uuid.UUID(share_id) # 404 on ValueError
|
|
share = await session.get(Share, sid)
|
|
if share is None or share.owner_id != current_user.id:
|
|
raise HTTPException(404, "Share not found") # IDOR pattern to mirror
|
|
|
|
# Route ordering: GET /received defined BEFORE DELETE /{share_id}
|
|
|
|
From backend/db/models.py (Share model — key fields):
|
|
Share.id: UUID
|
|
Share.document_id: UUID
|
|
Share.owner_id: UUID (FK to User)
|
|
Share.recipient_id: UUID (FK to User)
|
|
Share.permission: str # column exists, default "view" — no migration needed
|
|
|
|
From frontend/src/components/documents/DocumentCard.vue (line 31, buggy):
|
|
v-if="doc.share_count > 0" # BUG — backend sends is_shared: bool, not share_count
|
|
|
|
From frontend/src/components/sharing/ShareModal.vue (shares list row, line 75):
|
|
<span class="text-xs bg-gray-100 text-gray-600 px-2 py-1 rounded-full font-medium">view</span>
|
|
# This static "view" span must be replaced with the View/Edit toggle (C-2)
|
|
|
|
From frontend/src/stores/documents.js — existing share methods to reference:
|
|
shareDocument(docId, recipientHandle) — calls POST /api/shares
|
|
revokeShare(shareId) — calls DELETE /api/shares/{id}
|
|
listShares(docId) — calls GET /api/shares?document_id={docId}
|
|
</interfaces>
|
|
</context>
|
|
|
|
<tasks>
|
|
|
|
<task type="auto" tdd="true">
|
|
<name>Task 1: Backend — ShareCreate permission field + PATCH endpoint</name>
|
|
<files>backend/api/shares.py, backend/tests/test_shares.py</files>
|
|
<read_first>
|
|
- backend/api/shares.py — read the full file; understand ShareCreate model (line 38), grant_share handler (hardcoded permission="view" at line 97), revoke_share IDOR pattern (lines 239-265), route ordering comments
|
|
- backend/tests/test_shares.py — read the full file; understand async_client/auth_user/second_auth_user/db_session fixture pattern and _make_doc helper
|
|
- .planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-RESEARCH.md — Pattern 2 (PATCH IDOR-safe pattern) and Anti-Patterns section (route ordering)
|
|
</read_first>
|
|
<behavior>
|
|
- test_share_create_with_permission: POST /api/shares with {"permission": "edit"} returns 201 and body["permission"] == "edit"; POST with no permission field defaults to "view"
|
|
- test_share_patch_permission: PATCH /api/shares/{valid_id} with {"permission": "edit"} returns 200 and {"permission": "edit"}; a second PATCH with {"permission": "view"} returns 200 and {"permission": "view"}
|
|
- test_share_patch_idor: PATCH /api/shares/{id_owned_by_user_A} authenticated as user_B returns 404 (not 403, not 401)
|
|
</behavior>
|
|
<action>
|
|
Make two changes to backend/api/shares.py:
|
|
|
|
CHANGE 1 — ShareCreate model (add permission field):
|
|
Add `permission: str = "view"` to the ShareCreate model. Add a field_validator named `validate_permission` that checks the value is in `{"view", "edit"}` and raises ValueError otherwise. Import `field_validator` from pydantic if not already imported.
|
|
|
|
In the `grant_share` handler, change the hardcoded `permission="view"` in the Share(...) constructor (line 97) to `permission=body.permission`.
|
|
|
|
CHANGE 2 — Add SharePermissionPatch model and PATCH endpoint:
|
|
Add a new Pydantic model class `SharePermissionPatch(BaseModel)` with a single field `permission: str` and a `field_validator("permission")` classmethod that validates `v in {"view", "edit"}` (same pattern as above).
|
|
|
|
Add the PATCH endpoint `@router.patch("/{share_id}", status_code=200)` as `async def update_share_permission(...)`. Place it BEFORE the existing `@router.delete("/{share_id}", ...)` in the file (style consistency; method discrimination makes ordering safe, but before DELETE is conventional). The handler body:
|
|
- Parse `share_id` as `uuid.UUID(share_id)`, raising HTTPException(404) on ValueError
|
|
- `share = await session.get(Share, sid)` — 404 if None
|
|
- IDOR check: `if share is None or share.owner_id != current_user.id: raise HTTPException(404, "Share not found")` — mirrors revoke_share exactly (T-04-04-02)
|
|
- `share.permission = body.permission`
|
|
- `await session.commit()`
|
|
- Return `{"id": str(share.id), "permission": share.permission}`
|
|
|
|
Then in backend/tests/test_shares.py, promote the three xfail stubs added in Plan 06.2-01 to real tests. Replace the `pytest.xfail(...)` body with actual test logic following the _make_doc helper pattern and async_client fixture conventions already in the file.
|
|
</action>
|
|
<verify>
|
|
<automated>cd /Users/nik/Documents/Progamming/document_scanner/backend && pytest tests/test_shares.py::test_share_create_with_permission tests/test_shares.py::test_share_patch_permission tests/test_shares.py::test_share_patch_idor -x -v 2>&1 | tail -20</automated>
|
|
</verify>
|
|
<done>
|
|
- `pytest tests/test_shares.py -x -q` exits 0 — all 10 tests pass (7 pre-existing + 3 promoted)
|
|
- `grep "class SharePermissionPatch" backend/api/shares.py` returns a match
|
|
- `grep "share.owner_id != current_user.id" backend/api/shares.py` returns at least 2 matches (one in revoke_share, one in update_share_permission)
|
|
- PATCH /api/shares/{id} with wrong owner returns 404 (confirmed by test_share_patch_idor)
|
|
</done>
|
|
</task>
|
|
|
|
<task type="auto">
|
|
<name>Task 2: Frontend — is_shared badge fix + permission dropdown + View/Edit toggle</name>
|
|
<files>frontend/src/components/documents/DocumentCard.vue, frontend/src/components/sharing/ShareModal.vue, frontend/src/stores/documents.js</files>
|
|
<read_first>
|
|
- frontend/src/components/documents/DocumentCard.vue — read lines 25-40 to see the share_count bug at line 31 and surrounding template context
|
|
- frontend/src/components/sharing/ShareModal.vue — read the full file; understand the flex gap-2 creation row (lines 31-50), the static "view" span in the recipient list row (line 75), and how handleRevoke uses docsStore
|
|
- frontend/src/stores/documents.js — find shareDocument(), revokeShare(), and listShares() methods; understand the request() wrapper used by these methods so updateSharePermission follows the same pattern
|
|
- .planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-UI-SPEC.md — Component Contracts C-1 (permission dropdown markup), C-2 (View/Edit toggle markup), Copywriting Contract (label copy)
|
|
</read_first>
|
|
<action>
|
|
Make three frontend changes:
|
|
|
|
CHANGE 1 — DocumentCard.vue line 31 (per D-06):
|
|
Change `v-if="doc.share_count > 0"` to `v-if="doc.is_shared"`. This is a one-word change. No other modifications to DocumentCard.vue.
|
|
|
|
CHANGE 2 — ShareModal.vue — permission dropdown in creation row (per D-08, C-1 from UI-SPEC):
|
|
Add a `permission` reactive ref defaulting to `"view"` in the script setup section.
|
|
|
|
In the template, inside the `<div class="flex gap-2">` creation row (the row containing the handle input and the "Share document" button), insert a `<select>` element BETWEEN the handle `<input>` and the submit `<button>`. The select uses `v-model="permission"`, `aria-label="Permission level"`, and Tailwind classes: `border border-gray-300 rounded-lg px-3 py-2 text-sm bg-white focus:outline-none focus:ring-2 focus:ring-indigo-500 shrink-0`. Two options: `<option value="view">Can view</option>` and `<option value="edit">Can edit</option>`.
|
|
|
|
Pass `permission: permission.value` into the `docsStore.shareDocument(props.doc.id, trimmed, permission.value)` call (the store method needs updating — see below). Reset `permission.value = "view"` on successful submit.
|
|
|
|
CHANGE 3 — ShareModal.vue — View/Edit toggle per share row (per D-09, C-2 from UI-SPEC) and in-flight error state:
|
|
Add a reactive `permissionError` ref (string, null default) and a `updatingPermission` ref (Set or Object tracking in-flight share IDs) in script setup.
|
|
|
|
Replace the static `<span class="text-xs bg-gray-100 text-gray-600 px-2 py-1 rounded-full font-medium">view</span>` in each recipient list row with a View/Edit toggle group. The toggle group is a `<div>` with `role="group"` `aria-label="Permission"` containing two `<button>` elements ("View" and "Edit"). Each button:
|
|
- Active state classes: `bg-indigo-50 text-indigo-600 font-medium`
|
|
- Inactive state classes: `bg-gray-100 text-gray-600`
|
|
- Common classes: `text-xs px-2 py-1 rounded-full font-medium transition-colors`
|
|
- `aria-pressed` attribute reflecting whether the button's value matches `share.permission`
|
|
- `aria-label` pattern: "Change permission for {share.recipient_handle}"
|
|
- Disabled (opacity-50 pointer-events-none) when `updatingPermission.has(share.id)`
|
|
- On click of the inactive button: call `handlePermissionChange(share.id, 'view'|'edit')`
|
|
|
|
Add `handlePermissionChange(shareId, newPermission)` function:
|
|
- Optimistic: find the share in `shares.value`, set `share.permission = newPermission` immediately
|
|
- Mark in-flight: `updatingPermission.value.add(shareId)` (use `ref(new Set())`)
|
|
- Call `await docsStore.updateSharePermission(shareId, newPermission)`
|
|
- On error: revert `share.permission` to the old value, set `permissionError.value = "Failed to update permission."`
|
|
- Finally: `updatingPermission.value.delete(shareId)`
|
|
|
|
Show `permissionError` below the list (same `text-xs text-red-600 mt-2` pattern as the existing `error` display).
|
|
|
|
CHANGE 4 — documents.js store — add updateSharePermission action and update shareDocument signature:
|
|
Add `updateSharePermission(shareId, permission)` action that calls `PATCH /api/shares/${shareId}` with body `{ permission }` via the existing `request()` wrapper.
|
|
|
|
Update `shareDocument(docId, recipientHandle, permission = 'view')` to pass `{ document_id: docId, recipient_handle: recipientHandle, permission }` in the POST body (previously only `document_id` and `recipient_handle`).
|
|
</action>
|
|
<verify>
|
|
<automated>cd /Users/nik/Documents/Progamming/document_scanner/frontend && grep -n "doc.is_shared" src/components/documents/DocumentCard.vue | head -5</automated>
|
|
</verify>
|
|
<done>
|
|
- `grep "doc.is_shared" frontend/src/components/documents/DocumentCard.vue` returns a match (not share_count)
|
|
- `grep "Permission level" frontend/src/components/sharing/ShareModal.vue` returns a match
|
|
- `grep "handlePermissionChange" frontend/src/components/sharing/ShareModal.vue` returns a match
|
|
- `grep "updateSharePermission" frontend/src/stores/documents.js` returns a match
|
|
- `grep "share_count" frontend/src/components/documents/DocumentCard.vue` returns no match (old bug removed)
|
|
</done>
|
|
</task>
|
|
|
|
</tasks>
|
|
|
|
<threat_model>
|
|
## Trust Boundaries
|
|
|
|
| Boundary | Description |
|
|
|----------|-------------|
|
|
| browser → PATCH /api/shares/{id} | User-supplied share_id and permission value cross the API boundary |
|
|
| ShareModal → documents store | permission value must be one of the two literals before reaching the backend |
|
|
|
|
## STRIDE Threat Register
|
|
|
|
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
|
|-----------|----------|-----------|-------------|-----------------|
|
|
| T-06.2-02-01 | Elevation of Privilege | PATCH /api/shares/{id} | mitigate | `share.owner_id != current_user.id` → HTTPException(404) — mirrors existing revoke_share IDOR pattern; returns 404 not 403 to prevent share ID enumeration |
|
|
| T-06.2-02-02 | Tampering | SharePermissionPatch model | mitigate | `field_validator("permission")` checks value in `{"view", "edit"}` — no arbitrary string passthrough from request body to DB |
|
|
| T-06.2-02-03 | Tampering | ShareCreate.permission field | mitigate | Same field_validator as SharePermissionPatch — "view" is the server-enforced default if client omits the field |
|
|
| T-06.2-SC | Tampering | npm/pip/cargo installs | accept | No new packages installed in this plan |
|
|
</threat_model>
|
|
|
|
<verification>
|
|
After both tasks complete:
|
|
|
|
```
|
|
cd /Users/nik/Documents/Progamming/document_scanner/backend && pytest tests/test_shares.py -x -q
|
|
```
|
|
|
|
Expected: 10 passed (7 pre-existing + 3 promoted). No xfail in test_shares.py.
|
|
|
|
```
|
|
cd /Users/nik/Documents/Progamming/document_scanner/backend && pytest -v 2>&1 | tail -20
|
|
```
|
|
|
|
Expected: zero failures (pre-existing `test_extract_docx` xfail is allowed).
|
|
|
|
Frontend spot-checks (manual or via `grep`):
|
|
- DocumentCard.vue contains `v-if="doc.is_shared"` and NOT `share_count`
|
|
- ShareModal.vue contains `aria-label="Permission level"` and `handlePermissionChange`
|
|
- documents.js contains `updateSharePermission`
|
|
</verification>
|
|
|
|
<success_criteria>
|
|
- POST /api/shares with permission="edit" stores "edit" in DB — confirmed by test_share_create_with_permission
|
|
- PATCH /api/shares/{id} changes permission — confirmed by test_share_patch_permission
|
|
- PATCH /api/shares/{id} by wrong owner returns 404 — confirmed by test_share_patch_idor
|
|
- DocumentCard shows "Shared" pill based on doc.is_shared (not doc.share_count)
|
|
- ShareModal creation row has permission dropdown defaulting to "Can view"
|
|
- ShareModal share rows show View/Edit toggle instead of static "view" text
|
|
- All 10 test_shares.py tests pass
|
|
</success_criteria>
|
|
|
|
<output>
|
|
Create `.planning/phases/06.2-close-v1-sharing-cloud-delete-csv-export-gaps/06.2-02-SUMMARY.md` when done.
|
|
</output>
|