single query check

This commit is contained in:
Philipinho
2026-01-07 18:58:34 +00:00
parent 56c1cfe7a9
commit 4c635b4faf
4 changed files with 52 additions and 26 deletions
@@ -71,10 +71,10 @@ export class AuthenticationExtension implements Extension {
} }
// Check page-level permissions // Check page-level permissions
const { hasRestriction, canAccess, canEdit } = const { hasAnyRestriction, canAccess, canEdit } =
await this.pagePermissionRepo.getUserPageAccessLevel(user.id, page.id); await this.pagePermissionRepo.getUserPageAccessLevel(user.id, page.id);
if (hasRestriction) { if (hasAnyRestriction) {
if (!canAccess) { if (!canAccess) {
this.logger.warn( this.logger.warn(
`User ${user.id} denied page-level access to page: ${pageId}`, `User ${user.id} denied page-level access to page: ${pageId}`,
@@ -28,10 +28,10 @@ export class PageAccessService {
throw new ForbiddenException(); throw new ForbiddenException();
} }
const { hasRestriction, canAccess } = const { hasAnyRestriction, canAccess } =
await this.pagePermissionRepo.getUserPageAccessLevel(user.id, page.id); await this.pagePermissionRepo.getUserPageAccessLevel(user.id, page.id);
if (hasRestriction) { if (hasAnyRestriction) {
// Page has restrictions - use page-level permission // Page has restrictions - use page-level permission
if (!canAccess) { if (!canAccess) {
throw new ForbiddenException(); throw new ForbiddenException();
@@ -53,10 +53,10 @@ export class PageAccessService {
throw new ForbiddenException(); throw new ForbiddenException();
} }
const { hasRestriction, canEdit } = const { hasAnyRestriction, canEdit } =
await this.pagePermissionRepo.getUserPageAccessLevel(user.id, page.id); await this.pagePermissionRepo.getUserPageAccessLevel(user.id, page.id);
if (hasRestriction) { if (hasAnyRestriction) {
// Page has restrictions - use page-level permission // Page has restrictions - use page-level permission
if (!canEdit) { if (!canEdit) {
throw new ForbiddenException(); throw new ForbiddenException();
@@ -359,6 +359,8 @@ export class PagePermissionService {
* *
* Security: User must be a space member. Returns 404 for pages the user cannot view * Security: User must be a space member. Returns 404 for pages the user cannot view
* to avoid leaking existence of restricted pages. * to avoid leaking existence of restricted pages.
*
* Performance: Uses single optimized query to get all restriction/access data.
*/ */
async getPageRestrictionInfo( async getPageRestrictionInfo(
pageId: string, pageId: string,
@@ -378,23 +380,22 @@ export class PagePermissionService {
throw new ForbiddenException(); throw new ForbiddenException();
} }
const [hasDirectRestriction, hasAnyRestriction, canView, canEdit] = const {
await Promise.all([ hasDirectRestriction,
this.pagePermissionRepo.findPageAccessByPageId(pageId).then((r) => !!r), hasInheritedRestriction,
this.pagePermissionRepo.hasRestrictedAncestor(pageId), canAccess,
this.canViewPage(authUser.id, pageId), canEdit,
this.canEditPage(authUser.id, pageId), } = await this.pagePermissionRepo.getUserPageAccessLevel(
]); authUser.id,
pageId,
);
// Security: return 404 to avoid leaking existence of restricted pages // Security: return 404 to avoid leaking existence of restricted pages
if (!canView) { if (!canAccess) {
throw new NotFoundException('Page not found'); throw new NotFoundException('Permission not found');
} }
const hasInheritedRestriction = hasAnyRestriction && !hasDirectRestriction; const canManage = this.computeCanManage(ability, canEdit, canAccess);
// Determine if user can manage permissions
const canManage = this.computeCanManage(ability, canEdit, canView);
return { return {
id: page.id, id: page.id,
@@ -402,7 +403,7 @@ export class PagePermissionService {
hasDirectRestriction, hasDirectRestriction,
hasInheritedRestriction, hasInheritedRestriction,
userAccess: { userAccess: {
canView, canView: canAccess,
canEdit, canEdit,
canManage, canManage,
}, },
@@ -380,7 +380,9 @@ export class PagePermissionRepo {
/** /**
* Get user's access level for a page, checking ALL restricted ancestors. * Get user's access level for a page, checking ALL restricted ancestors.
* Returns: * Returns:
* - hasRestriction: whether page or any ancestor has restrictions * - hasDirectRestriction: whether this specific page has restrictions
* - hasInheritedRestriction: whether any ancestor (not self) has restrictions
* - 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 permission on all restricted ancestors (always true if no restrictions)
*/ */
@@ -388,14 +390,31 @@ export class PagePermissionRepo {
userId: string, userId: string,
pageId: string, pageId: string,
): Promise<{ ): Promise<{
hasRestriction: boolean; hasDirectRestriction: boolean;
hasInheritedRestriction: boolean;
hasAnyRestriction: boolean;
canAccess: boolean; canAccess: boolean;
canEdit: boolean; canEdit: boolean;
}> { }> {
const result = await this.db const result = await this.db
.selectFrom('pages') .selectFrom('pages')
.select((eb) => [ .select((eb) => [
// hasRestriction: any ancestor has page_access entry // hasDirectRestriction: this page itself has page_access entry
eb
.case()
.when(
eb.exists(
eb
.selectFrom('pageAccess')
.select('pageAccess.id')
.whereRef('pageAccess.pageId', '=', 'pages.id'),
),
)
.then(true)
.else(false)
.end()
.as('hasDirectRestriction'),
// hasInheritedRestriction: any ancestor (depth > 0) has page_access entry
eb eb
.case() .case()
.when( .when(
@@ -408,13 +427,14 @@ export class PagePermissionRepo {
'pageHierarchy.ancestorId', 'pageHierarchy.ancestorId',
) )
.select('pageAccess.id') .select('pageAccess.id')
.whereRef('pageHierarchy.descendantId', '=', 'pages.id'), .whereRef('pageHierarchy.descendantId', '=', 'pages.id')
.where('pageHierarchy.depth', '>', 0),
), ),
) )
.then(true) .then(true)
.else(false) .else(false)
.end() .end()
.as('hasRestriction'), .as('hasInheritedRestriction'),
// canAccess: no restricted ancestor without ANY permission // canAccess: no restricted ancestor without ANY permission
eb eb
.case() .case()
@@ -508,8 +528,13 @@ export class PagePermissionRepo {
.where('pages.id', '=', pageId) .where('pages.id', '=', pageId)
.executeTakeFirst(); .executeTakeFirst();
const hasDirectRestriction = Boolean(result?.hasDirectRestriction);
const hasInheritedRestriction = Boolean(result?.hasInheritedRestriction);
return { return {
hasRestriction: Boolean(result?.hasRestriction), hasDirectRestriction,
hasInheritedRestriction,
hasAnyRestriction: hasDirectRestriction || hasInheritedRestriction,
canAccess: Boolean(result?.canAccess), canAccess: Boolean(result?.canAccess),
canEdit: Boolean(result?.canEdit), canEdit: Boolean(result?.canEdit),
}; };