refactoring

This commit is contained in:
Philipinho
2026-02-22 06:48:54 +00:00
parent 9d154bf1f7
commit a2ec313878
4 changed files with 200 additions and 120 deletions
@@ -72,7 +72,7 @@ export class AuthenticationExtension implements Extension {
// Check page-level permissions // Check page-level permissions
const { hasAnyRestriction, canAccess, canEdit } = const { hasAnyRestriction, canAccess, canEdit } =
await this.pagePermissionRepo.getUserPageAccessLevel(user.id, page.id); await this.pagePermissionRepo.canUserEditPage(user.id, page.id);
if (hasAnyRestriction) { if (hasAnyRestriction) {
if (!canAccess) { if (!canAccess) {
@@ -28,16 +28,13 @@ export class PageAccessService {
throw new ForbiddenException(); throw new ForbiddenException();
} }
const { hasAnyRestriction, canAccess } = const canAccess = await this.pagePermissionRepo.canUserAccessPage(
await this.pagePermissionRepo.getUserPageAccessLevel(user.id, page.id); user.id,
page.id,
if (hasAnyRestriction) { );
// Page has restrictions - use page-level permission if (!canAccess) {
if (!canAccess) { throw new ForbiddenException();
throw new ForbiddenException();
}
} }
// No restriction - space membership (checked above) is sufficient for view
} }
/** /**
@@ -54,7 +51,7 @@ export class PageAccessService {
} }
const { hasAnyRestriction, canEdit } = const { hasAnyRestriction, canEdit } =
await this.pagePermissionRepo.getUserPageAccessLevel(user.id, page.id); await this.pagePermissionRepo.canUserEditPage(user.id, page.id);
if (hasAnyRestriction) { if (hasAnyRestriction) {
// Page has restrictions - use page-level permission // Page has restrictions - use page-level permission
@@ -229,27 +229,34 @@ export class PagePermissionService {
const userIds = dto.userIds ?? []; const userIds = dto.userIds ?? [];
const groupIds = dto.groupIds ?? []; const groupIds = dto.groupIds ?? [];
if (userIds.length > 0) { await executeTx(this.db, async (trx) => {
await this.pagePermissionRepo.deletePagePermissionsByUserIds( if (userIds.length > 0) {
pageAccess.id, await this.pagePermissionRepo.deletePagePermissionsByUserIds(
userIds, pageAccess.id,
); userIds,
} trx,
);
}
if (groupIds.length > 0) { if (groupIds.length > 0) {
await this.pagePermissionRepo.deletePagePermissionsByGroupIds( await this.pagePermissionRepo.deletePagePermissionsByGroupIds(
pageAccess.id, pageAccess.id,
groupIds, groupIds,
); trx,
} );
}
const writerCount = const writerCount =
await this.pagePermissionRepo.countWritersByPageAccessId(pageAccess.id); await this.pagePermissionRepo.countWritersByPageAccessId(
if (writerCount < 1) { pageAccess.id,
throw new BadRequestException( trx,
'There must be at least one user with "Can edit" permission', );
); if (writerCount < 1) {
} throw new BadRequestException(
'There must be at least one user with "Can edit" permission',
);
}
});
} }
async updatePagePermissionRole( async updatePagePermissionRole(
@@ -486,7 +493,10 @@ export class PagePermissionService {
throw new ForbiddenException(); throw new ForbiddenException();
} }
const canEdit = await this.canEditPage(user.id, page.id); const { canAccess, canEdit } = await this.canEditPage(user.id, page.id);
if (!canAccess) {
throw new ForbiddenException();
}
if (canEdit) { if (canEdit) {
return; return;
} }
@@ -517,18 +527,22 @@ export class PagePermissionService {
} }
/** /**
* Check if user can edit a page. * Check if user can edit a page based on page-level permissions.
* User must have WRITER permission on EVERY restricted ancestor. * Returns { hasAnyRestriction, canAccess, canEdit } from the nearest restricted ancestor logic.
* Returns true if:
* - No ancestors are restricted (defer to space permission)
* - User has writer permission on all restricted ancestors
*/ */
async canEditPage(userId: string, pageId: string): Promise<boolean> { async canEditPage(
userId: string,
pageId: string,
): Promise<{
hasAnyRestriction: boolean;
canAccess: boolean;
canEdit: boolean;
}> {
return this.pagePermissionRepo.canUserEditPage(userId, pageId); return this.pagePermissionRepo.canUserEditPage(userId, pageId);
} }
/** /**
* Check if user has writer permission on ALL restricted ancestors of a page. * Check if user has writer permission on the nearest restricted ancestor.
* Used for permission management operations. * Used for permission management operations.
*/ */
async hasWritePermission(userId: string, pageId: string): Promise<boolean> { async hasWritePermission(userId: string, pageId: string): Promise<boolean> {
@@ -539,7 +553,11 @@ export class PagePermissionService {
return false; // no restrictions, defer to space permissions return false; // no restrictions, defer to space permissions
} }
return this.pagePermissionRepo.canUserEditPage(userId, pageId); const { canEdit } = await this.pagePermissionRepo.canUserEditPage(
userId,
pageId,
);
return canEdit;
} }
async hasPageAccess(pageId: string): Promise<boolean> { async hasPageAccess(pageId: string): Promise<boolean> {
@@ -393,54 +393,60 @@ export class PagePermissionRepo {
} }
/** /**
* Check if user can edit a page by verifying they have WRITER permission on ALL restricted ancestors. * Check if user can edit a page.
* Single query: builds ancestor chain once, checks both traversal and nearest-restricted writer.
* - bool_and(pp.id IS NOT NULL): false if any restricted ancestor has no permission (traversal denied)
* - array_agg(role ORDER BY depth)[1]: role on the nearest restricted ancestor
* - Zero rows (no restricted ancestors): both NULL → defer to space permissions (true)
*/ */
async canUserEditPage(userId: string, pageId: string): Promise<boolean> { async canUserEditPage(
const deniedAncestor = await this.db userId: string,
.withRecursive('ancestors', (qb) => pageId: string,
qb ): Promise<{ hasAnyRestriction: boolean; canAccess: boolean; canEdit: boolean }> {
.selectFrom('pages') const result = await sql<{ canAccess: boolean | null; canEdit: boolean | null }>`
.select(['pages.id as ancestorId', 'pages.parentPageId']) WITH RECURSIVE ancestors AS (
.where('pages.id', '=', pageId) SELECT id AS ancestor_id, parent_page_id, 0 AS depth
.unionAll((eb) => FROM pages
eb WHERE id = ${pageId}::uuid
.selectFrom('pages') UNION ALL
.innerJoin('ancestors', 'ancestors.parentPageId', 'pages.id') SELECT p.id, p.parent_page_id, a.depth + 1
.select(['pages.id as ancestorId', 'pages.parentPageId']), FROM pages p
), JOIN ancestors a ON a.parent_page_id = p.id
) )
.selectFrom('ancestors') SELECT
.innerJoin('pageAccess', 'pageAccess.pageId', 'ancestors.ancestorId') bool_and(pp.id IS NOT NULL) AS "canAccess",
.leftJoin('pagePermissions', (join) => -- nearest restricted ancestor's highest role wins (DESC: 'writer' > 'reader', NULLS LAST: no-permission after real roles)
join (array_agg(pp.role ORDER BY a.depth ASC, pp.role DESC NULLS LAST))[1] = 'writer' AS "canEdit"
.onRef('pagePermissions.pageAccessId', '=', 'pageAccess.id') FROM ancestors a
.on('pagePermissions.role', '=', 'writer') JOIN page_access pa ON pa.page_id = a.ancestor_id
.on((eb) => LEFT JOIN page_permissions pp ON pp.page_access_id = pa.id
eb.or([ AND (
eb('pagePermissions.userId', '=', userId), pp.user_id = ${userId}::uuid
eb( OR pp.group_id IN (
'pagePermissions.groupId', SELECT gu.group_id FROM group_users gu WHERE gu.user_id = ${userId}::uuid
'in', )
this.userGroupIdsSubquery(eb, userId), )
), `.execute(this.db);
]),
),
)
.select('pageAccess.pageId')
.where('pagePermissions.id', 'is', null)
.executeTakeFirst();
return !deniedAncestor; const row = result.rows[0];
if (!row || row.canAccess === null) {
return { hasAnyRestriction: false, canAccess: true, canEdit: true };
}
return {
hasAnyRestriction: true,
canAccess: row.canAccess,
canEdit: row.canAccess && (row.canEdit ?? false),
};
} }
/** /**
* Get user's access level for a page, checking ALL restricted ancestors. * Get user's access level for a page.
* Returns: * Returns:
* - hasDirectRestriction: whether this specific page has restrictions * - hasDirectRestriction: whether this specific page has restrictions
* - hasInheritedRestriction: whether any ancestor (not self) has restrictions * - hasInheritedRestriction: whether any ancestor (not self) has restrictions
* - hasAnyRestriction: hasDirectRestriction || hasInheritedRestriction * - hasAnyRestriction: hasDirectRestriction || hasInheritedRestriction
* - canAccess: user has permission on all restricted ancestors (always true if no restrictions) * - canAccess: user has permission on all restricted ancestors (always true if no restrictions)
* - canEdit: user has writer permission on all restricted ancestors (always true if no restrictions) * - canEdit: user has writer on nearest restricted ancestor (always true if no restrictions)
*/ */
async getUserPageAccessLevel( async getUserPageAccessLevel(
userId: string, userId: string,
@@ -550,9 +556,43 @@ export class PagePermissionRepo {
.else(false) .else(false)
.end() .end()
.as('canAccess'), .as('canAccess'),
// canEdit: no restricted ancestor without WRITER permission // canEdit: nearest restricted ancestor determines edit capability
eb eb
.case() .case()
// traversal denied: any restricted ancestor without any permission
.when(
eb.exists(
eb
.selectFrom('ancestors')
.innerJoin(
'pageAccess',
'pageAccess.pageId',
'ancestors.ancestorId',
)
.leftJoin('pagePermissions', (join) =>
join
.onRef(
'pagePermissions.pageAccessId',
'=',
'pageAccess.id',
)
.on((eb2) =>
eb2.or([
eb2('pagePermissions.userId', '=', userId),
eb2(
'pagePermissions.groupId',
'in',
this.userGroupIdsSubquery(eb2, userId),
),
]),
),
)
.select('pageAccess.pageId')
.where('pagePermissions.id', 'is', null),
),
)
.then(false)
// no restricted ancestors at all → defer to space permissions
.when( .when(
eb.not( eb.not(
eb.exists( eb.exists(
@@ -563,31 +603,41 @@ export class PagePermissionRepo {
'pageAccess.pageId', 'pageAccess.pageId',
'ancestors.ancestorId', 'ancestors.ancestorId',
) )
.leftJoin('pagePermissions', (join) => .select('pageAccess.id'),
join
.onRef(
'pagePermissions.pageAccessId',
'=',
'pageAccess.id',
)
.on('pagePermissions.role', '=', 'writer')
.on((eb2) =>
eb2.or([
eb2('pagePermissions.userId', '=', userId),
eb2(
'pagePermissions.groupId',
'in',
this.userGroupIdsSubquery(eb2, userId),
),
]),
),
)
.select('pageAccess.pageId')
.where('pagePermissions.id', 'is', null),
), ),
), ),
) )
.then(true) .then(true)
// nearest restricted ancestor has writer for this user
.when(
eb.exists(
eb
.selectFrom('pagePermissions')
.select('pagePermissions.id')
.where('pagePermissions.role', '=', 'writer')
.where(
'pagePermissions.pageAccessId',
'=',
sql<string>`(
SELECT pa.id FROM ancestors a_nr
JOIN page_access pa ON pa.page_id = a_nr.ancestor_id
ORDER BY a_nr.depth ASC
LIMIT 1
)`,
)
.where((eb2) =>
eb2.or([
eb2('pagePermissions.userId', '=', userId),
eb2(
'pagePermissions.groupId',
'in',
this.userGroupIdsSubquery(eb2, userId),
),
]),
),
),
)
.then(true)
.else(false) .else(false)
.end() .end()
.as('canEdit'), .as('canEdit'),
@@ -703,6 +753,7 @@ export class PagePermissionRepo {
'pages.id as pageId', 'pages.id as pageId',
'pages.id as ancestorId', 'pages.id as ancestorId',
'pages.parentPageId', 'pages.parentPageId',
sql<number>`0`.as('depth'),
]) ])
.where(sql<SqlBool>`pages.id = ANY(${pageIds}::uuid[])`) .where(sql<SqlBool>`pages.id = ANY(${pageIds}::uuid[])`)
.unionAll((eb) => .unionAll((eb) =>
@@ -717,6 +768,7 @@ export class PagePermissionRepo {
'allAncestors.pageId', 'allAncestors.pageId',
'pages.id as ancestorId', 'pages.id as ancestorId',
'pages.parentPageId', 'pages.parentPageId',
sql<number>`"allAncestors".depth + 1`.as('depth'),
]), ]),
), ),
) )
@@ -725,6 +777,7 @@ export class PagePermissionRepo {
.select((eb) => .select((eb) =>
eb eb
.case() .case()
// no restricted ancestors for this page → defer to space
.when( .when(
eb.not( eb.not(
eb.exists( eb.exists(
@@ -735,37 +788,49 @@ export class PagePermissionRepo {
'pageAccess.pageId', 'pageAccess.pageId',
'allAncestors.ancestorId', 'allAncestors.ancestorId',
) )
.leftJoin('pagePermissions', (join) => .select('pageAccess.id')
join .whereRef('allAncestors.pageId', '=', 'pages.id'),
.onRef(
'pagePermissions.pageAccessId',
'=',
'pageAccess.id',
)
.on('pagePermissions.role', '=', 'writer')
.on((eb2) =>
eb2.or([
eb2('pagePermissions.userId', '=', userId),
eb2(
'pagePermissions.groupId',
'in',
this.userGroupIdsSubquery(eb2, userId),
),
]),
),
)
.select('pageAccess.pageId')
.whereRef('allAncestors.pageId', '=', 'pages.id')
.where('pagePermissions.id', 'is', null),
), ),
), ),
) )
.then(true) .then(true)
// nearest restricted ancestor has writer for this user
.when(
eb.exists(
eb
.selectFrom('pagePermissions')
.select('pagePermissions.id')
.where('pagePermissions.role', '=', 'writer')
.where(
'pagePermissions.pageAccessId',
'=',
sql<string>`(
SELECT pa.id FROM "allAncestors" aa
JOIN page_access pa ON pa.page_id = aa.ancestor_id
WHERE aa.page_id = pages.id
ORDER BY aa.depth ASC
LIMIT 1
)`,
)
.where((eb2) =>
eb2.or([
eb2('pagePermissions.userId', '=', userId),
eb2(
'pagePermissions.groupId',
'in',
this.userGroupIdsSubquery(eb2, userId),
),
]),
),
),
)
.then(true)
.else(false) .else(false)
.end() .end()
.as('canEdit'), .as('canEdit'),
) )
.where(sql<SqlBool>`pages.id = ANY(${pageIds}::uuid[])`) .where(sql<SqlBool>`pages.id = ANY(${pageIds}::uuid[])`)
// view filter: no restricted ancestor without any permission
.where(({ not, exists, selectFrom }) => .where(({ not, exists, selectFrom }) =>
not( not(
exists( exists(