From b5881c7114a88b6a43cabf55ba07c4ffc8a0b5df Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Mon, 6 Jul 2026 00:38:17 +0200 Subject: [PATCH] feat(calendar,addressbook): prepare authz engine --- ...0000_role_grants_calendar_address_book.sql | 68 +++++++++++++++ src/application/dtos/grant_dto.rs | 8 +- src/application/ports/authorization_ports.rs | 2 + .../services/magic_link_invite_service.rs | 35 +++++--- .../recipient_notification_service.rs | 12 +-- src/domain/services/authorization.rs | 35 +++++--- src/infrastructure/services/pg_acl_engine.rs | 85 ++++++++++++++++++- src/interfaces/api/handlers/grant_handler.rs | 8 ++ 8 files changed, 220 insertions(+), 33 deletions(-) create mode 100644 migrations/20260906000000_role_grants_calendar_address_book.sql diff --git a/migrations/20260906000000_role_grants_calendar_address_book.sql b/migrations/20260906000000_role_grants_calendar_address_book.sql new file mode 100644 index 00000000..d3307338 --- /dev/null +++ b/migrations/20260906000000_role_grants_calendar_address_book.sql @@ -0,0 +1,68 @@ +-- ───────────────────────────────────────────────────────────────────────── +-- Round 3 — admit 'calendar' and 'address_book' into +-- `storage.role_grants.resource_type`. +-- +-- Companion to the domain unblock in +-- `src/domain/services/authorization.rs` (Round 3 Phase 1). The +-- `Resource::Calendar(Uuid)` and `Resource::AddressBook(Uuid)` +-- variants can't be inserted into `role_grants` until the CHECK +-- constraint on `resource_type` permits their string discriminators. +-- +-- CalDAV and CardDAV surfaces have historically enforced access via +-- dedicated per-domain share tables (`caldav.calendar_shares`, +-- `carddav.address_book_shares`) and bespoke `check_calendar_access` +-- / `check_address_book_access` helpers. Round 3 folds both into the +-- unified ReBAC engine so: +-- +-- * A single ACL source of truth (`storage.role_grants`) covers +-- every OxiCloud resource type — files, folders, drives, +-- calendars, address books. +-- * Group subjects become a free feature on calendar/book shares +-- (falls out of `role_grants.subject_type='group'`). +-- * The `authz.require` audit line ("👮🏻‍♂️ perms: ⛔ …") fires on +-- denial with no per-domain retrofit. +-- +-- Migration of existing rows from `caldav.calendar_shares` and +-- `carddav.address_book_shares` into `role_grants` happens in the +-- next migration (Phase 2). The legacy tables stay in place through +-- this PR for rollback safety; they get dropped one release later. + +-- `resource_type` is a TEXT column with a CHECK constraint (not a PG +-- enum), so extending it is a DROP / ADD pair — no `ALTER TYPE` / +-- non-transactional migration issues. + +ALTER TABLE storage.role_grants + DROP CONSTRAINT IF EXISTS role_grants_resource_type_check; + +ALTER TABLE storage.role_grants + ADD CONSTRAINT role_grants_resource_type_check + CHECK (resource_type IN ('folder', 'file', 'drive', 'calendar', 'address_book')); + +-- Post-flight: introspect the live constraint definition and prove +-- both new values appear. Cheap read-only check with no INSERT. +DO $BODY$ +DECLARE + defn TEXT; +BEGIN + SELECT pg_get_constraintdef(c.oid) INTO defn + FROM pg_constraint c + JOIN pg_class t ON t.oid = c.conrelid + JOIN pg_namespace n ON n.oid = t.relnamespace + WHERE n.nspname = 'storage' + AND t.relname = 'role_grants' + AND c.conname = 'role_grants_resource_type_check'; + + IF defn IS NULL THEN + RAISE EXCEPTION + 'role_grants_resource_type_check not found on storage.role_grants'; + END IF; + IF position('calendar' IN defn) = 0 THEN + RAISE EXCEPTION + 'CHECK constraint does not admit ''calendar'': %', defn; + END IF; + IF position('address_book' IN defn) = 0 THEN + RAISE EXCEPTION + 'CHECK constraint does not admit ''address_book'': %', defn; + END IF; +END; +$BODY$; diff --git a/src/application/dtos/grant_dto.rs b/src/application/dtos/grant_dto.rs index f1e7cf93..c1941fa5 100644 --- a/src/application/dtos/grant_dto.rs +++ b/src/application/dtos/grant_dto.rs @@ -55,11 +55,13 @@ impl From for SubjectDto { } #[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] -#[serde(rename_all = "lowercase")] +#[serde(rename_all = "snake_case")] pub enum ResourceTypeDto { Folder, File, Drive, + Calendar, + AddressBook, } #[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] @@ -75,6 +77,8 @@ impl From for Resource { ResourceTypeDto::Folder => Resource::Folder(dto.id), ResourceTypeDto::File => Resource::File(dto.id), ResourceTypeDto::Drive => Resource::Drive(dto.id), + ResourceTypeDto::Calendar => Resource::Calendar(dto.id), + ResourceTypeDto::AddressBook => Resource::AddressBook(dto.id), } } } @@ -85,6 +89,8 @@ impl From for ResourceDto { Resource::Folder(id) => (ResourceTypeDto::Folder, id), Resource::File(id) => (ResourceTypeDto::File, id), Resource::Drive(id) => (ResourceTypeDto::Drive, id), + Resource::Calendar(id) => (ResourceTypeDto::Calendar, id), + Resource::AddressBook(id) => (ResourceTypeDto::AddressBook, id), }; ResourceDto { kind, id } } diff --git a/src/application/ports/authorization_ports.rs b/src/application/ports/authorization_ports.rs index 3c4e456e..6b10a143 100644 --- a/src/application/ports/authorization_ports.rs +++ b/src/application/ports/authorization_ports.rs @@ -62,6 +62,8 @@ pub trait AuthorizationEngine: Send + Sync + 'static { Resource::Folder(id) => ("Folder", id), Resource::File(id) => ("File", id), Resource::Drive(id) => ("Drive", id), + Resource::Calendar(id) => ("Calendar", id), + Resource::AddressBook(id) => ("AddressBook", id), }; // Audit-worthy: denials are the interesting signal. Routed // through the `audit` tracing target so log aggregators can diff --git a/src/application/services/magic_link_invite_service.rs b/src/application/services/magic_link_invite_service.rs index bc244e09..bc858863 100644 --- a/src/application/services/magic_link_invite_service.rs +++ b/src/application/services/magic_link_invite_service.rs @@ -307,20 +307,25 @@ impl MagicLinkInviteService { let (kind, resource_id) = match resource { Resource::Folder(id) => (MagicLinkResourceKind::Folder, id), Resource::File(id) => (MagicLinkResourceKind::File, id), - // Drive sharing — and therefore drive magic-link invitations — - // land in D2. The grant DTOs accept `Resource::Drive` from the - // wire today (see ResourceTypeDto) but no public API path - // actually grants on a drive in D0, so this arm is - // defensively unreachable. Treating it as an audit-logged - // no-op (grant is in place, mail suppressed) matches the - // ineligible-recipient branch above. - Resource::Drive(_) => { + // Drive / Calendar / AddressBook sharing is out-of-band for + // the magic-link flow. Drive shares land through + // `/api/drives/{id}/members`; Calendar / AddressBook shares + // through the Round-3 `/api/(calendars|address-books)/{id}/shares` + // endpoints. The DTOs accept every `Resource` variant on + // the wire (see `ResourceTypeDto`) but only file/folder + // grants trigger an invitation email. Treating the other + // arms as audit-logged suppressed no-ops keeps the grant + // in place while matching the ineligible-recipient branch + // above. + Resource::Drive(_) | Resource::Calendar(_) | Resource::AddressBook(_) => { tracing::info!( target: "audit", event = "magic_link.invitation_suppressed", - reason = "drive_resource_unsupported", + reason = "resource_kind_unsupported", user_id = %recipient.id(), - "📭 magic-link invitation suppressed: drive resources aren't invitable until D2", + resource_kind = %resource.type_str(), + "📭 magic-link invitation suppressed: {} resources aren't invitable via email", + resource.type_str(), ); return Ok(()); } @@ -347,10 +352,12 @@ impl MagicLinkInviteService { Resource::Folder(_) => "server.magic_link.email.kind_folder", Resource::File(_) => "server.magic_link.email.kind_file", // Unreachable — the early-return above exits before we get - // here for a Drive resource. The arm exists only to satisfy - // exhaustiveness; if you find this firing, the early-return - // was bypassed. - Resource::Drive(_) => "server.magic_link.email.kind_folder", + // here for Drive / Calendar / AddressBook resources. The + // arms exist only to satisfy exhaustiveness; if you find + // any firing, the early-return was bypassed. + Resource::Drive(_) | Resource::Calendar(_) | Resource::AddressBook(_) => { + "server.magic_link.email.kind_folder" + } }; // PR C: render in the recipient's preferred locale (set by UI // switcher, OIDC JIT claim, or inviter inheritance at row diff --git a/src/application/services/recipient_notification_service.rs b/src/application/services/recipient_notification_service.rs index b334c8dc..ab4880ee 100644 --- a/src/application/services/recipient_notification_service.rs +++ b/src/application/services/recipient_notification_service.rs @@ -472,11 +472,13 @@ impl RecipientNotificationService { let kind_key = match resource { Resource::Folder(_) => "server.magic_link.email.kind_folder", Resource::File(_) => "server.magic_link.email.kind_file", - // Drives don't generate share notifications in D0 — drive - // sharing lands in D2 and gets its own template key. Fall - // back to the folder label so any path that does reach - // here produces a readable, if generic, mail body. - Resource::Drive(_) => "server.magic_link.email.kind_folder", + // Drive / Calendar / AddressBook shares don't produce + // email notifications through this path. Fall back to the + // folder label so any code that does reach here still + // produces a readable (if generic) mail body. + Resource::Drive(_) | Resource::Calendar(_) | Resource::AddressBook(_) => { + "server.magic_link.email.kind_folder" + } }; let kind_label = self.i18n_or(kind_key, &locale, &[]).await; // Short form for the subject, long form (with email) for the diff --git a/src/domain/services/authorization.rs b/src/domain/services/authorization.rs index 0e9a6a3d..dccdf07c 100644 --- a/src/domain/services/authorization.rs +++ b/src/domain/services/authorization.rs @@ -78,11 +78,20 @@ pub enum Resource { /// membership and policy bag. Added in D0; membership lives in /// `storage.role_grants` (no separate `drive_members` table). Drive(Uuid), - // Reserved for future use: - // Calendar(Uuid), - // Reserved for future use: - // AddressBook(Uuid), - // Reserved for future use: + /// A CalDAV calendar. Membership + sharing lives in + /// `storage.role_grants` with `resource_type='calendar'` — + /// replaces the pre-Round-3 dedicated `caldav.calendar_shares` + /// table and the `check_calendar_access` bespoke helper. No + /// cascade parent (calendars are top-level per user); the engine + /// resolves directly against `role_grants` on the resource. + Calendar(Uuid), + /// A CardDAV address book. Same shape as `Calendar` — + /// `storage.role_grants` with `resource_type='address_book'` + /// replaces `carddav.address_book_shares` and the + /// `check_address_book_access` bespoke helper. + AddressBook(Uuid), + // Reserved for future use — same shape but tracked separately + // (music-service rewrite is its own PR): // Playlist(Uuid), } @@ -92,17 +101,19 @@ impl Resource { Resource::Folder(_) => "folder", Resource::File(_) => "file", Resource::Drive(_) => "drive", - //Resource::Calendar(_) => "calendar", - //Resource::AddressBook(_) => "adressbook", + Resource::Calendar(_) => "calendar", + Resource::AddressBook(_) => "address_book", //Resource::Playlist(_) => "playlist", } } pub fn id(&self) -> Uuid { match self { - Resource::Folder(id) | Resource::File(id) | Resource::Drive(id) => *id, - //| Resource::Calendar(id) - //| Resource::AddressBook(id) + Resource::Folder(id) + | Resource::File(id) + | Resource::Drive(id) + | Resource::Calendar(id) + | Resource::AddressBook(id) => *id, //| Resource::Playlist(id) } } @@ -112,8 +123,8 @@ impl Resource { "folder" => Some(Resource::Folder(id)), "file" => Some(Resource::File(id)), "drive" => Some(Resource::Drive(id)), - //"calendar" => Some(Resource::Calendar(id)), - //"adressbook" => Some(Resource::AddressBook(id)), + "calendar" => Some(Resource::Calendar(id)), + "address_book" => Some(Resource::AddressBook(id)), //"playlist" => Some(Resource::Playlist(id)), _ => None, } diff --git a/src/infrastructure/services/pg_acl_engine.rs b/src/infrastructure/services/pg_acl_engine.rs index 6d346ba4..708f8ba2 100644 --- a/src/infrastructure/services/pg_acl_engine.rs +++ b/src/infrastructure/services/pg_acl_engine.rs @@ -479,11 +479,18 @@ impl PgAclEngine { /// Returns the `drive_id` for a File / Folder. Drives don't have a parent /// drive — this returns `NotFound` for `Resource::Drive` and the caller /// must not invoke it on Drive resources. + /// + /// `Resource::Calendar` and `Resource::AddressBook` are top-level per + /// user with no drive ancestor; they also return `NotFound` and the + /// engine short-circuits to a direct `role_grants` lookup (no drive + /// precheck applies). async fn drive_of(&self, resource: Resource) -> Result { match resource { Resource::Folder(id) => self.folder_repo.get_folder_drive_id(&id.to_string()).await, Resource::File(id) => self.file_repo.get_file_drive_id(&id.to_string()).await, - Resource::Drive(_) => Err(DomainError::not_found("Drive", resource.id().to_string())), + Resource::Drive(_) | Resource::Calendar(_) | Resource::AddressBook(_) => Err( + DomainError::not_found(resource.type_str(), resource.id().to_string()), + ), } } @@ -519,6 +526,49 @@ impl PgAclEngine { /// permission — see `roles_implying()`. /// /// Uses the GiST index on `storage.folders.lpath` for O(log N) cascade. + /// Direct grant lookup with no cascade — used for top-level + /// resources whose ACL lives entirely on their own row + /// (`Resource::Calendar`, `Resource::AddressBook`). Same + /// role-array + subject-set shape as the cascade helpers so a + /// caller's group memberships still resolve, but no ltree / + /// folder ancestry / drive precheck applies. Calendars and + /// address books have no parent to inherit from. + async fn direct_grant_exists( + &self, + subject_types: &[&str], + subject_ids: &[Uuid], + permission: Permission, + resource_type: &'static str, + resource_id: Uuid, + counters: &QueryCounters, + ) -> Result { + counters.sql_queries.fetch_add(1, Ordering::Relaxed); + let roles = Self::roles_implying_strings(permission); + let exists: Option = sqlx::query_scalar( + r#" + SELECT 1 + FROM storage.role_grants g + WHERE g.subject_type = ANY($1) + AND g.subject_id = ANY($2) + AND g.role = ANY($3::storage.grant_role[]) + AND g.resource_type = $4 + AND g.resource_id = $5 + AND (g.expires_at IS NULL OR g.expires_at > NOW()) + LIMIT 1 + "#, + ) + .bind(subject_types) + .bind(subject_ids) + .bind(&roles) + .bind(resource_type) + .bind(resource_id) + .fetch_optional(self.pool.as_ref()) + .await + .map_err(|e| DomainError::internal_error("PgAcl", format!("direct grant: {e}")))?; + + Ok(exists.is_some()) + } + async fn folder_cascade_grant_exists( &self, subject_types: &[&str], @@ -819,6 +869,39 @@ impl PgAclEngine { .await? .is_some_and(|r| r.expand().contains(&permission))) } + // Top-level resources with no cascade parent — the ACL + // lives entirely on their own `role_grants` rows. Owner is + // an explicit grant seeded at MKCALENDAR / address-book + // create time (Round 3 phase 2 migration), so the common + // "owner accessing their own calendar" case is one SQL + // round-trip — no drive_role_cache short-circuit (no + // drive), no cascade. + Resource::Calendar(id) => { + let (subject_types, subject_ids) = + self.subject_match_set(subject, counters).await?; + self.direct_grant_exists( + &subject_types, + &subject_ids, + permission, + "calendar", + id, + counters, + ) + .await + } + Resource::AddressBook(id) => { + let (subject_types, subject_ids) = + self.subject_match_set(subject, counters).await?; + self.direct_grant_exists( + &subject_types, + &subject_ids, + permission, + "address_book", + id, + counters, + ) + .await + } } } } diff --git a/src/interfaces/api/handlers/grant_handler.rs b/src/interfaces/api/handlers/grant_handler.rs index bf6d190c..07d493ca 100644 --- a/src/interfaces/api/handlers/grant_handler.rs +++ b/src/interfaces/api/handlers/grant_handler.rs @@ -113,6 +113,14 @@ pub async fn create_grant( .get_by_id(id) .await .map(|d| d.drive.typed_policies()), + // Calendars and address books live outside the drive + // hierarchy (top-level per user), so no drive-level policy + // gates apply. If per-calendar / per-address-book policies + // ever ship, they'll live on the resource itself, not on a + // drive; the default-empty bag is the right no-op here. + Resource::Calendar(_) | Resource::AddressBook(_) => { + Ok(crate::domain::entities::drive::DrivePolicies::default()) + } }; let drive_policies = match drive_policies { Ok(p) => p,