diff --git a/apps/server/src/common/helpers/cache-keys.ts b/apps/server/src/common/helpers/cache-keys.ts index 570c96d88..38b24d20e 100644 --- a/apps/server/src/common/helpers/cache-keys.ts +++ b/apps/server/src/common/helpers/cache-keys.ts @@ -1,3 +1,11 @@ export const CacheKey = { LICENSE_VALID: (workspaceId: string) => `license:valid:${workspaceId}`, + SPACE_ROLES: (userId: string, spaceId: string) => + `perm:space-roles:${userId}:${spaceId}`, + PAGE_CAN_EDIT: (userId: string, pageId: string) => + `perm:can-edit:${userId}:${pageId}`, }; + +// Permission caches dedupe repeated checks within and across short request bursts. +// 5s keeps staleness on revocations bounded. +export const PERMISSION_CACHE_TTL_MS = 5_000; diff --git a/apps/server/src/common/helpers/with-cache.ts b/apps/server/src/common/helpers/with-cache.ts new file mode 100644 index 000000000..2db1d3c02 --- /dev/null +++ b/apps/server/src/common/helpers/with-cache.ts @@ -0,0 +1,27 @@ +import { Cache } from 'cache-manager'; + +export async function withCache( + cacheManager: Cache, + key: string, + ttlMs: number, + fn: () => Promise, +): Promise { + try { + const cached = await cacheManager.get<{ v: T }>(key); + if (cached !== undefined && cached !== null) { + return cached.v; + } + } catch (err) { + console.warn(`[withCache] get failed for "${key}", falling back to source`, err); + } + + const value = await fn(); + + try { + await cacheManager.set(key, { v: value }, ttlMs); + } catch (err) { + console.warn(`[withCache] set failed for "${key}"`, err); + } + + return value; +} diff --git a/apps/server/src/database/repos/page/page-permission.repo.ts b/apps/server/src/database/repos/page/page-permission.repo.ts index fbd7423ce..f753526cd 100644 --- a/apps/server/src/database/repos/page/page-permission.repo.ts +++ b/apps/server/src/database/repos/page/page-permission.repo.ts @@ -1,4 +1,6 @@ -import { Injectable } from '@nestjs/common'; +import { Inject, Injectable } from '@nestjs/common'; +import { CACHE_MANAGER } from '@nestjs/cache-manager'; +import { Cache } from 'cache-manager'; import { InjectKysely } from 'nestjs-kysely'; import { KyselyDB, KyselyTransaction } from '@docmost/db/types/kysely.types'; import { dbOrTx } from '@docmost/db/utils'; @@ -17,6 +19,11 @@ import { executeWithCursorPagination, } from '@docmost/db/pagination/cursor-pagination'; import { PagePermissionMember } from './types/page-permission.types'; +import { withCache } from '../../../common/helpers/with-cache'; +import { + CacheKey, + PERMISSION_CACHE_TTL_MS, +} from '../../../common/helpers/cache-keys'; export { PagePermissionMember } from './types/page-permission.types'; @@ -25,6 +32,7 @@ export class PagePermissionRepo { constructor( @InjectKysely() private readonly db: KyselyDB, private readonly groupRepo: GroupRepo, + @Inject(CACHE_MANAGER) private readonly cacheManager: Cache, ) {} async findPageAccessByPageId( @@ -361,40 +369,8 @@ export class PagePermissionRepo { * Check if user can access a page by verifying they have permission on ALL restricted ancestors. */ async canUserAccessPage(userId: string, pageId: string): Promise { - const deniedAncestor = await this.db - .withRecursive('ancestors', (qb) => - qb - .selectFrom('pages') - .select(['pages.id as ancestorId', 'pages.parentPageId']) - .where('pages.id', '=', pageId) - .unionAll((eb) => - eb - .selectFrom('pages') - .innerJoin('ancestors', 'ancestors.parentPageId', 'pages.id') - .select(['pages.id as ancestorId', 'pages.parentPageId']), - ), - ) - .selectFrom('ancestors') - .innerJoin('pageAccess', 'pageAccess.pageId', 'ancestors.ancestorId') - .leftJoin('pagePermissions', (join) => - join - .onRef('pagePermissions.pageAccessId', '=', 'pageAccess.id') - .on((eb) => - eb.or([ - eb('pagePermissions.userId', '=', userId), - eb( - 'pagePermissions.groupId', - 'in', - this.userGroupIdsSubquery(eb, userId), - ), - ]), - ), - ) - .select('pageAccess.pageId') - .where('pagePermissions.id', 'is', null) - .executeTakeFirst(); - - return !deniedAncestor; + const { canAccess } = await this.canUserEditPage(userId, pageId); + return canAccess; } /** @@ -412,43 +388,50 @@ export class PagePermissionRepo { canAccess: boolean; canEdit: boolean; }> { - const result = await sql<{ - canAccess: boolean | null; - canEdit: boolean | null; - }>` - WITH RECURSIVE ancestors AS ( - SELECT id AS ancestor_id, parent_page_id, 0 AS depth - FROM pages - WHERE id = ${pageId}::uuid - UNION ALL - SELECT p.id, p.parent_page_id, a.depth + 1 - FROM pages p - JOIN ancestors a ON a.parent_page_id = p.id - ) - SELECT - bool_and(pp.id IS NOT NULL) AS "canAccess", - -- nearest restricted ancestor's highest role wins (DESC: 'writer' > 'reader', NULLS LAST: no-permission after real roles) - (array_agg(pp.role ORDER BY a.depth ASC, pp.role DESC NULLS LAST))[1] = 'writer' AS "canEdit" - FROM ancestors a - JOIN page_access pa ON pa.page_id = a.ancestor_id - LEFT JOIN page_permissions pp ON pp.page_access_id = pa.id - AND ( - pp.user_id = ${userId}::uuid - OR pp.group_id IN ( - SELECT gu.group_id FROM group_users gu WHERE gu.user_id = ${userId}::uuid + return withCache( + this.cacheManager, + CacheKey.PAGE_CAN_EDIT(userId, pageId), + PERMISSION_CACHE_TTL_MS, + async () => { + const result = await sql<{ + canAccess: boolean | null; + canEdit: boolean | null; + }>` + WITH RECURSIVE ancestors AS ( + SELECT id AS ancestor_id, parent_page_id, 0 AS depth + FROM pages + WHERE id = ${pageId}::uuid + UNION ALL + SELECT p.id, p.parent_page_id, a.depth + 1 + FROM pages p + JOIN ancestors a ON a.parent_page_id = p.id ) - ) - `.execute(this.db); + SELECT + bool_and(pp.id IS NOT NULL) AS "canAccess", + -- nearest restricted ancestor's highest role wins (DESC: 'writer' > 'reader', NULLS LAST: no-permission after real roles) + (array_agg(pp.role ORDER BY a.depth ASC, pp.role DESC NULLS LAST))[1] = 'writer' AS "canEdit" + FROM ancestors a + JOIN page_access pa ON pa.page_id = a.ancestor_id + LEFT JOIN page_permissions pp ON pp.page_access_id = pa.id + AND ( + pp.user_id = ${userId}::uuid + OR pp.group_id IN ( + SELECT gu.group_id FROM group_users gu WHERE gu.user_id = ${userId}::uuid + ) + ) + `.execute(this.db); - 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), - }; + 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), + }; + }, + ); } /** diff --git a/apps/server/src/database/repos/space/space-member.repo.ts b/apps/server/src/database/repos/space/space-member.repo.ts index 50961802f..6711c30c7 100644 --- a/apps/server/src/database/repos/space/space-member.repo.ts +++ b/apps/server/src/database/repos/space/space-member.repo.ts @@ -1,4 +1,6 @@ -import { BadRequestException, Injectable } from '@nestjs/common'; +import { BadRequestException, Inject, Injectable } from '@nestjs/common'; +import { CACHE_MANAGER } from '@nestjs/cache-manager'; +import { Cache } from 'cache-manager'; import { InjectKysely } from 'nestjs-kysely'; import { KyselyDB, KyselyTransaction } from '@docmost/db/types/kysely.types'; import { dbOrTx } from '@docmost/db/utils'; @@ -13,6 +15,11 @@ import { MemberInfo, UserSpaceRole } from './types'; import { executeWithCursorPagination } from '@docmost/db/pagination/cursor-pagination'; import { GroupRepo } from '@docmost/db/repos/group/group.repo'; import { SpaceRepo } from '@docmost/db/repos/space/space.repo'; +import { withCache } from '../../../common/helpers/with-cache'; +import { + CacheKey, + PERMISSION_CACHE_TTL_MS, +} from '../../../common/helpers/cache-keys'; @Injectable() export class SpaceMemberRepo { @@ -20,6 +27,7 @@ export class SpaceMemberRepo { @InjectKysely() private readonly db: KyselyDB, private readonly groupRepo: GroupRepo, private readonly spaceRepo: SpaceRepo, + @Inject(CACHE_MANAGER) private readonly cacheManager: Cache, ) {} async insertSpaceMember( @@ -214,25 +222,36 @@ export class SpaceMemberRepo { userId: string, spaceId: string, ): Promise { - const roles = await this.db - .selectFrom('spaceMembers') - .select(['userId', 'role']) - .where('userId', '=', userId) - .where('spaceId', '=', spaceId) - .unionAll( - this.db + return withCache( + this.cacheManager, + CacheKey.SPACE_ROLES(userId, spaceId), + PERMISSION_CACHE_TTL_MS, + async () => { + const roles = await this.db .selectFrom('spaceMembers') - .innerJoin('groupUsers', 'groupUsers.groupId', 'spaceMembers.groupId') - .select(['groupUsers.userId', 'spaceMembers.role']) - .where('groupUsers.userId', '=', userId) - .where('spaceMembers.spaceId', '=', spaceId), - ) - .execute(); + .select(['userId', 'role']) + .where('userId', '=', userId) + .where('spaceId', '=', spaceId) + .unionAll( + this.db + .selectFrom('spaceMembers') + .innerJoin( + 'groupUsers', + 'groupUsers.groupId', + 'spaceMembers.groupId', + ) + .select(['groupUsers.userId', 'spaceMembers.role']) + .where('groupUsers.userId', '=', userId) + .where('spaceMembers.spaceId', '=', spaceId), + ) + .execute(); - if (!roles || roles.length === 0) { - return undefined; - } - return roles; + if (!roles || roles.length === 0) { + return undefined; + } + return roles; + }, + ); } async getUserIdsWithSpaceAccess(