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 e7c0ec75..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 @@ -130,166 +130,50 @@ impl From for PermissionDto { // Roles — the load-bearing model for ReBAC grants // ════════════════════════════════════════════════════════════════════════════ // -// Today a role is "DTO-layer sugar" — every grant write expands a role into -// N rows in `storage.access_grants`. The D-Prep refactor (see -// `docs/plan/drive.md` §Prerequisite + migration `20260730000000_role_grants.sql`) -// pushes the role down to storage (`storage.role_grants.role TEXT`); the -// engine reads the role and expands the bundle at query time via this same -// `expand()` function. Adding a role is now schema-free — one variant + one -// match arm here. +// 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. +/// 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 { - /// Read access only. The default "anyone can look but not touch". +pub enum RoleDto { Viewer, - /// Read + comment. Useful for review-only stakeholders. Commenter, - /// Read + create. The "drop-zone" role — uploads allowed, existing - /// content untouchable. Common for support-ticket attachments and - /// photo-submission folders. Contributor, - /// Read + create + update + comment. The standard collaboration role. Editor, - /// Full bundle: read, create, update, comment, delete, share — and - /// `Manage` for resource types that support it (drives, groups). This - /// is the highest user-grantable role; renamed from the historical - /// `Admin` to disambiguate from `UserRole::Admin` (the user-account - /// privilege) and to match Drive plan terminology. - /// - /// **Wire-format compat shim** (one release): also deserialises from - /// the legacy `"admin"` string so cached frontend clients keep - /// working until they refresh. Serialisation always emits `"owner"`. - /// Drop the alias in the cleanup PR. - #[serde(alias = "admin")] 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`). - /// - /// After D-Prep this is called at engine read time (1 row → bundle - /// expanded server-side). Pre-D-Prep it was called at API write time - /// (1 role → N rows fanned out). - 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, - ], +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, } } - - /// Lowercase string discriminator — matches the SQL `role` column values - /// in `storage.role_grants` and the JSON wire format. - 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 / JSON string discriminator. Returns - /// `None` for unknown values. Accepts the legacy `"admin"` spelling - /// for one release of API compat (clients that cached the old name - /// keep working; new responses always emit `"owner"`). - 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), - // Legacy compat: drop after one release once all clients are - // updated. Emits a debug log so we can track stragglers. - "admin" => { - tracing::debug!( - target: "oxicloud::grants", - "Role::parse: accepted legacy 'admin' string as Role::Owner" - ); - Some(Role::Owner) - } - _ => None, - } - } - - /// Every role, in a stable order. Used by `roles_implying` and exposed - /// to the UI so the share modal can render the full picker without - /// hardcoding the list. - /// - /// **UI scope today**: the share dialog renders only `Viewer`, - /// `Editor`, and `Owner` (matches the existing 3-button UX). The - /// `Commenter` and `Contributor` variants are implemented server- - /// side and accepted on the API surface, reserved for future UI - /// exposure when a real use case asks for them. Until then they - /// stay invisible to end users — no picker option, no documentation - /// surface. - /// - /// Any role can be granted on any resource type. Permission bundles - /// that include capabilities the resource type doesn't check for - /// (e.g. `Manage` on a folder, `Create` on a file) simply produce - /// harmless no-ops — no separate validation layer is needed. - pub const ALL: [Role; 5] = [ - Role::Viewer, - Role::Commenter, - Role::Contributor, - Role::Editor, - Role::Owner, - ]; } -/// 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?": -/// -/// ```ignore -/// SELECT resource_id FROM role_grants -/// WHERE subject_id = $1 -/// AND resource_type = 'drive' -/// AND role IN (roles_implying(Permission::Read)); -/// ``` -/// -/// Precomputed in code rather than stored in the DB — `Role` and -/// `Permission` are both small fixed enums, the table can never grow -/// beyond a handful of rows, and keeping it in-code makes "what changes -/// when I add a Permission?" a single grep target. -pub fn roles_implying(permission: Permission) -> &'static [Role] { - use Permission::*; - match permission { - // Every role grants Read — viewer is the floor. - 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 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, + } } } @@ -325,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>, } @@ -345,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>, @@ -360,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")] @@ -373,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, diff --git a/src/application/ports/authorization_ports.rs b/src/application/ports/authorization_ports.rs index b0b9b1c7..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 @@ -182,20 +160,10 @@ pub trait AuthorizationEngine: Send + Sync + 'static { /// /group is deleted. Returns the count of rows removed. async fn revoke_all_for_subject(&self, subject: Subject) -> Result; - // ── Role-keyed grant operations (D-Prep dual-write) ──────────────────── - // These manage `storage.role_grants`, the role-keyed table introduced - // by the D-Prep refactor (see `docs/plan/drive.md` §Prerequisite). - // - // During the dual-write window both tables stay populated; the engine - // reads from `access_grants` until the read-path pivot lands. After - // the cleanup PR (which drops `access_grants`), these two methods - // become the ONLY grant write path — the per-permission `grant` / - // `revoke` above are removed at that point. - // - // The handler layer drives these (it knows the Role); lifecycle hook - // bulk-deletes (`revoke_all_for_*` above) wipe role_grants in lockstep - // inside their own implementation, so callers using those paths don't - // need to invoke `clear_role` separately. + // ── 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)` @@ -207,10 +175,10 @@ pub trait AuthorizationEngine: Send + Sync + 'static { &self, granted_by: Uuid, subject: Subject, - role: crate::application::dtos::grant_dto::Role, + role: Role, resource: Resource, expires_at: Option>, - ) -> Result<(), DomainError>; + ) -> Result; /// Remove the role for a `(subject, resource)` pair. Idempotent — /// succeeds whether or not the row existed. Called after `revoke` 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 7e8b5503..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)), @@ -200,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)] @@ -208,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()) @@ -225,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 1d46db3b..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 //! @@ -37,14 +38,13 @@ use uuid::Uuid; use moka::future::Cache; use sqlx::PgPool; -use crate::application::dtos::grant_dto::roles_implying; use crate::application::ports::authorization_ports::AuthorizationEngine; use crate::common::errors::DomainError; 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; @@ -206,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. @@ -239,8 +239,8 @@ impl PgAclEngine { } /// Convert a `Permission` into the array of role strings whose bundle - /// includes it — used to bind the `g.role = ANY($N::text[])` filter on - /// every cascade / lookup query that reads `storage.role_grants`. + /// 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 @@ -264,11 +264,10 @@ impl PgAclEngine { /// `subject_ids` is the expanded set returned by `expand_user` (or a /// single-element vec for non-user callers). /// - /// **D-Prep**: now reads `storage.role_grants` (1 row per role - /// assignment) instead of `storage.access_grants` (N rows per role). - /// The permission filter `g.permission = $3` becomes - /// `g.role = ANY($3::text[])` where the array is the set of roles whose - /// bundle includes the requested permission — see `roles_implying()`. + /// 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( @@ -288,7 +287,7 @@ impl PgAclEngine { JOIN storage.folders gf ON gf.id = g.resource_id WHERE g.subject_type = ANY($1) AND g.subject_id = ANY($2) - AND g.role = ANY($3::text[]) + 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) @@ -329,7 +328,7 @@ impl PgAclEngine { FROM storage.role_grants WHERE subject_type = ANY($1) AND subject_id = ANY($2) - AND role = ANY($3::text[]) + 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 @@ -340,7 +339,7 @@ impl PgAclEngine { JOIN storage.files target_f ON target_f.id = $4 WHERE g.subject_type = ANY($1) AND g.subject_id = ANY($2) - AND g.role = ANY($3::text[]) + 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 @@ -361,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()) @@ -410,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: ( @@ -430,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, @@ -554,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::< _, ( @@ -579,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 @@ -621,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) @@ -653,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)) @@ -725,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} @@ -765,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} @@ -793,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 @@ -875,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, @@ -897,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::< _, ( @@ -913,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 "#, ) @@ -942,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 @@ -951,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 @@ -1040,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 @@ -1055,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 @@ -1129,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 @@ -1160,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 @@ -1180,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 { ( @@ -1209,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 @@ -1246,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 @@ -1284,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 ), @@ -1297,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 @@ -1380,7 +1398,7 @@ impl AuthorizationEngine for PgAclEngine { grant_id, granted_at, expires_at, - perm_str, + role_str, _, _, has_password, @@ -1389,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); @@ -1409,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); + } } } @@ -1498,7 +1518,7 @@ impl AuthorizationEngine for PgAclEngine { grant_id, granted_at, expires_at, - perm_str, + role_str, _, _, has_password, @@ -1507,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; }; @@ -1531,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); + } } } @@ -1578,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::< _, ( @@ -1594,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) @@ -1608,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 { @@ -1630,149 +1714,17 @@ impl AuthorizationEngine for PgAclEngine { Option>, ), >( - 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 - RETURNING id, subject_type, subject_id, resource_type, resource_id, - permission, 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(granted_by) - .bind(expires_at) - .fetch_one(self.pool.as_ref()) - .await - .map_err(|e| DomainError::internal_error("PgAcl", format!("insert grant: {e}")))?; - - Self::row_to_grant(row) - } - - async fn set_expiry_for_subject( - &self, - subject: Subject, - expires_at: Option>, - ) -> 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 \ - WHERE subject_type = $1 AND subject_id = $2 \ - AND resource_type = $4 AND resource_id = $5", - ) - .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}")) - })?; - 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 { - // D-Prep dual-write: wipe role_grants for this resource too. - // Idempotent — succeeds whether or not any row existed. - 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 role_grants for resource: {e}")) - })?; - - 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 { - // D-Prep dual-write: wipe role_grants for this subject too. - 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 role_grants for subject: {e}")) - })?; - - 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) - } - - // ── D-Prep role_grants writes ────────────────────────────────────────── - - async fn set_role( - &self, - granted_by: Uuid, - subject: Subject, - role: crate::application::dtos::grant_dto::Role, - resource: Resource, - expires_at: Option>, - ) -> Result<(), DomainError> { - sqlx::query( r#" INSERT INTO storage.role_grants (subject_type, subject_id, resource_type, resource_id, role, granted_by, expires_at) - VALUES ($1, $2, $3, $4, $5, $6, $7) + 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, + role::text, granted_by, granted_at, expires_at "#, ) .bind(subject.type_str()) @@ -1782,11 +1734,11 @@ impl AuthorizationEngine for PgAclEngine { .bind(role.as_str()) .bind(granted_by) .bind(expires_at) - .execute(self.pool.as_ref()) + .fetch_one(self.pool.as_ref()) .await .map_err(|e| DomainError::internal_error("PgAcl", format!("set_role: {e}")))?; - Ok(()) + Self::row_to_grant(row) } async fn clear_role(&self, subject: Subject, resource: Resource) -> Result<(), DomainError> { 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 21617cb5..7fc39ab3 100644 --- a/src/interfaces/api/handlers/grant_handler.rs +++ b/src/interfaces/api/handlers/grant_handler.rs @@ -21,9 +21,9 @@ 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, Role, 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,41 +66,7 @@ pub async fn create_grant( let authz = &state.authorization; let caller_id = auth_user.id; - // Validate: exactly one of permissions/role. Capture BOTH the - // permission list (for the per-permission access_grants writes that - // keep the old engine read path working) AND the role (for the new - // role_grants `set_role` dual-write that lands after the per- - // permission loop). - let (permissions, role): (Vec, Role) = match (dto.permissions, dto.role) { - (Some(perms), None) if !perms.is_empty() => { - let perms: Vec = perms.into_iter().map(Into::into).collect(); - // Derive the closest matching role from the raw permission set - // so we have ONE role to mirror into role_grants. `Role::parse` - // always succeeds here because `role_from_permissions` only - // emits known role strings. - let role = Role::parse(role_from_permissions(&perms)) - .expect("role_from_permissions returns a known role string"); - (perms, role) - } - (None, Some(role)) => (role.expand().to_vec(), role), - (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; @@ -165,41 +131,20 @@ pub async fn create_grant( } }; - let mut results: Vec = Vec::with_capacity(permissions.len()); - for perm in permissions { - // `storage.access_grants.permission` CHECK constraint predates - // `Permission::Manage`; it accepts only the original 6 values. - // Manage exists in the Owner bundle for engine read-path use - // (via `roles_implying`) and gets persisted via the role_grants - // dual-write below. Skipping it here keeps the access_grants - // safety net populated without tripping the CHECK; the cleanup - // PR that drops access_grants also drops this skip. - if perm == Permission::Manage { - continue; - } - 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(); - } - } - } - - // D-Prep dual-write: mirror the role assignment into storage.role_grants. - // ON CONFLICT UPDATE makes this idempotent — repeated POSTs with the - // same (subject, resource) update the role in place, matching the - // PATCH-style semantics callers will get after the engine read pivot. - if let Err(err) = authz + // 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 { - error!("set_role dual-write failed: {err}"); - return AppError::from(err).into_response(); - } + Ok(g) => g, + Err(err) => { + error!("set_role write failed: {err}"); + return AppError::from(err).into_response(); + } + }; + let grants = vec![GrantDto::from(grant)]; tracing::info!( target: "audit", @@ -210,7 +155,6 @@ pub async fn create_grant( resource_type = resource.type_str(), resource_id = %resource.id(), role = role.as_str(), - permission_count = results.len(), expires_at = ?expires_at, "🤝 grant created with role '{}'", role.as_str(), ); @@ -282,7 +226,7 @@ pub async fn create_grant( ( StatusCode::CREATED, Json(CreateGrantResponseDto { - grants: results, + grants, notification, }), ) @@ -560,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 @@ -572,86 +515,16 @@ 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 { + // 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 + { 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. `Permission::Manage` is excluded from both sides - // because the historical `access_grants.permission` CHECK doesn't - // accept it — see the matching skip in `create_grant`. The role - // assignment captures Manage via the role_grants `set_role` call - // further down; the engine's read-path uses `roles_implying(Manage)` - // → `[Owner]` and never goes through per-permission rows. - let to_add: Vec = target_perms - .difference(¤t_perms) - .copied() - .filter(|p| *p != Permission::Manage) - .collect(); - let to_remove: Vec = current_perms - .difference(&target_perms) - .copied() - .filter(|p| *p != Permission::Manage) - .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) - .await - { - return AppError::from(e).into_response(); - } - - // D-Prep dual-write: mirror the resulting role into storage.role_grants. - // The per-permission diff above keeps access_grants converged; this - // single UPSERT keeps role_grants in sync with the OVERALL outcome - // (one row carrying the role + expiry). After the engine read pivot - // and the access_grants drop, the per-permission diff above goes - // away and this call becomes the only mutation the handler performs. - if let Err(e) = authz - .set_role(caller_id, subject, dto.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(); tracing::info!( target: "audit", @@ -661,34 +534,22 @@ pub async fn set_role( subject_id = %subject.id(), resource_type = resource.type_str(), resource_id = %resource.id(), - role = dto.role.as_str(), - permissions_added = to_add.len(), - permissions_removed = to_remove.len(), + role = role.as_str(), expires_at = ?expires_at, - "🔁 role set to '{}' (+{} -{})", - dto.role.as_str(), - to_add.len(), - to_remove.len(), + "🔁 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" @@ -696,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/shareModal.js b/static/js/components/shareModal.js index dc5b246a..6c876301 100644 --- a/static/js/components/shareModal.js +++ b/static/js/components/shareModal.js @@ -73,23 +73,6 @@ function _looksLikeEmail(q) { return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(q); } -/** - * Permissions that belong to each role (mirrors `Role::expand()` in - * `src/application/dtos/grant_dto.rs`). The share modal only renders - * Viewer/Editor/Owner as picker buttons today; `commenter` and - * `contributor` are kept here for fidelity with the server-side enum so - * a future UI exposure doesn't need a mirror-table update. The `manage` - * permission in the owner bundle is reserved for Drive- and Group-level - * admin actions (no-op on file/folder resources). - */ -const ROLE_PERMISSIONS = { - viewer: ['read'], - commenter: ['read', 'comment'], - contributor: ['read', 'create'], - editor: ['read', 'comment', 'create', 'update'], - owner: ['read', 'comment', 'create', 'update', 'share', 'delete', 'manage'] -}; - /** * Fetch up to ~8 ReBAC subject groups whose name matches `q`. Authenticated * endpoint; returns `[]` on any failure so the autocomplete degrades to @@ -117,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 'owner'; - 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'; } @@ -616,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({ diff --git a/static/js/core/types.js b/static/js/core/types.js index 3a719a2b..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'|'commenter'|'contributor'|'editor'|'owner'} 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 {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. @@ -464,7 +470,11 @@ * 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 index 4a0f4ede..e5a218e1 100644 --- a/tests/api/role_grants.hurl +++ b/tests/api/role_grants.hurl @@ -4,17 +4,15 @@ # Pins the D-Prep refactor behaviours that don't fit naturally into # the existing `grants.hurl` (which is API-shape-focused). Specifically: # -# 1. New wire-format role names: +# 1. Wire-format role names: # - "owner" is accepted on POST and emitted on response -# - "admin" still accepted on POST for one release (compat shim -# in `Role::parse`); the server normalises it to Owner +# - "admin" is REJECTED with 422 (compat alias retired in the +# cleanup PR — see Step 6a) # -# 2. Dual-write proof: granting a role and then exercising a +# 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` because the engine now reads from there -# for authz decisions (see `folder_cascade_grant_exists` post- -# D-Prep). If dual-write failed, the engine would see no row and -# reject the check. +# `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). @@ -177,10 +175,9 @@ HTTP 204 # ───────────────────────────────────────────────────────────── -# Step 6 — Legacy "admin" string compat. The server's Role::parse -# accepts "admin" and normalises to Owner during the D-Prep -# dual-write window (one release). Verify by granting sam -# with role="admin" — sam should then be able to Delete too. +# 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}} @@ -191,15 +188,26 @@ Content-Type: application/json "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] -# Capture the first per-permission grant id from the response so -# Step 10's revoke doesn't have to query the My Shares endpoint -# with awkward JSONPath filtering. Any single grant_id works: -# the revoke handler calls `clear_role` which wipes the entire -# role_grants row for (sam, test_folder), so the engine reads -# return false for sam afterwards regardless of how many -# access_grants rows still exist. +# 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" @@ -326,9 +334,8 @@ body not contains "\"role\":\"admin\"" # ───────────────────────────────────────────────────────────── -# Step 10 — Revoke: removing sam's grant. The handler clears the -# access_grants rows AND calls clear_role to wipe the -# role_grants row in the same flow. +# 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 @@ -336,7 +343,7 @@ body not contains "\"role\":\"admin\"" # at this folder). # ───────────────────────────────────────────────────────────── -# sam_grant_id was captured at Step 6 from the create response. +# sam_grant_id was captured at Step 6b from the create response. DELETE {{base_url}}/api/grants/{{sam_grant_id}} Authorization: Bearer {{admin_token}} 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