Change condition for multi-workspace check (#17938)
When deploying with ``` IS_WORKSPACE_CREATION_LIMITED_TO_SERVER_ADMINS=true IS_MULTIWORKSPACE_ENABLED=true ``` The first workspace can be created successfully. However, any attempt to create additional workspaces as admin fails with the error: `Workspace creation is restricted to admins` because `canAccessFullAdminPanel` is **false** If these flags are set to false during the initial deployment and restarting the Docker container, workspace creation works normally. Problem is caused by `canAccessFullAdminPanel` --------- Co-authored-by: ehconitin <nitinkoche03@gmail.com> Co-authored-by: Félix Malfait <felix@twenty.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Félix Malfait <felix.malfait@gmail.com>
This commit is contained in:
manstfu
co-authored by
ehconitin
Félix Malfait
Cursor
Félix Malfait
ehconitin
Félix Malfait
Cursor
Félix Malfait
parent
7523143f12
commit
e3db73ef46
@@ -506,10 +506,6 @@ export class AuthResolver {
|
||||
@AuthUser() currentUser: UserEntity,
|
||||
@AuthProvider() authProvider: AuthProviderEnum,
|
||||
): Promise<SignUpOutput> {
|
||||
await this.signInUpService.checkWorkspaceCreationIsAllowedOrThrow(
|
||||
currentUser,
|
||||
);
|
||||
|
||||
const { user, workspace } = await this.signInUpService.signUpOnNewWorkspace(
|
||||
{ type: 'existingUser', existingUser: currentUser },
|
||||
);
|
||||
|
||||
+304
@@ -0,0 +1,304 @@
|
||||
import {
|
||||
AuthException,
|
||||
AuthExceptionCode,
|
||||
} from 'src/engine/core-modules/auth/auth.exception';
|
||||
import { type SignInUpNewUserPayload } from 'src/engine/core-modules/auth/types/signInUp.type';
|
||||
import { AuthProviderEnum } from 'src/engine/core-modules/workspace/types/workspace.type';
|
||||
|
||||
import { SignInUpService } from './sign-in-up.service';
|
||||
|
||||
const mockPartialUserPayload: SignInUpNewUserPayload = {
|
||||
email: 'first.user@acme.dev',
|
||||
firstName: 'First',
|
||||
lastName: 'User',
|
||||
locale: 'en',
|
||||
isEmailAlreadyVerified: true,
|
||||
};
|
||||
|
||||
type MockConfigurationValues = {
|
||||
IS_MULTIWORKSPACE_ENABLED: boolean;
|
||||
IS_WORKSPACE_CREATION_LIMITED_TO_SERVER_ADMINS: boolean;
|
||||
SERVER_URL: string;
|
||||
};
|
||||
|
||||
const createSignInUpServiceForTests = () => {
|
||||
const mockUserRepository = {
|
||||
create: jest.fn((user) => user),
|
||||
save: jest.fn(async (user) => ({ id: 'saved-user-id', ...user })),
|
||||
count: jest.fn(),
|
||||
};
|
||||
|
||||
const mockWorkspaceRepository = {
|
||||
count: jest.fn(),
|
||||
create: jest.fn(),
|
||||
};
|
||||
|
||||
const mockConfigurationValues: MockConfigurationValues = {
|
||||
IS_MULTIWORKSPACE_ENABLED: true,
|
||||
IS_WORKSPACE_CREATION_LIMITED_TO_SERVER_ADMINS: false,
|
||||
SERVER_URL: 'http://localhost:3000',
|
||||
};
|
||||
|
||||
const mockTwentyConfigService = {
|
||||
get: jest.fn(
|
||||
(configKey: keyof MockConfigurationValues) =>
|
||||
mockConfigurationValues[configKey],
|
||||
),
|
||||
};
|
||||
|
||||
const queryRunnerMock = {
|
||||
manager: {
|
||||
save: jest.fn(),
|
||||
},
|
||||
connect: jest.fn(),
|
||||
startTransaction: jest.fn(),
|
||||
commitTransaction: jest.fn(),
|
||||
rollbackTransaction: jest.fn(),
|
||||
release: jest.fn(),
|
||||
};
|
||||
|
||||
const service = new SignInUpService(
|
||||
mockUserRepository as any,
|
||||
mockWorkspaceRepository as any,
|
||||
{
|
||||
validatePersonalInvitation: jest.fn(),
|
||||
invalidateWorkspaceInvitation: jest.fn(),
|
||||
} as any,
|
||||
{
|
||||
create: jest.fn(),
|
||||
checkUserWorkspaceExists: jest.fn(),
|
||||
} as any,
|
||||
{
|
||||
setOnboardingCreateProfilePending: jest.fn(),
|
||||
setOnboardingInviteTeamPending: jest.fn(),
|
||||
createOnboardingStatusForWorkspaceMember: jest.fn(),
|
||||
} as any,
|
||||
{
|
||||
emitCustomBatchEvent: jest.fn(),
|
||||
} as any,
|
||||
{
|
||||
getHttpClient: jest.fn(),
|
||||
} as any,
|
||||
mockTwentyConfigService as any,
|
||||
{
|
||||
generateSubdomain: jest.fn(),
|
||||
} as any,
|
||||
{
|
||||
findUserByEmail: jest.fn(),
|
||||
findByEmail: jest.fn(),
|
||||
markEmailAsVerified: jest.fn(),
|
||||
} as any,
|
||||
{
|
||||
incrementCounter: jest.fn(),
|
||||
} as any,
|
||||
{
|
||||
invalidateAndRecompute: jest.fn(),
|
||||
} as any,
|
||||
{
|
||||
createWorkspaceCustomApplication: jest.fn(),
|
||||
} as any,
|
||||
{
|
||||
createQueryRunner: jest.fn(() => queryRunnerMock),
|
||||
} as any,
|
||||
);
|
||||
|
||||
return {
|
||||
service,
|
||||
mockUserRepository,
|
||||
mockWorkspaceRepository,
|
||||
mockConfigurationValues,
|
||||
};
|
||||
};
|
||||
|
||||
describe('SignInUpService workspace-creation policy', () => {
|
||||
it('grants bootstrap owner server permissions when multi-workspace is enabled and unrestricted', async () => {
|
||||
const {
|
||||
service,
|
||||
mockUserRepository,
|
||||
mockWorkspaceRepository,
|
||||
mockConfigurationValues,
|
||||
} = createSignInUpServiceForTests();
|
||||
|
||||
mockConfigurationValues.IS_MULTIWORKSPACE_ENABLED = true;
|
||||
mockConfigurationValues.IS_WORKSPACE_CREATION_LIMITED_TO_SERVER_ADMINS =
|
||||
false;
|
||||
mockWorkspaceRepository.count.mockResolvedValue(0);
|
||||
mockUserRepository.count.mockResolvedValue(0);
|
||||
jest
|
||||
.spyOn((service as any).userService, 'findUserByEmail')
|
||||
.mockResolvedValue(null);
|
||||
|
||||
await service.signUpWithoutWorkspace(mockPartialUserPayload, {
|
||||
provider: AuthProviderEnum.Google,
|
||||
} as any);
|
||||
|
||||
expect(mockUserRepository.create).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
canImpersonate: true,
|
||||
canAccessFullAdminPanel: true,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('grants bootstrap owner server permissions when multi-workspace is enabled and restricted', async () => {
|
||||
const {
|
||||
service,
|
||||
mockUserRepository,
|
||||
mockWorkspaceRepository,
|
||||
mockConfigurationValues,
|
||||
} = createSignInUpServiceForTests();
|
||||
|
||||
mockConfigurationValues.IS_MULTIWORKSPACE_ENABLED = true;
|
||||
mockConfigurationValues.IS_WORKSPACE_CREATION_LIMITED_TO_SERVER_ADMINS =
|
||||
true;
|
||||
mockWorkspaceRepository.count.mockResolvedValue(0);
|
||||
mockUserRepository.count.mockResolvedValue(0);
|
||||
jest
|
||||
.spyOn((service as any).userService, 'findUserByEmail')
|
||||
.mockResolvedValue(null);
|
||||
|
||||
await service.signUpWithoutWorkspace(mockPartialUserPayload, {
|
||||
provider: AuthProviderEnum.Google,
|
||||
} as any);
|
||||
|
||||
expect(mockUserRepository.create).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
canImpersonate: true,
|
||||
canAccessFullAdminPanel: true,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('assigns default non-admin permissions after bootstrap in multi-workspace mode', async () => {
|
||||
const {
|
||||
service,
|
||||
mockUserRepository,
|
||||
mockWorkspaceRepository,
|
||||
mockConfigurationValues,
|
||||
} = createSignInUpServiceForTests();
|
||||
|
||||
mockConfigurationValues.IS_MULTIWORKSPACE_ENABLED = true;
|
||||
mockConfigurationValues.IS_WORKSPACE_CREATION_LIMITED_TO_SERVER_ADMINS =
|
||||
false;
|
||||
mockWorkspaceRepository.count.mockResolvedValue(1);
|
||||
mockUserRepository.count.mockResolvedValue(1);
|
||||
jest
|
||||
.spyOn((service as any).userService, 'findUserByEmail')
|
||||
.mockResolvedValue(null);
|
||||
|
||||
await service.signUpWithoutWorkspace(mockPartialUserPayload, {
|
||||
provider: AuthProviderEnum.Google,
|
||||
} as any);
|
||||
|
||||
expect(mockUserRepository.create).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
canImpersonate: false,
|
||||
canAccessFullAdminPanel: false,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('does not grant admin to second user signing up before any workspace exists', async () => {
|
||||
const {
|
||||
service,
|
||||
mockUserRepository,
|
||||
mockWorkspaceRepository,
|
||||
mockConfigurationValues,
|
||||
} = createSignInUpServiceForTests();
|
||||
|
||||
mockConfigurationValues.IS_MULTIWORKSPACE_ENABLED = true;
|
||||
mockConfigurationValues.IS_WORKSPACE_CREATION_LIMITED_TO_SERVER_ADMINS =
|
||||
false;
|
||||
mockWorkspaceRepository.count.mockResolvedValue(0);
|
||||
mockUserRepository.count.mockResolvedValue(1);
|
||||
jest
|
||||
.spyOn((service as any).userService, 'findUserByEmail')
|
||||
.mockResolvedValue(null);
|
||||
|
||||
await service.signUpWithoutWorkspace(mockPartialUserPayload, {
|
||||
provider: AuthProviderEnum.Google,
|
||||
} as any);
|
||||
|
||||
expect(mockUserRepository.create).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
canImpersonate: false,
|
||||
canAccessFullAdminPanel: false,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('throws forbidden when a non-admin existing user creates workspace in restricted mode after bootstrap', async () => {
|
||||
const { service, mockWorkspaceRepository, mockConfigurationValues } =
|
||||
createSignInUpServiceForTests();
|
||||
|
||||
mockConfigurationValues.IS_MULTIWORKSPACE_ENABLED = true;
|
||||
mockConfigurationValues.IS_WORKSPACE_CREATION_LIMITED_TO_SERVER_ADMINS =
|
||||
true;
|
||||
mockWorkspaceRepository.count.mockResolvedValue(1);
|
||||
|
||||
const nonAdminExistingUser = {
|
||||
id: 'existing-user-id',
|
||||
email: 'existing.user@acme.dev',
|
||||
canAccessFullAdminPanel: false,
|
||||
};
|
||||
|
||||
await expect(
|
||||
service.signUpOnNewWorkspace({
|
||||
type: 'existingUser',
|
||||
existingUser: nonAdminExistingUser as any,
|
||||
}),
|
||||
).rejects.toMatchObject({
|
||||
code: AuthExceptionCode.FORBIDDEN_EXCEPTION,
|
||||
});
|
||||
});
|
||||
|
||||
it('throws SIGNUP_DISABLED when creating workspace in single-workspace mode after bootstrap', async () => {
|
||||
const { service, mockWorkspaceRepository, mockConfigurationValues } =
|
||||
createSignInUpServiceForTests();
|
||||
|
||||
mockConfigurationValues.IS_MULTIWORKSPACE_ENABLED = false;
|
||||
mockConfigurationValues.IS_WORKSPACE_CREATION_LIMITED_TO_SERVER_ADMINS =
|
||||
false;
|
||||
mockWorkspaceRepository.count.mockResolvedValue(1);
|
||||
|
||||
await expect(
|
||||
service.signUpOnNewWorkspace({
|
||||
type: 'existingUser',
|
||||
existingUser: {
|
||||
id: 'existing-user-id',
|
||||
email: 'existing.user@acme.dev',
|
||||
canAccessFullAdminPanel: true,
|
||||
} as any,
|
||||
}),
|
||||
).rejects.toMatchObject({
|
||||
code: AuthExceptionCode.SIGNUP_DISABLED,
|
||||
});
|
||||
});
|
||||
|
||||
it('keeps single-workspace SIGNUP_DISABLED behavior after first workspace exists', async () => {
|
||||
const { service, mockWorkspaceRepository, mockConfigurationValues } =
|
||||
createSignInUpServiceForTests();
|
||||
|
||||
mockConfigurationValues.IS_MULTIWORKSPACE_ENABLED = false;
|
||||
mockConfigurationValues.IS_WORKSPACE_CREATION_LIMITED_TO_SERVER_ADMINS =
|
||||
false;
|
||||
mockWorkspaceRepository.count.mockResolvedValue(1);
|
||||
jest
|
||||
.spyOn((service as any).userService, 'findUserByEmail')
|
||||
.mockResolvedValue(null);
|
||||
|
||||
await expect(
|
||||
service.signUpWithoutWorkspace(mockPartialUserPayload, {
|
||||
provider: AuthProviderEnum.Google,
|
||||
} as any),
|
||||
).rejects.toBeInstanceOf(AuthException);
|
||||
|
||||
await expect(
|
||||
service.signUpWithoutWorkspace(mockPartialUserPayload, {
|
||||
provider: AuthProviderEnum.Google,
|
||||
} as any),
|
||||
).rejects.toMatchObject({
|
||||
code: AuthExceptionCode.SIGNUP_DISABLED,
|
||||
});
|
||||
});
|
||||
});
|
||||
+66
-64
@@ -4,7 +4,7 @@ import { InjectDataSource, InjectRepository } from '@nestjs/typeorm';
|
||||
import { msg } from '@lingui/core/macro';
|
||||
import { TWENTY_ICONS_BASE_URL } from 'twenty-shared/constants';
|
||||
import { WorkspaceActivationStatus } from 'twenty-shared/workspace';
|
||||
import { type DataSource, type QueryRunner, Repository } from 'typeorm';
|
||||
import { Repository, type DataSource, type QueryRunner } from 'typeorm';
|
||||
import { v4 } from 'uuid';
|
||||
|
||||
import { USER_SIGNUP_EVENT_NAME } from 'src/engine/api/graphql/workspace-query-runner/constants/user-signup-event-name.constants';
|
||||
@@ -379,55 +379,68 @@ export class SignInUpService {
|
||||
return savedUser;
|
||||
}
|
||||
|
||||
private async setDefaultImpersonateAndAccessFullAdminPanel() {
|
||||
if (!this.twentyConfigService.get('IS_MULTIWORKSPACE_ENABLED')) {
|
||||
const workspacesCount = await this.workspaceRepository.count();
|
||||
private async isSignUpEnabled(): Promise<boolean> {
|
||||
const workspaceCount = await this.workspaceRepository.count();
|
||||
|
||||
// let the creation of the first workspace
|
||||
if (workspacesCount > 0) {
|
||||
throw new AuthException(
|
||||
'New workspace setup is disabled',
|
||||
AuthExceptionCode.SIGNUP_DISABLED,
|
||||
);
|
||||
}
|
||||
|
||||
return { canImpersonate: true, canAccessFullAdminPanel: true };
|
||||
}
|
||||
|
||||
return { canImpersonate: false, canAccessFullAdminPanel: false };
|
||||
}
|
||||
|
||||
private isWorkspaceCreationLimitedToServerAdmins(): boolean {
|
||||
return this.twentyConfigService.get(
|
||||
'IS_WORKSPACE_CREATION_LIMITED_TO_SERVER_ADMINS',
|
||||
return (
|
||||
this.twentyConfigService.get('IS_MULTIWORKSPACE_ENABLED') ||
|
||||
workspaceCount === 0
|
||||
);
|
||||
}
|
||||
|
||||
private async isFirstWorkspaceInSystem(): Promise<boolean> {
|
||||
const count = await this.workspaceRepository.count();
|
||||
|
||||
return count === 0;
|
||||
}
|
||||
|
||||
async checkWorkspaceCreationIsAllowedOrThrow(
|
||||
currentUser: UserEntity,
|
||||
): Promise<void> {
|
||||
if (!this.isWorkspaceCreationLimitedToServerAdmins()) return;
|
||||
|
||||
// Only allow bypass during initial system bootstrap (no workspaces exist yet)
|
||||
if (await this.isFirstWorkspaceInSystem()) return;
|
||||
|
||||
if (!currentUser.canAccessFullAdminPanel) {
|
||||
private async assertSignUpEnabled(): Promise<void> {
|
||||
if (!(await this.isSignUpEnabled())) {
|
||||
throw new AuthException(
|
||||
'Workspace creation is restricted to admins',
|
||||
AuthExceptionCode.FORBIDDEN_EXCEPTION,
|
||||
{
|
||||
userFriendlyMessage: msg`Workspace creation is restricted to admins`,
|
||||
},
|
||||
'New workspace setup is disabled',
|
||||
AuthExceptionCode.SIGNUP_DISABLED,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
private async hasServerAdmin(): Promise<boolean> {
|
||||
const adminCount = await this.userRepository.count({
|
||||
where: { canAccessFullAdminPanel: true },
|
||||
});
|
||||
|
||||
return adminCount > 0;
|
||||
}
|
||||
|
||||
private async assertWorkspaceCreationAllowed(
|
||||
userData: ExistingUserOrPartialUserWithPicture['userData'],
|
||||
): Promise<void> {
|
||||
await this.assertSignUpEnabled();
|
||||
|
||||
const workspaceCount = await this.workspaceRepository.count();
|
||||
|
||||
if (workspaceCount === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (
|
||||
!this.twentyConfigService.get(
|
||||
'IS_WORKSPACE_CREATION_LIMITED_TO_SERVER_ADMINS',
|
||||
)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
const isExistingAdmin =
|
||||
userData.type === 'existingUser' &&
|
||||
userData.existingUser.canAccessFullAdminPanel;
|
||||
|
||||
if (isExistingAdmin) {
|
||||
return;
|
||||
}
|
||||
|
||||
throw new AuthException(
|
||||
'Workspace creation is restricted to admins',
|
||||
AuthExceptionCode.FORBIDDEN_EXCEPTION,
|
||||
{
|
||||
userFriendlyMessage: msg`Workspace creation is restricted to admins`,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
async signUpOnNewWorkspace(
|
||||
userData: ExistingUserOrPartialUserWithPicture['userData'],
|
||||
) {
|
||||
@@ -446,27 +459,9 @@ export class SignInUpService {
|
||||
);
|
||||
}
|
||||
|
||||
if (
|
||||
this.isWorkspaceCreationLimitedToServerAdmins() &&
|
||||
!(await this.isFirstWorkspaceInSystem())
|
||||
) {
|
||||
const isExistingAdmin =
|
||||
userData.type === 'existingUser' &&
|
||||
userData.existingUser.canAccessFullAdminPanel;
|
||||
await this.assertWorkspaceCreationAllowed(userData);
|
||||
|
||||
if (!isExistingAdmin) {
|
||||
throw new AuthException(
|
||||
'Workspace creation is restricted to admins',
|
||||
AuthExceptionCode.FORBIDDEN_EXCEPTION,
|
||||
{
|
||||
userFriendlyMessage: msg`Workspace creation is restricted to admins`,
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
const { canImpersonate, canAccessFullAdminPanel } =
|
||||
await this.setDefaultImpersonateAndAccessFullAdminPanel();
|
||||
const shouldGrantServerAdmin = !(await this.hasServerAdmin());
|
||||
|
||||
const logoUrl = `${TWENTY_ICONS_BASE_URL}/${getDomainNameByEmail(email)}`;
|
||||
const isLogoUrlValid = async () => {
|
||||
@@ -523,8 +518,8 @@ export class SignInUpService {
|
||||
: await this.saveNewUser(
|
||||
userData.newUserWithPicture,
|
||||
{
|
||||
canImpersonate,
|
||||
canAccessFullAdminPanel,
|
||||
canImpersonate: shouldGrantServerAdmin,
|
||||
canAccessFullAdminPanel: shouldGrantServerAdmin,
|
||||
},
|
||||
queryRunner,
|
||||
);
|
||||
@@ -584,9 +579,16 @@ export class SignInUpService {
|
||||
);
|
||||
}
|
||||
|
||||
await this.assertSignUpEnabled();
|
||||
|
||||
const shouldGrantServerAdmin = !(await this.hasServerAdmin());
|
||||
|
||||
return this.saveNewUser(
|
||||
await this.computePartialUserFromUserPayload(newUserParams, authParams),
|
||||
await this.setDefaultImpersonateAndAccessFullAdminPanel(),
|
||||
{
|
||||
canImpersonate: shouldGrantServerAdmin,
|
||||
canAccessFullAdminPanel: shouldGrantServerAdmin,
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user