diff --git a/packages/backend/server/src/__tests__/team.e2e.ts b/packages/backend/server/src/__tests__/team.e2e.ts index 8ac092afa1..482fdaf0ef 100644 --- a/packages/backend/server/src/__tests__/team.e2e.ts +++ b/packages/backend/server/src/__tests__/team.e2e.ts @@ -987,7 +987,7 @@ test('should be able to grant and revoke doc user role', async t => { revokeDocUserRoles(input: { workspaceId: "${ws.id}", docId: "${pageId}", - userIds: ["${external.id}"] + userId: "${external.id}" }) } `, @@ -1010,7 +1010,7 @@ test('should be able to grant and revoke doc user role', async t => { revokeDocUserRoles(input: { workspaceId: "${ws.id}", docId: "${pageId}", - userIds: ["${read.id}"] + userId: "${read.id}" }) } `, @@ -1019,7 +1019,7 @@ test('should be able to grant and revoke doc user role', async t => { t.like(externalRes.body, { errors: [ { - message: `You do not have permission to access Space ${ws.id}.`, + message: `You do not have permission to access doc ${pageId} under Space ${ws.id}.`, }, ], }); diff --git a/packages/backend/server/src/base/error/def.ts b/packages/backend/server/src/base/error/def.ts index d58ba9e33c..a00b2cef5d 100644 --- a/packages/backend/server/src/base/error/def.ts +++ b/packages/backend/server/src/base/error/def.ts @@ -474,6 +474,10 @@ export const USER_FRIENDLY_ERRORS = { type: 'invalid_input', message: 'Page default role can not be owner.', }, + can_not_batch_grant_page_owner_permissions: { + type: 'invalid_input', + message: 'Can not batch grant page owner permissions.', + }, // Subscription Errors unsupported_subscription_plan: { diff --git a/packages/backend/server/src/base/error/errors.gen.ts b/packages/backend/server/src/base/error/errors.gen.ts index 9ba77fbaab..4dd76c8d78 100644 --- a/packages/backend/server/src/base/error/errors.gen.ts +++ b/packages/backend/server/src/base/error/errors.gen.ts @@ -411,6 +411,12 @@ export class PageDefaultRoleCanNotBeOwner extends UserFriendlyError { super('invalid_input', 'page_default_role_can_not_be_owner', message); } } + +export class CanNotBatchGrantPageOwnerPermissions extends UserFriendlyError { + constructor(message?: string) { + super('invalid_input', 'can_not_batch_grant_page_owner_permissions', message); + } +} @ObjectType() class UnsupportedSubscriptionPlanDataType { @Field() plan!: string @@ -768,6 +774,7 @@ export enum ErrorNames { FAILED_TO_UPSERT_SNAPSHOT, ACTION_FORBIDDEN_ON_NON_TEAM_WORKSPACE, PAGE_DEFAULT_ROLE_CAN_NOT_BE_OWNER, + CAN_NOT_BATCH_GRANT_PAGE_OWNER_PERMISSIONS, UNSUPPORTED_SUBSCRIPTION_PLAN, FAILED_TO_CHECKOUT, INVALID_CHECKOUT_PARAMETERS, diff --git a/packages/backend/server/src/core/permission/service.ts b/packages/backend/server/src/core/permission/service.ts index 372bb5962e..671da4363c 100644 --- a/packages/backend/server/src/core/permission/service.ts +++ b/packages/backend/server/src/core/permission/service.ts @@ -1,15 +1,15 @@ import { Injectable, Logger } from '@nestjs/common'; -import type { Prisma } from '@prisma/client'; +import type { Prisma, WorkspacePageUserPermission } from '@prisma/client'; import { PrismaClient, WorkspaceMemberStatus } from '@prisma/client'; import { groupBy } from 'lodash-es'; import { + CanNotBatchGrantPageOwnerPermissions, DocAccessDenied, EventBus, OnEvent, SpaceAccessDenied, SpaceOwnerNotFound, - SpaceShouldHaveOnlyOneOwner, WorkspacePermissionNotFound, } from '../../base'; import { @@ -737,17 +737,15 @@ export class PermissionService { ].filter(Boolean) as Prisma.PrismaPromise[] ); - return p; + return p as WorkspacePageUserPermission; } - async revokePage(ws: string, page: string, users: string[]) { + async revokePage(ws: string, page: string, user: string) { const result = await this.prisma.workspacePageUserPermission.deleteMany({ where: { workspaceId: ws, pageId: page, - userId: { - in: users, - }, + userId: user, type: { // We shouldn't revoke owner permission, should auto deleted by workspace/user delete cascading not: DocRole.Owner, @@ -758,74 +756,30 @@ export class PermissionService { return result.count > 0; } - async grantPagePermission( + async batchGrantPage( workspaceId: string, pageId: string, userIds: string[], role: DocRole ) { if (userIds.length === 0) { - return []; - } - if (role === DocRole.Owner && userIds.length > 1) { - throw new SpaceShouldHaveOnlyOneOwner({ spaceId: workspaceId }); + return 0; } - return await this.prisma.$transaction(async tx => - Promise.all( - userIds.map(id => - tx.workspacePageUserPermission.upsert({ - where: { - workspaceId_pageId_userId: { - workspaceId, - pageId, - userId: id, - }, - }, - create: { - workspaceId, - pageId, - userId: id, - type: role, - }, - update: { - type: role, - }, - }) - ) - ) - ); - } + if (role === DocRole.Owner) { + throw new CanNotBatchGrantPageOwnerPermissions(); + } - async updatePagePermission( - workspaceId: string, - pageId: string, - userId: string, - role: DocRole - ) { - const permission = await this.prisma.workspacePageUserPermission.findFirst({ - where: { + const result = await this.prisma.workspacePageUserPermission.createMany({ + skipDuplicates: true, + data: userIds.map(id => ({ workspaceId, pageId, - userId, - }, - }); - - if (!permission) { - return this.grantPage(workspaceId, pageId, userId, role); - } - - return await this.prisma.workspacePageUserPermission.update({ - where: { - workspaceId_pageId_userId: { - workspaceId, - pageId, - userId, - }, - }, - data: { + userId: id, type: role, - }, + })), }); + + return result.count; } } diff --git a/packages/backend/server/src/core/workspaces/resolvers/page.ts b/packages/backend/server/src/core/workspaces/resolvers/page.ts index c17cd781d8..d095c5f16a 100644 --- a/packages/backend/server/src/core/workspaces/resolvers/page.ts +++ b/packages/backend/server/src/core/workspaces/resolvers/page.ts @@ -37,7 +37,6 @@ import { mapDocRoleToPermissions, PermissionService, PublicPageMode, - WorkspaceRole, } from '../../permission'; import { PublicUserType } from '../../user'; import { DocID } from '../../utils/doc'; @@ -94,15 +93,15 @@ class UpdateDocUserRoleInput { } @InputType() -class RevokeDocUserRolesInput { +class RevokeDocUserRoleInput { @Field(() => String) docId!: string; @Field(() => String) workspaceId!: string; - @Field(() => [String]) - userIds!: string[]; + @Field(() => String) + userId!: string; } @InputType() @@ -263,10 +262,18 @@ export class PagePermissionResolver { complexity: 4, }) async pageGrantedUsersList( + @CurrentUser() user: CurrentUser, @Parent() workspace: WorkspaceType, @Args('pageId') pageId: string, @Args('pagination') pagination: PaginationInput ): Promise { + await this.permission.checkPagePermission( + workspace.id, + pageId, + 'Doc.Users.Read', + user.id + ); + const docId = new DocID(pageId, workspace.id); const [permissions, totalCount] = await this.prisma.$transaction(tx => { return Promise.all([ @@ -454,7 +461,7 @@ export class PagePermissionResolver { 'Doc.Users.Manage', user.id ); - await this.permission.grantPagePermission( + await this.permission.batchGrantPage( doc.workspace, doc.guid, input.userIds, @@ -471,7 +478,7 @@ export class PagePermissionResolver { @Mutation(() => Boolean) async revokeDocUserRoles( @CurrentUser() user: CurrentUser, - @Args('input') input: RevokeDocUserRolesInput + @Args('input') input: RevokeDocUserRoleInput ): Promise { const doc = new DocID(input.docId, input.workspaceId); const pairs = { @@ -488,15 +495,16 @@ export class PagePermissionResolver { 'Expect doc not to be workspace' ); } - await this.permission.checkWorkspace( + await this.permission.checkPagePermission( doc.workspace, - user.id, - WorkspaceRole.Collaborator + doc.guid, + 'Doc.Users.Manage', + user.id ); - await this.permission.revokePage(doc.workspace, doc.guid, input.userIds); + await this.permission.revokePage(doc.workspace, doc.guid, input.userId); this.logger.log('Revoke doc user roles', { ...pairs, - userIds: input.userIds, + userId: input.userId, }); return true; } @@ -521,38 +529,36 @@ export class PagePermissionResolver { 'Expect doc not to be workspace' ); } - await this.permission.checkWorkspace( + + await this.permission.checkPagePermission( doc.workspace, - user.id, - WorkspaceRole.Collaborator + doc.guid, + input.role === DocRole.Owner ? 'Doc.TransferOwner' : 'Doc.Users.Manage', + user.id ); + + await this.permission.grantPage( + doc.workspace, + doc.guid, + input.userId, + input.role + ); + if (input.role === DocRole.Owner) { - const ret = await this.permission.grantPagePermission( - doc.workspace, - doc.guid, - [input.userId], - input.role - ); this.logger.log('Transfer doc owner', { ...pairs, userId: input.userId, role: input.role, }); - return ret.length > 0; } else { - await this.permission.updatePagePermission( - doc.workspace, - doc.guid, - input.userId, - input.role - ); this.logger.log('Update doc user role', { ...pairs, userId: input.userId, role: input.role, }); - return true; } + + return true; } @Mutation(() => Boolean) @@ -580,7 +586,7 @@ export class PagePermissionResolver { ); } try { - await this.permission.checkCloudPagePermission( + await this.permission.checkPagePermission( doc.workspace, doc.guid, 'Doc.Users.Manage', diff --git a/packages/backend/server/src/schema.gql b/packages/backend/server/src/schema.gql index 7ac8da4e76..436a6edc35 100644 --- a/packages/backend/server/src/schema.gql +++ b/packages/backend/server/src/schema.gql @@ -254,6 +254,7 @@ enum ErrorNames { CANNOT_DELETE_ALL_ADMIN_ACCOUNT CANNOT_DELETE_OWN_ACCOUNT CANT_UPDATE_ONETIME_PAYMENT_SUBSCRIPTION + CAN_NOT_BATCH_GRANT_PAGE_OWNER_PERMISSIONS CAPTCHA_VERIFICATION_FAILED COPILOT_ACTION_TAKEN COPILOT_FAILED_TO_CREATE_MESSAGE @@ -629,7 +630,7 @@ type Mutation { removeWorkspaceFeature(feature: FeatureType!, workspaceId: String!): Boolean! resumeSubscription(idempotencyKey: String @deprecated(reason: "use header `Idempotency-Key`"), plan: SubscriptionPlan = Pro, workspaceId: String): SubscriptionType! revoke(userId: String!, workspaceId: String!): Boolean! - revokeDocUserRoles(input: RevokeDocUserRolesInput!): Boolean! + revokeDocUserRoles(input: RevokeDocUserRoleInput!): Boolean! revokeInviteLink(workspaceId: String!): Boolean! revokePage(pageId: String!, workspaceId: String!): Boolean! @deprecated(reason: "use revokePublicPage") revokePublicPage(pageId: String!, workspaceId: String!): WorkspacePage! @@ -809,9 +810,9 @@ type RemoveAvatar { success: Boolean! } -input RevokeDocUserRolesInput { +input RevokeDocUserRoleInput { docId: String! - userIds: [String!]! + userId: String! workspaceId: String! }