diff --git a/docs/architecture/rebac-authorization.md b/docs/architecture/rebac-authorization.md index ca5c927e..2a1d70d9 100644 --- a/docs/architecture/rebac-authorization.md +++ b/docs/architecture/rebac-authorization.md @@ -107,9 +107,19 @@ storage.access_grants expires_at TIMESTAMPTZ NULL ``` -One row per `(subject, permission, resource)` triple. An "admin role on folder +One row per `(subject, permission, resource)` triple. An "owner role on folder X for user Y" is 6 rows; a "viewer role" is 1 row. +> **Note (D-Prep, 2026-06-17):** the role assignment has since pivoted into +> a separate `storage.role_grants` table that stores **one row per role +> assignment** rather than one per permission. `access_grants` stays +> populated via dual-write during the transition; the engine reads the +> role-keyed table for authz decisions. The cleanup PR drops +> `access_grants` after the dual-write window. The historical role name +> `Admin` was renamed to `Owner` at the same time, to disambiguate from +> `UserRole::Admin` (user-account privilege) and match Drive plan +> terminology. + Cleanup is trigger-driven (`trg_cleanup_grants_folder`, …): when a resource or subject is deleted, all referencing grants disappear in the same transaction. diff --git a/docs/plan/drive.md b/docs/plan/drive.md new file mode 100644 index 00000000..e567c00f --- /dev/null +++ b/docs/plan/drive.md @@ -0,0 +1,1401 @@ +# Drives — design proposal + +> **Status**: design proposal, locked but not implemented. Reviewed Jun 2026 with +> Ed; all blocking decisions answered. Open items listed at the end of the file. + +## Context + +Today every resource (folder, file) in OxiCloud has a single user owner +recorded directly on the row (`storage.folders.user_id`, +`storage.files.user_id`). Sharing is layered on top via ReBAC grants in +`storage.access_grants`. The model is simple and has carried us this far, +but it has two structural problems: + +1. **Owner = user, full stop.** When a user leaves the company, their + files leave with them. The team's de-facto shared folders are + technically owned by a single person; transferring ownership requires + moving every resource one-by-one and re-issuing every grant. There is + no "this folder belongs to the Engineering team" concept. + +2. **Quota is per-user, full stop.** A user with personal storage who + also collaborates in a 1 TB team space currently has both counted + against their personal quota (or the team data costs are absorbed by + whoever owns the folder). There is no way to bill team storage to the + team. + +The proposal is to introduce **drives** as a Google-Workspace-style +container concept: + +- Every resource belongs to exactly one drive. +- A drive has owners (users **and/or** groups) with roles. +- Each user automatically gets a personal drive at registration. +- Shared drives can be owned by groups, so membership changes + automatically follow the group — "Bob left" no longer needs an admin + to chase down which folders to reassign. +- Quotas move from users to drives. +- Per-drive policies enable team-level rules (forbid public links, + forbid sharing, forbid cross-drive moves, future: timeboxed sessions, + end-to-end encryption). + +The shift is large but the model is well-understood — it's the same +pattern Google Workspace's Shared Drives and Microsoft SharePoint +Document Libraries use. OxiCloud users coming from those services will +recognise it instantly. + +## Prerequisite — PR D-Prep: `access_grants → role_grants` + +**Status (2026-06-17): scope complete, ready to PR.** The schema migration +runs cleanly against real sandbox data (38 role_grants rows produced, +matching the audit's 29 viewer / 5 editor / 4 owner distribution, zero +bundle mismatches). End-to-end Hurl test (`tests/api/role_grants.hurl`) +covers create / atomic role update / revoke with both the canonical +`"owner"` and the legacy `"admin"` compat wire format. Engine reads +pivot to `role_grants` for authz decisions; `access_grants` stays +populated via dual-write as the safety net for one release cycle. +**Ratified 2026-06-16.** A separate PR lands BEFORE D0 that refactors +`storage.access_grants` into `storage.role_grants` with role-bundle +semantics: + +```sql +storage.role_grants + id uuid PRIMARY KEY DEFAULT gen_random_uuid() + subject_type text NOT NULL CHECK (subject_type IN ('user','group')) + subject_id uuid NOT NULL + resource_type text NOT NULL -- 'folder','file','drive','group',… + resource_id uuid NOT NULL + role text NOT NULL CHECK (role IN ('viewer','editor','owner', …)) + expires_at timestamptz NULL + granted_by uuid NOT NULL FK → auth.users(id) + granted_at timestamptz NOT NULL DEFAULT now() +``` + +Roles map to permission bundles via a single +server-side function `role_bundle(role) -> &[Permission]`: + +| Role | Bundle | +|---|---| +| `viewer` | `Read` | +| `editor` | `Read, Create, Update, Comment` | +| `owner` | `Read, Create, Update, Comment, Delete, Share, Manage` | + +`Manage` is a new Permission introduced in this refactor — covers +"configure this resource's settings, add/remove members, change role +assignments". Future-friendly for Group-as-Resource. + +**Why first**: +1. Refactor has standalone value beyond Drive (cleaner API, audit + log, UI surface). +2. Doing it AFTER Drive would force migrating both `access_grants` + AND `drive_members` — solving the unification first means D0 + migrates one table, not two. +3. **The `drive_members` table planned by earlier drafts goes away.** + Drive membership becomes rows in `role_grants` with + `resource_type='drive'`. One table, one engine, one cache. + +**The sections below describe the Drive model assuming D-Prep has +landed.** References to "drive membership" mean rows in +`role_grants` with `resource_type='drive'`, not a separate table. + +**Gating policy**: D-Prep must ship, bake in production, and pass +validation across the API surface (grant endpoints, audit log +events) and the UI surface (My Shares dialog, share modals, admin +group/role views) before any of the Drive PRs (D0–D7) begin. The +goal is to surface and absorb refactor-related issues against the +existing single-resource model — not to discover them mid-flight +while the Drive migration is also in motion. If D-Prep needs +follow-up patches after deploy, they land as standalone fixes +before D0 starts. + +## Locked design (decisions ratified in design discussion) + +### 1. Ownership pivot + +- A new table `storage.drives` becomes the ownership anchor. +- `storage.folders` and `storage.files` lose their `user_id` column; + they gain `drive_id NOT NULL`. +- The owner of a resource is computed via `resource.drive_id → + drive.drive_members WHERE role='owner'` — a JOIN, not a denormalised + column. The drives table will be tiny (one row per internal user + + handful of shared drives), so the join is cheap with a proper index. +- **Single source of truth** confirmed. `user_id` is dropped from + resources after a migration phase (see Migration below). + +### 2. Drive membership + +Drive membership lives in `storage.role_grants` (introduced by PR +D-Prep — see prerequisite above) with `resource_type='drive'` and +`resource_id=`. There is **no** separate +`drive_members` table. One row per `(subject, drive)` pair carries +the role; the role expands to a permission bundle via the same +`role_bundle()` function used everywhere else. + +Concretely, a drive membership is: + +```sql +INSERT INTO storage.role_grants + (subject_type, subject_id, resource_type, resource_id, role, granted_by) +VALUES ('user', $user_id, 'drive', $drive_id, 'editor', $admin_id); +``` + +The `PgAclEngine` reads this row no differently than any other +grant; the drive-membership concept exists only at the application +layer (`DriveService` enforces the personal-drive invariants, etc.) +— there's no separate engine path or cache keyspace. + +- A **shared** drive (`kind='shared'`) can have **0..N user + owners + 0..N group owners** (mixed freely) plus any number of + editors/viewers. +- A **personal** drive (`kind='personal'`) has exactly **one** + member row in `drive_members`, fixed to `(subject_type='user', + subject_id=, role='owner')`. The rule applies to + every `kind='personal'` row, whether it carries + `default_for_user IS NOT NULL` (the user's default drive) or + `default_for_user IS NULL` (a secondary personal silo, e.g. a + SQL-created sibling root folder promoted by migration §10). + Secondary does **not** loosen the single-user constraint. + - Personal drives **cannot** be added to, role-changed, or have + their sole owner removed. They cannot be co-owned. They cannot + be transferred to another user. This matches Google Drive's + "My Drive" and Microsoft's "OneDrive" semantics: a personal + drive is a single-user namespace; collaboration happens through + per-resource grants, by moving content into a shared drive, + or by promoting a secondary personal drive to `kind='shared'` + (capability matrix in §3). + - Enforcement is at the application layer: + `DriveService::add_member` refuses when `kind='personal'`; + `remove_member` refuses when the drive is personal. User + deletion cleans up the user's **default** personal drive via + the `default_for_user` FK cascade, and the user's + **secondary** personal drives via an application-layer pass + (those have no FK to cascade through — see §6 lifecycle). +- **Shared-drive last-owner protection**: removing the final + `role='owner'` member of a shared drive is refused at the + application layer. The check counts remaining owner-role members + (expanding group owners to their members) and rejects the delete + if the count would become zero. +- A shared drive can have **0 viewers** and **0 editors** — only + the ≥1-owner invariant matters. + +### 3. Drive entity + +```sql +storage.drives + id uuid PRIMARY KEY DEFAULT gen_random_uuid() + name text NOT NULL -- "Personal" or user-chosen + kind text NOT NULL CHECK (kind IN ('personal','shared')) + default_for_user uuid NULL FK → auth.users(id) ON DELETE CASCADE + quota_bytes bigint NULL -- NULL = unlimited + used_bytes bigint NOT NULL DEFAULT 0 + policies jsonb NOT NULL DEFAULT '{}' + created_at timestamptz NOT NULL DEFAULT now() + updated_at timestamptz NOT NULL DEFAULT now() + -- `default_for_user` may ONLY be set on personal drives. + -- Shared drives have it NULL by definition. + CONSTRAINT drives_default_marker_personal_only + CHECK (default_for_user IS NULL OR kind = 'personal') + +CREATE UNIQUE INDEX drives_default_for_user_idx + ON storage.drives (default_for_user) + WHERE default_for_user IS NOT NULL; -- one DEFAULT personal drive per user +``` + +#### Two orthogonal properties: `kind` and `default_for_user` + +- **`kind`** = drive capability shape (see §2 for the rules): + - `'personal'` = single-user single-owner; cannot have members + added; sharing happens only through per-resource grants. + - `'shared'` = multi-owner / multi-member; supports user and + group members at every role. +- **`default_for_user`** = "this is the default drive for user + X". Set on exactly one `kind='personal'` drive per user (the + partial unique index enforces it). Used by: + - UI default-drive redirect (`/` → `/drive/`). + - NC default-drive resolution when the credential doesn't pin + a specific drive. + - The `ON DELETE CASCADE` on this column cleans up the user's + default drive in one step when the user is deleted. + - Canonical lookup: `SELECT id FROM storage.drives WHERE + default_for_user = $1` — single-row, index-backed. + +A user can have **multiple `kind='personal'` drives** — at most +one of them carries `default_for_user`, the others sit as +"secondary personal" silos (e.g. SQL-created root folders +promoted by migration §10). Secondary personals follow the same +single-owner rules as the default; they just aren't the default +landing target. + +External users **never** get a personal drive (lifecycle §6). +There is no constraint to write — externals simply have no row +in `storage.drives` with `default_for_user = `, and +nothing tries to create one. + +#### Drive naming — `name` is a label, identity lives in `kind` + `default_for_user` + +`name` is owner-editable for every drive (personal or shared). A +user who renames their drive from "Personal" → "Ed's space" does +**not** stop having a personal drive, and does not stop having a +default. The `kind` flag + `default_for_user` pointer carry the +identity; the name is purely a display label. + +Why this matters: +- UI and NC default-drive resolution MUST query on `kind` / + `default_for_user`, never on `name = 'Personal'`. The latter + would silently break the moment the user renames. +- The initial migration sets `name = 'Personal'` on the default + personal drive for back-compat with the label users see today; + further renames go through the normal drive-rename endpoint + and persist on the same row. Secondary personal drives carry + whatever name the sibling root folder had (e.g. `Archive`, + `2024 Projects`). + +#### Capabilities matrix + +| Capability | `kind='personal'`, default (`default_for_user` set) | `kind='personal'`, secondary (`default_for_user IS NULL`) | `kind='shared'` | +|---|---|---|---| +| Membership shape | exactly 1 user-owner row | exactly 1 user-owner row | 0..N users + 0..N groups at any role | +| `add_member` | refused | refused | allowed | +| `remove_member` | refused (sole owner is fixed) | refused (sole owner is fixed) | allowed except sole owner | +| Rename | allowed (by the owner) | allowed (by the owner) | allowed (by any owner) | +| Delete via API | **refused** — deleting this loses all the user's files; the only path is user-delete cascade | allowed (it's just a silo) | allowed (by an owner; CASCADEs the drive's contents) | +| Default-drive lookup result | this drive | never | never | +| On user-delete | `ON DELETE CASCADE` via `default_for_user` FK (free) | application-layer cleanup: enumerate via drive_members and delete | member rows referencing the user are dropped; refuse user-delete if any shared drive would lose its last owner | +| Group ownership | no | no | yes | +| Per-resource grant outward | yes (subject to drive policies) | yes | yes | +| Cross-drive move | yes (subject to `forbid_cross_drive_move`) | yes | yes | +| Kind conversion | no — always default-personal | yes → may be promoted to `kind='shared'` later (drops the single-user restriction, picks up members) | no | + +### 4. Roles → permission bundles + +Drive-level roles map to existing ReBAC `Permission` values via union +expansion: + +| Role | Permissions implied on every resource inside the drive | +|---|---| +| `viewer` | `Read` | +| `editor` | `Read`, `Create`, `Update`, `Comment` | +| `owner` | `Read`, `Create`, `Update`, `Comment`, `Delete`, `Share`, *and* drive-level admin (rename, edit policies, manage members, change quota) | + +### 5. Permission resolution — additive over `role_grants` + +A user has permission `P` on resource `R` if any of the +`role_grants` rows reachable from them (directly, via group +membership, or via drive membership) carries a role whose bundle +includes `P`. Specifically: + +- A direct grant: `(subject_type='user', subject_id=$user, resource_id=$R, role=…)` +- A group-mediated grant: any `role_grants` row whose + `subject_type='group'` points at a group the user is a transitive + member of. +- A drive-mediated grant: any `role_grants` row whose + `resource_type='drive'` and `resource_id=$R.drive_id`, with + the same per-row role bundle. + +Permissions are **additive**. Drive role is the baseline floor for +every resource in the drive; explicit per-resource grants only add +on top. There is no "revoke for this file even though you're a +drive editor" concept, matching Google Workspace. + +The auth engine reads `role_grants` as a single source of truth. +Cache invalidation has one keyspace (rows in `role_grants`) instead +of two. The drive-membership lookup ("what drives can this caller +read?") is a `WHERE subject_id=$1 AND resource_type='drive'` scan +against the same table. + +### 6. Lifecycle rules + +| Event | Behaviour | +|---|---| +| New internal user registers | Auto-create a default personal drive (`kind='personal'`, `name='Personal'`, `default_for_user=`, `quota_bytes=`), insert the single `drive_members (drive_id, user, , owner)` row. | +| External user invited (magic-link only) | **No personal drive created.** External users are grant-only recipients with no storage. | +| External user converts to internal (future flow) | Default personal drive created at conversion time. | +| User deleted | **Default** personal drive cascade-deletes via `ON DELETE CASCADE` on `default_for_user`. **Secondary** personal drives (`kind='personal' AND default_for_user IS NULL` and whose sole `drive_members` row points at the user) are deleted by an application-layer pass in the same transaction. Member rows referencing the deleted user are removed from all shared drives. If a removal would leave a shared drive with zero owners, deletion is refused — admin must transfer first. | +| Group deleted | Refuse if the group is a member of any shared drive that would lose its last owner. Admin must transfer or remove the group's role from those drives first. (Groups can't be members of personal drives.) | +| Add member to personal drive | Refuse. Personal drives are single-user — collaborate via per-resource grants or by moving content into a shared drive. | +| Remove sole owner of personal drive | Refuse. The only deletion path for a personal drive is user-deletion via cascade. | +| Delete personal drive | Refuse from the API. Only ON DELETE CASCADE (user deletion) drops it. | +| Rename personal or shared drive | Allowed for any owner-role caller. `name` is a label only. | +| Remove last owner of shared drive | Refuse — drive must always have ≥1 owner. App-layer check on `DELETE FROM drive_members`. | + +### 7. Quota model + +The per-user `auth.users.storage_quota_bytes` field is **migrated to +the user's personal drive's `quota_bytes`** in one step, then the +column is deprecated (kept for one release cycle as a no-op, dropped +in a later migration). + +After the cutover: +- Every drive owns its quota. Files inside a drive count against that + drive's `used_bytes` only. +- A user who collaborates in a 1 TB shared drive sees their personal + drive's quota as "their" quota; the shared drive's quota is owned + by the team. +- New drives default to a tenant-configured `OXICLOUD_DEFAULT_DRIVE_QUOTA_BYTES` + setting (separate env var, replacing today's per-user equivalent). + +`used_bytes` is maintained incrementally on every file insert/delete +(plus a periodic reconciliation job to fix drift, similar to the +existing per-user accounting). + +**Chunk dedup vs per-drive quota.** With the CDC chunk store landed +in v0.7.0 (see `delta_upload_service`, `upload_ingest`, instant +upload by hash), a single chunk can be referenced by files in +multiple drives. The accounting decision: **each drive counts the +file's logical size in full against its own `used_bytes`** — dedup +savings are server-side only and never visible in the per-drive +quota number. This matches the existing per-user blob-dedup model +and avoids the alternative "pro-rated quota" trap (which makes +quota math depend on cross-drive content and breaks the user's +mental model of "I have 1 TB free"). Reconciliation job sums file +sizes per drive, not chunk allocations. + +### 8. Policies (JSONB, extensible) + +Each drive carries a `policies` JSON object. Five known keys for v1: + +```jsonc +{ + "forbid_sharing": false, // disables per-resource grants on this drive + "forbid_external_sharing": false, // blocks grants to is_external=true subjects + "forbid_public_links": false, // blocks token-share (anonymous link) creation + "forbid_cross_drive_move": false // blocks MOVE when src.drive_id != dst.drive_id +} +``` + +Enforcement points (one place per policy — single grep target): + +| Policy | Enforcement callsite | +|---|---| +| `forbid_sharing` | `grant_handler::create_grant` — checks `resource.drive_id`'s policy before insertion | +| `forbid_external_sharing` | `magic_link_invite_service::resolve_or_create_recipient` and `grant_handler::create_grant` (when subject is `is_external=true`) | +| `forbid_public_links` | `share_handler::create_shared_link` | +| `forbid_cross_drive_move` | `file_handler::move_file` and `folder_handler::move_folder` — refuse when `src.drive_id != dst.drive_id` | + +Default to `false` (everything allowed) — opt-in by drive owner via +the drive settings UI. + +#### Policy semantics — subtleties to remember + +- **`forbid_sharing`** disables **per-resource** grants on resources + in the drive. Drive owners can still add **drive-level** members + (otherwise the drive becomes uneditable except by the original + owner). The policy means "no fine-grained sharing of individual + files; access happens through drive membership only". +- **`forbid_cross_drive_move`** protects against exfiltration via UI + move. It does **not** stop download + re-upload (that's a different + category of policy — file-egress, future). UI surface should make + this explicit so users don't read it as data-leak protection. + +#### Future policy keys (out of scope for v1 — but the JSONB shape +accommodates them without schema migration) + +- `timeboxed_session` — drive contents require re-auth after N + minutes. Significant UX/middleware lift; defer. +- `end_to_end_encrypted` — client-side encryption; massive scope. + +### 9. URL surface + +#### Frontend + +| URL | Resolves to | +|---|---| +| `/` | Redirect to the caller's personal drive UUID | +| `/drive/` | Drive root view | +| `/drive//` | Folder inside the drive | + +#### Native WebDAV (`/webdav/...`) + +| URL | Resolves to | +|---|---| +| `/webdav/` | Caller's personal drive root + `` (back-compat with today's behaviour) | +| `/webdav/drives//` | Specific drive root + `` | + +Today's `/webdav/` handler implicitly looks up the caller's +home folder and prepends it. Post-drives, the same handler looks up +the caller's personal drive and resolves paths inside it. **Zero +breakage** for existing native WebDAV clients. + +The `drives` path segment is **reserved**: a folder literally named +`drives` cannot exist at the top level of any drive. Migration +pre-check refuses to start if existing data violates this — operator +must rename before upgrading. (Conservative estimate: zero existing +folders are named exactly `drives`. The migration script reports any +collisions for manual fix-up.) + +#### NextCloud-compat WebDAV (`/remote.php/dav/...`) + +> **The path-segment `/drives//` form is NOT used on the NC +> surface.** It is reserved for the native WebDAV surface (see +> table above). The NC surface keeps the URL shape +> `/remote.php/dav/files//` and carries the drive +> selector in the **credential**, not the URL. +> +> NC desktop / mobile clients store credentials per +> `(host, username)` and offer a single sync root per saved +> account. A path-segment scheme would require NC clients to grow +> multi-root awareness, which they don't have. Two +> credential-side mechanisms are valid here: +> +> 1. **Username discriminator (`{user}~{drive-uuid}`)** — the +> chroot POC on `feat/nextcloud-drive` (commit `137169b7`). +> The Basic Auth username carries the drive UUID after a `~` +> separator; the URL stays under `/remote.php/dav/files/{user}~{uuid}/`. +> Explicit on the wire, no per-credential server state needed +> beyond the existing app-password row. +> +> 2. **App-password ↔ drive binding** — store the chosen drive +> UUID directly on the `auth.app_passwords` row at issuance +> time. The Basic Auth username stays as `{user}` (clean +> NextCloud UX, no `~` to explain). The auth middleware looks +> up the app-password row, reads its `drive_id` binding, and +> uses that as the drive context. Each drive a user wants to +> sync gets its own app-password. +> +> Both are workable; option (2) is the cleaner UX (username +> matches what users type, no extra character to explain) but +> requires a schema add on `auth.app_passwords` and one extra +> JOIN in the hot auth path. Option (1) is the smallest possible +> change but exposes the `~` to the user. They're not mutually +> exclusive — the issuance flow can produce credentials in either +> shape. Decide before D1 ships which is the **default** the +> Login Flow v2 picker produces. + +| URL | Resolves to | +|---|---| +| `/remote.php/dav/files//` | That user's personal drive — unchanged. (App-password drive-binding NULL ⇒ personal.) | +| `/remote.php/dav/files/~/` | Option 1: explicit drive in the URL. | +| `/remote.php/dav/files//` *(with `auth.app_passwords.drive_id` set)* | Option 2: drive resolved from the credential row. URL is indistinguishable from the unchanged personal case to the client. | + +In either case, the auth middleware asserts the caller is a member +of the resolved drive before serving any DAV verb. Pre-existing NC +clients pointed at `/remote.php/dav/files//` continue +syncing the user's personal drive without reconfiguration — +regardless of which option is chosen as the default. + +#### Username/UUID collision — defused + +The `/remote.php/dav/files//` vs `/remote.php/dav/drives//` +split solves the worry about username/UUID ambiguity. The +discriminator is the literal segment (`files` vs `drives`), never +the value of ``. A user happening to have a UUID-shaped username +is no longer a problem. + +### 10. Storage paths — wrapper folder retired + +Today `storage.folders.path` is e.g. `My Folder - admin/Docs`. The +"My Folder - admin" wrapper is the user's home folder, created at +registration via `format!("My Folder - {}", username)`. + +Post-drives, **the wrapper goes away**. The drive itself is the +root; folders and files that used to live inside the wrapper sit +directly under the drive with no intermediate folder: + +``` +Drive "Personal" (uuid=…, kind=personal, owner=admin) ← was the "My Folder - admin" wrapper +├── Docs/ +└── aa.pdf +``` + +Same for shared drives — they already had no wrapper: + +``` +Drive "Engineering" (uuid=…, kind=shared, owners=group:engineering) +├── Specs/ +├── Roadmap.md +└── archive/ +``` + +The two surfaces share one rule: **drive root = `parent_id IS NULL` +within the drive's `drive_id`**. The "personal vs shared" branch +disappears from path resolution — both kinds resolve the same way. + +#### Why this is client-safe + +The wrapper was already invisible to WebDAV / NC clients pre-drive: + +- NC clients hit `/remote.php/dav/files//` and + `nc_to_internal_path` prepended `My Folder - /` internally + before talking to the storage layer. The client never saw the + wrapper segment in its URL. +- Native `/webdav/` was implicitly chrooted to the user's + home by `resolve_webdav_path`. Same story. + +So URL-level back-compat is preserved trivially — clients keep +asking for `/remote.php/dav/files/admin/Docs/foo.pdf`, the +resolver no longer prepends the wrapper, and the storage row's +path is now `Docs/foo.pdf` instead of `My Folder - admin/Docs/foo.pdf`. +Net effect on the wire: zero. + +#### Why this is a better model + +The original plan kept the wrapper "for back-compat" but it has +no value beyond the storage layer (clients never see it, the +filesystem mirror is happy either way). Keeping it forced path +resolution to always know whether the caller is in a personal or +shared drive and conditionally prepend a segment. Dropping it +collapses that branch and makes the personal-vs-shared distinction +purely a metadata concern (kind, quota source, member shape) — +**not** a path-shape concern. + +#### Sibling root folders also become drives + +Today the only way to get a `parent_id IS NULL` folder is via the +home-folder creation path, which produces exactly one per user. +However, manual SQL has already been used (and may still be used +by ops) to create additional top-level folders — folders that sit +beside `My Folder - ` at `parent_id IS NULL`. These are +**not** addressable today through any handler (the UI assumes +exactly one root); they exist as data only. + +Post-migration, the model has to absorb them. Rule: + +- For each user, find every row with `parent_id IS NULL AND user_id + = `. +- The one named `My Folder - ` becomes the **default + personal drive**: `kind='personal'`, + `default_for_user=`, sole member = the user. +- Every **other** sibling becomes a fresh **secondary personal + drive**: `kind='personal'`, `default_for_user=NULL`, sole + member = the user, name carried over from the folder's `name` + column, quota initialised from the user's quota + (`auth.users.storage_quota_bytes`) — same default as the + user's primary personal drive. Membership rules from §2 apply: + the user cannot invite co-owners while the drive remains + personal. To open the silo up, the user can later **convert** + the secondary personal to `kind='shared'` (an + application-layer operation that flips the kind and lifts the + single-user restriction so the membership API can add other + users / groups). +- The folder row is deleted (the drive replaces it as the root). + Its children get their `parent_id` set to `NULL` *within their + new `drive_id`*. + +The chroot POC's "pick a drive at login" picker on +`feat/nextcloud-drive` already produces the right shape for this: +users with one drive auto-select Personal silently, users with +N drives get a real picker. No POC change needed — it just sees +real drive rows instead of folder UUIDs. + +### 11. Content search index — drive-aware filtering + +v0.7.0 added an embedded Tantivy full-text content index (see +`infrastructure/services/search_index/tantivy_content_index.rs` +and migration `20260701000000_content_search_index.sql`). Today +every indexed document carries the owning user as a filter field +and queries restrict by that field at query time. + +When ownership pivots to `drive_id`, the index has to follow — +otherwise search leaks content across drives the moment D7 drops +`user_id`: + +1. **Schema update**: every indexed document gains a `drive_id` + field stamped at ingest time. Existing documents need a + one-shot reindex pass during the D0 migration (read each row, + look up its new `drive_id`, update the index entry). Cheap on + small instances; the migration script should report a progress + count for larger ones. +2. **Query path**: instead of filtering by `user_id = caller`, + expand `caller → set of drive_ids the caller can read` + (personal + every shared-drive membership) and filter by + `drive_id ∈ that set`. Expansion reuses the drive-role check + already required by `PgAclEngine`. +3. **Treat as a blocking step of D0**, not a D4/D5-era polish + item — otherwise the index is the silent leak path during the + dual-write window. + +Filtering on a low-cardinality `drive_id` is something Tantivy +handles natively; this is bookkeeping, not a query-plan risk. + +### 12. Trash — per-drive scoped, owner-actioned + +Today trash is per-user: `storage.files` / `storage.folders` +carry `is_trashed BOOLEAN` + `trashed_at` + `original_parent_id` +(soft-delete in place), and `storage.trash_items` is a VIEW that +UNIONs them. The listing endpoint (`GET /api/trash/resources`) +filters by `user_id = caller`. + +Post-drives, trash becomes **per-drive**: + +- **Storage shape is unchanged.** The `drive_id` column added to + `storage.files` / `storage.folders` in Phase A already + identifies which drive a trashed row belongs to. No new + trash table, no schema work beyond updating the + `storage.trash_items` VIEW to surface `drive_id` alongside + (or replacing) `user_id`. +- **Trash listing query** filters by drive(s) the caller can + read. Default listing returns trash from every drive the + caller has membership on; a `?drive_id=` parameter + scopes to one drive. UI shows a drive picker above the trash + list, same as the main file view. +- **Trash mutations are owner-only.** Per the §4 role-bundle, + `Delete` is in the owner bundle only — so today's "anyone + who can delete the original can act on its trash entry" is + already drive-owner-scoped. Specifically: + - **Send to trash** — any drive owner (carries `Delete`). + Personal drive: the user themselves. + - **Restore** — any drive owner. Operation reverses + `is_trashed`, sets `parent_id` back to `original_parent_id` + when that ancestor is still in the same drive (otherwise to + the drive root with a name conflict resolver). + - **Permanent delete** — any drive owner. Clears the row and + decrements drive `used_bytes`. + - **View trash** — any drive member (viewer / editor / + owner). Viewers can see what was deleted from a drive they + have access to; only owners can act on it. Mirrors Google + Drive's per-shared-drive trash UX. +- **Cross-drive moves carry their trash home with them**: + when a file moves from drive A → drive B and is later + trashed, the row's `drive_id` is B's, so trash for B sees + it (not A's, which is the natural and expected answer). +- **Cascade on drive deletion**: when a shared drive is + deleted (D3 will land the delete-drive flow), every + `storage.files` / `storage.folders` row with that + `drive_id` cascades, trashed or not. There's no need to + "drain the trash first" — the whole drive disappears in + one CASCADE. +- **Personal-drive trash** follows the same model: bound to + the personal drive, sole owner (= the user) does + everything. No new UX divergence between personal and + shared. + +The orphan/aborted-upload sweep introduced in v0.7.0 +(`944c8337`, periodic trash job) must become drive-aware so +it doesn't accidentally sweep across drives the caller +shouldn't see. It already keys off ownership; the rewrite is +a per-drive pass instead of per-user. + +### 13. Upload paths and quota timing + +Four upload protocols, each with different "when do we know the +size" and "when do we know the destination" answers. The Drive +migration pivots quota from user-scoped to drive-scoped without +changing protocol shapes — but one path (NC chunked) carries a +pre-existing wart that the chroot POC's `~` username (or the +`app_passwords.drive_id` binding) lets us finally fix. + +| Protocol | Size known | Quota check fires | Destination / drive known | +|---|---|---|---| +| Default multipart (`POST /api/files/upload`) | At request start (Content-Length / multipart `size`). | `file_upload_service.rs:185` — `check_storage_quota(caller_id, metadata.size)` before any bytes are stored. | At request start (form field `folder_id`). Drive derives from `folder.drive_id`. | +| Native chunked (`POST /api/uploads`) | At session create — client declares `total_size` in JSON. | `chunked_upload_handler.rs:213` — at session creation against declared `total_size`. | At session creation (`folder_id` in JSON). Drive derives from `folder.drive_id`. | +| **NextCloud chunked** (`/remote.php/dav/uploads/{user}/{session}/...`) | Never declared. MKCOL creates empty session, PUT chunks arrive one at a time, client decides "done". | **Today: only at the final MOVE (assemble)** — `handle_assemble` → `file_upload_service::ingest_stream_to_cas` → quota check on the assembled size. Wasted-bandwidth wart: a client over quota can upload GB before the server can refuse. | **Today**: only at MOVE (parsed from the `Destination:` header). **With the chroot POC** (`{user}~{drive-uuid}` username, see §9): known at MKCOL — the auth middleware already split the username. **With `app_passwords.drive_id` binding**: known at MKCOL — the credential row pins the drive. | +| Delta protocol (`/api/files/delta/*`) | At `negotiate` (client provides manifest with `total_size`). | `delta_upload_service.rs:331` — at commit against `total_size`. | At negotiate (target file_id or `folder_id`). Drive derives accordingly. | + +#### Decision — per-chunk incremental quota check on the NC chunked path + +The three non-NC paths trivially pivot to drive-scoped quota: +replace `check_storage_quota(caller_id, size)` with +`check_drive_quota(drive_id, size)`. Destination is known at +handler entry; drive falls out of the destination's `drive_id` +column. No protocol change. + +For NC chunked, the Drive migration **also closes the +wasted-bandwidth wart** because the drive identity is now known +at MKCOL (via `~` username or app-password binding — either NC +credential-side scheme from §9 surfaces it). Approach: + +1. **MKCOL guard** — if `drive.used_bytes >= drive.quota_bytes`, + refuse the session creation with `507 Insufficient Storage`. + No point letting the client even start. +2. **Per-chunk PUT check** — track cumulative bytes received in + the session (sum of on-disk chunk sizes, maintained by the + chunked-uploads service). On each PUT, before writing the + chunk: + ```text + if drive.used_bytes + session.bytes_so_far + chunk.size > drive.quota_bytes: + refuse with 507 Insufficient Storage + ``` + The first chunk that would push us over is refused; client + sees the error within one chunk's worth of wasted upload + (typically a few MB) instead of after the whole multi-GB file. +3. **Assemble-time re-check** stays as a defence-in-depth (in + case two concurrent sessions on the same drive each got past + the per-chunk check but their sum exceeds quota at MOVE). This + matches today's structure. +4. **Unlimited quota** (`quota_bytes IS NULL`) short-circuits all + three checks — no work. + +The per-chunk check is O(1) amortised: each session tracks its +cumulative size as it goes. The drive's `used_bytes` is read from +the row once per chunk; with the v0.7.0 incremental-update +pattern (`b5b80549`, `d6987329`) that's a single primary-key +lookup, not an aggregate query. + +Net effect on NC clients: nothing changes for in-quota uploads; +over-quota clients get a clear 507 within seconds instead of +after the whole upload finishes. + +#### Editor-role delete and trash — call out the UX tension + +§4's role bundle gives `Delete` only to `owner`. That means +in a shared drive, an editor who uploads a typo file CANNOT +send it to trash themselves — they have to ask an owner. This +is already flagged as Open Question 4 (revisit before D2) and +the answer there determines the trash UX for editors too. If +editors get a `Delete` capability (or a dedicated "trash own +content" capability), the trash mutation rules become "trash +your own files" + "drive owners can act on anyone's trashed +files". Until that's decided, the conservative answer above +(owner-only mutations) holds. + +### 14. File provenance — `created_by` / `updated_by` + +Today `storage.files.user_id` (and `storage.folders.user_id`) +quietly doubles as **both** the ownership pointer (whose files are +these?) AND the provenance signal (who created this?). The Drive +migration cleanly separates ownership onto `drive_id`, but +provenance has to come with us — in a shared drive where Ed, +Alice, and Bob all upload files, "who put this here?" remains a +load-bearing question for UI, audit, and account-cleanup workflows. + +The split: + +```sql +-- on both storage.folders and storage.files: +drive_id uuid NOT NULL -- ownership +created_by uuid NULL FK → auth.users(id) ON DELETE SET NULL +created_at timestamptz NOT NULL DEFAULT now() -- existing +updated_by uuid NULL FK → auth.users(id) ON DELETE SET NULL +updated_at timestamptz NOT NULL DEFAULT now() -- existing +``` + +- **`created_by`** is set once at row insert (every upload path: + multipart, native chunked, NC chunked, streaming CDC, delta, + instant-upload-by-hash) — the caller_id stamps it. Never + changes after. +- **`updated_by`** is set whenever `updated_at` is touched — + rename, overwrite, move, restore from trash, PROPPATCH on + favorites. Same write-path discipline as `updated_at` today. +- **`ON DELETE SET NULL`** on both FKs. When a user is deleted, + files they created or edited in shared drives stay (they belong + to the drive, not the deleter); the FK nulls out, the UI renders + "Unknown user" (or "Deleted user" with a small tombstone table — + future polish, see Open Question 12). + +#### Why this lands in D0, not D7 + +If we waited until D7 to introduce these columns, every existing +file/folder row would have NULL `created_by` after the migration — +provenance permanently lost for pre-D7 content. Adding the columns +in D0 and **backfilling from `user_id`** during the dual-write +phase gives every existing row a real `created_by` value (the user +we know created it, because that's what `user_id` meant). New +writes during the dual-write window populate both `user_id` AND +`created_by` / `updated_by`. By D7 the columns are self-sufficient +and dropping `user_id` loses nothing. + +#### UI display semantics + +- File details panel: "Created by Alice on 2026-01-15", "Last + edited by Bob 3 hours ago" — both fields drive a normal user + avatar + name lookup. NULL → "Unknown user" placeholder. +- Activity log on a shared drive: aggregates `updated_by` over + recent rows to show "who's been active here lately". +- Account-cleanup workflow: when an admin deletes a user, show a + pre-flight summary "Bob authored 47 files and last edited 12 + more across drives X, Y, Z — proceeding will mark those entries + as authored by 'Deleted user'." Lets the admin choose to + reassign or simply confirm. + +#### Engine touch-point + +The "who touched updated_at" rule applies uniformly across all +mutation paths. Centralise it: every service that bumps +`updated_at` must also set `updated_by = caller_id` in the same +SQL statement. The `FileRepository::update_*` and +`FolderRepository::update_*` methods are the natural choke points +— a single audit reveals every callsite to verify. + +The async `tree_etag_queue` (v0.7.0) propagates an etag up the +ancestry but **does NOT** propagate `updated_by` — ETags are +fingerprints of structure, not authorship. Only the direct +mutation site updates `updated_by`. + +## Migration strategy + +A drive-id column on every resource is a database surgery touching +every storage query. We phase it for safety: + +### Phase A — additive (PR D0) + +1. Create `storage.drives` and `storage.drive_members`. +2. Add `drive_id uuid NULL` to `storage.folders` and `storage.files`. +3. **Per-user root-folder sweep**: for each internal user, list + every `storage.folders` row where `parent_id IS NULL AND + user_id = `. Exactly one is expected to be + `My Folder - `; any extras are SQL-created siblings + (see §10). + - The `My Folder - ` row → becomes the **default + personal drive**: `INSERT INTO storage.drives (name='Personal', + kind='personal', default_for_user=, quota_bytes=)` + and insert one `(drive_id, user=, role='owner')` + member row. + - Every other sibling row → becomes a fresh **secondary + personal drive**: `kind='personal'`, `default_for_user=NULL`, + name carried over from the folder's `name`, + `quota_bytes=`, and one + `(drive_id, user=, role='owner')` member row. + Membership rules from §2 apply (single-owner, no `add_member`); + the user can later promote one to `kind='shared'` to invite + collaborators. +4. **Promote children, drop the wrapper**: for every folder/file + row that has `parent_id = `, set + `drive_id = ` and `parent_id = NULL` + (the drive itself is the new root, not a folder). Then DELETE + the root folder rows from step 3 — they no longer exist as + folders, the drive replaces them. +5. **Cascade `drive_id` down the tree** — for every remaining + folder/file row, set `drive_id` by walking the ancestry to + whichever root the row descends from. After this step every + row has the same `drive_id` as its `parent_id`'s row, which + chains up to a drive set in step 3/4. +6. **Full path-metadata reconstruction**. The `path` column on + **every** row in `storage.folders` and `storage.files` gets + rewritten. For rows that descended from `My Folder - `, + strip that prefix; for rows that descended from a sibling root, + strip that sibling's `name`. The path column now contains only + the in-drive path (e.g. `Docs/foo.pdf`, never + `My Folder - admin/Docs/foo.pdf`). + - **This is the bulk of D0's runtime cost.** Personal-drive + scope = every folder/file the user owns. A 100k-file user + gets 100k UPDATEs. Use a single `UPDATE … WHERE drive_id = + ` per drive, not a row-at-a-time loop. The ltree-path + change is what every downstream subsystem keys off, so doing + this in one transaction per drive (not per row) is also a + correctness boundary. + - **Downstream caches and indexes** — audit each for path or + path-derived keys: + - **Tantivy content index (§11)** — the index does NOT + store paths (see `tantivy_content_index.rs`: indexed + fields are `file_id`, `user_id`, `name` (basename only), + `content`. No `path` field, the wrapper folder name was + never a term). So the wrapper removal alone requires no + reindex. The reindex §11 calls for is driven by the + schema gaining `drive_id` and the query filter pivoting + from `user_id` to `drive_id` — NOT by the path rewrite. + Same migration window, but for a different reason. + - **Thumbnail cache** — if keyed by path rather than + file_id, invalidate; preferably switch to file_id-keyed + during this migration so the issue doesn't recur. Audit + before D0 starts. + - **Folder ETag queue (`async_tree_etag_queue`,** see Open + Question 8) — flush or recompute; ETags derived from old + paths are stale. + - **Recent-items / favorites** — referenced by file_id, not + path; probably fine. Verify. + - **On-disk storage mirror** — see Open Question 10. If the + filesystem layout is path-mirrored, every file moves on + disk too; if content-addressable, the FS is untouched. + Audit before D0 starts. +7. Verify: every row has `drive_id IS NOT NULL`, no row has + `parent_id` pointing at a non-existent folder, no `path` value + contains the legacy `My Folder - ` prefix. +8. Add `NOT NULL` constraint on `drive_id`. + +**Keep `user_id`** on resources alongside `drive_id` for the entire +Phase A release cycle. Code is updated to read `drive_id` everywhere; +`user_id` is dual-written for one release as a safety net. If +something breaks we can roll back without data loss. + +Pre-flight checks the migration script runs before any writes: +- Report how many sibling-root folders exist per user. Don't + refuse — extras become drives — but surface the count so an + operator can sanity-check ("Ed has 4 root folders, expected + ≤1; verify those are real and intended before promoting them + to drives"). +- Refuse if any sibling root folder is literally named `drives` + (would collide with the reserved URL segment on the native + `/webdav/drives//...` surface). Operator renames first. +- Refuse if any user has `storage_used_bytes > storage_quota_bytes` + by an amount that wouldn't fit the destination drive's quota + semantics (sanity check). + +### Phase B — cleanup (PR D7, one release later) + +1. Drop `user_id` from `storage.folders` and `storage.files`. +2. Drop dual-write code paths. +3. Deprecate `auth.users.storage_quota_bytes` (or drop — quotas live + on drives now). + +Phase B is the point of no return; deferring it by one release gives +us a real rollback window while the new model bakes in production. + +## PR sequencing + +| PR | Scope | Risk | +|---|---|---| +| **D-Prep — role_grants refactor** | `access_grants → role_grants` schema migration with role-bundle semantics. `Manage` Permission added to the enum + role bundle. Engine reads role_grants only; `access_grants` removed (after one dual-write release if compat is needed). API gains `role` parameter on grant endpoints; audit log emits one `role_grant.*` event per role assignment instead of N permission events. **No Drive concept yet.** Sets the foundation that all subsequent PRs build on. **Data shape confirmed**: empirical audit shows >99% of existing `access_grants` rows already cluster into the standard bundles (viewer/editor/owner) — the migration is mechanical for the vast majority of data; the <1% edge cases get absorbed by shipping `commenter` and `contributor` roles on day one or get an explicit per-row migration decision logged. | **Medium** — touches the load-bearing authorisation table, but the data shape removes the main migration risk | +| **D0 — foundation** | `storage.drives` schema (no `drive_members` — uses `role_grants` from D-Prep); `Drive` domain entity; migration creating personal drives + backfilling `drive_id` on every resource; read-only `GET /api/drives` listing the caller's drives (single query: `SELECT … FROM role_grants WHERE subject_id=$caller AND resource_type='drive'`). Dual-write `user_id` alongside `drive_id` for safety. **No new UI.** **Every upload path stamps `drive_id` at insert**: classic multipart (`file_handler::upload`), chunked NC (`uploads_handler`), streaming CDC (`upload_ingest`), delta upload (`delta_upload_service`), instant upload by hash. Tantivy reindex (see §11) is part of this PR. **Provenance columns added** (see §14): `created_by` and `updated_by` on both `storage.folders` and `storage.files`, FK to `auth.users` with `ON DELETE SET NULL`; backfilled from `user_id` so pre-Drive content has provenance from day one; every mutation path that touches `updated_at` also sets `updated_by`. | **High** — every storage query touches, all upload paths touched | +| **D1 — UI switcher + URL routing** | Sidebar drive picker, `/drive//` frontend routes, default-drive redirect from `/`. WebDAV path dispatcher recognising `drives/` as the drive-explicit prefix on both `/webdav/` and `/remote.php/dav/`. | Medium | +| **D2 — drive membership API + per-drive trash auth** | `POST /api/drives/{id}/members`, `DELETE`, `PUT` for role changes — thin handlers that translate to `role_grants` INSERT/DELETE/UPDATE with `resource_type='drive'`. `Resource::Drive(Uuid)` (added in D-Prep at the enum level) gets its specialised handler surface here. Shared-drive last-owner protection. Group-as-subject support reuses the existing `subject_groups` machinery. **Personal-drive guards** (`add_member`, `remove_member`, `delete_drive` refuse on `kind='personal'` — see §2). **Per-drive trash authorisation** (§12): trash listing filters by drive(s) the caller can read; trash mutations (send/restore/permanent-delete) require `role='owner'` on the drive; `storage.trash_items` VIEW updated to surface `drive_id`; orphan/aborted-upload sweep becomes per-drive. | Medium | +| **D3 — group-owned shared drives** | "Create shared drive" flow — admin or group owner triggers, drive created with `kind='shared'`, initial owner row is the group. Group-deletion guard refuses if the group is the last owner of any drive. Drive-rename, drive-delete. | Low | +| **D4 — per-drive quota** | Move storage accounting off `auth.users.storage_used_bytes` onto `storage.drives.used_bytes`. **Re-point the existing per-user incremental CTE** (introduced in v0.7.0 — see `b5b80549`, `d6987329`) at drive rows; don't reinvent the counting logic. Upload paths check `drive.quota_bytes` instead of (or in addition to) the user's quota for the dual-write window. **Per-chunk incremental quota check on the NC chunked path** (see §13): MKCOL refuses when the drive is already over quota; each PUT chunk runs an O(1) `used + session_so_far + chunk_size > quota` test and refuses with 507 within one chunk of wasted upload. Closes a pre-existing wart where NC clients could upload GB before learning they were over quota. Reconciliation job runs once per day to fix drift. | Medium | +| **D5 — policies** | JSONB policies column + enforcement at the four known callsites. Owner-only UI in drive settings. Ship policies one at a time if you want fine-grained rollout — `forbid_public_links` first (lowest risk), then `forbid_external_sharing`, then `forbid_sharing`, then `forbid_cross_drive_move`. | Low | +| **D6 — cross-drive move + audit** | Move folder/file between drives (allowed by default; gated by `forbid_cross_drive_move` policy on the source drive). Audit events for every drive lifecycle event (`drive.created`, `drive.member_added`, `drive.member_removed`, `drive.policy_changed`, `drive.deleted`, `resource.moved_between_drives`). | Low | +| **D7 — back-compat sweep** | Drop `user_id` from `storage.folders` / `storage.files`. Drop dual-write code. Drop or deprecate `auth.users.storage_quota_bytes`. **Provenance columns (`created_by`, `updated_by`) stay** — they were populated from D0 and are now the sole source of authorship signal. | Low — but the point of no return | + +Approximate total: 4–6 weeks of focused work, depending on test +coverage depth. + +## Out of scope for v1 (worth noting so we don't accidentally invite scope creep) + +- **Timeboxed session policy** — drives that auto-lock after N + minutes. Big middleware lift. +- **End-to-end encryption policy** — client-side encryption with + server holding only ciphertext. Massive scope. +- **Per-drive sync targets** — sync client design ("1 drive = 1 + target like 'family-computer'"). Schema-friendly today (drive has + stable UUID), implementation is its own design. +- **Cross-drive search UI** — start with per-drive scoping. "Search + everywhere" comes later. +- **Drive templates** — "create a drive pre-populated with these + folders" is nice but not foundation work. + + + +## Open questions to revisit before D0 lands + +1. **Naming of the default personal drive**. + - Locked: "Personal" (i18n key, neutral, doesn't break on username + rename). + - But: should it be renameable by the user? Default-rename allowed? + I'd say yes — drive name is just a label. + +2. **`auth.users.storage_quota_bytes` — drop or keep?** + - Drop entirely (cleanest). + - Keep as the **default initial quota** for newly-created personal + drives (so admins still have a single tunable). + - I lean toward keep-as-default-initial. + +3. **Can a user be a member of their own personal drive in more than one + way?** (e.g. directly AND via a group they belong to) + - Should the membership rows allow that? Or dedup? + - Easy answer: allow; permission resolution naturally unions, so + duplicate paths to the same role are idempotent. + +4. **What happens when you DELETE a folder/file in a shared drive + you have only editor role on?** + - Today: ReBAC checks Delete permission on the resource. + - With drives: editor role implies Create + Update + Comment but + **not** Delete (per the role-bundle table above). + - So editor cannot delete by default. Should they? Some teams want + "editor can do anything except change drive settings". Worth a + dedicated decision before D2 — possibly add a separate "can + delete" toggle, or split editor into `editor` / `contributor`. + +5. **Drive icons / colour customisation** — visual differentiator + between drives in the sidebar. Out of scope for v1 but worth + noting; the `drives` table can carry a small `display` JSONB column + that accumulates this kind of cosmetic config without schema + churn. + +6. **WebDAV path resolution edge case**: when a user is a member of a + shared drive and someone shares a single file inside it explicitly + with them via a per-resource grant, what path do they see in their + client? + - The drive's path (they have access via membership), full stop. + - Per-resource grants are additive; they don't create a separate + "shared with me" listing for files that already live in a drive + the user can access. + - Confirm during D2 that this is what users expect. + +7. **Search-everywhere from the file picker**. When a user goes to + share a file, today the picker lists their own files. With drives, + should the picker default to "current drive" or "all drives I have + access to"? UX call, not a foundation decision. + +8. **`async_tree_etag_queue` — drive-pinning assumption audit**. + v0.7.0 introduced `migrations/20260626000000_tree_etag_statement_triggers.sql` + + `20260627000000_async_tree_etag_queue.sql` for ETag + propagation up folder ancestry. The queue likely keys off + folder path / owner; before D0 starts, confirm it doesn't + embed assumptions that fold `drive_id` away (e.g. computing + ancestry across a path that crosses a drive boundary). + 5-minute audit to lock the question down. + +9. **NC credential ↔ drive binding default**. Section 9 leaves + open whether the Login Flow v2 picker defaults to issuing + `{user}~{uuid}` Basic Auth usernames (option 1) or + `auth.app_passwords.drive_id`-bound app-passwords (option 2) + for non-personal drives. Decide before D1 ships; the answer + determines whether `auth.app_passwords` gets a new column or + not. + +10. **On-disk storage mirror — does the file path under + `OXICLOUD_STORAGE_PATH` change too?** Phase A step 6 strips + the `My Folder - /` prefix from `storage.folders.path` + / `storage.files.path` columns. If the on-disk layout mirrors + these paths (`//My Folder - admin/Docs/foo.pdf`), + the migration also has to `mv` every file on disk. If on-disk + is content-addressable (BLAKE3-keyed), the columns can be + rewritten without touching the filesystem. **Audit the + storage adapter before starting D0** and decide whether the + migration script: + - just renames the path columns (CAS layout — cheap), or + - renames the path columns AND issues a `mv` per file + (path-mirrored layout — expensive on big instances). + The blob store is content-addressable as of v0.7.0 so most + file content lives under `.blobs//` and is + already wrapper-agnostic; the concern is only the + metadata-projection / thumbnail-cache trees if they're + path-shaped. + +11. **Promoting sibling root folders — UX after migration**. + Users whose home was a single `My Folder - ` end + up with one drive (Personal). Users with SQL-added siblings + end up with multiple drives, one named per the original + folder name. The drive name carries over verbatim — should + the migration log who got more than one drive, so an + operator can DM them and explain the new picker? Operational + nicety, not a correctness concern. + +12. **Deleted-user tombstone for provenance**. §14 sets + `created_by` / `updated_by` to NULL when the referenced user + is deleted (`ON DELETE SET NULL`). The UI then shows "Unknown + user", which is correct but information-poor — for compliance + / audit / "who left this?" purposes, a small `deleted_users` + tombstone table (`id, last_known_username, deleted_at`) would + let the UI render "Bob (deleted 2026-04-12)" instead of just + "Unknown user". Out of scope for v1 but easy to add later + without schema rework (NULL `created_by` stays NULL; a + parallel lookup against the tombstone table on display). + +## Existing code to reuse + +- **ReBAC `Resource` enum** at `src/domain/services/authorization.rs:74` + already has a tagged-union shape; adding `Drive(Uuid)` is one variant + + `from_parts` arm + `type_str` arm. +- **ReBAC `Permission` enum** already has the bundle we need + (`Read`, `Create`, `Update`, `Delete`, `Share`, `Comment`). The + role-to-bundle mapping for drives lives in a new `drive_role.rs` + helper. +- **`PgAclEngine` Moka cache** at + `src/infrastructure/services/pg_acl_engine.rs:88` handles ReBAC + grant caching. The drive-role check piggybacks on the same cache + with a separate keyspace (`drive_perm:::`). +- **`SubjectGroupService::list_transitive_users`** at + `src/application/services/subject_group_service.rs:385` already + expands a group to its transitive user members. The drive-membership + check uses this to resolve group-owner subjects. +- **`folder_service::create_home_folder`** at + `src/application/services/folder_service.rs:644` is where the + per-user wrapper folder is created today. Post-migration this + function **goes away** — there is no wrapper folder anymore. The + user-create lifecycle hook now creates a Drive row directly and + inserts the owner-role member row. The lifecycle path is the same; + the work it does shrinks. +- **NC path resolver `nc_to_internal_path`** at + `src/interfaces/nextcloud/webdav_handler.rs:51` and the native + resolver `resolve_webdav_path` at + `src/interfaces/api/handlers/webdav_handler.rs:188` are the two + callsites that learn about drives. Both gain a "drive context" + parameter resolved from the URL prefix (`/files//` or + `{user}~{uuid}` for NC; `/webdav/` or `/webdav/drives//` + for native). **Neither resolver prepends `My Folder - /` + anymore** — the storage path IS the in-drive path. Both + functions also get simpler, not more complex, despite gaining + the drive parameter (the personal-vs-shared branch is now a + metadata lookup, not a path-shape decision). +- **`MagicLinkInviteService`** and the share-notification pipeline + (`RecipientNotificationService`) need the new policy checks + (`forbid_external_sharing`, `forbid_sharing`) wired in at their + respective callsites. +- **Migration timestamp convention**: as of v0.7.0 the head + migration is `20260702000000_drop_dead_file_indexes.sql`, and + `20260701000000_content_search_index.sql` already exists (the + Tantivy index). D0's migration becomes `20260801000000_drives.sql` + (or whatever date D0 actually starts) — the original + `20260701000000_drives.sql` slot is taken. + +## Verification + +Each PR carries its own verification block. The bar across all of +them: **(a)** new behaviour proven by a focused test, **(b)** the +existing Hurl + Playwright baselines still pass green (`bash +tests/api/run.sh && bash tests/webdav/run.sh && cd tests/e2e && npm +test`), **(c)** `cargo fmt && cargo clippy --all-features +--all-targets -- -D warnings` clean. + +### D-Prep +- **Unit**: `role_bundle(role)` returns the expected permission set + for each role; `roles_implying(permission)` returns the expected + role list; round-trip a grant insert→read→compare for every role. +- **Data audit**: query `access_grants` and confirm that >99% of + rows cluster into the standard bundles (the empirical figure that + unlocked Sequence A). Log the <1% edge cases per row for review. +- **Migration round-trip**: roll forward against a populated DB → + every prior `access_grants` row is represented by exactly one + `role_grants` row → roll back → original `access_grants` rows + recovered byte-identically. +- **API**: new Hurl test `tests/api/role_grants.hurl` — + - `POST /api/grants` with `role='editor'` creates a single row + with the expected bundle when expanded. + - `PUT /api/grants/{id}` with `role='viewer'` is atomic (no race + window where the user has zero permissions). + - Compat shim: `POST /api/grants` with the legacy `permission` + field still works for one release and maps to the closest role. + - Audit log emits `role_grant.created`, `role_grant.role_changed`, + `role_grant.revoked` events with the role name carried through. +- **UI smoke (Playwright)**: My Shares dialog renders roles instead + of permission checkboxes; share modal preset buttons map to roles; + changing a member's role from editor → viewer fires exactly one + PATCH and the UI reflects the change without a race-window blank + state. +- **All existing Hurl + WebDAV + Playwright tests still pass** — the + refactor must be invisible to every non-grant-handling test. + +### D0 +- **Unit**: `Drive` entity (kind + `default_for_user` CHECK + constraints), `default_for_user` partial unique index. Personal- + drive `add_member` / `remove_member` / `delete_drive` all refuse + cleanly with the expected `DomainError`. +- **Migration round-trip**: roll forward against a populated DB → + every existing folder/file row has `drive_id` set (no NULLs); + every user has exactly one drive with `default_for_user` set; + sibling root folders became secondary `kind='personal'` drives; + every `storage.folders.path` and `storage.files.path` value has + the `My Folder - /` prefix stripped → roll back via + `sqlx migrate revert` → `drive_id` column gone, `user_id` intact + thanks to dual-write, original paths recovered. +- **Storage check**: post-migration `bash tests/api/storage_cleanup_check.sh` + still reports a clean tree (no orphans). +- **Tantivy reindex**: every indexed doc carries a `drive_id`; + search query filtered by `drive_id ∈ caller's drives` returns the + expected hits; cross-drive isolation confirmed (search as Alice, + Bob's content never appears). +- **API**: `tests/api/drives_foundation.hurl` — admin lists + `/api/drives`, sees their default personal drive with the correct + quota, `kind='personal'`, `default_for_user` matching their own + uuid. Creating a folder still works; the folder's `drive_id` + matches the personal drive. +- **All upload paths set `drive_id`**: targeted Hurl tests for + multipart, native chunked, NC chunked, delta upload, instant + upload by hash. Each verifies the resulting file row has the + expected `drive_id`. + +### D1 +- **Routing**: `cargo build` clean; WebDAV dispatcher routes + `/webdav/drives//...` correctly; `/webdav/` still + resolves to the caller's default drive (back-compat). +- **NC client back-compat**: a real NC sync client pointed at + `/remote.php/dav/files/admin/` continues syncing the user's default + personal drive without reconfiguration. The chroot POC's `~` + username (or app-password binding) lands a sync into the chosen + drive transparently. +- **Manual smoke**: open `/`, get redirected to + `/drive/`. Click sidebar drive switcher → URL + updates, listing reloads. Drive picker shows all of the caller's + drives (default first), each with its quota usage. +- **Playwright**: a new `tests/e2e/drive-switching.spec.ts` exercises + sidebar → URL → listing → cross-drive isolation (folders in + drive A don't appear in drive B's listing). + +### D2 +- **Membership API**: `POST /api/drives/{id}/members` with user, with + group, and with role changes; refuses on personal drives; + shared-drive last-owner protection. Group expansion (transitive + members count toward the owner total). +- **Trash per drive**: trash listing scopes correctly to drive(s) + the caller can read; mutations refuse without owner role; the + `storage.trash_items` VIEW surfaces `drive_id`. +- **Updated NC + WebDAV regression baselines** — the 105+ scenarios + in `BASELINE_TESTS_NC_WEBDAV.md` still pass green; per-drive + trash and membership changes don't regress existing protocol + behaviour. + +### D3–D7 +- Per-PR verification authored when the PR is drafted. Each PR adds + at least one new Hurl/Playwright test for the headline capability + and proves zero regression on the previous baselines. + +### Cross-cutting regression — runs on every Drive PR + +A standing checklist independent of the headline capability of each +PR: + +1. `bash tests/api/run.sh` green (>100 scenarios). +2. `bash tests/webdav/run.sh` green (NC + native WebDAV baselines). +3. `cd tests/e2e && npm test` green (Playwright). +4. `tests/api/storage_cleanup_check.sh` clean. +5. No new `cargo clippy` warnings. +6. Tantivy index returns no cross-drive results for any caller. +7. `/api/dedup/stats` shows blob ref-counts consistent with the + number of files referencing each blob across all drives. + +## UI design — outline for D1 and D3 + +The Drive concept reshapes three load-bearing UI elements. Each +gets a small design pass alongside the relevant PR — the bullets +here lock the **intended shape**; the pixel work happens when D1 / +D3 draft. + +### Sidebar — drives as children of a "Drive" section + +Today the sidebar shows a single root view (the user's home +folder). Post-Drive it gains a top-level **Drive** section whose +children are the drives the caller has access to: + +``` +📁 Drive + ⭐ Personal ← kind='personal', default_for_user=caller (★ marks default) + Family Archive ← kind='personal', secondary (no star) + Engineering ← kind='shared', caller is owner/editor/viewer + 2025 Marketing Sprint ← kind='shared' + Recent + Favorites + Shared with me + Trash +``` + +- The user's **default personal drive** is marked with a star (or + bolded — UI choice). Clicking it lands on the default landing + view (today's "Files" experience). +- Other drives — secondary personals, shared drives — appear below + with the same visual weight. Clicking switches the context (URL + changes to `/drive//...`, listing reloads). +- The sidebar is collapsible per-drive; users with many drives can + hide the drives they aren't actively in. +- **No drive icons / colours in v1** (deferred to a future polish + PR — see "Out of scope" item 5). +- The "Drive" section header itself is non-interactive (just a + grouping label); the action affordance is "Create shared drive" + via the `+` button at the end of the list (admin / group-owner + scoped; lands in D3). + +### Breadcrumb — drive-rooted + +Today's breadcrumb is path-rooted: `Home / Docs / Q3 / Report.pdf`. +Post-Drive the breadcrumb starts at the selected drive: + +``` +[Personal ▾] / Docs / Q3 / Report.pdf +``` + +- The drive name is the **first** element. Clicking it returns to + that drive's root listing. +- The `▾` chevron next to the drive name opens a quick-switcher + picker (same list as the sidebar). Lets users jump between drives + without using the sidebar. +- For paths inside shared drives, the drive name still leads: + `[Engineering ▾] / Specs / Q3 OKRs.md`. +- The breadcrumb's overflow / truncation behaviour for deep paths is + unchanged from today — only the root element is new. +- **Drive name follows server-side rename** — the breadcrumb queries + the drive's current `name`; renaming the drive updates the + breadcrumb on next reload without any client-side cache work. + +### "Owners" section — review and redesign + +Today's UI has an "Owner" surface in several places (file details +sidebar, share dialog, folder properties). It currently shows "the +user who owns this file" — a single name. + +Post-Drive, ownership has multiple shapes: + +- For a file in a personal drive: still one owner (the drive's sole + user-owner). Display stays the same — "Owner: Ed". +- For a file in a shared drive: ownership is the drive's **owner- + role members** (possibly multiple users + groups). Display + becomes "Owner: Engineering team (3 members + 2 groups)". +- For a drive itself (when displayed in a settings view): same as + above, but in a list form: "Owners: Ed, Alice, Engineering + group". + +Open questions for the D3 PR to settle: +- **Show the drive name as the effective owner?** "Owner: + Engineering" reads cleanly but conflates "the drive" with "the + drive's owners" — clearer for end users, less precise. +- **Should the file details sidebar expand owners on click?** + Click "Engineering team" → see the owner roster. Useful for + large drives where the owner list doesn't fit inline. +- **Audit/My Shares dialog**: the "Shared with me" view currently + groups by sharer. Post-Drive it can group by drive instead + (your shared-drive access shows once per drive, not once per + file). UX win — decide which grouping is default and whether + both are togglable. + +These three UI surfaces are independent enough that they can ship +in separate PRs (sidebar in D1, breadcrumb in D1, owners review +in D3 alongside the create-shared-drive flow). The sidebar and +breadcrumb are essentially mechanical given the new model; the +owners review is the only one with genuine design questions. + +## File map (anticipated, not yet created) + +``` +migrations/ + 20260730000000_role_grants.sql ← D-Prep (rename of access_grants + to role_grants with role bundles) + 20260801000000_drives.sql ← D0 (drives table only; + membership is rows in role_grants) + 20260901000000_drop_user_id_on_resources.sql ← D7 + +src/domain/entities/ + drive.rs ← D0 + role.rs ← D-Prep (role enum + bundle map) + +src/domain/services/ + authorization.rs (modify) ← D-Prep adds `Manage` Permission + + `Resource::Drive(Uuid)` variant + +src/application/services/ + drive_service.rs ← D0–D3 (CRUD + policy enforcement, + membership operations translate + to role_grants writes) + +src/infrastructure/repositories/pg/ + drive_pg_repository.rs ← D0 + role_grant_pg_repository.rs ← D-Prep (replaces + access_grant_pg_repository) + +src/infrastructure/services/pg_acl_engine.rs (modify) + reads role_grants only ← D-Prep + Resource::Drive routing ← D0 + +src/interfaces/api/handlers/ + grant_handler.rs (modify) ← D-Prep (accepts role parameter) + drive_handler.rs ← D0 (list), D2 (members), D3 (create/delete shared) + +src/interfaces/api/handlers/webdav_handler.rs (modify) + drive-aware path resolution ← D1 + +src/interfaces/nextcloud/webdav_handler.rs (modify) + drive-aware path resolution ← D1 + +static/js/views/drive/ ← D1 (sidebar switcher, drive view) +static/js/model/drives.js ← D1 (REST client) +static/css/components/driveSwitcher.css ← D1 +``` + +## Glossary (for the next reader) + +- **Drive** — a top-level container that owns folders and files. +- **Personal drive** — the auto-created drive a user gets at + registration. One per internal user. `kind='personal'`. +- **Shared drive** — a drive whose membership includes at least one + group (or multiple users). `kind='shared'`. +- **Drive member** — a row in `storage.role_grants` with + `resource_type='drive'`, carrying a subject (user or group) + and a role. There is **no** separate `drive_members` table — see + the D-Prep prerequisite at the top of this plan for the storage + pivot. +- **Owner / editor / viewer** — drive roles. Map to ReBAC + permission bundles for resources inside the drive. +- **Drive policy** — JSONB key on the drive that toggles a sharing / + movement restriction. +- **Drive context** in WebDAV — the drive whose root the request is + rooted at. Native WebDAV resolves it from the URL prefix + (`/webdav/` → caller's personal drive, + `/webdav/drives//` → explicit drive). NC resolves + it from the credential (Basic Auth `{user}~{uuid}` username, or + the `auth.app_passwords.drive_id` binding — see §9). +- **Wrapper folder** — historical name for + `My Folder - `, the folder created at registration + via `format!("My Folder - {}", username)`. **Retired** in the + Drive migration: drive root replaces it. Every reference to + "wrapper" in older comments / docs is by definition pre-Drive. diff --git a/docs/plan/plan-ReBAC-Permissions-Grants-Cascading.md b/docs/plan/plan-ReBAC-Permissions-Grants-Cascading.md index 5a0a6506..9a55d1cb 100644 --- a/docs/plan/plan-ReBAC-Permissions-Grants-Cascading.md +++ b/docs/plan/plan-ReBAC-Permissions-Grants-Cascading.md @@ -602,20 +602,29 @@ impl Role { pub fn expand(self) -> &'static [Permission] { match self { Role::Viewer => &[Permission::Read], - Role::Commenter => &[Permission::Read, Permission::Comment], - Role::Editor => &[Permission::Read, Permission::Comment, - Permission::Create, Permission::Update], - Role::Manager => &[Permission::Read, Permission::Comment, - Permission::Create, Permission::Update, - Permission::Share], - Role::Admin => &[Permission::Read, Permission::Comment, - Permission::Create, Permission::Update, - Permission::Share, Permission::Delete], + Role::Commenter => &[Permission::Read, Permission::Comment], + Role::Contributor => &[Permission::Read, Permission::Create], + Role::Editor => &[Permission::Read, Permission::Comment, + Permission::Create, Permission::Update], + Role::Owner => &[Permission::Read, Permission::Comment, + Permission::Create, Permission::Update, + Permission::Share, Permission::Delete, + Permission::Manage], } } } ``` +> **Note (D-Prep, 2026-06-17):** the `Manager` role was retired before shipping +> (its bundle was a strict subset of `Owner`); the historical `Admin` role was +> renamed to `Owner` to disambiguate from `UserRole::Admin` (the user-account +> privilege) and match Drive plan terminology. `Contributor` is the new +> drop-zone role. The actual on-the-wire enum lives in +> `src/application/dtos/grant_dto.rs`; that file is the canonical source of +> truth for bundle expansion. The pivot to role-keyed storage (`role_grants` +> table) also happened in D-Prep — see +> `docs/architecture/rebac-authorization.md` for the dual-write timeline. + ### `POST /api/grants` accepts either shape ```json diff --git a/migrations/20260730000000_role_grants.sql b/migrations/20260730000000_role_grants.sql new file mode 100644 index 00000000..afdc1921 --- /dev/null +++ b/migrations/20260730000000_role_grants.sql @@ -0,0 +1,244 @@ +-- ════════════════════════════════════════════════════════════════════════════ +-- D-Prep: storage.role_grants — role-bundle replacement for access_grants +-- ════════════════════════════════════════════════════════════════════════════ +-- Refactor #1 of the Drive sequence (see `docs/plan/drive.md` § Prerequisite). +-- +-- Today every role assignment is stored as N rows in `storage.access_grants` +-- (one row per Permission in the role's bundle — editor = 4 rows, owner = 6). +-- This migration introduces `storage.role_grants` where each role assignment +-- is ONE row carrying the role name; permission expansion happens at engine +-- read time via the in-code `role_bundle()` function. +-- +-- The five roles shipped on day one: +-- viewer = {read} +-- commenter = {comment, read} ← new +-- contributor = {create, read} ← new +-- editor = {comment, create, read, update} +-- owner = {comment, create, delete, read, share, update} +-- (post-Drive: + manage, when Group-as-Resource lands) +-- +-- This migration is **additive**: `storage.access_grants` stays populated as +-- a dual-write safety net until a follow-up cleanup PR drops it after the +-- new model has baked in production. The down migration just drops +-- role_grants — access_grants is untouched, so rollback is trivial. +-- +-- Pre-flight: the migration REFUSES to run if `access_grants` contains any +-- non-bundle clusters (permission sets that don't match one of the five +-- roles above). Run `tools/audit-grants-bundle-shape.sql` first to confirm +-- the data is clean — Ed's audit on 2026-06-17 returned 100% bundle-shaped. + + +-- ── 1. Pre-flight assertion ───────────────────────────────────────────────── +-- Refuse to migrate if there are any non-bundle clusters. The five known +-- bundles are listed here verbatim; keep them in sync with the in-code +-- `role_bundle()` function. + +DO $BODY$ +DECLARE + bad_count BIGINT; +BEGIN + WITH cluster AS ( + SELECT subject_type, subject_id, resource_type, resource_id, + array_agg(permission ORDER BY permission) AS perms + FROM storage.access_grants + GROUP BY 1, 2, 3, 4 + ) + SELECT count(*) INTO bad_count + FROM cluster + WHERE perms NOT IN ( + ARRAY['read']::text[], + ARRAY['comment','read']::text[], + ARRAY['create','read']::text[], + ARRAY['comment','create','read','update']::text[], + ARRAY['comment','create','delete','read','share','update']::text[] + ); + + IF bad_count > 0 THEN + RAISE EXCEPTION + 'D-Prep migration refused: % (subject,resource) clusters in ' + 'storage.access_grants have non-bundle permission sets. Run ' + 'tools/audit-grants-bundle-shape.sql section 3 to inspect them, ' + 'then either resolve manually or extend the bundle list above ' + 'with a new named role before retrying.', bad_count; + END IF; +END $BODY$; + + +-- ── 2. The role_grants table ──────────────────────────────────────────────── + +CREATE TABLE IF NOT EXISTS storage.role_grants ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + + -- Subject (who has the role) + -- 'user' → auth.users.id + -- 'group' → storage.subject_groups.id + -- 'token' → storage.shares.id (anonymous link — always 'viewer') + subject_type TEXT NOT NULL + CHECK (subject_type IN ('user', 'group', 'token')), + subject_id UUID NOT NULL, + + -- Resource (what the role is on) + -- 'drive' and 'group' join later as Drive + Group-as-Resource land. + resource_type TEXT NOT NULL + CHECK (resource_type IN ('folder', 'file')), + resource_id UUID NOT NULL, + + -- Role — expands to a permission bundle via the in-code `role_bundle()` + -- function. The CHECK lists the day-one role roster; adding a new + -- role is a single ALTER TABLE DROP CONSTRAINT / ADD CONSTRAINT pair + -- (or replace with a foreign key into a lookup table if instance- + -- defined roles ever land). + -- + -- Universal roster: ANY role can be granted on ANY resource_type. + -- Permission bundles include capabilities the resource type may not + -- check for (e.g. `Manage` on a folder, `Create` on a file); those + -- produce harmless no-ops at engine read time — no per-resource-type + -- validation needed at the DB layer. + -- + -- The UI exposes only Viewer/Editor/Owner in the share dialog today + -- (matches the existing 3-button UX). Commenter and Contributor stay + -- in the enum for server-side use + future UI exposure when a real + -- use case asks for them. + role TEXT NOT NULL + CHECK (role IN ('viewer', 'commenter', 'contributor', 'editor', 'owner')), + + -- Audit + lifecycle + granted_by UUID NOT NULL, + granted_at TIMESTAMPTZ NOT NULL DEFAULT now(), + expires_at TIMESTAMPTZ, + + -- Exactly one role per (subject, resource). Atomic role changes become + -- a single UPDATE; no DELETE+INSERT race. + UNIQUE (subject_type, subject_id, resource_type, resource_id) +); + +COMMENT ON TABLE storage.role_grants IS + 'Role-based ReBAC grants. One row = one role assignment. Permission ' + 'bundle expansion is in-code; see role_bundle() in ' + 'src/application/dtos/grant_dto.rs. Replaces storage.access_grants; ' + 'both tables coexist during the D-Prep dual-write window.'; +COMMENT ON COLUMN storage.role_grants.role IS + 'One of viewer / commenter / contributor / editor / owner. Expanded to ' + 'a Permission bundle by the in-code role_bundle() function at engine ' + 'read time.'; + + +-- ── 3. Indexes — match the hot-path queries ───────────────────────────────── + +-- "What does this caller have access to?" — every WebDAV / NC request, +-- every UI default-drive resolution (post-Drive) hits this. +CREATE INDEX IF NOT EXISTS idx_role_grants_subject + ON storage.role_grants (subject_type, subject_id); + +-- "Who has access to this resource?" — share dialogs, audit views. +CREATE INDEX IF NOT EXISTS idx_role_grants_resource + ON storage.role_grants (resource_type, resource_id); + +-- Partial index on expiry — only rows that actually expire (mirrors the +-- access_grants index pattern, same rationale). +CREATE INDEX IF NOT EXISTS idx_role_grants_expires_at + ON storage.role_grants (expires_at) WHERE expires_at IS NOT NULL; + +-- For GET /api/grants/outgoing/resources (who granted what). +CREATE INDEX IF NOT EXISTS idx_role_grants_granted_by + ON storage.role_grants (granted_by); + + +-- ── 4. Backfill from access_grants ───────────────────────────────────────── +-- For each (subject, resource) cluster in access_grants, write one +-- role_grants row with the matching role. The CASE expression mirrors +-- `Role::expand()` exactly — when that function changes (new role added), +-- update both this CASE and the CHECK constraint above. +-- +-- expires_at: take MIN across the cluster (most conservative — the role +-- assignment expires at the earliest expiry of any of its constituent +-- grants). granted_at: MIN (when the role assignment started). granted_by: +-- the granter of the earliest row (preserves attribution to the admin who +-- initially set the role up). + +WITH cluster AS ( + SELECT subject_type, + subject_id, + resource_type, + resource_id, + array_agg(permission ORDER BY permission) AS perms, + MIN(granted_at) AS earliest_granted_at, + MIN(expires_at) AS earliest_expires_at + FROM storage.access_grants + GROUP BY 1, 2, 3, 4 +), +earliest_grantor AS ( + SELECT DISTINCT ON (subject_type, subject_id, resource_type, resource_id) + subject_type, + subject_id, + resource_type, + resource_id, + granted_by + FROM storage.access_grants + ORDER BY subject_type, subject_id, resource_type, resource_id, granted_at ASC +) +INSERT INTO storage.role_grants + (subject_type, subject_id, resource_type, resource_id, + role, granted_by, granted_at, expires_at) +SELECT + c.subject_type, + c.subject_id, + c.resource_type, + c.resource_id, + CASE c.perms + WHEN ARRAY['read']::text[] + THEN 'viewer' + WHEN ARRAY['comment','read']::text[] + THEN 'commenter' + WHEN ARRAY['create','read']::text[] + THEN 'contributor' + WHEN ARRAY['comment','create','read','update']::text[] + THEN 'editor' + WHEN ARRAY['comment','create','delete','read','share','update']::text[] + THEN 'owner' + END AS role, + eg.granted_by, + c.earliest_granted_at, + c.earliest_expires_at +FROM cluster c +JOIN earliest_grantor eg USING (subject_type, subject_id, resource_type, resource_id) +ON CONFLICT (subject_type, subject_id, resource_type, resource_id) DO NOTHING; + + +-- ── 5. Post-flight consistency check ─────────────────────────────────────── +-- Assert that the backfill landed one role_grants row per (subject, +-- resource) cluster in access_grants. Any mismatch means a bundle pattern +-- silently failed to match — refuses to commit, surfacing the bug. + +DO $BODY$ +DECLARE + expected_clusters BIGINT; + actual_role_grants BIGINT; + null_roles BIGINT; +BEGIN + SELECT count(*) INTO expected_clusters + FROM ( + SELECT 1 FROM storage.access_grants + GROUP BY subject_type, subject_id, resource_type, resource_id + ) c; + + SELECT count(*) INTO actual_role_grants FROM storage.role_grants; + + IF expected_clusters != actual_role_grants THEN + RAISE EXCEPTION + 'D-Prep backfill consistency check failed: expected % role_grants ' + 'rows (one per distinct (subject, resource) cluster in access_grants), ' + 'got %. Investigate before declaring the migration successful.', + expected_clusters, actual_role_grants; + END IF; + + -- Defensive: NULL role would mean the CASE expression failed to match. + -- Pre-flight already refuses this, but double-check. + SELECT count(*) INTO null_roles FROM storage.role_grants WHERE role IS NULL; + IF null_roles > 0 THEN + RAISE EXCEPTION + 'D-Prep backfill produced % role_grants rows with NULL role — ' + 'a bundle pattern slipped past the pre-flight check. Investigate.', + null_roles; + END IF; +END $BODY$; diff --git a/migrations/20260801000000_role_grants_enum.sql b/migrations/20260801000000_role_grants_enum.sql new file mode 100644 index 00000000..81ac0f16 --- /dev/null +++ b/migrations/20260801000000_role_grants_enum.sql @@ -0,0 +1,72 @@ +-- ════════════════════════════════════════════════════════════════════════════ +-- Cleanup #1: storage.role_grants.role — TEXT → storage.grant_role ENUM +-- ════════════════════════════════════════════════════════════════════════════ +-- D-Prep shipped `role_grants.role` as TEXT + CHECK constraint. Promoting it +-- to a native PostgreSQL ENUM gives us three things at once: +-- +-- 1. Index-driven sort by role strength. The ENUM values are declared in +-- strength order — owner first, viewer last. `ORDER BY role ASC` then +-- yields the UX-mandated "strongest first" ordering (Owner → Editor → +-- Contributor → Commenter → Viewer) without a CASE expression. The +-- `idx_role_grants_subject` / `idx_role_grants_resource` indexes can be +-- extended (or composite-augmented) with the role column for index-only +-- ordered scans. +-- +-- 2. Type-level safety. The CHECK constraint goes away; invalid roles fail +-- at the column type, not at row insertion. One contract instead of two +-- (column type AND check constraint). +-- +-- 3. Cleaner query shape. Every listing query that used the strength CASE +-- becomes a plain `ORDER BY role` after this migration. +-- +-- Trade-off accepted: PostgreSQL ENUMs allow ADD VALUE (with BEFORE / AFTER +-- positional anchors) and RENAME VALUE, but not DROP VALUE or arbitrary +-- reorder. The OxiCloud role roster is intentionally stable — new roles get +-- appended, none get reordered or removed. Confirmed with Ed. +-- +-- This migration must run BEFORE the access_grants drop, since it's purely +-- about role_grants.role. + +-- ── 1. Create the ENUM type ──────────────────────────────────────────────── +-- Declaration order = sort order. Strongest first so `ORDER BY role ASC` +-- matches the UX requirement (max permission → least permission). + +CREATE TYPE storage.grant_role AS ENUM ( + 'owner', -- ordinal 0, sorts first + 'editor', -- ordinal 1 + 'contributor', -- ordinal 2 + 'commenter', -- ordinal 3 + 'viewer' -- ordinal 4, sorts last +); + +COMMENT ON TYPE storage.grant_role IS + 'Role-keyed grant strength. Declaration order is sort order: ORDER BY ' + 'role ASC yields owner → viewer (strongest → weakest), matching the ' + 'share-dialog and shared-with-me UX. Adding a new role is ALTER TYPE ' + 'ADD VALUE; renaming is ALTER TYPE RENAME VALUE. Dropping or reordering ' + 'is not supported — adjust the roster only by append.'; + + +-- ── 2. Drop the redundant CHECK constraint ───────────────────────────────── +-- The inline CHECK on role_grants.role was auto-named +-- `role_grants_role_check` by PostgreSQL. Drop it before the type swap — +-- the ENUM now enforces the same invariant at the column level. + +ALTER TABLE storage.role_grants + DROP CONSTRAINT IF EXISTS role_grants_role_check; + + +-- ── 3. Convert role TEXT → storage.grant_role ────────────────────────────── +-- USING cast: text values are guaranteed to be one of the five valid labels +-- (the dropped CHECK enforced this; the D-Prep backfill only produced these +-- five values). If a stray value slipped through, the cast errors out and +-- the migration aborts — preferable to silently coercing. + +ALTER TABLE storage.role_grants + ALTER COLUMN role TYPE storage.grant_role + USING role::storage.grant_role; + +COMMENT ON COLUMN storage.role_grants.role IS + 'One of owner / editor / contributor / commenter / viewer. Expanded to ' + 'a Permission bundle by the in-code role_bundle() function at engine ' + 'read time. Sort order matches declaration order in storage.grant_role.'; diff --git a/migrations/20260801000001_role_grants_cascade_triggers.sql b/migrations/20260801000001_role_grants_cascade_triggers.sql new file mode 100644 index 00000000..1fca5043 --- /dev/null +++ b/migrations/20260801000001_role_grants_cascade_triggers.sql @@ -0,0 +1,122 @@ +-- ════════════════════════════════════════════════════════════════════════════ +-- Cleanup #2: cascade triggers for storage.role_grants +-- ════════════════════════════════════════════════════════════════════════════ +-- The D-Prep migration created `storage.role_grants` but no cascade triggers. +-- Until now, role_grants stayed consistent because the application-layer +-- lifecycle hooks (`engine.revoke_all_for_resource` / `_subject`) wiped rows +-- on the canonical delete paths, AND the existing `trg_cleanup_grants_*` +-- triggers kept `storage.access_grants` clean as a defence-in-depth net. +-- +-- The follow-up cleanup PR drops `access_grants` (and its triggers) entirely. +-- Without this migration that drop would leave `role_grants` without any +-- DB-level safety net — direct SQL, future codepaths that forget to call the +-- engine hooks, and any other bypass route could orphan rows whose subject +-- or resource has already been deleted. +-- +-- This migration mirrors the four forward + one reverse triggers from +-- `20260520000000_rebac_access_grants.sql` and `20260612000001_share_grant_ +-- reverse_cascade.sql`, retargeted at `storage.role_grants`. Same shape, same +-- AFTER-DELETE semantics, same idempotent CREATE OR REPLACE patterns. +-- +-- During the transition window (this migration applied; `access_grants` not +-- yet dropped) both sets of triggers coexist — they target different tables +-- and don't conflict. Once `access_grants` is dropped, the old triggers and +-- their helper functions vanish in the same migration. + +-- ── 1. Forward cascade: resource delete → cleanup role_grants ────────────── +-- Fires AFTER DELETE on storage.folders / storage.files; deletes every +-- role_grants row referencing that resource. TG_ARGV[0] discriminates which +-- resource_type the trigger is wired for. + +CREATE OR REPLACE FUNCTION storage.cleanup_role_grants_on_resource_delete() +RETURNS TRIGGER AS $$ +BEGIN + DELETE FROM storage.role_grants + WHERE resource_type = TG_ARGV[0] + AND resource_id = OLD.id; + RETURN OLD; +END; +$$ LANGUAGE plpgsql; + +DROP TRIGGER IF EXISTS trg_cleanup_role_grants_folder ON storage.folders; +CREATE TRIGGER trg_cleanup_role_grants_folder + AFTER DELETE ON storage.folders + FOR EACH ROW + EXECUTE FUNCTION storage.cleanup_role_grants_on_resource_delete('folder'); + +DROP TRIGGER IF EXISTS trg_cleanup_role_grants_file ON storage.files; +CREATE TRIGGER trg_cleanup_role_grants_file + AFTER DELETE ON storage.files + FOR EACH ROW + EXECUTE FUNCTION storage.cleanup_role_grants_on_resource_delete('file'); + + +-- ── 2. Forward cascade: subject delete → cleanup role_grants ─────────────── +-- Fires AFTER DELETE on auth.users / storage.shares; deletes every +-- role_grants row referencing that subject. Groups are NOT wired here — +-- `subject_group_service::delete()` performs that cascade transactionally +-- in application code, mirroring the historical access_grants behaviour. + +CREATE OR REPLACE FUNCTION storage.cleanup_role_grants_on_subject_delete() +RETURNS TRIGGER AS $$ +BEGIN + DELETE FROM storage.role_grants + WHERE subject_type = TG_ARGV[0] + AND subject_id = OLD.id; + RETURN OLD; +END; +$$ LANGUAGE plpgsql; + +DROP TRIGGER IF EXISTS trg_cleanup_role_grants_user ON auth.users; +CREATE TRIGGER trg_cleanup_role_grants_user + AFTER DELETE ON auth.users + FOR EACH ROW + EXECUTE FUNCTION storage.cleanup_role_grants_on_subject_delete('user'); + +DROP TRIGGER IF EXISTS trg_cleanup_role_grants_token ON storage.shares; +CREATE TRIGGER trg_cleanup_role_grants_token + AFTER DELETE ON storage.shares + FOR EACH ROW + EXECUTE FUNCTION storage.cleanup_role_grants_on_subject_delete('token'); + + +-- ── 3. Reverse cascade: last-token-grant delete → cleanup storage.shares ─── +-- A caller hitting DELETE /api/grants/{id} on a token's role grant would +-- otherwise leave the storage.shares row stranded — the token still +-- resolves to "no access" (cascade query finds no rows), but the metadata +-- row accumulates forever. +-- +-- With role_grants the UNIQUE (subject, resource) constraint guarantees a +-- token has at most ONE role grant per resource, so "the last grant for a +-- token" collapses to "the only grant for that token". The NOT EXISTS +-- guard still works correctly — it just always evaluates the same way for +-- token subjects. +-- +-- The DELETE on storage.shares is a no-op when the share row is already +-- gone (the forward cascade `trg_cleanup_role_grants_token` is in flight +-- and already removed it). Idempotent in both directions. + +CREATE OR REPLACE FUNCTION storage.cleanup_share_on_last_role_grant_delete() +RETURNS trigger AS $$ +BEGIN + IF OLD.subject_type = 'token' THEN + DELETE FROM storage.shares s + WHERE s.id = OLD.subject_id + AND NOT EXISTS ( + SELECT 1 FROM storage.role_grants rg + WHERE rg.subject_type = 'token' + AND rg.subject_id = OLD.subject_id + ); + END IF; + RETURN OLD; +END; +$$ LANGUAGE plpgsql; + +DROP TRIGGER IF EXISTS trg_cleanup_share_on_role_grant_delete ON storage.role_grants; +CREATE TRIGGER trg_cleanup_share_on_role_grant_delete + AFTER DELETE ON storage.role_grants + FOR EACH ROW + EXECUTE FUNCTION storage.cleanup_share_on_last_role_grant_delete(); + +COMMENT ON FUNCTION storage.cleanup_share_on_last_role_grant_delete() IS + 'Reverse cascade: deletes storage.shares row when its last token role grant is removed. Pairs with trg_cleanup_role_grants_token (forward direction).'; diff --git a/migrations/20260801000002_drop_access_grants.sql b/migrations/20260801000002_drop_access_grants.sql new file mode 100644 index 00000000..4162b626 --- /dev/null +++ b/migrations/20260801000002_drop_access_grants.sql @@ -0,0 +1,63 @@ +-- ════════════════════════════════════════════════════════════════════════════ +-- Cleanup #3: drop storage.access_grants (and everything attached to it) +-- ════════════════════════════════════════════════════════════════════════════ +-- The final step of the role-keyed ReBAC cleanup. By the time this migration +-- runs: +-- +-- * Every read path goes through `storage.role_grants` (cleanup #1 / #2). +-- * The engine no longer has a `grant()` method; `set_role()` / +-- `clear_role()` are the only writes. +-- * The HTTP surface (`POST /api/grants`, `PUT /api/grants/role`) only +-- accepts role-keyed shapes. +-- * `share_service`, `subject_group_service`, `auth_application_service`, +-- `share_pg_repository`, and `integration_test_support` all read +-- `role_grants` exclusively. +-- * `storage.role_grants` has its own cascade triggers +-- (`trg_cleanup_role_grants_*`) and reverse-cascade +-- (`trg_cleanup_share_on_role_grant_delete`), added in cleanup #2. +-- +-- So `access_grants` is fully unreferenced — we can drop it together with +-- the helper triggers + functions defined in +-- `20260520000000_rebac_access_grants.sql` and +-- `20260612000001_share_grant_reverse_cascade.sql`. +-- +-- Roll-back posture: this is destructive. There is no down migration. The +-- D-Prep backfill is one-way (role-keyed rows are derived from +-- permission-keyed clusters; the reverse reconstruction would need a fixed +-- bundle mapping that may have shifted between releases). Recovering +-- requires restoring from a backup taken before this migration runs. + +-- ── 1. Drop the access_grants triggers FROM their source tables ──────────── +-- These triggers live on storage.folders / storage.files / auth.users / +-- storage.shares. Dropping access_grants doesn't implicitly remove them +-- (the trigger row points at the source table; the body references the +-- target table, and that body is what breaks once access_grants is gone). +-- Drop them explicitly so subsequent DELETEs on those source tables don't +-- error out. + +DROP TRIGGER IF EXISTS trg_cleanup_grants_folder ON storage.folders; +DROP TRIGGER IF EXISTS trg_cleanup_grants_file ON storage.files; +DROP TRIGGER IF EXISTS trg_cleanup_grants_user ON auth.users; +DROP TRIGGER IF EXISTS trg_cleanup_grants_token ON storage.shares; + +-- The reverse-cascade trigger is ON access_grants and goes away with the +-- table — but the IF EXISTS makes this safe regardless of drop order. +DROP TRIGGER IF EXISTS trg_cleanup_share_on_grant_delete ON storage.access_grants; + + +-- ── 2. Drop the trigger helper functions ──────────────────────────────────── +-- No other code references these — the `cleanup_role_grants_*` equivalents +-- defined in cleanup #2 carry the same behaviour against role_grants. + +DROP FUNCTION IF EXISTS storage.cleanup_grants_on_resource_delete(); +DROP FUNCTION IF EXISTS storage.cleanup_grants_on_subject_delete(); +DROP FUNCTION IF EXISTS storage.cleanup_share_on_last_token_grant_delete(); + + +-- ── 3. Drop the table ────────────────────────────────────────────────────── +-- CASCADE removes any remaining dependent objects (indexes, comments, and +-- the reverse-cascade trigger if it survived step 1). With every Rust code +-- path already routed through role_grants, nothing in the application +-- layer will notice. + +DROP TABLE IF EXISTS storage.access_grants CASCADE; diff --git a/src/application/dtos/grant_dto.rs b/src/application/dtos/grant_dto.rs index ac74a225..62c42bf5 100644 --- a/src/application/dtos/grant_dto.rs +++ b/src/application/dtos/grant_dto.rs @@ -11,7 +11,7 @@ use uuid::Uuid; use crate::application::dtos::cursor::{CursorListResponse, CursorQuery, PageCursor}; use crate::application::dtos::file_dto::FileDto; use crate::application::dtos::folder_dto::FolderDto; -use crate::domain::services::authorization::{Grant, Permission, Resource, Subject}; +use crate::domain::services::authorization::{Grant, Permission, Resource, Role, Subject}; // ════════════════════════════════════════════════════════════════════════════ // Subject / Resource / Permission DTOs @@ -95,6 +95,7 @@ pub enum PermissionDto { Comment, Delete, Update, + Manage, } impl From for Permission { @@ -106,6 +107,7 @@ impl From for Permission { PermissionDto::Comment => Permission::Comment, PermissionDto::Delete => Permission::Delete, PermissionDto::Update => Permission::Update, + PermissionDto::Manage => Permission::Manage, } } } @@ -119,57 +121,58 @@ impl From for PermissionDto { Permission::Comment => PermissionDto::Comment, Permission::Delete => PermissionDto::Delete, Permission::Update => PermissionDto::Update, + Permission::Manage => PermissionDto::Manage, } } } // ════════════════════════════════════════════════════════════════════════════ -// Roles (DTO-layer sugar) +// Roles — the load-bearing model for ReBAC grants // ════════════════════════════════════════════════════════════════════════════ +// +// One row per role assignment in `storage.role_grants.role` (a +// `storage.grant_role` ENUM). The engine expands the bundle at query time +// via `Role::expand()` on the domain enum. Adding a role is two edits: +// the variant + match arm on `Role`, and an `ALTER TYPE +// storage.grant_role ADD VALUE 'name'` migration. -#[derive(Debug, Clone, Copy, Serialize, Deserialize, ToSchema)] +/// Wire-format wrapper around the domain `Role` enum. Carries the +/// serde/utoipa derives. Maps 1:1 to/from `Role` via `From`. +/// +/// The historical `"admin"` alias for `Owner` (used during the D-Prep +/// dual-write window for cached clients) has been retired in the cleanup +/// PR — the OxiCloud UI emits `"owner"` exclusively. Stragglers receive +/// a 422 on POST/PUT, which surfaces the upgrade cleanly. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize, ToSchema)] #[serde(rename_all = "lowercase")] -pub enum Role { +pub enum RoleDto { Viewer, - //Commenter, + Commenter, + Contributor, Editor, - //Manager, - Admin, + Owner, } -impl Role { - /// Expands a role into its constituent raw permissions. Storage and - /// engine know nothing about roles — the server normalizes here before - /// writing rows. - pub fn expand(self) -> &'static [Permission] { - match self { - Role::Viewer => &[Permission::Read], - /* reserved for future - Role::Commenter => &[Permission::Read, Permission::Comment], - */ - Role::Editor => &[ - Permission::Read, - Permission::Comment, - Permission::Create, - Permission::Update, - ], - /* reserved for future - Role::Manager => &[ - Permission::Read, - Permission::Comment, - Permission::Create, - Permission::Update, - Permission::Share, - ], - */ - Role::Admin => &[ - Permission::Read, - Permission::Comment, - Permission::Create, - Permission::Update, - Permission::Share, - Permission::Delete, - ], +impl From for Role { + fn from(r: RoleDto) -> Self { + match r { + RoleDto::Viewer => Role::Viewer, + RoleDto::Commenter => Role::Commenter, + RoleDto::Contributor => Role::Contributor, + RoleDto::Editor => Role::Editor, + RoleDto::Owner => Role::Owner, + } + } +} + +impl From for RoleDto { + fn from(r: Role) -> Self { + match r { + Role::Viewer => RoleDto::Viewer, + Role::Commenter => RoleDto::Commenter, + Role::Contributor => RoleDto::Contributor, + Role::Editor => RoleDto::Editor, + Role::Owner => RoleDto::Owner, } } } @@ -206,17 +209,18 @@ pub enum SubjectInputDto { }, } -/// `POST /api/grants` — accepts either `permissions` (explicit) or `role`. -/// Server-side validation requires exactly one of the two to be present. +/// `POST /api/grants` — create or refresh a role assignment. +/// +/// Strictly role-keyed since the cleanup PR: callers send exactly one +/// role; the engine writes a single row in `storage.role_grants`. The +/// historical per-permission shape (`permissions: [...]`) was dropped — +/// the OxiCloud UI is the only known caller and it already sends `role`. #[derive(Debug, Deserialize, ToSchema)] pub struct CreateGrantDto { pub subject: SubjectInputDto, pub resource: ResourceDto, - #[serde(default)] - pub permissions: Option>, - #[serde(default)] - pub role: Option, - /// Optional expiry for every grant in this request. RFC 3339 / ISO 8601. + pub role: RoleDto, + /// Optional expiry for the grant. RFC 3339 / ISO 8601. #[serde(default)] pub expires_at: Option>, } @@ -226,7 +230,7 @@ pub struct CreateGrantDto { pub struct UpdateRoleDto { pub subject: SubjectDto, pub resource: ResourceDto, - pub role: Role, + pub role: RoleDto, /// Optional expiry applied to every grant written or updated by this call. #[serde(default)] pub expires_at: Option>, @@ -241,7 +245,10 @@ pub struct GrantDto { pub id: Uuid, pub subject: SubjectDto, pub resource: ResourceDto, - pub permission: PermissionDto, + /// Role-keyed since D-Prep cleanup — one row in `storage.role_grants` + /// is one `GrantDto`. The bundle of underlying permissions is implied + /// by the role and recomputed client-side from the same lookup table. + pub role: RoleDto, pub granted_by: Uuid, pub granted_at: chrono::DateTime, #[serde(skip_serializing_if = "Option::is_none")] @@ -254,7 +261,7 @@ impl From for GrantDto { id: g.id, subject: g.subject.into(), resource: g.resource.into(), - permission: g.permission.into(), + role: g.role.into(), granted_by: g.granted_by, granted_at: g.granted_at, expires_at: g.expires_at, @@ -430,12 +437,34 @@ pub struct SharedWithMeItemDto { } /// Derive the closest-matching role label from a set of permissions. -/// Maps the permission set to `"admin"`, `"editor"`, or `"viewer"`. +/// +/// **Legacy helper for the dual-write window.** Once D-Prep ships and the +/// engine reads `role_grants.role` directly, this function becomes unused +/// and is dropped in the cleanup PR. Kept here so callers that still hit +/// `access_grants` and reconstruct a role for display can stay working +/// during the transition. +/// +/// Emits the new five-role roster on output (`"viewer"` / `"commenter"` / +/// `"contributor"` / `"editor"` / `"owner"`). Note this is **lossy** for +/// permission sets that don't match a bundle exactly — but D-Prep's +/// pre-flight refuses to migrate any such cluster, so post-migration data +/// only contains bundle-shaped sets. pub fn role_from_permissions(perms: &[Permission]) -> &'static str { - if perms.contains(&Permission::Delete) && perms.contains(&Permission::Share) { - "admin" - } else if perms.contains(&Permission::Create) || perms.contains(&Permission::Update) { + let has_read = perms.contains(&Permission::Read); + let has_comment = perms.contains(&Permission::Comment); + let has_create = perms.contains(&Permission::Create); + let has_update = perms.contains(&Permission::Update); + let has_delete = perms.contains(&Permission::Delete); + let has_share = perms.contains(&Permission::Share); + + if has_delete && has_share { + "owner" + } else if has_create && has_update { "editor" + } else if has_read && has_create && !has_update { + "contributor" + } else if has_read && has_comment && !has_create && !has_update { + "commenter" } else { "viewer" } @@ -457,7 +486,12 @@ pub struct OutgoingResourceGrantDto { pub subject_id: Uuid, /// Human-readable label (username for users, share name for tokens). pub subject_display: String, - /// Derived role label: `"viewer"` | `"editor"` | `"admin"`. + /// Role label: `"viewer"` | `"commenter"` | `"contributor"` | `"editor"` + /// | `"owner"`. Emitted by `role_from_permissions()` during the dual-write + /// window; once D-Prep cleanup lands this is read directly from + /// `storage.role_grants.role`. The legacy `"admin"` spelling is no longer + /// emitted — clients that cached it must accept `"owner"` too (the API + /// `Role::parse` still accepts `"admin"` on input for one release). pub role: String, pub granted_at: chrono::DateTime, #[serde(skip_serializing_if = "Option::is_none")] diff --git a/src/application/ports/authorization_ports.rs b/src/application/ports/authorization_ports.rs index e310c3e1..494267af 100644 --- a/src/application/ports/authorization_ports.rs +++ b/src/application/ports/authorization_ports.rs @@ -13,7 +13,7 @@ use uuid::Uuid; use crate::common::errors::DomainError; use crate::domain::services::authorization::{ Grant, GrantCursor, IncomingGrantSummary, OutgoingResourceSummary, Permission, Resource, - ResourceKind, Subject, + ResourceKind, Role, Subject, }; pub trait AuthorizationEngine: Send + Sync + 'static { @@ -90,11 +90,7 @@ pub trait AuthorizationEngine: Send + Sync + 'static { /// Resources explicitly granted to `subject`. Direct grants only — no /// cascade expansion. Used by `GET /api/grants/incoming`. - async fn list_incoming_grants( - &self, - subject: Subject, - permission_filter: Option, - ) -> Result, DomainError>; + async fn list_incoming_grants(&self, subject: Subject) -> Result, DomainError>; /// Cursor-paginated list of resources explicitly granted to `subject`, /// optionally filtered by resource kind. Multiple permission rows for the @@ -139,38 +135,20 @@ pub trait AuthorizationEngine: Send + Sync + 'static { reverse: bool, ) -> Result<(Vec, Option), DomainError>; - /// Create a grant. Idempotent — duplicates are absorbed by the UNIQUE - /// constraint; if the row already exists its `expires_at` is updated. - async fn grant( - &self, - granted_by: Uuid, - subject: Subject, - permission: Permission, - resource: Resource, - expires_at: Option>, - ) -> Result; - - /// Update `expires_at` on every grant row for the given subject. - /// Used when a share's expiry is changed — one call updates all - /// permission rows for that token in a single UPDATE. + /// Update `expires_at` for every role grant belonging to `subject`. + /// Used by `share_service` when a token-share's expiry is refreshed — + /// the subject (token) maps to a small fixed set of role grants, so a + /// single UPDATE covers them. Resource-scoped expiry changes go through + /// `set_role` (which carries `expires_at` as part of its UPSERT). async fn set_expiry_for_subject( &self, subject: Subject, expires_at: Option>, ) -> Result<(), DomainError>; - /// Update `expires_at` on every grant row for the given `(subject, resource)` - /// pair. Used by `set_role` to sync the expiry of retained grants when the - /// caller changes expiry without changing permissions. - async fn set_expiry_on_resource( - &self, - subject: Subject, - resource: Resource, - expires_at: Option>, - ) -> Result<(), DomainError>; - - /// Revoke a specific grant by its UUID. Returns `Ok(())` whether or not - /// the row existed (idempotent revoke). + /// Revoke a single role grant by its UUID. Idempotent — returns `Ok(())` + /// whether or not the row existed. The id comes from a prior listing + /// or `find_grant_full_by_id` lookup. async fn revoke(&self, grant_id: Uuid) -> Result<(), DomainError>; /// Removes every grant whose `resource` matches. Called by lifecycle @@ -181,4 +159,30 @@ pub trait AuthorizationEngine: Send + Sync + 'static { /// Removes every grant whose `subject` matches. Called when a user/token /// /group is deleted. Returns the count of rows removed. async fn revoke_all_for_subject(&self, subject: Subject) -> Result; + + // ── Role-keyed grant operations ──────────────────────────────────────── + // These are the only grant write path. Lifecycle hook bulk-deletes + // (`revoke_all_for_*` above) wipe matching rows directly, so callers + // using those paths don't need to invoke `clear_role` separately. + + /// Set the role for a `(subject, resource)` pair. Idempotent via the + /// UNIQUE `(subject_type, subject_id, resource_type, resource_id)` + /// constraint — `ON CONFLICT` updates the role + expires_at if they + /// changed, which is exactly the right semantics for an atomic role + /// change (e.g. promoting Viewer → Editor in one UPDATE with no race + /// window, no DELETE+INSERT). + async fn set_role( + &self, + granted_by: Uuid, + subject: Subject, + role: Role, + resource: Resource, + expires_at: Option>, + ) -> Result; + + /// Remove the role for a `(subject, resource)` pair. Idempotent — + /// succeeds whether or not the row existed. Called after `revoke` + /// succeeds to keep the two tables in sync during dual-write; after + /// cleanup this is the canonical role-revocation entry point. + async fn clear_role(&self, subject: Subject, resource: Resource) -> Result<(), DomainError>; } diff --git a/src/application/services/auth_application_service.rs b/src/application/services/auth_application_service.rs index 36584601..a77e7125 100644 --- a/src/application/services/auth_application_service.rs +++ b/src/application/services/auth_application_service.rs @@ -1388,7 +1388,7 @@ impl AuthApplicationService { /// Visibility rule, evaluated top-to-bottom: /// 1. **Self lookup** — `caller_id == target_id` always succeeds. /// 2. **Shared-grant relationship** — caller and target appear - /// together on at least one row of `storage.access_grants`, + /// together on at least one row of `storage.role_grants`, /// either direction (caller-as-granter / target-as-subject, /// or target-as-granter / caller-as-subject). Applies to both /// internal and external callers. This is what lets an @@ -1454,7 +1454,7 @@ impl AuthApplicationService { let related: Option = sqlx::query_scalar( r#" SELECT 1 - FROM storage.access_grants + FROM storage.role_grants WHERE (granted_by = $1 AND subject_type = 'user' AND subject_id = $2) OR (granted_by = $2 AND subject_type = 'user' AND subject_id = $1) LIMIT 1 diff --git a/src/application/services/share_service.rs b/src/application/services/share_service.rs index 3250c7fe..da411815 100644 --- a/src/application/services/share_service.rs +++ b/src/application/services/share_service.rs @@ -5,7 +5,7 @@ use tokio::sync::Semaphore; use uuid::Uuid; use crate::domain::repositories::folder_repository::FolderRepository; -use crate::domain::services::authorization::{Permission, Resource, Subject}; +use crate::domain::services::authorization::{Resource, Role, Subject}; use crate::infrastructure::repositories::pg::SharePgRepository; use crate::infrastructure::repositories::pg::file_blob_read_repository::FileBlobReadRepository; use crate::infrastructure::repositories::pg::folder_db_repository::FolderDbRepository; @@ -254,9 +254,9 @@ impl ShareUseCase for ShareService { .await .map_err(|e| ShareServiceError::Repository(e.to_string()))?; - // Create one Read-only grant for the token subject, carrying expires_at. - // Tokens are always read-only. The DELETE trigger `trg_cleanup_grants_token` - // cleans up this grant when the share is later deleted. + // Anonymous link tokens always get the Viewer role (read-only). + // The `trg_cleanup_grants_token` trigger cleans up this grant when + // the share row is later deleted. let item_id_uuid = Uuid::parse_str(saved_share.item_id()) .map_err(|_| ShareServiceError::Validation("Invalid item UUID".to_string()))?; let resource = match saved_share.item_type() { @@ -267,10 +267,10 @@ impl ShareUseCase for ShareService { .expires_at .and_then(|ts| chrono::DateTime::from_timestamp(ts as i64, 0)); self.authorization - .grant( + .set_role( user_id, Subject::Token(saved_share.id()), - Permission::Read, + Role::Viewer, resource, expires_dt, ) diff --git a/src/application/services/subject_group_service.rs b/src/application/services/subject_group_service.rs index 89576334..41e0d7c3 100644 --- a/src/application/services/subject_group_service.rs +++ b/src/application/services/subject_group_service.rs @@ -5,7 +5,7 @@ //! - Name validation runs (defence-in-depth alongside the DB CHECK). //! - Virtual groups (e.g. `Internal`) are protected from mutation. //! - Audit events are emitted via `tracing::info!(target = "audit", ...)`. -//! - Cascading delete of `storage.access_grants` rows referencing this +//! - Cascading delete of `storage.role_grants` rows referencing this //! group runs in the same transaction as the group delete. //! //! See `migrations/20260612000000_subject_groups.sql` for the schema. @@ -172,9 +172,9 @@ impl SubjectGroupService { /// Delete the group; cascades to: /// - `auth.subject_group_members` rows (FK CASCADE). - /// - `storage.access_grants` rows where `subject_type='group'` and + /// - `storage.role_grants` rows where `subject_type='group'` and /// `subject_id = id` (handled here, no FK exists between - /// `access_grants` and `subject_groups`). + /// `role_grants` and `subject_groups`). pub async fn delete(&self, id: Uuid, caller_id: Uuid) -> Result<(), DomainError> { let existing = self.get_by_id(id).await?; if existing.is_virtual { @@ -196,7 +196,7 @@ impl SubjectGroupService { })?; let grants_deleted = sqlx::query( - "DELETE FROM storage.access_grants + "DELETE FROM storage.role_grants WHERE subject_type = 'group' AND subject_id = $1", ) .bind(id) @@ -519,9 +519,9 @@ mod integration_tests { // ── 13. Grants are revoked atomically when a group is deleted ────────── // - // The plan said "FK CASCADE", but there's no FK between `access_grants` - // and `subject_groups` (different schemas; the cascade is handled by the - // service's transactional DELETE). This test pins that behaviour. + // There is no FK between `storage.role_grants` and `auth.subject_groups` + // (different schemas); the cascade is handled by the service's + // transactional DELETE. This test pins that behaviour. #[tokio::test] async fn test_grants_revoked_when_group_deleted() { let svc = make_service().await; @@ -534,21 +534,21 @@ mod integration_tests { .unwrap(); let resource_id = Uuid::new_v4(); sqlx::query( - "INSERT INTO storage.access_grants \ + "INSERT INTO storage.role_grants \ (subject_type, subject_id, resource_type, resource_id, \ - permission, granted_by) \ - VALUES ('group', $1, 'folder', $2, 'read', $3)", + role, granted_by) \ + VALUES ('group', $1, 'folder', $2, 'viewer', $3)", ) .bind(group.id) .bind(resource_id) .bind(admin) .execute(svc.pool.as_ref()) .await - .expect("insert grant row"); + .expect("insert role_grants row"); // Sanity: the grant exists. let pre: i64 = sqlx::query_scalar( - "SELECT COUNT(*) FROM storage.access_grants \ + "SELECT COUNT(*) FROM storage.role_grants \ WHERE subject_type = 'group' AND subject_id = $1", ) .bind(group.id) @@ -561,7 +561,7 @@ mod integration_tests { svc.delete(group.id, admin).await.unwrap(); let post: i64 = sqlx::query_scalar( - "SELECT COUNT(*) FROM storage.access_grants \ + "SELECT COUNT(*) FROM storage.role_grants \ WHERE subject_type = 'group' AND subject_id = $1", ) .bind(group.id) diff --git a/src/domain/entities/share.rs b/src/domain/entities/share.rs index 24a09f5c..93d965e4 100644 --- a/src/domain/entities/share.rs +++ b/src/domain/entities/share.rs @@ -12,7 +12,7 @@ pub struct Share { item_type: ShareItemType, token: String, password_hash: Option, - /// Derived from `storage.access_grants.expires_at` — not stored on the share row. + /// Derived from `storage.role_grants.expires_at` — not stored on the share row. expires_at: Option, created_at: u64, created_by: Uuid, diff --git a/src/domain/entities/subject_group.rs b/src/domain/entities/subject_group.rs index 2ada7cf7..8845eb5b 100644 --- a/src/domain/entities/subject_group.rs +++ b/src/domain/entities/subject_group.rs @@ -2,7 +2,7 @@ //! //! Subject groups are root-owned (no `owner_id`), globally named with an //! RFC 5321 local-part shape, and able to contain users *or* other groups. -//! Grants in `storage.access_grants` with `subject_type = 'group'` reference +//! Grants in `storage.role_grants` with `subject_type = 'group'` reference //! a row in `auth.subject_groups`. //! //! Cycle prevention and depth-cap (`MAX_GROUP_DEPTH`) are enforced at the diff --git a/src/domain/repositories/subject_group_repository.rs b/src/domain/repositories/subject_group_repository.rs index 96212b07..cc799954 100644 --- a/src/domain/repositories/subject_group_repository.rs +++ b/src/domain/repositories/subject_group_repository.rs @@ -93,8 +93,8 @@ pub trait SubjectGroupRepository: Send + Sync + 'static { ) -> Result; /// Delete the group. Cascades to `subject_group_members` and to - /// `storage.access_grants` rows referencing this group as subject (via - /// the application service — there is no FK between `access_grants` and + /// `storage.role_grants` rows referencing this group as subject (via + /// the application service — there is no FK between `role_grants` and /// `subject_groups`, so the service performs the cascade explicitly in /// the same transaction). async fn delete(&self, id: Uuid) -> Result<(), SubjectGroupRepositoryError>; diff --git a/src/domain/services/authorization.rs b/src/domain/services/authorization.rs index 2e095e59..06cf0607 100644 --- a/src/domain/services/authorization.rs +++ b/src/domain/services/authorization.rs @@ -3,7 +3,7 @@ //! These types are storage-agnostic — they describe the relationship between //! a subject (who), a resource (what), and a permission (action). The //! `AuthorizationEngine` port consumes them and the `PgAclEngine` implementation -//! maps them to / from `storage.access_grants` rows. +//! maps them to / from `storage.role_grants` rows. use crate::application::dtos::cursor::PageCursor; use std::fmt; @@ -49,7 +49,7 @@ impl Subject { /// `"external"` is no longer accepted: PR-2 of the external-users /// work folded the federated-identity case into `Subject::User(uuid)` /// with `auth.users.is_external = TRUE`. The DB CHECK constraint - /// on `storage.access_grants.subject_type` was narrowed to match. + /// on `storage.role_grants.subject_type` was narrowed to match. pub fn from_parts(subject_type: &str, id: Uuid) -> Option { match subject_type { "user" => Some(Subject::User(id)), @@ -140,18 +140,29 @@ pub enum Permission { Delete, /// Modify the resource (rename, move, edit content). Update, + /// Configure the resource's settings, add/remove members, change role + /// assignments. Used by: + /// - Drive owners managing drive membership and policies. + /// - Group owners managing the group itself (Group-as-Resource, future). + /// + /// Folder and file resources do not currently surface a `Manage` check; + /// the permission lives in the enum because the role bundle (`Owner`) + /// includes it, and the resource types that DO check it (`Drive`, + /// `Group`) are added in subsequent PRs (see `docs/plan/drive.md`). + Manage, } impl Permission { /// Every permission, in a stable order. Used by `Role::expand()` and SQL /// `permission = ANY(...)` lookups. - pub const ALL: [Permission; 6] = [ + pub const ALL: [Permission; 7] = [ Permission::Read, Permission::Create, Permission::Share, Permission::Comment, Permission::Delete, Permission::Update, + Permission::Manage, ]; pub fn as_str(&self) -> &'static str { @@ -162,6 +173,7 @@ impl Permission { Permission::Comment => "comment", Permission::Delete => "delete", Permission::Update => "update", + Permission::Manage => "manage", } } @@ -175,6 +187,7 @@ impl Permission { "comment" => Some(Permission::Comment), "delete" => Some(Permission::Delete), "update" => Some(Permission::Update), + "manage" => Some(Permission::Manage), _ => None, } } @@ -187,7 +200,7 @@ impl fmt::Display for Permission { } // ════════════════════════════════════════════════════════════════════════════ -// Grant — a row in storage.access_grants +// Grant — a row in storage.role_grants // ════════════════════════════════════════════════════════════════════════════ #[derive(Clone, Debug)] @@ -195,12 +208,132 @@ pub struct Grant { pub id: Uuid, pub subject: Subject, pub resource: Resource, - pub permission: Permission, + /// Role-keyed since D-Prep cleanup: one `Grant` represents the role + /// row in `storage.role_grants` rather than a single permission. The + /// engine and HTTP surface no longer carry per-permission rows; + /// callers that need permissions use `role.expand()`. + pub role: Role, pub granted_by: Uuid, pub granted_at: chrono::DateTime, pub expires_at: Option>, } +// ════════════════════════════════════════════════════════════════════════════ +// Role — a named bundle of permissions +// ════════════════════════════════════════════════════════════════════════════ +// +// Roles are the load-bearing model for ReBAC grants since D-Prep. Each +// `storage.role_grants` row stores one role; the engine expands the bundle +// at read time via `Role::expand()`. Adding a role is two edits: +// 1. a variant here + match arm in `expand()` / `as_str()` / `parse()` +// 2. an `ALTER TYPE storage.grant_role ADD VALUE 'name'` migration +// +// `RoleDto` (DTO layer) carries the wire-format derives + the legacy +// `"admin"` alias for backwards compat. + +#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)] +pub enum Role { + Viewer, + Commenter, + Contributor, + Editor, + Owner, +} + +impl Role { + /// Expand the role into its permission bundle. Single source of truth — + /// any code that needs "does this role include Permission X?" routes + /// through here (or its inverse, `roles_implying`). + pub fn expand(self) -> &'static [Permission] { + match self { + Role::Viewer => &[Permission::Read], + Role::Commenter => &[Permission::Read, Permission::Comment], + Role::Contributor => &[Permission::Read, Permission::Create], + Role::Editor => &[ + Permission::Read, + Permission::Comment, + Permission::Create, + Permission::Update, + ], + Role::Owner => &[ + Permission::Read, + Permission::Comment, + Permission::Create, + Permission::Update, + Permission::Share, + Permission::Delete, + Permission::Manage, + ], + } + } + + /// Lowercase discriminator — matches the SQL `role` ENUM values in + /// `storage.role_grants` (after the `::text` cast). + pub fn as_str(self) -> &'static str { + match self { + Role::Viewer => "viewer", + Role::Commenter => "commenter", + Role::Contributor => "contributor", + Role::Editor => "editor", + Role::Owner => "owner", + } + } + + /// Parse a role from its SQL discriminator. Returns `None` for unknown + /// values. The `"admin"` legacy alias is handled by `RoleDto` at the + /// wire boundary — the database only ever stores the canonical names. + pub fn parse(s: &str) -> Option { + match s { + "viewer" => Some(Role::Viewer), + "commenter" => Some(Role::Commenter), + "contributor" => Some(Role::Contributor), + "editor" => Some(Role::Editor), + "owner" => Some(Role::Owner), + _ => None, + } + } + + /// Every role, in declaration order. Mirrors the `storage.grant_role` + /// ENUM order in PG, which is weakest-to-strongest as written here for + /// historical reasons (`storage.grant_role` declares strongest first). + pub const ALL: [Role; 5] = [ + Role::Viewer, + Role::Commenter, + Role::Contributor, + Role::Editor, + Role::Owner, + ]; +} + +impl fmt::Display for Role { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.as_str()) + } +} + +/// Inverse of [`Role::expand`]: returns every role whose bundle contains +/// the given permission. Used by the engine to build the SQL +/// `WHERE role IN (...)` filter on hot-path queries like "what drives can +/// this caller read?". +pub fn roles_implying(permission: Permission) -> &'static [Role] { + use Permission::*; + match permission { + Read => &[ + Role::Viewer, + Role::Commenter, + Role::Contributor, + Role::Editor, + Role::Owner, + ], + Comment => &[Role::Commenter, Role::Editor, Role::Owner], + Create => &[Role::Contributor, Role::Editor, Role::Owner], + Update => &[Role::Editor, Role::Owner], + Delete => &[Role::Owner], + Share => &[Role::Owner], + Manage => &[Role::Owner], + } +} + impl Grant { pub fn is_expired(&self) -> bool { self.expires_at.is_some_and(|exp| exp < chrono::Utc::now()) @@ -212,7 +345,7 @@ impl Grant { // ════════════════════════════════════════════════════════════════════════════ /// Resource type without an id — used to filter paginated grant queries by -/// type. Mirrors the `resource_type` column values in `storage.access_grants`. +/// type. Mirrors the `resource_type` column values in `storage.role_grants`. /// Add new variants here when new resource types are supported. #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum ResourceKind { diff --git a/src/infrastructure/repositories/pg/share_pg_repository.rs b/src/infrastructure/repositories/pg/share_pg_repository.rs index d5c30183..8660324e 100644 --- a/src/infrastructure/repositories/pg/share_pg_repository.rs +++ b/src/infrastructure/repositories/pg/share_pg_repository.rs @@ -38,7 +38,7 @@ impl SharePgRepository { /// Maps a [`sqlx::postgres::PgRow`] to the domain [`Share`] entity. /// Expects columns: id, item_id, item_name, item_type, token, password_hash, - /// expires_at (derived from access_grants subquery), created_at, created_by, access_count. + /// expires_at (derived from role_grants subquery), created_at, created_by, access_count. fn row_to_entity(row: &sqlx::postgres::PgRow) -> Result { let id: Uuid = row .try_get("id") @@ -54,7 +54,7 @@ impl SharePgRepository { DomainError::internal_error("Share", format!("Failed to read token: {e}")) })?; let password_hash: Option = row.try_get("password_hash").unwrap_or(None); - // expires_at derived from access_grants subquery (unix seconds as i64) + // expires_at derived from role_grants subquery (unix seconds as i64) let expires_at: Option = row.try_get("expires_at").unwrap_or(None); let created_at: i64 = row.try_get("created_at").map_err(|e| { DomainError::internal_error("Share", format!("Failed to read created_at: {e}")) @@ -98,7 +98,7 @@ impl ShareStoragePort for SharePgRepository { RETURNING id, item_id, item_name, item_type, token, password_hash, (SELECT MIN(EXTRACT(EPOCH FROM ag.expires_at)::BIGINT) - FROM storage.access_grants ag + FROM storage.role_grants ag WHERE ag.subject_type = 'token' AND ag.subject_id = id) AS expires_at, created_at, created_by, access_count "#, @@ -127,7 +127,7 @@ impl ShareStoragePort for SharePgRepository { r#" SELECT s.id, s.item_id, s.item_name, s.item_type, s.token, s.password_hash, (SELECT MIN(EXTRACT(EPOCH FROM ag.expires_at)::BIGINT) - FROM storage.access_grants ag + FROM storage.role_grants ag WHERE ag.subject_type = 'token' AND ag.subject_id = s.id) AS expires_at, s.created_at, s.created_by, s.access_count FROM storage.shares s @@ -160,7 +160,7 @@ impl ShareStoragePort for SharePgRepository { r#" SELECT s.id, s.item_id, s.item_name, s.item_type, s.token, s.password_hash, (SELECT MIN(EXTRACT(EPOCH FROM ag.expires_at)::BIGINT) - FROM storage.access_grants ag + FROM storage.role_grants ag WHERE ag.subject_type = 'token' AND ag.subject_id = s.id) AS expires_at, s.created_at, s.created_by, s.access_count FROM storage.shares s @@ -218,7 +218,7 @@ impl ShareStoragePort for SharePgRepository { r#" SELECT s.id, s.item_id, s.item_name, s.item_type, s.token, s.password_hash, (SELECT MIN(EXTRACT(EPOCH FROM ag.expires_at)::BIGINT) - FROM storage.access_grants ag + FROM storage.role_grants ag WHERE ag.subject_type = 'token' AND ag.subject_id = s.id) AS expires_at, s.created_at, s.created_by, s.access_count FROM storage.shares s @@ -250,7 +250,7 @@ impl ShareStoragePort for SharePgRepository { RETURNING id, item_id, item_name, item_type, token, password_hash, (SELECT MIN(EXTRACT(EPOCH FROM ag.expires_at)::BIGINT) - FROM storage.access_grants ag + FROM storage.role_grants ag WHERE ag.subject_type = 'token' AND ag.subject_id = storage.shares.id) AS expires_at, created_at, created_by, access_count "#, @@ -286,7 +286,7 @@ impl ShareStoragePort for SharePgRepository { r#" SELECT s.id, s.item_id, s.item_name, s.item_type, s.token, s.password_hash, (SELECT MIN(EXTRACT(EPOCH FROM ag.expires_at)::BIGINT) - FROM storage.access_grants ag + FROM storage.role_grants ag WHERE ag.subject_type = 'token' AND ag.subject_id = s.id) AS expires_at, s.created_at, s.created_by, s.access_count, COUNT(*) OVER() AS total_count diff --git a/src/infrastructure/repositories/pg/subject_group_pg_repository.rs b/src/infrastructure/repositories/pg/subject_group_pg_repository.rs index 7484a155..6f3d0f60 100644 --- a/src/infrastructure/repositories/pg/subject_group_pg_repository.rs +++ b/src/infrastructure/repositories/pg/subject_group_pg_repository.rs @@ -272,8 +272,8 @@ impl SubjectGroupRepository for SubjectGroupPgRepository { async fn delete(&self, id: Uuid) -> Result<(), SubjectGroupRepositoryError> { // The application service is responsible for clearing related - // `storage.access_grants` rows in the same transaction (there's no - // FK between access_grants and subject_groups). The subject_group_members + // `storage.role_grants` rows in the same transaction (there's no + // FK between role_grants and subject_groups). The subject_group_members // rows cascade automatically via FK. let result = sqlx::query("DELETE FROM auth.subject_groups WHERE id = $1") .bind(id) diff --git a/src/infrastructure/services/pg_acl_engine.rs b/src/infrastructure/services/pg_acl_engine.rs index b60ba832..652d54e3 100644 --- a/src/infrastructure/services/pg_acl_engine.rs +++ b/src/infrastructure/services/pg_acl_engine.rs @@ -1,13 +1,14 @@ //! PostgreSQL-backed implementation of `AuthorizationEngine`. //! -//! Stores grants in `storage.access_grants` (see migration -//! `20260520000000_rebac_access_grants.sql`). Cascading is resolved at check -//! time via PostgreSQL `ltree` `@>` (ancestor-of) on `storage.folders.lpath`, -//! using the existing GiST index for O(log N) traversal. +//! Stores grants in `storage.role_grants` (one role per (subject, resource) +//! pair; the role's permission bundle is expanded in code via +//! `Role::expand()`). Cascading is resolved at check time via PostgreSQL +//! `ltree` `@>` (ancestor-of) on `storage.folders.lpath`, using the +//! existing GiST index for O(log N) traversal. //! //! Owner is implicit — `storage.folders.user_id` / `storage.files.user_id` //! are checked first via dedicated helpers; if the caller is the owner, no -//! SQL against `access_grants` happens. +//! SQL against `role_grants` happens. //! //! ## Lifecycle cleanup //! @@ -43,7 +44,7 @@ use crate::domain::entities::subject_group::INTERNAL_GROUP_ID; use crate::domain::repositories::subject_group_repository::SubjectGroupRepository; use crate::domain::services::authorization::{ Grant, GrantCursor, IncomingGrantSummary, OutgoingGrantEntry, OutgoingResourceSummary, - Permission, Resource, ResourceKind, Subject, + Permission, Resource, ResourceKind, Role, Subject, roles_implying, }; use crate::infrastructure::repositories::pg::SubjectGroupPgRepository; use crate::infrastructure::repositories::pg::file_blob_read_repository::FileBlobReadRepository; @@ -205,7 +206,7 @@ impl PgAclEngine { } /// Expand a caller's `Subject` into the `(subject_types, subject_ids)` - /// pair that should be matched in `storage.access_grants`. For User + /// pair that should be matched in `storage.role_grants`. For User /// callers this is `(["user","group"], [uid, …transitive groups, INTERNAL])`; /// for any non-user subject (Token / External / Group as direct caller) /// it's a single-element pair with no cascade. @@ -237,6 +238,22 @@ impl PgAclEngine { } } + /// Convert a `Permission` into the array of role strings whose bundle + /// includes it — bound as `ANY($N::storage.grant_role[])` so the + /// ENUM-typed `role` column compares without an implicit text cast. + /// + /// This is the inverse of `Role::expand()`, precomputed via + /// `grant_dto::roles_implying()`. The mapping is small and static (≤5 + /// roles per permission today); resolving it in code keeps the SQL + /// path simple and lets us add new roles without touching every + /// query site. + fn roles_implying_strings(permission: Permission) -> Vec<&'static str> { + roles_implying(permission) + .iter() + .map(|r| r.as_str()) + .collect() + } + /// Cascading check for folders: is there a grant on any ancestor folder /// (including the target itself) for any of the given subject IDs and /// any of the given subject types? @@ -247,6 +264,11 @@ impl PgAclEngine { /// `subject_ids` is the expanded set returned by `expand_user` (or a /// single-element vec for non-user callers). /// + /// Reads `storage.role_grants` (1 row per role assignment); a permission + /// filter `g.permission = $3` becomes `g.role = ANY($3::storage.grant_role[])` where + /// the array is the set of roles whose bundle includes the requested + /// permission — see `roles_implying()`. + /// /// Uses the GiST index on `storage.folders.lpath` for O(log N) cascade. async fn folder_cascade_grant_exists( &self, @@ -257,14 +279,15 @@ impl PgAclEngine { counters: &QueryCounters, ) -> Result { counters.sql_queries.fetch_add(1, Ordering::Relaxed); + let roles = Self::roles_implying_strings(permission); let exists: Option = sqlx::query_scalar( r#" SELECT 1 - FROM storage.access_grants g + FROM storage.role_grants g JOIN storage.folders gf ON gf.id = g.resource_id WHERE g.subject_type = ANY($1) AND g.subject_id = ANY($2) - AND g.permission = $3 + AND g.role = ANY($3::storage.grant_role[]) AND g.resource_type = 'folder' AND (g.expires_at IS NULL OR g.expires_at > NOW()) AND gf.lpath @> (SELECT lpath FROM storage.folders WHERE id = $4) @@ -273,7 +296,7 @@ impl PgAclEngine { ) .bind(subject_types) .bind(subject_ids) - .bind(permission.as_str()) + .bind(&roles) .bind(folder_id) .fetch_optional(self.pool.as_ref()) .await @@ -285,7 +308,7 @@ impl PgAclEngine { /// Cascading check for files: either a direct file grant OR a grant on /// any ancestor folder of the file's containing folder. See /// `folder_cascade_grant_exists` for the meaning of `subject_types` / - /// `subject_ids`. + /// `subject_ids` and the D-Prep role-array migration. async fn file_cascade_grant_exists( &self, subject_types: &[&str], @@ -295,27 +318,28 @@ impl PgAclEngine { counters: &QueryCounters, ) -> Result { counters.sql_queries.fetch_add(1, Ordering::Relaxed); + let roles = Self::roles_implying_strings(permission); let exists: Option = sqlx::query_scalar( r#" SELECT 1 FROM ( -- direct file grant SELECT 1 - FROM storage.access_grants + FROM storage.role_grants WHERE subject_type = ANY($1) AND subject_id = ANY($2) - AND permission = $3 + AND role = ANY($3::storage.grant_role[]) AND resource_type = 'file' AND resource_id = $4 AND (expires_at IS NULL OR expires_at > NOW()) UNION ALL -- cascading from any ancestor folder of the file's containing folder SELECT 1 - FROM storage.access_grants g + FROM storage.role_grants g JOIN storage.folders gf ON gf.id = g.resource_id JOIN storage.files target_f ON target_f.id = $4 WHERE g.subject_type = ANY($1) AND g.subject_id = ANY($2) - AND g.permission = $3 + AND g.role = ANY($3::storage.grant_role[]) AND g.resource_type = 'folder' AND (g.expires_at IS NULL OR g.expires_at > NOW()) AND target_f.folder_id IS NOT NULL @@ -327,7 +351,7 @@ impl PgAclEngine { ) .bind(subject_types) .bind(subject_ids) - .bind(permission.as_str()) + .bind(&roles) .bind(file_id) .fetch_optional(self.pool.as_ref()) .await @@ -336,39 +360,16 @@ impl PgAclEngine { Ok(exists.is_some()) } - /// Look up a single grant by id. Returns `(resource, granted_by)` so - /// the REST `DELETE /api/grants/{id}` handler can decide authorization - /// without a second round-trip. Returns `Ok(None)` if no such grant. - pub async fn find_grant_by_id( - &self, - grant_id: Uuid, - ) -> Result, DomainError> { - let row: Option<(String, Uuid, Uuid)> = sqlx::query_as( - "SELECT resource_type, resource_id, granted_by FROM storage.access_grants WHERE id = $1", - ) - .bind(grant_id) - .fetch_optional(self.pool.as_ref()) - .await - .map_err(|e| DomainError::internal_error("PgAcl", format!("find_grant_by_id: {e}")))?; - - let Some((rt, rid, granter)) = row else { - return Ok(None); - }; - let res = Resource::from_parts(&rt, rid) - .ok_or_else(|| DomainError::internal_error("PgAcl", "unknown resource_type"))?; - Ok(Some((res, granter))) - } - - /// Variant of `find_grant_by_id` that also returns the subject — - /// needed by `POST /api/grants/{id}/notify` to resolve who to email. - /// Returns `(subject, resource, granted_by)` or `None`. + /// Look up a single role grant by id, returning the actors a revoke / + /// notify handler needs to make a decision without a second round-trip. + /// Returns `(subject, resource, granted_by)` or `None` if no such row. pub async fn find_grant_full_by_id( &self, grant_id: Uuid, ) -> Result, DomainError> { let row: Option<(String, Uuid, String, Uuid, Uuid)> = sqlx::query_as( "SELECT subject_type, subject_id, resource_type, resource_id, granted_by \ - FROM storage.access_grants WHERE id = $1", + FROM storage.role_grants WHERE id = $1", ) .bind(grant_id) .fetch_optional(self.pool.as_ref()) @@ -385,8 +386,14 @@ impl PgAclEngine { Ok(Some((subject, resource, granter))) } - /// Row type for all full-grant SELECT queries: - /// (id, subject_type, subject_id, resource_type, resource_id, permission, granted_by, granted_at, expires_at) + /// Row type for `storage.role_grants` SELECTs: + /// (id, subject_type, subject_id, resource_type, resource_id, role, granted_by, granted_at, expires_at). + /// + /// Builds a single role-keyed `Grant` per row. `Grant` is role-keyed + /// since the D-Prep cleanup PR — every listing method returns role + /// rows directly; bundle expansion to per-permission Grants no longer + /// happens here. Callers that need the permission set use + /// `grant.role.expand()` at the call site. #[allow(clippy::type_complexity)] fn row_to_grant( row: ( @@ -405,13 +412,13 @@ impl PgAclEngine { .ok_or_else(|| DomainError::internal_error("PgAcl", "unknown subject_type"))?; let resource = Resource::from_parts(&row.3, row.4) .ok_or_else(|| DomainError::internal_error("PgAcl", "unknown resource_type"))?; - let permission = Permission::parse(&row.5) - .ok_or_else(|| DomainError::internal_error("PgAcl", "unknown permission"))?; + let role = Role::parse(&row.5) + .ok_or_else(|| DomainError::internal_error("PgAcl", "unknown role"))?; Ok(Grant { id: row.0, subject, resource, - permission, + role, granted_by: row.6, granted_at: row.7, expires_at: row.8, @@ -529,15 +536,14 @@ impl AuthorizationEngine for PgAclEngine { result } - async fn list_incoming_grants( - &self, - subject: Subject, - permission_filter: Option, - ) -> Result, DomainError> { - let perm_str = permission_filter.map(|p| p.as_str().to_string()); + async fn list_incoming_grants(&self, subject: Subject) -> Result, DomainError> { let counters = QueryCounters::default(); let (subject_types, subject_ids) = self.subject_match_set(subject, &counters).await?; + // `ORDER BY role ASC` exploits the `storage.grant_role` ENUM + // declared as `(owner, editor, contributor, commenter, viewer)`, + // so the sort order matches the UX requirement ("Owner > Editor + // > Contributor > Commenter > Viewer") without a per-row CASE. let rows = sqlx::query_as::< _, ( @@ -554,18 +560,16 @@ impl AuthorizationEngine for PgAclEngine { >( r#" SELECT id, subject_type, subject_id, resource_type, resource_id, - permission, granted_by, granted_at, expires_at - FROM storage.access_grants + role::text, granted_by, granted_at, expires_at + FROM storage.role_grants WHERE subject_type = ANY($1) AND subject_id = ANY($2) - AND ($3::text IS NULL OR permission = $3) - ORDER BY granted_at DESC - LIMIT $4 + ORDER BY role ASC, granted_at DESC + LIMIT $3 "#, ) .bind(&subject_types) .bind(&subject_ids) - .bind(perm_str) .bind(MAX_GRANT_ROWS + 1) .fetch_all(self.pool.as_ref()) .await @@ -596,7 +600,12 @@ impl AuthorizationEngine for PgAclEngine { // NULL otherwise. This lets every sort mode share a single query_as call. // 0 resource_type String // 1 resource_id Uuid - // 2 permissions Vec + // 2 roles Vec — every distinct role granting access to this + // resource (post-D-Prep). Expanded to permissions + // in `IncomingGrantSummary` via `Role::expand()`. + // Multiple entries possible when a user has both + // a direct grant and a group-mediated grant on + // the same resource. // 3 granted_at DateTime // 4 granted_by Uuid // 5 sort_str Option — resource_name (name/type) or owner_name (granted_by) @@ -628,14 +637,20 @@ impl AuthorizationEngine for PgAclEngine { // is `(["user","group"], [uid, …transitive groups, INTERNAL])` so the // listing includes every resource the user can reach via a group // grant (matching what `check()` allows). See `subject_match_set`. + // + // Post-D-Prep this reads `storage.role_grants` and aggregates the + // ENUM-typed `role` column into a text array. Multiple roles can + // appear per resource when the caller reaches it via both a direct + // grant and a group-mediated grant — the union of role bundles + // produces the displayed permission set in Rust below. const AGG: &str = r#"agg AS ( SELECT resource_type, resource_id, - array_agg(DISTINCT permission ORDER BY permission) AS permissions, + array_agg(DISTINCT role::text ORDER BY role::text) AS roles, MIN(granted_at) AS granted_at, (array_agg(granted_by ORDER BY granted_at))[1] AS granted_by - FROM storage.access_grants + FROM storage.role_grants WHERE subject_type = ANY($1) AND subject_id = ANY($2) AND ($3::text[] IS NULL OR resource_type = ANY($3)) @@ -700,7 +715,7 @@ impl AuthorizationEngine for PgAclEngine { LEFT JOIN storage.folders f ON f.id = agg.resource_id AND agg.resource_type = 'folder' LEFT JOIN storage.files fi ON fi.id = agg.resource_id AND agg.resource_type = 'file' ) - SELECT resource_type, resource_id, permissions, granted_at, granted_by, sort_str, sort_int + SELECT resource_type, resource_id, roles, granted_at, granted_by, sort_str, sort_int FROM named WHERE {where_clause} ORDER BY {order_clause} @@ -740,7 +755,7 @@ impl AuthorizationEngine for PgAclEngine { FROM agg LEFT JOIN auth.users u ON u.id = agg.granted_by ) - SELECT resource_type, resource_id, permissions, granted_at, granted_by, sort_str, sort_int + SELECT resource_type, resource_id, roles, granted_at, granted_by, sort_str, sort_int FROM owner_named WHERE {where_clause} ORDER BY {order_clause} @@ -768,7 +783,7 @@ impl AuthorizationEngine for PgAclEngine { }; format!( r#"WITH {AGG} - SELECT resource_type, resource_id, permissions, granted_at, granted_by, + SELECT resource_type, resource_id, roles, granted_at, granted_by, NULL::text AS sort_str, NULL::bigint AS sort_int FROM agg @@ -850,14 +865,21 @@ impl AuthorizationEngine for PgAclEngine { }; // ── Convert rows to domain summaries ────────────────────────────────── + // Post-D-Prep: the SQL aggregate produces a `roles` text array. We + // expand each role's bundle and union them — direct grants and + // group-mediated grants on the same resource collapse to a single + // deduplicated permission set, matching the pre-pivot behaviour. let summaries = rows .into_iter() - .filter_map(|(rt, rid, perms_str, granted_at, granted_by, _, _)| { + .filter_map(|(rt, rid, roles_str, granted_at, granted_by, _, _)| { let resource_type = ResourceKind::parse(&rt)?; - let permissions = perms_str + let mut permissions: Vec = roles_str .into_iter() - .filter_map(|s| Permission::parse(&s)) + .filter_map(|s| Role::parse(&s)) + .flat_map(|r| r.expand().iter().copied()) .collect(); + permissions.sort_by_key(|p| p.as_str()); + permissions.dedup(); Some(IncomingGrantSummary { resource_type, resource_id: rid, @@ -872,6 +894,14 @@ impl AuthorizationEngine for PgAclEngine { } async fn list_grants_on_resource(&self, resource: Resource) -> Result, DomainError> { + // Pivoted to `storage.role_grants` (see `list_incoming_grants`). + // Each role row expands to N permission-keyed `Grant` rows via + // `role_row_to_grants` until the public `Grant` shape becomes + // role-keyed. + // + // `ORDER BY role ASC` exploits the `storage.grant_role` ENUM's + // declaration order (owner first → viewer last) so the share + // dialog's "who has access" list shows strongest grants on top. let rows = sqlx::query_as::< _, ( @@ -888,11 +918,11 @@ impl AuthorizationEngine for PgAclEngine { >( r#" SELECT id, subject_type, subject_id, resource_type, resource_id, - permission, granted_by, granted_at, expires_at - FROM storage.access_grants + role::text, granted_by, granted_at, expires_at + FROM storage.role_grants WHERE resource_type = $1 AND resource_id = $2 - ORDER BY granted_at DESC + ORDER BY role ASC, granted_at DESC LIMIT $3 "#, ) @@ -917,7 +947,10 @@ impl AuthorizationEngine for PgAclEngine { ) -> Result<(Vec, Option), DomainError> { let fetch_limit = (limit as i64) + 1; - // Row shape — one row per (resource, subject, permission). + // Row shape — post-D-Prep, one row per (resource, subject) since + // `storage.role_grants` carries exactly one role per pair (UNIQUE + // constraint). Permission bundles are expanded in the row consumer + // via `Role::expand()`. // Columns: // 0 resource_type String // 1 resource_id Uuid @@ -926,9 +959,9 @@ impl AuthorizationEngine for PgAclEngine { // 4 subject_id Uuid // 5 subject_display String — username or share item_name // 6 grant_id Uuid - // 7 granted_at DateTime — this (subject, perm) row + // 7 granted_at DateTime — this (subject, role) row // 8 expires_at Option> - // 9 permission String + // 9 role String — `grant_role` ENUM as text // 10 sort_str Option // 11 sort_int Option // 12 has_password bool — token: shares.password_hash IS NOT NULL @@ -1015,7 +1048,7 @@ impl AuthorizationEngine for PgAclEngine { CASE WHEN ag.resource_type = 'file' THEN fi.name END ) AS sort_str, {sort_int_expr} AS sort_int - FROM storage.access_grants ag + FROM storage.role_grants ag LEFT JOIN storage.folders f ON f.id = ag.resource_id AND ag.resource_type = 'folder' LEFT JOIN storage.files fi ON fi.id = ag.resource_id AND ag.resource_type = 'file' WHERE ag.granted_by = $1 @@ -1030,12 +1063,12 @@ impl AuthorizationEngine for PgAclEngine { SELECT ag.resource_type, ag.resource_id, rp.first_shared_at, ag.subject_type, ag.subject_id, COALESCE(u.username, u.email, sg.name::text, sh.item_name, fi.name, fld.name, ag.subject_id::text) AS subject_display, - ag.id AS grant_id, ag.granted_at, ag.expires_at, ag.permission, + ag.id AS grant_id, ag.granted_at, ag.expires_at, ag.role::text AS role, rp.sort_str, rp.sort_int, (sh.password_hash IS NOT NULL) AS has_password, COALESCE(u.is_external, FALSE) AS is_external FROM rp - JOIN storage.access_grants ag + JOIN storage.role_grants ag ON ag.resource_type = rp.resource_type AND ag.resource_id = rp.resource_id AND ag.granted_by = $1 LEFT JOIN auth.users u ON ag.subject_type = 'user' AND u.id = ag.subject_id @@ -1104,7 +1137,7 @@ impl AuthorizationEngine for PgAclEngine { ELSE 3 END)::bigint AS sort_int, MIN(ag.granted_at) AS first_granted_at - FROM storage.access_grants ag + FROM storage.role_grants ag LEFT JOIN auth.users u ON ag.subject_type = 'user' AND u.id = ag.subject_id LEFT JOIN auth.subject_groups sg @@ -1135,13 +1168,13 @@ impl AuthorizationEngine for PgAclEngine { ag.id AS grant_id, ag.granted_at, ag.expires_at, - ag.permission, + ag.role::text AS role, LOWER(rp.subject_display) AS sort_str, rp.sort_int, rp.has_password, rp.is_external FROM rp - JOIN storage.access_grants ag + JOIN storage.role_grants ag ON ag.resource_type = rp.resource_type AND ag.resource_id = rp.resource_id AND ag.subject_type = rp.subject_type @@ -1155,7 +1188,12 @@ impl AuthorizationEngine for PgAclEngine { // Page on (role_order, subject_display, resource_id) triples so that all // of one person's grants within a role are contiguous — enabling aggregation // ("Bob on Folder A, Folder B") to work correctly across cursor pages. - // role_order: 0 = admin (has delete+share), 1 = editor (has create or update), 2 = viewer + // + // role_order matches the `storage.grant_role` ENUM declaration + // order (strongest first) via `array_position`, so + // `sort_int ASC` matches the UX requirement: 1 = owner, + // 2 = editor, 3 = contributor, 4 = commenter, 5 = viewer. + // 1-based because `array_position` is. // Cursor: sort_int=role_order, resource_name=LOWER(subject_display), resource_id let (page_where, page_order) = if reverse { ( @@ -1184,15 +1222,20 @@ impl AuthorizationEngine for PgAclEngine { MAX(COALESCE(u.username, u.email, sh.item_name, ag.subject_id::text)) AS subject_display, BOOL_OR(sh.password_hash IS NOT NULL) AS has_password, COALESCE(BOOL_OR(u.is_external), FALSE) AS is_external, - CASE - WHEN BOOL_OR(ag.permission = 'delete') - AND BOOL_OR(ag.permission = 'share') THEN 0 - WHEN BOOL_OR(ag.permission = 'create') - OR BOOL_OR(ag.permission = 'update') THEN 1 - ELSE 2 - END::bigint AS sort_int, + -- One role per (resource, subject) post-D-Prep + -- (UNIQUE constraint on role_grants), so MAX + -- returns that single row's role. `array_position` + -- against the ENUM's declaration order produces a + -- 1-based rank: owner=1 → viewer=5. Strength + -- ordering tracks the ENUM declaration — adding + -- a new role between owner and viewer doesn't + -- need a parallel CASE update here. + array_position( + enum_range(NULL::storage.grant_role), + MAX(ag.role) + )::bigint AS sort_int, MIN(ag.granted_at) AS first_granted_at - FROM storage.access_grants ag + FROM storage.role_grants ag LEFT JOIN auth.users u ON ag.subject_type = 'user' AND u.id = ag.subject_id LEFT JOIN storage.shares sh @@ -1221,13 +1264,13 @@ impl AuthorizationEngine for PgAclEngine { ag.id AS grant_id, ag.granted_at, ag.expires_at, - ag.permission, + ag.role::text AS role, LOWER(rp.subject_display) AS sort_str, rp.sort_int, rp.has_password, rp.is_external FROM rp - JOIN storage.access_grants ag + JOIN storage.role_grants ag ON ag.resource_type = rp.resource_type AND ag.resource_id = rp.resource_id AND ag.subject_type = rp.subject_type @@ -1259,7 +1302,7 @@ impl AuthorizationEngine for PgAclEngine { SELECT resource_type, resource_id, MIN(granted_at) AS first_shared_at, NULL::text AS sort_str, NULL::bigint AS sort_int - FROM storage.access_grants + FROM storage.role_grants WHERE granted_by = $1 GROUP BY resource_type, resource_id ), @@ -1272,12 +1315,12 @@ impl AuthorizationEngine for PgAclEngine { SELECT ag.resource_type, ag.resource_id, rp.first_shared_at, ag.subject_type, ag.subject_id, COALESCE(u.username, u.email, sh.item_name, fi.name, fld.name, ag.subject_id::text) AS subject_display, - ag.id AS grant_id, ag.granted_at, ag.expires_at, ag.permission, + ag.id AS grant_id, ag.granted_at, ag.expires_at, ag.role::text AS role, NULL::text AS sort_str, NULL::bigint AS sort_int, (sh.password_hash IS NOT NULL) AS has_password, COALESCE(u.is_external, FALSE) AS is_external FROM rp - JOIN storage.access_grants ag + JOIN storage.role_grants ag ON ag.resource_type = rp.resource_type AND ag.resource_id = rp.resource_id AND ag.granted_by = $1 LEFT JOIN auth.users u ON ag.subject_type = 'user' AND u.id = ag.subject_id @@ -1355,7 +1398,7 @@ impl AuthorizationEngine for PgAclEngine { grant_id, granted_at, expires_at, - perm_str, + role_str, _, _, has_password, @@ -1364,7 +1407,7 @@ impl AuthorizationEngine for PgAclEngine { let Some(resource_type) = ResourceKind::parse(&rt_str) else { continue; }; - let Some(perm) = Permission::parse(&perm_str) else { + let Some(role) = Role::parse(&role_str) else { continue; }; let key = (resource_id, subj_id); @@ -1384,8 +1427,10 @@ impl AuthorizationEngine for PgAclEngine { }, ) }); - if !entry.permissions.contains(&perm) { - entry.permissions.push(perm); + for &perm in role.expand() { + if !entry.permissions.contains(&perm) { + entry.permissions.push(perm); + } } } @@ -1473,7 +1518,7 @@ impl AuthorizationEngine for PgAclEngine { grant_id, granted_at, expires_at, - perm_str, + role_str, _, _, has_password, @@ -1482,7 +1527,7 @@ impl AuthorizationEngine for PgAclEngine { let Some(resource_type) = ResourceKind::parse(&rt_str) else { continue; }; - let Some(perm) = Permission::parse(&perm_str) else { + let Some(role) = Role::parse(&role_str) else { continue; }; @@ -1506,8 +1551,10 @@ impl AuthorizationEngine for PgAclEngine { has_password, is_external, }); - if !entry.permissions.contains(&perm) { - entry.permissions.push(perm); + for &perm in role.expand() { + if !entry.permissions.contains(&perm) { + entry.permissions.push(perm); + } } } @@ -1553,6 +1600,11 @@ impl AuthorizationEngine for PgAclEngine { } async fn list_outgoing_grants(&self, granted_by: Uuid) -> Result, DomainError> { + // Pivoted to `storage.role_grants` (see `list_incoming_grants`). + // Group membership doesn't apply on the outgoing side — we + // filter by `granted_by` directly. Bundle expansion still + // happens at read time via `role_row_to_grants` until the + // public `Grant` shape becomes role-keyed. let rows = sqlx::query_as::< _, ( @@ -1569,10 +1621,10 @@ impl AuthorizationEngine for PgAclEngine { >( r#" SELECT id, subject_type, subject_id, resource_type, resource_id, - permission, granted_by, granted_at, expires_at - FROM storage.access_grants + role::text, granted_by, granted_at, expires_at + FROM storage.role_grants WHERE granted_by = $1 - ORDER BY granted_at DESC + ORDER BY role ASC, granted_at DESC "#, ) .bind(granted_by) @@ -1583,11 +1635,68 @@ impl AuthorizationEngine for PgAclEngine { rows.into_iter().map(Self::row_to_grant).collect() } - async fn grant( + async fn set_expiry_for_subject( + &self, + subject: Subject, + expires_at: Option>, + ) -> Result<(), DomainError> { + sqlx::query( + "UPDATE storage.role_grants SET expires_at = $3 \ + WHERE subject_type = $1 AND subject_id = $2", + ) + .bind(subject.type_str()) + .bind(subject.id()) + .bind(expires_at) + .execute(self.pool.as_ref()) + .await + .map_err(|e| { + DomainError::internal_error("PgAcl", format!("set_expiry_for_subject: {e}")) + })?; + Ok(()) + } + + async fn revoke(&self, grant_id: Uuid) -> Result<(), DomainError> { + sqlx::query("DELETE FROM storage.role_grants WHERE id = $1") + .bind(grant_id) + .execute(self.pool.as_ref()) + .await + .map_err(|e| DomainError::internal_error("PgAcl", format!("revoke: {e}")))?; + Ok(()) + } + + async fn revoke_all_for_resource(&self, resource: Resource) -> Result { + let result = sqlx::query( + "DELETE FROM storage.role_grants WHERE resource_type = $1 AND resource_id = $2", + ) + .bind(resource.type_str()) + .bind(resource.id()) + .execute(self.pool.as_ref()) + .await + .map_err(|e| DomainError::internal_error("PgAcl", format!("revoke for resource: {e}")))?; + + Ok(result.rows_affected() as usize) + } + + async fn revoke_all_for_subject(&self, subject: Subject) -> Result { + let result = sqlx::query( + "DELETE FROM storage.role_grants WHERE subject_type = $1 AND subject_id = $2", + ) + .bind(subject.type_str()) + .bind(subject.id()) + .execute(self.pool.as_ref()) + .await + .map_err(|e| DomainError::internal_error("PgAcl", format!("revoke for subject: {e}")))?; + + Ok(result.rows_affected() as usize) + } + + // ── D-Prep role_grants writes ────────────────────────────────────────── + + async fn set_role( &self, granted_by: Uuid, subject: Subject, - permission: Permission, + role: Role, resource: Resource, expires_at: Option>, ) -> Result { @@ -1606,104 +1715,48 @@ impl AuthorizationEngine for PgAclEngine { ), >( r#" - INSERT INTO storage.access_grants - (subject_type, subject_id, resource_type, resource_id, permission, granted_by, expires_at) - VALUES ($1, $2, $3, $4, $5, $6, $7) - ON CONFLICT (subject_type, subject_id, resource_type, resource_id, permission) - DO UPDATE SET expires_at = EXCLUDED.expires_at + INSERT INTO storage.role_grants + (subject_type, subject_id, resource_type, resource_id, + role, granted_by, expires_at) + VALUES ($1, $2, $3, $4, $5::storage.grant_role, $6, $7) + ON CONFLICT (subject_type, subject_id, resource_type, resource_id) + DO UPDATE SET role = EXCLUDED.role, + expires_at = EXCLUDED.expires_at, + granted_by = EXCLUDED.granted_by RETURNING id, subject_type, subject_id, resource_type, resource_id, - permission, granted_by, granted_at, expires_at + role::text, granted_by, granted_at, expires_at "#, ) .bind(subject.type_str()) .bind(subject.id()) .bind(resource.type_str()) .bind(resource.id()) - .bind(permission.as_str()) + .bind(role.as_str()) .bind(granted_by) .bind(expires_at) .fetch_one(self.pool.as_ref()) .await - .map_err(|e| DomainError::internal_error("PgAcl", format!("insert grant: {e}")))?; + .map_err(|e| DomainError::internal_error("PgAcl", format!("set_role: {e}")))?; Self::row_to_grant(row) } - async fn set_expiry_for_subject( - &self, - subject: Subject, - expires_at: Option>, - ) -> Result<(), DomainError> { + async fn clear_role(&self, subject: Subject, resource: Resource) -> Result<(), DomainError> { sqlx::query( - "UPDATE storage.access_grants SET expires_at = $3 WHERE subject_type = $1 AND subject_id = $2", - ) - .bind(subject.type_str()) - .bind(subject.id()) - .bind(expires_at) - .execute(self.pool.as_ref()) - .await - .map_err(|e| DomainError::internal_error("PgAcl", format!("set_expiry_for_subject: {e}")))?; - Ok(()) - } - - async fn set_expiry_on_resource( - &self, - subject: Subject, - resource: Resource, - expires_at: Option>, - ) -> Result<(), DomainError> { - sqlx::query( - "UPDATE storage.access_grants SET expires_at = $3 \ + "DELETE FROM storage.role_grants \ WHERE subject_type = $1 AND subject_id = $2 \ - AND resource_type = $4 AND resource_id = $5", + AND resource_type = $3 AND resource_id = $4", ) .bind(subject.type_str()) .bind(subject.id()) - .bind(expires_at) .bind(resource.type_str()) .bind(resource.id()) .execute(self.pool.as_ref()) .await - .map_err(|e| { - DomainError::internal_error("PgAcl", format!("set_expiry_on_resource: {e}")) - })?; + .map_err(|e| DomainError::internal_error("PgAcl", format!("clear_role: {e}")))?; + Ok(()) } - - async fn revoke(&self, grant_id: Uuid) -> Result<(), DomainError> { - sqlx::query("DELETE FROM storage.access_grants WHERE id = $1") - .bind(grant_id) - .execute(self.pool.as_ref()) - .await - .map_err(|e| DomainError::internal_error("PgAcl", format!("revoke: {e}")))?; - Ok(()) - } - - async fn revoke_all_for_resource(&self, resource: Resource) -> Result { - let result = sqlx::query( - "DELETE FROM storage.access_grants WHERE resource_type = $1 AND resource_id = $2", - ) - .bind(resource.type_str()) - .bind(resource.id()) - .execute(self.pool.as_ref()) - .await - .map_err(|e| DomainError::internal_error("PgAcl", format!("revoke for resource: {e}")))?; - - Ok(result.rows_affected() as usize) - } - - async fn revoke_all_for_subject(&self, subject: Subject) -> Result { - let result = sqlx::query( - "DELETE FROM storage.access_grants WHERE subject_type = $1 AND subject_id = $2", - ) - .bind(subject.type_str()) - .bind(subject.id()) - .execute(self.pool.as_ref()) - .await - .map_err(|e| DomainError::internal_error("PgAcl", format!("revoke for subject: {e}")))?; - - Ok(result.rows_affected() as usize) - } } // ───────────────────────────────────────────────────────────────────────────── diff --git a/src/integration_test_support.rs b/src/integration_test_support.rs index 8e2e2e3a..8b4cf043 100644 --- a/src/integration_test_support.rs +++ b/src/integration_test_support.rs @@ -42,7 +42,7 @@ pub fn test_db_url() -> String { /// OnceCell so concurrent test threads block until the first caller /// finishes; subsequent calls are zero-cost. /// -/// Order matters: `storage.access_grants` rows go first because there's +/// Order matters: `storage.role_grants` rows go first because there's /// no FK from there to `auth.subject_groups` (the service's `delete` /// path does this transactionally; here we bypass the service). static CLEANUP_ONCE: tokio::sync::OnceCell<()> = tokio::sync::OnceCell::const_new(); @@ -51,7 +51,7 @@ pub async fn ensure_clean_test_db(pool: &PgPool) { CLEANUP_ONCE .get_or_init(|| async { let _ = sqlx::query( - "DELETE FROM storage.access_grants + "DELETE FROM storage.role_grants WHERE subject_type = 'group' AND subject_id IN ( SELECT id FROM auth.subject_groups WHERE name LIKE 'rust-test-%' diff --git a/src/interfaces/api/handlers/grant_handler.rs b/src/interfaces/api/handlers/grant_handler.rs index adc8c5c3..7fc39ab3 100644 --- a/src/interfaces/api/handlers/grant_handler.rs +++ b/src/interfaces/api/handlers/grant_handler.rs @@ -14,16 +14,16 @@ use axum::{ use futures::future::join_all; use serde::Deserialize; use std::sync::Arc; -use tracing::{error, info, warn}; +use tracing::{error, warn}; use utoipa::IntoParams; use uuid::Uuid; use crate::application::dtos::cursor::PageCursor; use crate::application::dtos::grant_dto::{ CreateGrantDto, CreateGrantResponseDto, GrantDto, MySharesDto, NotifyOutcomeSetDto, - OutgoingResourceGrantDto, OutgoingResourceItemDto, PermissionDto, ResourceContentDto, - ResourceDto, ResourceTypeDto, SharedWithMeDto, SharedWithMeItemDto, SharedWithMeQuery, - SubjectDto, SubjectInputDto, UpdateRoleDto, role_from_permissions, + OutgoingResourceGrantDto, OutgoingResourceItemDto, ResourceContentDto, ResourceDto, + ResourceTypeDto, SharedWithMeDto, SharedWithMeItemDto, SharedWithMeQuery, SubjectDto, + SubjectInputDto, UpdateRoleDto, role_from_permissions, }; use crate::application::ports::authorization_ports::AuthorizationEngine; use crate::application::ports::file_ports::FileRetrievalUseCase; @@ -35,7 +35,7 @@ use crate::common::errors::DomainError; use crate::domain::errors::ErrorKind; use crate::domain::services::authorization::{ GrantCursor, IncomingGrantSummary, OutgoingResourceSummary, Permission, Resource, ResourceKind, - Subject, + Role, Subject, }; use crate::interfaces::errors::AppError; use crate::interfaces::middleware::auth::AuthUser; @@ -66,28 +66,7 @@ pub async fn create_grant( let authz = &state.authorization; let caller_id = auth_user.id; - // Validate: exactly one of permissions/role - let permissions: Vec = match (dto.permissions, dto.role) { - (Some(perms), None) if !perms.is_empty() => perms.into_iter().map(Into::into).collect(), - (None, Some(role)) => role.expand().to_vec(), - (Some(_), Some(_)) => { - return AppError::new( - StatusCode::BAD_REQUEST, - "Provide either 'permissions' or 'role', not both", - "InvalidInput", - ) - .into_response(); - } - _ => { - return AppError::new( - StatusCode::BAD_REQUEST, - "Either 'permissions' (non-empty) or 'role' is required", - "InvalidInput", - ) - .into_response(); - } - }; - + let role: Role = dto.role.into(); let resource: Resource = dto.resource.into(); let expires_at = dto.expires_at; @@ -152,25 +131,32 @@ pub async fn create_grant( } }; - let mut results: Vec = Vec::with_capacity(permissions.len()); - for perm in permissions { - match authz - .grant(caller_id, subject, perm, resource, expires_at) - .await - { - Ok(grant) => results.push(grant.into()), - Err(err) => { - error!("grant insert failed for {perm:?}: {err}"); - return AppError::from(err).into_response(); - } + // Single role row in `storage.role_grants`. `ON CONFLICT UPDATE` in + // the engine makes repeated POSTs with the same (subject, resource) + // a role refresh, matching the PATCH-style semantics callers expect. + let grant = match authz + .set_role(caller_id, subject, role, resource, expires_at) + .await + { + Ok(g) => g, + Err(err) => { + error!("set_role write failed: {err}"); + return AppError::from(err).into_response(); } - } - info!( - "Created {} grant(s) for subject={:?} on resource={:?} by user {}", - results.len(), - subject, - resource, - caller_id + }; + let grants = vec![GrantDto::from(grant)]; + + tracing::info!( + target: "audit", + event = "role_grant.created", + caller_id = %caller_id, + subject_type = subject.type_str(), + subject_id = %subject.id(), + resource_type = resource.type_str(), + resource_id = %resource.id(), + role = role.as_str(), + expires_at = ?expires_at, + "🤝 grant created with role '{}'", role.as_str(), ); // PR N1 — route the post-grant notification through the unified @@ -240,7 +226,7 @@ pub async fn create_grant( ( StatusCode::CREATED, Json(CreateGrantResponseDto { - grants: results, + grants, notification, }), ) @@ -274,17 +260,20 @@ pub async fn revoke_grant( Err(_) => return AppError::not_found(format!("Grant {id} not found")).into_response(), }; - // Look up the grant to find the underlying resource (and granter). - let on_resource = match authz.find_grant_by_id(grant_id).await { - Ok(Some((res, granter))) => (res, granter), + // Look up the grant to find the subject, resource, and granter. + // `find_grant_full_by_id` returns the subject too — needed for the + // `clear_role` dual-write below (role_grants is keyed by (subject, + // resource), not by access_grants id). + let (subject, resource, granter) = match authz.find_grant_full_by_id(grant_id).await { + Ok(Some(triple)) => triple, Ok(None) => return StatusCode::NO_CONTENT.into_response(), // idempotent Err(e) => return AppError::from(e).into_response(), }; // Caller is authorized if they are the granter OR have Share on the resource. - if on_resource.1 != caller_id + if granter != caller_id && let Err(e) = authz - .require(Subject::User(caller_id), Permission::Share, on_resource.0) + .require(Subject::User(caller_id), Permission::Share, resource) .await { return AppError::from(e).into_response(); @@ -293,7 +282,36 @@ pub async fn revoke_grant( if let Err(e) = authz.revoke(grant_id).await { return AppError::from(e).into_response(); } - info!("Revoked grant {grant_id} (caller {caller_id})"); + + // D-Prep dual-write: clear the role_grants row for this (subject, + // resource). Idempotent — succeeds whether or not the row existed. + // + // Today's API revokes one access_grants row by id; the role_grants + // row models the WHOLE (subject, resource) cluster. Calling clear_role + // here effectively revokes the WHOLE role assignment in role_grants, + // even if other per-permission access_grants rows remain. This is the + // correct semantics for the eventual cleanup-PR model (role_grants is + // role-keyed; once access_grants goes away, "revoke" means "drop the + // role"). During the dual-write window the two tables can drift + // briefly if a caller revokes only some permissions of a role, but + // the engine still reads access_grants so behaviour is unchanged. + if let Err(e) = authz.clear_role(subject, resource).await { + return AppError::from(e).into_response(); + } + + tracing::info!( + target: "audit", + event = "role_grant.revoked", + caller_id = %caller_id, + grant_id = %grant_id, + subject_type = subject.type_str(), + subject_id = %subject.id(), + resource_type = resource.type_str(), + resource_id = %resource.id(), + granter_id = %granter, + self_revoke = (granter == caller_id), + "🗑️ grant revoked", + ); StatusCode::NO_CONTENT.into_response() } @@ -486,9 +504,8 @@ pub async fn set_role( let caller_id = auth_user.id; let subject: Subject = dto.subject.into(); let resource: Resource = dto.resource.into(); + let role: Role = dto.role.into(); let expires_at = dto.expires_at; - let target_perms: std::collections::HashSet = - dto.role.expand().iter().copied().collect(); // Caller must have Share on the resource. if let Err(e) = authz @@ -498,84 +515,41 @@ pub async fn set_role( return AppError::from(e).into_response(); } - // Fetch current grants on the resource for this subject. - let current = match authz.list_grants_on_resource(resource).await { - Ok(g) => g, - Err(e) => return AppError::from(e).into_response(), - }; - let current_perms: std::collections::HashSet = current - .iter() - .filter(|g| g.subject == subject) - .map(|g| g.permission) - .collect(); - - // Diff and apply. - let to_add: Vec = target_perms.difference(¤t_perms).copied().collect(); - let to_remove: Vec = current_perms.difference(&target_perms).copied().collect(); - - for perm in &to_remove { - if let Some(g) = current - .iter() - .find(|g| g.subject == subject && g.permission == *perm) - && let Err(e) = authz.revoke(g.id).await - { - return AppError::from(e).into_response(); - } - } - for perm in &to_add { - if let Err(e) = authz - .grant(caller_id, subject, *perm, resource, expires_at) - .await - { - return AppError::from(e).into_response(); - } - } - - // Sync expiry on all remaining grants for this (subject, resource) pair — - // includes newly added ones and any that were already present (retained). - // Callers that omit expires_at will clear any existing expiry; this is - // intentional: it keeps all permission rows for the pair consistent. - if let Err(e) = authz - .set_expiry_on_resource(subject, resource, expires_at) + // Atomic role refresh. UNIQUE on (subject, resource) + ON CONFLICT + // UPDATE in `set_role` turns this into a single UPSERT — no diff, + // no race window. Returns the resulting role row. + let grant = match authz + .set_role(caller_id, subject, role, resource, expires_at) .await { - return AppError::from(e).into_response(); - } - - // Return the new full set. - let after = match authz.list_grants_on_resource(resource).await { Ok(g) => g, Err(e) => return AppError::from(e).into_response(), }; - let mine: Vec = after - .into_iter() - .filter(|g| g.subject == subject) - .map(Into::into) - .collect(); - info!( - "Role applied: caller={} subject={:?} resource={:?} added={:?} removed={:?}", - caller_id, subject, resource, to_add, to_remove + tracing::info!( + target: "audit", + event = "role_grant.role_set", + caller_id = %caller_id, + subject_type = subject.type_str(), + subject_id = %subject.id(), + resource_type = resource.type_str(), + resource_id = %resource.id(), + role = role.as_str(), + expires_at = ?expires_at, + "🔁 role set to '{}'", role.as_str(), ); - (StatusCode::OK, Json(mine)).into_response() + (StatusCode::OK, Json(vec![GrantDto::from(grant)])).into_response() } // ════════════════════════════════════════════════════════════════════════════ // GET /api/grants/incoming // ════════════════════════════════════════════════════════════════════════════ -#[derive(Debug, Deserialize, IntoParams)] -pub struct IncomingQuery { - #[serde(default)] - pub permission: Option, -} - #[utoipa::path( get, path = "/api/grants/incoming", - params(IncomingQuery), responses( - (status = 200, description = "Direct grants targeting the caller", body = Vec), + (status = 200, description = "Direct role grants targeting the caller", body = Vec), ), security(("bearerAuth" = [])), tag = "grants" @@ -583,12 +557,11 @@ pub struct IncomingQuery { pub async fn list_incoming( State(state): State, auth_user: AuthUser, - Query(q): Query, ) -> impl IntoResponse { let caller_id = auth_user.id; match state .authorization - .list_incoming_grants(Subject::User(caller_id), q.permission.map(Into::into)) + .list_incoming_grants(Subject::User(caller_id)) .await { Ok(grants) => { diff --git a/src/interfaces/api/mod.rs b/src/interfaces/api/mod.rs index e30113f3..2c07d61f 100644 --- a/src/interfaces/api/mod.rs +++ b/src/interfaces/api/mod.rs @@ -23,7 +23,7 @@ use crate::application::dtos::folder_dto::{ use crate::application::dtos::folder_listing_dto::FolderListingDto; use crate::application::dtos::grant_dto::{ CreateGrantDto, GrantDto, OutgoingResourceItemDto, PermissionDto, ResourceContentDto, - ResourceDto, ResourceTypeDto, Role, SharedWithMeDto, SharedWithMeItemDto, SubjectDto, + ResourceDto, ResourceTypeDto, RoleDto, SharedWithMeDto, SharedWithMeItemDto, SubjectDto, SubjectTypeDto, UpdateRoleDto, }; use crate::application::dtos::i18n_dto::{ @@ -351,7 +351,7 @@ use crate::interfaces::api::handlers::file_handler::MoveFilePayload; ResourceTypeDto, ResourceDto, PermissionDto, - Role, + RoleDto, CreateGrantDto, UpdateRoleDto, GrantDto, diff --git a/static/js/components/mySharesList.js b/static/js/components/mySharesList.js index 6c374406..1db6b02f 100644 --- a/static/js/components/mySharesList.js +++ b/static/js/components/mySharesList.js @@ -454,7 +454,7 @@ class MySharesList { ); menu.appendChild(this._menuSeparator()); - for (const role of /** @type {('admin'|'editor'|'viewer')[]} */ (['admin', 'editor', 'viewer'])) { + for (const role of /** @type {('owner'|'editor'|'viewer')[]} */ (['owner', 'editor', 'viewer'])) { const isCurrent = grant.role === role; const mi = this._menuItem(isCurrent ? 'fas fa-check' : '', roleLabel(role), false, async () => { menu.remove(); diff --git a/static/js/components/roleChip.js b/static/js/components/roleChip.js index de37dac9..c2eea359 100644 --- a/static/js/components/roleChip.js +++ b/static/js/components/roleChip.js @@ -23,7 +23,7 @@ import { i18n } from '../core/i18n.js'; * @returns {'manage'|'edit'|'view'} */ function roleMod(role) { - if (role === 'admin') return 'manage'; + if (role === 'owner') return 'manage'; if (role === 'editor') return 'edit'; return 'view'; } @@ -31,14 +31,17 @@ function roleMod(role) { /** * Translate a role identifier into a localized human-readable label. * Exported so callers that just want the label (e.g. context-menu rows) - * can reuse the same wording the chip uses. + * can reuse the same wording the chip uses. Unknown roles fall back to + * the raw role string — `commenter` and `contributor` exist server-side + * but aren't surfaced in the UI today, so they'll display as-is until a + * future UI exposure adds proper labels. * @param {string} role * @returns {string} */ export function roleLabel(role) { /** @type {Record} */ const m = { - admin: i18n.t('share.role.canManage', 'Can manage'), + owner: i18n.t('share.role.canManage', 'Can manage'), editor: i18n.t('share.role.canEdit', 'Can edit'), viewer: i18n.t('share.role.canView', 'Can view') }; @@ -51,7 +54,7 @@ export function roleLabel(role) { * @returns {string} */ function roleIcon(role) { - if (role === 'admin') return 'fa-crown'; + if (role === 'owner') return 'fa-crown'; if (role === 'editor') return 'fa-pencil-alt'; return 'fa-eye'; } diff --git a/static/js/components/shareModal.js b/static/js/components/shareModal.js index 2b3febdd..6c876301 100644 --- a/static/js/components/shareModal.js +++ b/static/js/components/shareModal.js @@ -73,13 +73,6 @@ function _looksLikeEmail(q) { return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(q); } -/** Permissions that belong to each role (must mirror the Rust DTO). */ -const ROLE_PERMISSIONS = { - viewer: ['read'], - editor: ['read', 'comment', 'create', 'update'], - admin: ['read', 'comment', 'create', 'update', 'share', 'delete'] -}; - /** * Fetch up to ~8 ReBAC subject groups whose name matches `q`. Authenticated * endpoint; returns `[]` on any failure so the autocomplete degrades to @@ -107,14 +100,20 @@ async function _searchGroups(q) { } /** - * Derive the highest role a set of grants represents for one subject. + * Pick the displayed role for a member row. Server-side every Grant + * carries an explicit role since the cleanup PR, so this just reads it. + * The server may emit `commenter` or `contributor` (full enum), but the + * picker only exposes Viewer/Editor/Owner — collapse the two unexposed + * roles to the closest neighbour so the UI never renders an unknown + * option. * @param {Grant[]} subjectGrants * @returns {ShareRoleEnum} */ function _roleFromGrants(subjectGrants) { - const perms = new Set(subjectGrants.map((g) => g.permission)); - if (perms.has('delete') || perms.has('share')) return 'admin'; - if (perms.has('create') || perms.has('update')) return 'editor'; + const role = subjectGrants[0]?.role; + if (role === 'owner' || role === 'editor' || role === 'viewer') return role; + if (role === 'commenter') return 'viewer'; + if (role === 'contributor') return 'editor'; return 'viewer'; } @@ -363,7 +362,7 @@ const shareModal = { for (const [val, label] of [ ['viewer', i18n.t('share.role.canView', 'Can view')], ['editor', i18n.t('share.role.canEdit', 'Can edit')], - ['admin', i18n.t('share.role.canManage', 'Can manage')] + ['owner', i18n.t('share.role.canManage', 'Can manage')] ]) { const opt = document.createElement('option'); opt.value = val; @@ -606,7 +605,7 @@ const shareModal = { granted_at: '', granted_by: '', subject: { type: subjectType, id: contact.id }, - permission: /** @type {import('../core/types.js').PermissionTypeEnum} */ (ROLE_PERMISSIONS[this._stagedRole][0]), + role: this._stagedRole, resource: { type: this._itemType, id: this._item?.id ?? '' } }; this._localMembers.push({ @@ -649,7 +648,7 @@ const shareModal = { // matching the UX contract and the kebab-menu / role-select dropdown // order. Renaming the labels from "Manager"/"Editor"/"Viewer" to // "Can manage"/"Can edit"/"Can view" left this iteration order stale. - const groups = /** @type {ShareRoleEnum[]} */ (['admin', 'editor', 'viewer']); + const groups = /** @type {ShareRoleEnum[]} */ (['owner', 'editor', 'viewer']); let memberIndex = 0; for (const role of groups) { @@ -663,7 +662,7 @@ const shareModal = { header.className = 'smd-group-header'; const labelMap = { - admin: i18n.t('share.role.canManage', 'Can manage'), + owner: i18n.t('share.role.canManage', 'Can manage'), editor: i18n.t('share.role.canEdit', 'Can edit'), viewer: i18n.t('share.role.canView', 'Can view') }; @@ -711,7 +710,7 @@ const shareModal = { for (const [val, label] of [ ['viewer', i18n.t('share.role.canView', 'Can view')], ['editor', i18n.t('share.role.canEdit', 'Can edit')], - ['admin', i18n.t('share.role.canManage', 'Can manage')] + ['owner', i18n.t('share.role.canManage', 'Can manage')] ]) { const opt = document.createElement('option'); opt.value = val; diff --git a/static/js/core/types.js b/static/js/core/types.js index 594919c0..0c12efb2 100644 --- a/static/js/core/types.js +++ b/static/js/core/types.js @@ -302,21 +302,27 @@ * @property {String} id */ +/** + * Server-side role enum — every grantable role the backend recognises. + * The share modal's UI picker only exposes a subset (see `ShareRoleEnum`); + * the wire format may carry any of these values on a Grant. + * @typedef {'viewer'|'commenter'|'contributor'|'editor'|'owner'} GrantRoleEnum + */ + /** * @typedef {Object} Grant * @property {string} id * @property {string} granted_at - ISO-8601 datetime string. * @property {string} granted_by * @property {Subject} subject - * @property {PermissionTypeEnum} permission + * @property {GrantRoleEnum} role - Role-keyed grant. One Grant = one role + * assignment in `storage.role_grants`. The implied permission bundle + * is derived client-side from the same lookup table used by + * `Role::expand()` on the server (see `ROLE_PERMISSIONS` in shareModal). * @property {Resource} resource * @property {string|null} [expires_at] - ISO-8601 datetime string, or absent/null for no expiry. */ -/** - * Roles: `viewer`, `commenter`, `editor`, `manager`, `admin` - */ - /** * Configuration for `ResourceListComponent`. * @typedef {Object} ResourceListConfig @@ -367,7 +373,7 @@ * @property {'user'|'group'|'token'|'external'} subject_type * @property {string} subject_id * @property {string} subject_display - Username (users) or share name (tokens). - * @property {'viewer'|'editor'|'admin'} role + * @property {GrantRoleEnum} role - Server-emitted role string. `commenter` and `contributor` are reserved for future UI exposure; today the share modal only renders `viewer`/`editor`/`owner` (see `ShareRoleEnum`). * @property {string} granted_at - ISO-8601 * @property {string|null} [expires_at] - ISO-8601 or absent. * @property {boolean} has_password - True when a token subject has a password set. @@ -453,15 +459,22 @@ // ------------------- share modal /** - * Share roles (DTO-layer sugar for the ReBAC permission sets). - * @typedef {'viewer'|'editor'|'admin'} ShareRoleEnum + * Share-modal-exposed roles. The server's `Role` enum also includes + * `commenter` and `contributor` (see `OutgoingResourceGrant.role`); those + * are reserved for future UI exposure and are not offered as picker options + * today. The "Can manage" UI label maps to `owner`. + * @typedef {'viewer'|'editor'|'owner'} ShareRoleEnum */ /** * One collaborator row in the share modal's People section. * @typedef {Object} MemberEntry * @property {Grant} grant - Representative grant (used for subject/resource info). - * @property {Grant[]} _grants - All grants for this subject on the resource (may be > 1). + * @property {Grant[]} _grants - All grants for this subject on the resource. Post-pivot + * this is at most one entry (`storage.role_grants` UNIQUE on + * `(subject, resource)`); the array shape is preserved so the existing + * "revoke every grant on remove" loop in `_applyAll` still works + * without a special-case for empty / new entries. * @property {ShareRoleEnum} role - Derived role label shown in the UI. * @property {'keep'|'remove'|'change'|'new'} _op - Pending local operation. * @property {string|null} [expires_at] - YYYY-MM-DD expiry date string, or null for no expiry. diff --git a/static/js/model/grants.js b/static/js/model/grants.js index a6890ff0..68dd8bb3 100644 --- a/static/js/model/grants.js +++ b/static/js/model/grants.js @@ -164,7 +164,9 @@ const grants = { /** * Create a new grant. - * Body mirrors `CreateGrantDto`: `{ subject, resource, role }` OR `{ subject, resource, permissions }`. + * Body mirrors `CreateGrantDto`: `{ subject, resource, role, expires_at? }`. + * Strictly role-keyed since the cleanup PR — the per-permission shape + * is no longer accepted. * * Response shape (PR N1 — `CreateGrantResponseDto`): * diff --git a/static/sw.js b/static/sw.js index 17197a43..d6915295 100644 --- a/static/sw.js +++ b/static/sw.js @@ -1,6 +1,6 @@ // OxiCloud Service Worker // FIXME: generate cache name according build ? -const CACHE_NAME = 'oxicloud-cache-v27'; +const CACHE_NAME = 'oxicloud-cache-v28'; // Only cache static assets — NOT HTML files. // HTML files are served network-first so browsers always get the latest diff --git a/tests/api/grants.hurl b/tests/api/grants.hurl index 3c5214ac..6c503eb6 100644 --- a/tests/api/grants.hurl +++ b/tests/api/grants.hurl @@ -122,11 +122,11 @@ Content-Type: application/json } HTTP 201 -# PR N1: POST /api/grants now wraps results in -# `CreateGrantResponseDto { grants, notification }`. +# Cleanup PR: one role row per (subject, resource). `CreateGrantResponseDto` +# wraps a single role-keyed Grant in `.grants[0]`. [Asserts] jsonpath "$.grants" count == 1 -jsonpath "$.grants[0].permission" == "read" +jsonpath "$.grants[0].role" == "viewer" # ───────────────────────────────────────────────────────────── @@ -148,12 +148,13 @@ Authorization: Bearer {{dave_token}} HTTP 200 [Asserts] -jsonpath "$[?(@.resource.id=='{{shared_folder_id}}')].permission" == "read" +jsonpath "$[?(@.resource.id=='{{shared_folder_id}}')].role" == "viewer" # ───────────────────────────────────────────────────────────── -# Step 9 — Promote Bob to Admin (adds comment, create, update, share, delete). -# PUT /api/grants/role reconciles the row set in one call. +# Step 9 — Promote Bob to Owner (covers comment, create, update, share, +# delete, manage). PUT /api/grants/role replaces the role in one +# UPSERT against `storage.role_grants`. # ───────────────────────────────────────────────────────────── PUT {{base_url}}/api/grants/role Authorization: Bearer {{alice_token}} @@ -161,16 +162,17 @@ Content-Type: application/json { "subject": { "type": "user", "id": "{{dave_user_id}}" }, "resource": { "type": "folder", "id": "{{shared_folder_id}}" }, - "role": "admin" + "role": "owner" } HTTP 200 [Asserts] -jsonpath "$" count == 6 +jsonpath "$" count == 1 +jsonpath "$[0].role" == "owner" # ───────────────────────────────────────────────────────────── -# Step 10 — Bob can now rename (Manager includes update). +# Step 10 — Bob can now rename (Owner includes update). # ───────────────────────────────────────────────────────────── PUT {{base_url}}/api/folders/{{shared_folder_id}}/rename Authorization: Bearer {{dave_token}} @@ -194,7 +196,7 @@ HTTP 200 # ───────────────────────────────────────────────────────────── -# Step 12 — Bob re-shares to Carol (he has Share via Admin). +# Step 12 — Bob re-shares to Carol (he has Share via Owner). # ───────────────────────────────────────────────────────────── POST {{base_url}}/api/grants Authorization: Bearer {{dave_token}} @@ -218,7 +220,7 @@ Authorization: Bearer {{eve_token}} HTTP 200 [Asserts] -jsonpath "$[?(@.resource.id=='{{shared_folder_id}}')].permission" == "read" +jsonpath "$[?(@.resource.id=='{{shared_folder_id}}')].role" == "viewer" # ───────────────────────────────────────────────────────────── @@ -247,7 +249,7 @@ Content-Type: application/json HTTP 200 [Asserts] jsonpath "$" count == 1 -jsonpath "$[0].permission" == "read" +jsonpath "$[0].role" == "viewer" # ───────────────────────────────────────────────────────────── @@ -263,8 +265,9 @@ HTTP 404 # ───────────────────────────────────────────────────────────── # Step 17 — Lifecycle: Alice deletes the folder. The DB trigger -# trg_cleanup_grants_folder removes both bob's and carol's -# grants automatically (also for the cascade-deleted child). +# trg_cleanup_role_grants_folder removes both bob's and +# carol's grants automatically (also for the cascade-deleted +# child). # ───────────────────────────────────────────────────────────── DELETE {{base_url}}/api/folders/{{child_folder_id}} Authorization: Bearer {{alice_token}} @@ -767,7 +770,7 @@ HTTP 404 # ════════════════════════════════════════════════════════════════════ -# Phase 2D — Promote adam to Admin (all 6 permissions). Delete OK. +# Phase 2D — Promote adam to Owner (full bundle, incl. delete). Delete OK. # ════════════════════════════════════════════════════════════════════ PUT {{base_url}}/api/grants/role Authorization: Bearer {{alice_token}} @@ -775,7 +778,7 @@ Content-Type: application/json { "subject": { "type": "user", "id": "{{adam_user_id}}" }, "resource": { "type": "folder", "id": "{{perm_folder_id}}" }, - "role": "admin" + "role": "owner" } HTTP 200 @@ -788,7 +791,7 @@ HTTP 204 # ════════════════════════════════════════════════════════════════════ # Phase 2E — Lifecycle cleanup. Alice (still the owner) trashes & -# empties; the trigger removes all access_grants rows. +# empties; the trigger removes all role_grants rows. # ════════════════════════════════════════════════════════════════════ DELETE {{base_url}}/api/folders/{{perm_folder_id}} Authorization: Bearer {{alice_token}} @@ -1176,12 +1179,12 @@ Content-Type: application/json { "subject": { "type": "user", "id": "{{frank_user_id}}" }, "resource": { "type": "folder", "id": "{{batch_root_id}}" }, - "role": "admin" + "role": "owner" } HTTP 200 -# Frank (Admin grant = Delete) trashes batch_file_2 — item goes to +# Frank (Owner role includes Delete) trashes batch_file_2 — item goes to # Alice's trash because file.user_id is unchanged (Alice is still owner). POST {{base_url}}/api/batch/trash Authorization: Bearer {{frank_token}} diff --git a/tests/api/grants_nested_groups.hurl b/tests/api/grants_nested_groups.hurl index e11c6282..83a8b585 100644 --- a/tests/api/grants_nested_groups.hurl +++ b/tests/api/grants_nested_groups.hurl @@ -285,7 +285,7 @@ HTTP 201 # `CreateGrantResponseDto { grants, notification }`. [Asserts] jsonpath "$.grants" count == 1 -jsonpath "$.grants[0].permission" == "read" +jsonpath "$.grants[0].role" == "viewer" jsonpath "$.grants[0].subject.type" == "group" jsonpath "$.grants[0].subject.id" == "{{group_a_id}}" @@ -355,7 +355,7 @@ Authorization: Bearer {{henry_token}} HTTP 200 [Asserts] -jsonpath "$[?(@.resource.id=='{{perm_folder_id}}')].permission" == "read" +jsonpath "$[?(@.resource.id=='{{perm_folder_id}}')].role" == "viewer" jsonpath "$[?(@.resource.id=='{{perm_folder_id}}')].subject.type" == "group" jsonpath "$[?(@.resource.id=='{{perm_folder_id}}')].subject.id" == "{{group_a_id}}" @@ -533,7 +533,7 @@ HTTP 404 # ════════════════════════════════════════════════════════════════════ -# Phase D — Promote group A's grant to Admin (all 6 permissions). +# Phase D — Promote group A's grant to Owner (full bundle). # Delete now succeeds for henry, still flowing through B → A. # ════════════════════════════════════════════════════════════════════ PUT {{base_url}}/api/grants/role @@ -542,7 +542,7 @@ Content-Type: application/json { "subject": { "type": "group", "id": "{{group_a_id}}" }, "resource": { "type": "folder", "id": "{{perm_folder_id}}" }, - "role": "admin" + "role": "owner" } HTTP 200 diff --git a/tests/api/role_grants.hurl b/tests/api/role_grants.hurl new file mode 100644 index 00000000..e5a218e1 --- /dev/null +++ b/tests/api/role_grants.hurl @@ -0,0 +1,395 @@ +# ============================================================= +# OxiCloud — D-Prep: role_grants dual-write + new wire format +# ============================================================= +# Pins the D-Prep refactor behaviours that don't fit naturally into +# the existing `grants.hurl` (which is API-shape-focused). Specifically: +# +# 1. Wire-format role names: +# - "owner" is accepted on POST and emitted on response +# - "admin" is REJECTED with 422 (compat alias retired in the +# cleanup PR — see Step 6a) +# +# 2. Role-keyed write proof: granting a role and then exercising a +# permission from its bundle works → proves the row landed in +# `storage.role_grants` and the engine read path expands the +# bundle correctly (see `folder_cascade_grant_exists`). +# +# 3. Atomic role updates via PUT /api/grants/role — the role flips +# in a single SQL update (no DELETE+INSERT race window). +# +# 4. Clean revoke: DELETE /api/grants/{id} clears role_grants too. +# +# Self-contained: creates its own users, folders, and files so it +# can run in any position relative to other test files. +# ============================================================= + + +# ───────────────────────────────────────────────────────────── +# Step 1 — admin login + home folder lookup +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ "username": "{{username}}", "password": "{{password}}" } + +HTTP 200 +[Captures] +admin_token: jsonpath "$.access_token" +admin_user_id: jsonpath "$.user.id" + +GET {{base_url}}/api/folders +Authorization: Bearer {{admin_token}} + +HTTP 200 +[Captures] +admin_home_id: jsonpath "$[0].id" + + +# ───────────────────────────────────────────────────────────── +# Step 2 — Create two fresh test users (renee, sam) so this file +# doesn't depend on cross-file fixtures. Use the +# legacy-compat path that POSTs to /api/admin/users. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/admin/users +Authorization: Bearer {{admin_token}} +Content-Type: application/json +{ "username": "renee", "password": "ReneePassword1!", "email": "renee@example.com", "role": "user" } + +HTTP 201 +[Captures] +renee_user_id: jsonpath "$.id" + + +POST {{base_url}}/api/admin/users +Authorization: Bearer {{admin_token}} +Content-Type: application/json +{ "username": "sam", "password": "SamPassword1!", "email": "sam@example.com", "role": "user" } + +HTTP 201 +[Captures] +sam_user_id: jsonpath "$.id" + + +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ "username": "renee", "password": "ReneePassword1!" } + +HTTP 200 +[Captures] +renee_token: jsonpath "$.access_token" + + +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ "username": "sam", "password": "SamPassword1!" } + +HTTP 200 +[Captures] +sam_token: jsonpath "$.access_token" + + +# ───────────────────────────────────────────────────────────── +# Step 3 — admin creates a folder "role-grants-test" + a file +# inside it to use as the authz target throughout the file. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/folders +Authorization: Bearer {{admin_token}} +Content-Type: application/json +{ "name": "role-grants-test", "parent_id": "{{admin_home_id}}" } + +HTTP 201 +[Captures] +test_folder_id: jsonpath "$.id" + + +POST {{base_url}}/api/files/upload +Authorization: Bearer {{admin_token}} +[MultipartFormData] +folder_id: {{test_folder_id}} +file: file,fixtures/hello.txt; text/plain + +HTTP 201 +[Captures] +test_file_id: jsonpath "$.id" + +# Rename immediately so subsequent throwaway uploads of `hello.txt` +# to the same folder don't 409. Each throwaway upload below applies +# the same pattern (upload → rename → use) to keep the namespace +# clean for the next one. +PUT {{base_url}}/api/files/{{test_file_id}}/rename +Authorization: Bearer {{admin_token}} +Content-Type: application/json +{ "name": "step3-anchor.txt" } + +HTTP 200 + + +# ───────────────────────────────────────────────────────────── +# Step 4 — Grant renee role="owner" on the folder. New wire format. +# Response should echo back the canonical name. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/grants +Authorization: Bearer {{admin_token}} +Content-Type: application/json +{ + "subject": { "type": "user", "id": "{{renee_user_id}}" }, + "resource": { "type": "folder", "id": "{{test_folder_id}}" }, + "role": "owner" +} + +HTTP 201 + + +# ───────────────────────────────────────────────────────────── +# Step 5 — Dual-write proof: renee (now Owner of the folder) can +# DELETE a file inside it. Owner's bundle includes Delete; +# the engine reads from role_grants → if dual-write didn't +# land the row, the cascade query returns empty and the +# delete is refused. +# +# We upload + delete a throwaway file to avoid removing the +# test_file we'll need for later steps. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/files/upload +Authorization: Bearer {{admin_token}} +[MultipartFormData] +folder_id: {{test_folder_id}} +file: file,fixtures/hello.txt; text/plain + +HTTP 201 +[Captures] +throwaway_file_id: jsonpath "$.id" + +# Rename so subsequent uploads in this folder don't 409 on "hello.txt". +PUT {{base_url}}/api/files/{{throwaway_file_id}}/rename +Authorization: Bearer {{admin_token}} +Content-Type: application/json +{ "name": "renee-throwaway.txt" } + +HTTP 200 + + +DELETE {{base_url}}/api/files/{{throwaway_file_id}} +Authorization: Bearer {{renee_token}} + +HTTP 204 + + +# ───────────────────────────────────────────────────────────── +# Step 6a — Reject the legacy "admin" string. The cleanup PR +# retired the `#[serde(alias = "admin")]` compat shim on +# `RoleDto::Owner`; the deserialiser now refuses it with 422. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/grants +Authorization: Bearer {{admin_token}} +Content-Type: application/json +{ + "subject": { "type": "user", "id": "{{sam_user_id}}" }, + "resource": { "type": "folder", "id": "{{test_folder_id}}" }, + "role": "admin" +} + +HTTP 422 + + +# ───────────────────────────────────────────────────────────── +# Step 6b — Grant sam Owner with the canonical role string. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/grants +Authorization: Bearer {{admin_token}} +Content-Type: application/json +{ + "subject": { "type": "user", "id": "{{sam_user_id}}" }, + "resource": { "type": "folder", "id": "{{test_folder_id}}" }, + "role": "owner" +} + +HTTP 201 +[Captures] +# The single role-keyed Grant returned in `.grants[0]` is the +# `storage.role_grants` row id. Step 10's revoke uses it to +# `clear_role` and wipe the row. +sam_grant_id: jsonpath "$.grants[0].id" + + +# Sam should now have Owner-equivalent access — Delete works. +POST {{base_url}}/api/files/upload +Authorization: Bearer {{admin_token}} +[MultipartFormData] +folder_id: {{test_folder_id}} +file: file,fixtures/hello.txt; text/plain + +HTTP 201 +[Captures] +sam_throwaway_id: jsonpath "$.id" + +PUT {{base_url}}/api/files/{{sam_throwaway_id}}/rename +Authorization: Bearer {{admin_token}} +Content-Type: application/json +{ "name": "sam-throwaway.txt" } + +HTTP 200 + + +DELETE {{base_url}}/api/files/{{sam_throwaway_id}} +Authorization: Bearer {{sam_token}} + +HTTP 204 + + +# ───────────────────────────────────────────────────────────── +# Step 7 — Atomic role update: demote renee from Owner to Viewer +# via PUT /api/grants/role. Single SQL UPDATE on +# role_grants — no DELETE+INSERT race. +# +# After: renee can still Read but should be refused Delete. +# ───────────────────────────────────────────────────────────── +PUT {{base_url}}/api/grants/role +Authorization: Bearer {{admin_token}} +Content-Type: application/json +{ + "subject": { "type": "user", "id": "{{renee_user_id}}" }, + "resource": { "type": "folder", "id": "{{test_folder_id}}" }, + "role": "viewer" +} + +HTTP 200 + + +# Renee can still read the file (Viewer's bundle includes Read). +GET {{base_url}}/api/files/{{test_file_id}} +Authorization: Bearer {{renee_token}} + +HTTP 200 + + +# Renee CANNOT delete — Viewer's bundle excludes Delete; the +# folder_cascade_grant_exists query for Delete returns empty. +POST {{base_url}}/api/files/upload +Authorization: Bearer {{admin_token}} +[MultipartFormData] +folder_id: {{test_folder_id}} +file: file,fixtures/hello.txt; text/plain + +HTTP 201 +[Captures] +post_demote_file_id: jsonpath "$.id" + +PUT {{base_url}}/api/files/{{post_demote_file_id}}/rename +Authorization: Bearer {{admin_token}} +Content-Type: application/json +{ "name": "post-demote.txt" } + +HTTP 200 + + +DELETE {{base_url}}/api/files/{{post_demote_file_id}} +Authorization: Bearer {{renee_token}} + +HTTP * +[Asserts] +status >= 400 +status < 500 + + +# ───────────────────────────────────────────────────────────── +# Step 8 — Promote renee back to Editor (one role change, atomic) +# ───────────────────────────────────────────────────────────── +PUT {{base_url}}/api/grants/role +Authorization: Bearer {{admin_token}} +Content-Type: application/json +{ + "subject": { "type": "user", "id": "{{renee_user_id}}" }, + "resource": { "type": "folder", "id": "{{test_folder_id}}" }, + "role": "editor" +} + +HTTP 200 + + +# Editor's bundle includes Update — renaming a file should work. +PUT {{base_url}}/api/files/{{post_demote_file_id}}/rename +Authorization: Bearer {{renee_token}} +Content-Type: application/json +{ "name": "renamed-by-renee.txt" } + +HTTP 200 + + +# ───────────────────────────────────────────────────────────── +# Step 9 — My Shares response shape: the outgoing-resources endpoint +# must emit role strings from the new roster ("viewer" / +# "editor" / "owner"), never the legacy "admin". +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/grants/outgoing/resources +Authorization: Bearer {{admin_token}} + +HTTP 200 +[Asserts] +# The response body MUST contain "owner" (sam's role) and "editor" +# (renee's current role after the promote). It MUST NOT contain the +# legacy "admin" role string for any grant emitted by the server. +body contains "\"role\":\"owner\"" +body contains "\"role\":\"editor\"" +body not contains "\"role\":\"admin\"" + + +# ───────────────────────────────────────────────────────────── +# Step 10 — Revoke: removing sam's grant. `engine.revoke()` DELETEs +# the single `storage.role_grants` row by id. +# +# After: sam's Delete attempt should be refused (proof +# the role_grants row is gone — the cascade query for +# Delete returns empty because sam has no row pointing +# at this folder). +# ───────────────────────────────────────────────────────────── + +# sam_grant_id was captured at Step 6b from the create response. +DELETE {{base_url}}/api/grants/{{sam_grant_id}} +Authorization: Bearer {{admin_token}} + +HTTP 204 + + +# Post-revoke: sam can no longer Delete in this folder. +POST {{base_url}}/api/files/upload +Authorization: Bearer {{admin_token}} +[MultipartFormData] +folder_id: {{test_folder_id}} +file: file,fixtures/hello.txt; text/plain + +HTTP 201 +[Captures] +post_revoke_file_id: jsonpath "$.id" + +PUT {{base_url}}/api/files/{{post_revoke_file_id}}/rename +Authorization: Bearer {{admin_token}} +Content-Type: application/json +{ "name": "post-revoke.txt" } + +HTTP 200 + + +DELETE {{base_url}}/api/files/{{post_revoke_file_id}} +Authorization: Bearer {{sam_token}} + +HTTP * +[Asserts] +status >= 400 +status < 500 + + +# ───────────────────────────────────────────────────────────── +# Step 11 — Teardown: clean up users + folder so this file +# leaves no residue for the storage_cleanup_check. +# ───────────────────────────────────────────────────────────── +DELETE {{base_url}}/api/folders/{{test_folder_id}} +Authorization: Bearer {{admin_token}} +HTTP 204 + +DELETE {{base_url}}/api/admin/users/{{renee_user_id}} +Authorization: Bearer {{admin_token}} +HTTP 200 + +DELETE {{base_url}}/api/admin/users/{{sam_user_id}} +Authorization: Bearer {{admin_token}} +HTTP 200 diff --git a/tests/api/run.sh b/tests/api/run.sh index 1524b63e..d07cfbb9 100755 --- a/tests/api/run.sh +++ b/tests/api/run.sh @@ -146,6 +146,7 @@ hurl --variables-file "$API_DIR/test.env" --file-root "$REPO_ROOT/tests" --test "$API_DIR/public_shares.hurl" \ "$API_DIR/permissions.hurl" \ "$API_DIR/grants.hurl" \ + "$API_DIR/role_grants.hurl" \ "$API_DIR/subject_groups.hurl" \ "$API_DIR/groups_effective_members.hurl" \ "$API_DIR/grants_nested_groups.hurl" \ diff --git a/tests/api/subject_groups.hurl b/tests/api/subject_groups.hurl index aa958bb0..93fe1680 100644 --- a/tests/api/subject_groups.hurl +++ b/tests/api/subject_groups.hurl @@ -221,7 +221,7 @@ Content-Type: application/json { "subject": { "type": "group", "id": "{{engineers_id}}" }, "resource": { "type": "folder", "id": "{{shared_folder_id}}" }, - "permissions": ["read"] + "role": "viewer" } HTTP 201