From 936c0de7fe5d9fa113b86ce5b5c84c010dbc7696 Mon Sep 17 00:00:00 2001 From: Philipinho <16838612+Philipinho@users.noreply.github.com> Date: Tue, 28 Apr 2026 19:21:46 +0100 Subject: [PATCH] refactor(base): drop SpaceCaslSubject.Base, route base permissions through Page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bases are pages (isBase=true) and the casl rules already granted the exact same Manage/Read level on the Base subject as on Page for every space role (admin, writer, reader). The `Base` subject was therefore pure duplication: any caller that needed to check base access either went through pageAccessService (which uses Page internally) or did a direct Page-equivalent ability.cannot(..., Base) check that produced the same outcome as Page would have. Drop SpaceCaslSubject.Base entirely — server enum, server union, server factory rules, client enum, client union — and switch the two remaining direct callers to Page: - base.controller.ts `create` and list checks now use Page (matching page.controller.ts's create/list). - base-table.tsx's `canSave` now reads Page edit ability. Net effect: one source of truth for "can this user view/edit/manage content in this space," whether the content is a regular page or a base. Existing role assignments behave identically; no migration needed because permissions are computed per-request from the role, not stored. --- apps/client/src/features/base/components/base-table.tsx | 4 +++- .../src/features/space/permissions/permissions.type.ts | 7 ++++--- apps/server/src/core/base/controllers/base.controller.ts | 9 +++++++-- .../src/core/casl/abilities/space-ability.factory.ts | 3 --- .../src/core/casl/interfaces/space-ability.type.ts | 9 ++++++--- 5 files changed, 20 insertions(+), 12 deletions(-) diff --git a/apps/client/src/features/base/components/base-table.tsx b/apps/client/src/features/base/components/base-table.tsx index 393e38121..b7db5bd53 100644 --- a/apps/client/src/features/base/components/base-table.tsx +++ b/apps/client/src/features/base/components/base-table.tsx @@ -109,9 +109,11 @@ export function BaseTable({ pageId, embedded }: BaseTableProps) { // use-history-restore.tsx for the same pattern. const { data: space } = useSpaceQuery(base?.spaceId ?? ""); const spaceAbility = useSpaceAbility(space?.membership?.permissions); + // Bases are pages — gate save on the same Page subject the rest of + // the app uses; the dedicated Base subject was redundant. const canSave = spaceAbility.can( SpaceCaslAction.Edit, - SpaceCaslSubject.Base, + SpaceCaslSubject.Page, ); // Hold the rows query until `base` has loaded. Otherwise the query diff --git a/apps/client/src/features/space/permissions/permissions.type.ts b/apps/client/src/features/space/permissions/permissions.type.ts index 44a21ef43..cf5dbc355 100644 --- a/apps/client/src/features/space/permissions/permissions.type.ts +++ b/apps/client/src/features/space/permissions/permissions.type.ts @@ -9,11 +9,12 @@ export enum SpaceCaslSubject { Settings = "settings", Member = "member", Page = "page", - Base = "base", } +// Bases are pages and inherit Page permissions — a separate Base +// subject was redundant and has been dropped from the server's casl +// rules too. Anything that used to check Base now checks Page. export type SpaceAbility = | [SpaceCaslAction, SpaceCaslSubject.Settings] | [SpaceCaslAction, SpaceCaslSubject.Member] - | [SpaceCaslAction, SpaceCaslSubject.Page] - | [SpaceCaslAction, SpaceCaslSubject.Base]; + | [SpaceCaslAction, SpaceCaslSubject.Page]; diff --git a/apps/server/src/core/base/controllers/base.controller.ts b/apps/server/src/core/base/controllers/base.controller.ts index 83b79572d..072bafb9d 100644 --- a/apps/server/src/core/base/controllers/base.controller.ts +++ b/apps/server/src/core/base/controllers/base.controller.ts @@ -56,8 +56,11 @@ export class BaseController { @AuthUser() user: User, @AuthWorkspace() workspace: Workspace, ) { + // Bases are pages — use the same SpaceCaslSubject.Page check the + // page controller uses for its own create endpoint, so a single + // role definition (admin/writer/reader) governs both. const ability = await this.spaceAbility.createForUser(user, dto.spaceId); - if (ability.cannot(SpaceCaslAction.Create, SpaceCaslSubject.Base)) { + if (ability.cannot(SpaceCaslAction.Create, SpaceCaslSubject.Page)) { throw new ForbiddenException(); } @@ -110,8 +113,10 @@ export class BaseController { @Body() pagination: PaginationOptions, @AuthUser() user: User, ) { + // Same Page-subject check the page controller's list-equivalents + // use; a base is a page (isBase=true) so reader access aligns. const ability = await this.spaceAbility.createForUser(user, dto.spaceId); - if (ability.cannot(SpaceCaslAction.Read, SpaceCaslSubject.Base)) { + if (ability.cannot(SpaceCaslAction.Read, SpaceCaslSubject.Page)) { throw new ForbiddenException(); } diff --git a/apps/server/src/core/casl/abilities/space-ability.factory.ts b/apps/server/src/core/casl/abilities/space-ability.factory.ts index 6837fb945..53a57a0c2 100644 --- a/apps/server/src/core/casl/abilities/space-ability.factory.ts +++ b/apps/server/src/core/casl/abilities/space-ability.factory.ts @@ -46,7 +46,6 @@ function buildSpaceAdminAbility() { can(SpaceCaslAction.Manage, SpaceCaslSubject.Member); can(SpaceCaslAction.Manage, SpaceCaslSubject.Page); can(SpaceCaslAction.Manage, SpaceCaslSubject.Share); - can(SpaceCaslAction.Manage, SpaceCaslSubject.Base); return build(); } @@ -58,7 +57,6 @@ function buildSpaceWriterAbility() { can(SpaceCaslAction.Read, SpaceCaslSubject.Member); can(SpaceCaslAction.Manage, SpaceCaslSubject.Page); can(SpaceCaslAction.Manage, SpaceCaslSubject.Share); - can(SpaceCaslAction.Manage, SpaceCaslSubject.Base); return build(); } @@ -70,6 +68,5 @@ function buildSpaceReaderAbility() { can(SpaceCaslAction.Read, SpaceCaslSubject.Member); can(SpaceCaslAction.Read, SpaceCaslSubject.Page); can(SpaceCaslAction.Read, SpaceCaslSubject.Share); - can(SpaceCaslAction.Read, SpaceCaslSubject.Base); return build(); } diff --git a/apps/server/src/core/casl/interfaces/space-ability.type.ts b/apps/server/src/core/casl/interfaces/space-ability.type.ts index aa32648ca..72ec42c6c 100644 --- a/apps/server/src/core/casl/interfaces/space-ability.type.ts +++ b/apps/server/src/core/casl/interfaces/space-ability.type.ts @@ -10,12 +10,15 @@ export enum SpaceCaslSubject { Member = 'member', Page = 'page', Share = 'share', - Base = 'base', } +// Bases are pages (isBase=true) and inherit Page permissions. There +// used to be a separate `Base` subject here; it duplicated Page in +// every role's casl rules, so callers that needed to check base +// access just used Page (via pageAccessService) anyway. Dropped to +// keep one source of truth. export type ISpaceAbility = | [SpaceCaslAction, SpaceCaslSubject.Settings] | [SpaceCaslAction, SpaceCaslSubject.Member] | [SpaceCaslAction, SpaceCaslSubject.Page] - | [SpaceCaslAction, SpaceCaslSubject.Share] - | [SpaceCaslAction, SpaceCaslSubject.Base]; + | [SpaceCaslAction, SpaceCaslSubject.Share];