From f4fcf39eb53749f15a3cd19a77e536df382d5faa Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Wed, 5 Mar 2025 16:47:41 +0100 Subject: [PATCH] [permissions] Prepare for roll-out (#10676) Closes https://github.com/twentyhq/core-team-issues/issues/469 and https://github.com/twentyhq/core-team-issues/issues/500 In this PR 1. stop conditioning permission initialization for a workspace to env variable value. Instead we want to create and assign permissions and roles in all new workspaces. For now that will be totally silent. 2. temporarily, the default role is set to the admin role for new workspaces. it will also be the case for existing workspaces through the backfill command. Member role is still being created though. (when we will do the final roll-out we will update this so that future workspaces have the member role as default role. our goal here is not to break any current behaviour for users, that today have all have the equivalent of admin rights). --- .../0-44-initialize-permissions.command.ts | 112 +++++------------- .../auth/services/sign-in-up.service.ts | 8 +- .../environment/environment-variables.ts | 10 -- .../permissions/permissions.service.ts | 4 - .../workspace-manager.service.ts | 18 +-- 5 files changed, 37 insertions(+), 115 deletions(-) diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/0-44/0-44-initialize-permissions.command.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/0-44/0-44-initialize-permissions.command.ts index e67879232..4a8b1d76b 100644 --- a/packages/twenty-server/src/database/commands/upgrade-version-command/0-44/0-44-initialize-permissions.command.ts +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/0-44/0-44-initialize-permissions.command.ts @@ -3,7 +3,7 @@ import { InjectRepository } from '@nestjs/typeorm'; import chalk from 'chalk'; import { Command } from 'nest-commander'; import { isDefined } from 'twenty-shared'; -import { IsNull, Repository } from 'typeorm'; +import { Repository } from 'typeorm'; import { ActiveOrSuspendedWorkspacesMigrationCommandOptions, @@ -62,36 +62,28 @@ export class InitializePermissionsCommand extends ActiveOrSuspendedWorkspacesMig }); } - await this.assignAdminRole({ + await this.assignAdminRoleToMembers({ workspaceId, adminRoleId, options, }); - let memberRoleId: string | undefined; + await this.setAdminRoleAsDefaultRole({ + workspaceId, + adminRoleId, + options, + }); - memberRoleId = workspaceRoles.find( + const memberRole = workspaceRoles.find( (role) => role.label === MEMBER_ROLE_LABEL, - )?.id; + ); - if (!isDefined(memberRoleId)) { - memberRoleId = await this.createMemberRole({ + if (!isDefined(memberRole)) { + await this.createMemberRole({ workspaceId, options, }); } - - await this.setMemberRoleAsDefaultRole({ - workspaceId, - memberRoleId, - options, - }); - - await this.assignMemberRoleToUserWorkspacesWithoutRole({ - workspaceId, - memberRoleId, - options, - }); } catch (error) { this.logger.log( chalk.red(`Error in workspace ${workspaceId} - ${error.message}`), @@ -143,39 +135,7 @@ export class InitializePermissionsCommand extends ActiveOrSuspendedWorkspacesMig return memberRole.id; } - private async setMemberRoleAsDefaultRole({ - workspaceId, - memberRoleId, - options, - }: { - workspaceId: string; - memberRoleId: string; - options: ActiveOrSuspendedWorkspacesMigrationCommandOptions; - }) { - const workspaceDefaultRole = await this.workspaceRepository.findOne({ - where: { - id: workspaceId, - }, - }); - - if (!isDefined(workspaceDefaultRole?.defaultRoleId)) { - this.logger.log( - chalk.green( - `Setting member role as default role ${options.dryRun ? '(dry run)' : ''}`, - ), - ); - - if (options.dryRun) { - return; - } - - await this.workspaceRepository.update(workspaceId, { - defaultRoleId: memberRoleId, - }); - } - } - - private async assignAdminRole({ + private async setAdminRoleAsDefaultRole({ workspaceId, adminRoleId, options, @@ -184,28 +144,25 @@ export class InitializePermissionsCommand extends ActiveOrSuspendedWorkspacesMig adminRoleId: string; options: ActiveOrSuspendedWorkspacesMigrationCommandOptions; }) { - const oldestUserWorkspace = await this.userWorkspaceRepository.findOne({ + const workspaceDefaultRole = await this.workspaceRepository.findOne({ where: { - workspaceId, - deletedAt: IsNull(), - }, - relations: { - user: true, - }, - order: { - user: { - createdAt: 'ASC', - }, + id: workspaceId, }, }); - if (!oldestUserWorkspace) { - throw new Error('No user workspace found'); + if (isDefined(workspaceDefaultRole?.defaultRoleId)) { + this.logger.log( + chalk.green( + 'Workspace already has a default role. Skipping setting admin role as default role', + ), + ); + + return; } this.logger.log( chalk.green( - `Assigning admin role to user ${oldestUserWorkspace.id} ${options.dryRun ? '(dry run)' : ''}`, + `Setting admin role as default role ${options.dryRun ? '(dry run)' : ''}`, ), ); @@ -213,29 +170,24 @@ export class InitializePermissionsCommand extends ActiveOrSuspendedWorkspacesMig return; } - await this.userRoleService.assignRoleToUserWorkspace({ - roleId: adminRoleId, - userWorkspaceId: oldestUserWorkspace.id, - workspaceId, + await this.workspaceRepository.update(workspaceId, { + defaultRoleId: adminRoleId, }); } - private async assignMemberRoleToUserWorkspacesWithoutRole({ + private async assignAdminRoleToMembers({ workspaceId, - memberRoleId, + adminRoleId, options, }: { workspaceId: string; - memberRoleId: string; + adminRoleId: string; options: ActiveOrSuspendedWorkspacesMigrationCommandOptions; }) { const userWorkspaces = await this.userWorkspaceRepository.find({ where: { workspaceId, }, - relations: { - user: true, - }, }); const rolesByUserWorkspace = @@ -247,7 +199,6 @@ export class InitializePermissionsCommand extends ActiveOrSuspendedWorkspacesMig }); for (const userWorkspace of userWorkspaces) { - // If userWorkspace has a role, do nothing if ( rolesByUserWorkspace .get(userWorkspace.id) @@ -255,7 +206,7 @@ export class InitializePermissionsCommand extends ActiveOrSuspendedWorkspacesMig ) { this.logger.log( chalk.green( - `User workspace ${userWorkspace.id} already has a role. Skipping member role assignation`, + `User workspace ${userWorkspace.id} already has a role. Skipping role assignation`, ), ); continue; @@ -263,7 +214,7 @@ export class InitializePermissionsCommand extends ActiveOrSuspendedWorkspacesMig this.logger.log( chalk.green( - `Assigning member role to user workspace ${userWorkspace.id} ${options.dryRun ? '(dry run)' : ''}`, + `Assigning admin role to workspace member ${userWorkspace.id} ${options.dryRun ? '(dry run)' : ''}`, ), ); @@ -271,9 +222,8 @@ export class InitializePermissionsCommand extends ActiveOrSuspendedWorkspacesMig continue; } - // Otherwise, assign member role to userWorkspace await this.userRoleService.assignRoleToUserWorkspace({ - roleId: memberRoleId, + roleId: adminRoleId, userWorkspaceId: userWorkspace.id, workspaceId, }); diff --git a/packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.ts b/packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.ts index a8e7f0487..9e99ff0de 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.ts @@ -31,7 +31,6 @@ import { } from 'src/engine/core-modules/auth/types/signInUp.type'; import { DomainManagerService } from 'src/engine/core-modules/domain-manager/services/domain-manager.service'; import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service'; -import { FeatureFlagKey } from 'src/engine/core-modules/feature-flag/enums/feature-flag-key.enum'; import { FeatureFlagService } from 'src/engine/core-modules/feature-flag/services/feature-flag.service'; import { FileUploadService } from 'src/engine/core-modules/file/file-upload/services/file-upload.service'; import { OnboardingService } from 'src/engine/core-modules/onboarding/onboarding.service'; @@ -273,12 +272,7 @@ export class SignInUpService { await this.activateOnboardingForUser(user, params.workspace); } - const isPermissionsEnabled = await this.featureFlagService.isFeatureEnabled( - FeatureFlagKey.IsPermissionsEnabled, - params.workspace.id, - ); - - if (isPermissionsEnabled && params.workspace.defaultRoleId) { + if (params.workspace.defaultRoleId) { await this.userRoleService.assignRoleToUserWorkspace({ workspaceId: params.workspace.id, userWorkspaceId: userWorkspace.id, diff --git a/packages/twenty-server/src/engine/core-modules/environment/environment-variables.ts b/packages/twenty-server/src/engine/core-modules/environment/environment-variables.ts index 86e871c1c..ec1f963d2 100644 --- a/packages/twenty-server/src/engine/core-modules/environment/environment-variables.ts +++ b/packages/twenty-server/src/engine/core-modules/environment/environment-variables.ts @@ -860,16 +860,6 @@ export class EnvironmentVariables { @IsBoolean() IS_MULTIWORKSPACE_ENABLED = false; - @EnvironmentVariablesMetadata({ - group: EnvironmentVariablesGroup.Other, - description: - 'Use as a feature flag for the new permission feature we are working on.', - }) - @CastToBoolean() - @IsOptional() - @IsBoolean() - PERMISSIONS_ENABLED = false; - @EnvironmentVariablesMetadata({ group: EnvironmentVariablesGroup.Other, description: diff --git a/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.service.ts b/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.service.ts index 31402dedc..48602ec74 100644 --- a/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.service.ts @@ -145,10 +145,6 @@ export class PermissionsService { return roleOfUserWorkspace?.[roleColumn] === true; } - public async isPermissionsEnabled(): Promise { - return this.environmentService.get('PERMISSIONS_ENABLED') === true; - } - private getRoleColumnForRequiredPermission( requiredPermission: PermissionsOnAllObjectRecords, ): keyof RoleEntity { diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-manager.service.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-manager.service.ts index 3292191dc..280dfda88 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-manager.service.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-manager.service.ts @@ -99,12 +99,8 @@ export class WorkspaceManagerService { ); const permissionsEnabledStart = performance.now(); - const permissionsEnabled = - await this.permissionsService.isPermissionsEnabled(); - if (permissionsEnabled === true) { - await this.initPermissions({ workspaceId, userId }); - } + await this.initPermissions({ workspaceId, userId }); const permissionsEnabledEnd = performance.now(); @@ -168,12 +164,7 @@ export class WorkspaceManagerService { dataSourceId: dataSourceMetadata.id, }); - const permissionsEnabled = - await this.permissionsService.isPermissionsEnabled(); - - if (permissionsEnabled === true) { - await this.initPermissionsDev(workspaceId); - } + await this.initPermissionsDev(workspaceId); } /** @@ -296,12 +287,13 @@ export class WorkspaceManagerService { roleId: adminRole.id, }); - const memberRole = await this.roleService.createMemberRole({ + await this.roleService.createMemberRole({ workspaceId, }); + // Temporary - after permissions are rolled-out we will set member role as the default role await this.workspaceRepository.update(workspaceId, { - defaultRoleId: memberRole.id, + defaultRoleId: adminRole.id, }); }