diff --git a/packages/twenty-server/src/engine/core-modules/auth/auth.resolver.ts b/packages/twenty-server/src/engine/core-modules/auth/auth.resolver.ts index 994662fda..0cf3190a7 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/auth.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/auth.resolver.ts @@ -176,10 +176,14 @@ export class AuthResolver { workspaceId: signUpInput.workspaceId, }); - const invitation = await this.authService.findInvitationForSignInUp({ - currentWorkspace, - workspacePersonalInviteToken: signUpInput.workspacePersonalInviteToken, - }); + const invitation = + currentWorkspace && signUpInput.workspacePersonalInviteToken + ? await this.authService.findInvitationForSignInUp({ + currentWorkspace, + workspacePersonalInviteToken: + signUpInput.workspacePersonalInviteToken, + }) + : undefined; const existingUser = await this.userRepository.findOne({ where: { diff --git a/packages/twenty-server/src/engine/core-modules/auth/controllers/google-auth.controller.ts b/packages/twenty-server/src/engine/core-modules/auth/controllers/google-auth.controller.ts index fe89ad551..644f857ab 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/controllers/google-auth.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/controllers/google-auth.controller.ts @@ -66,11 +66,14 @@ export class GoogleAuthController { }); try { - const invitation = await this.authService.findInvitationForSignInUp({ - currentWorkspace, - workspacePersonalInviteToken, - email, - }); + const invitation = + currentWorkspace && workspacePersonalInviteToken && email + ? await this.authService.findInvitationForSignInUp({ + currentWorkspace, + workspacePersonalInviteToken, + email, + }) + : undefined; const existingUser = await this.userRepository.findOne({ where: { email }, diff --git a/packages/twenty-server/src/engine/core-modules/auth/controllers/microsoft-auth.controller.ts b/packages/twenty-server/src/engine/core-modules/auth/controllers/microsoft-auth.controller.ts index 2485b1699..5e5a4e708 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/controllers/microsoft-auth.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/controllers/microsoft-auth.controller.ts @@ -64,11 +64,14 @@ export class MicrosoftAuthController { }); try { - const invitation = await this.authService.findInvitationForSignInUp({ - currentWorkspace, - workspacePersonalInviteToken, - email, - }); + const invitation = + currentWorkspace && workspacePersonalInviteToken && email + ? await this.authService.findInvitationForSignInUp({ + currentWorkspace, + workspacePersonalInviteToken, + email, + }) + : undefined; const existingUser = await this.userRepository.findOne({ where: { email }, diff --git a/packages/twenty-server/src/engine/core-modules/auth/controllers/sso-auth.controller.ts b/packages/twenty-server/src/engine/core-modules/auth/controllers/sso-auth.controller.ts index b41a09b54..5a8c35f75 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/controllers/sso-auth.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/controllers/sso-auth.controller.ts @@ -153,10 +153,13 @@ export class SSOAuthController { ); } - const invitation = await this.authService.findInvitationForSignInUp({ - currentWorkspace: identityProvider.workspace, - email: payload.email, - }); + const invitation = + payload.email && identityProvider.workspace + ? await this.authService.findInvitationForSignInUp({ + currentWorkspace: identityProvider.workspace, + email: payload.email, + }) + : undefined; const existingUser = await this.userRepository.findOne({ where: { diff --git a/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.spec.ts b/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.spec.ts index 36d8ed949..31ec9b0c8 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.spec.ts @@ -19,6 +19,10 @@ import { WorkspaceInvitationService } from 'src/engine/core-modules/workspace-in import { SocialSsoService } from 'src/engine/core-modules/auth/services/social-sso.service'; import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { ExistingUserOrNewUser } from 'src/engine/core-modules/auth/types/signInUp.type'; +import { + AuthException, + AuthExceptionCode, +} from 'src/engine/core-modules/auth/auth.exception'; import { AuthService } from './auth.service'; @@ -29,7 +33,7 @@ const UserWorkspaceFindOneByMock = jest.fn(); const userWorkspaceServiceCheckUserWorkspaceExistsMock = jest.fn(); const workspaceInvitationGetOneWorkspaceInvitationMock = jest.fn(); -const workspaceInvitationValidateInvitationMock = jest.fn(); +const workspaceInvitationValidatePersonalInvitationMock = jest.fn(); const userWorkspaceAddUserToWorkspaceMock = jest.fn(); const environmentServiceGetMock = jest.fn(); @@ -112,7 +116,8 @@ describe('AuthService', () => { useValue: { getOneWorkspaceInvitation: workspaceInvitationGetOneWorkspaceInvitationMock, - validateInvitation: workspaceInvitationValidateInvitationMock, + validatePersonalInvitation: + workspaceInvitationValidatePersonalInvitationMock, }, }, { @@ -193,7 +198,7 @@ describe('AuthService', () => { userWorkspaceServiceCheckUserWorkspaceExistsMock.mockReturnValueOnce(false); workspaceInvitationGetOneWorkspaceInvitationMock.mockReturnValueOnce({}); - workspaceInvitationValidateInvitationMock.mockReturnValueOnce({}); + workspaceInvitationValidatePersonalInvitationMock.mockReturnValueOnce({}); userWorkspaceAddUserToWorkspaceMock.mockReturnValueOnce({}); const response = await service.challenge( @@ -216,40 +221,20 @@ describe('AuthService', () => { expect( workspaceInvitationGetOneWorkspaceInvitationMock, ).toHaveBeenCalledTimes(1); - expect(workspaceInvitationValidateInvitationMock).toHaveBeenCalledTimes(1); + expect( + workspaceInvitationValidatePersonalInvitationMock, + ).toHaveBeenCalledTimes(1); expect(userWorkspaceAddUserToWorkspaceMock).toHaveBeenCalledTimes(1); expect(UserFindOneMock).toHaveBeenCalledTimes(1); }); - it('checkAccessForSignIn - allow signin for existing user who target a workspace with right access', async () => { - const spy = jest - .spyOn(userService, 'hasUserAccessToWorkspaceOrThrow') - .mockResolvedValue(); + describe('checkAccessForSignIn', () => { + it('checkAccessForSignIn - allow signin for existing user who target a workspace with right access', async () => { + const spy = jest + .spyOn(userService, 'hasUserAccessToWorkspaceOrThrow') + .mockResolvedValue(); - await service.checkAccessForSignIn({ - userData: { - type: 'existingUser', - existingUser: { - id: 'user-id', - }, - } as ExistingUserOrNewUser['userData'], - invitation: undefined, - workspaceInviteHash: undefined, - workspace: { - id: 'workspace-id', - } as Workspace, - }); - - expect(spy).toHaveBeenCalledTimes(1); - }); - - it('checkAccessForSignIn - trigger error on existing user signin in unauthorized workspace', async () => { - const spy = jest - .spyOn(userService, 'hasUserAccessToWorkspaceOrThrow') - .mockRejectedValue(new Error('Access denied')); - - await expect( - service.checkAccessForSignIn({ + await service.checkAccessForSignIn({ userData: { type: 'existingUser', existingUser: { @@ -260,87 +245,143 @@ describe('AuthService', () => { workspaceInviteHash: undefined, workspace: { id: 'workspace-id', + isPublicInviteLinkEnabled: true, } as Workspace, - }), - ).rejects.toThrow(new Error('Access denied')); + }); - expect(spy).toHaveBeenCalledTimes(1); - }); - - it("checkAccessForSignIn - allow signup for new user who don't target a workspace", async () => { - const spy = jest - .spyOn(userService, 'hasUserAccessToWorkspaceOrThrow') - .mockResolvedValue(); - - await service.checkAccessForSignIn({ - userData: { - type: 'newUser', - newUserPayload: {}, - } as ExistingUserOrNewUser['userData'], - invitation: undefined, - workspaceInviteHash: undefined, - workspace: undefined, + expect(spy).toHaveBeenCalledTimes(1); }); - expect(spy).toHaveBeenCalledTimes(0); - }); + it('checkAccessForSignIn - trigger error on existing user signin in unauthorized workspace', async () => { + const spy = jest + .spyOn(userService, 'hasUserAccessToWorkspaceOrThrow') + .mockRejectedValue(new Error('Access denied')); - it("checkAccessForSignIn - allow signup for existing user who don't target a workspace", async () => { - const spy = jest - .spyOn(userService, 'hasUserAccessToWorkspaceOrThrow') - .mockResolvedValue(); + await expect( + service.checkAccessForSignIn({ + userData: { + type: 'existingUser', + existingUser: { + id: 'user-id', + }, + } as ExistingUserOrNewUser['userData'], + invitation: undefined, + workspaceInviteHash: undefined, + workspace: { + id: 'workspace-id', + isPublicInviteLinkEnabled: true, + } as Workspace, + }), + ).rejects.toThrow(new Error('Access denied')); - await service.checkAccessForSignIn({ - userData: { - type: 'existingUser', - existingUser: { - id: 'user-id', - }, - } as ExistingUserOrNewUser['userData'], - invitation: undefined, - workspaceInviteHash: undefined, - workspace: undefined, + expect(spy).toHaveBeenCalledTimes(1); }); - expect(spy).toHaveBeenCalledTimes(0); - }); + it('checkAccessForSignIn - trigger an error when a user attempts to sign up using a public link in a workspace where public links are disabled', async () => { + const spy = jest.spyOn(userService, 'hasUserAccessToWorkspaceOrThrow'); - it('checkAccessForSignIn - allow signup for new user who target a workspace with invitation', async () => { - const spy = jest - .spyOn(userService, 'hasUserAccessToWorkspaceOrThrow') - .mockResolvedValue(); + await expect( + service.checkAccessForSignIn({ + userData: { + type: 'existingUser', + existingUser: { + id: 'user-id', + }, + } as ExistingUserOrNewUser['userData'], + invitation: undefined, + workspaceInviteHash: 'workspaceInviteHash', + workspace: { + id: 'workspace-id', + isPublicInviteLinkEnabled: false, + } as Workspace, + }), + ).rejects.toThrow( + new AuthException( + 'Public invite link is disabled for this workspace', + AuthExceptionCode.FORBIDDEN_EXCEPTION, + ), + ); - await service.checkAccessForSignIn({ - userData: { - type: 'existingUser', - existingUser: { - id: 'user-id', - }, - } as ExistingUserOrNewUser['userData'], - invitation: {} as AppToken, - workspaceInviteHash: undefined, - workspace: {} as Workspace, + expect(spy).toHaveBeenCalledTimes(0); }); - expect(spy).toHaveBeenCalledTimes(0); - }); + it("checkAccessForSignIn - allow signup for new user who don't target a workspace", async () => { + const spy = jest + .spyOn(userService, 'hasUserAccessToWorkspaceOrThrow') + .mockResolvedValue(); - it('checkAccessForSignIn - allow signup for new user who target a workspace with workspaceInviteHash', async () => { - const spy = jest - .spyOn(userService, 'hasUserAccessToWorkspaceOrThrow') - .mockResolvedValue(); + await service.checkAccessForSignIn({ + userData: { + type: 'newUser', + newUserPayload: {}, + } as ExistingUserOrNewUser['userData'], + invitation: undefined, + workspaceInviteHash: undefined, + workspace: undefined, + }); - await service.checkAccessForSignIn({ - userData: { - type: 'newUser', - newUserPayload: {}, - } as ExistingUserOrNewUser['userData'], - invitation: undefined, - workspaceInviteHash: 'workspaceInviteHash', - workspace: {} as Workspace, + expect(spy).toHaveBeenCalledTimes(0); }); - expect(spy).toHaveBeenCalledTimes(0); + it("checkAccessForSignIn - allow signup for existing user who don't target a workspace", async () => { + const spy = jest + .spyOn(userService, 'hasUserAccessToWorkspaceOrThrow') + .mockResolvedValue(); + + await service.checkAccessForSignIn({ + userData: { + type: 'existingUser', + existingUser: { + id: 'user-id', + }, + } as ExistingUserOrNewUser['userData'], + invitation: undefined, + workspaceInviteHash: undefined, + workspace: undefined, + }); + + expect(spy).toHaveBeenCalledTimes(0); + }); + + it('checkAccessForSignIn - allow signup for new user who target a workspace with invitation', async () => { + const spy = jest + .spyOn(userService, 'hasUserAccessToWorkspaceOrThrow') + .mockResolvedValue(); + + await service.checkAccessForSignIn({ + userData: { + type: 'existingUser', + existingUser: { + id: 'user-id', + }, + } as ExistingUserOrNewUser['userData'], + invitation: {} as AppToken, + workspaceInviteHash: undefined, + workspace: {} as Workspace, + }); + + expect(spy).toHaveBeenCalledTimes(0); + }); + + it('checkAccessForSignIn - allow signup for new user who target a workspace with public invitation', async () => { + const spy = jest + .spyOn(userService, 'hasUserAccessToWorkspaceOrThrow') + .mockResolvedValue(); + + await service.checkAccessForSignIn({ + userData: { + type: 'newUser', + newUserPayload: {}, + } as ExistingUserOrNewUser['userData'], + invitation: undefined, + workspaceInviteHash: 'workspaceInviteHash', + workspace: { + isPublicInviteLinkEnabled: true, + } as Workspace, + }); + + expect(spy).toHaveBeenCalledTimes(0); + }); }); it('findWorkspaceForSignInUp - signup password auth', async () => { diff --git a/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.ts b/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.ts index 8a1e6ed4d..252fc04a2 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.ts @@ -98,7 +98,7 @@ export class AuthService { ); if (invitation) { - await this.workspaceInvitationService.validateInvitation({ + await this.workspaceInvitationService.validatePersonalInvitation({ workspacePersonalInviteToken: invitation.value, email: user.email, }); @@ -480,25 +480,20 @@ export class AuthService { workspacePersonalInviteToken, email, }: { - currentWorkspace?: Workspace | null; + currentWorkspace: Workspace | null; workspacePersonalInviteToken?: string; email?: string; }) { - if (!currentWorkspace) return undefined; + if (!currentWorkspace || !workspacePersonalInviteToken) return undefined; - const qr = this.appTokenRepository.createQueryBuilder('appToken'); - - qr.where('"appToken"."workspaceId" = :workspaceId', { - workspaceId: currentWorkspace.id, - }).andWhere('"appToken".type = :type', { - type: AppTokenType.InvitationToken, - }); - - if (email) { - qr.andWhere('"appToken".context->>\'email\' = :email', { - email, + const qr = this.appTokenRepository + .createQueryBuilder('appToken') + .where('"appToken"."workspaceId" = :workspaceId', { + workspaceId: currentWorkspace.id, + }) + .andWhere('"appToken".type = :type', { + type: AppTokenType.InvitationToken, }); - } if (workspacePersonalInviteToken) { qr.andWhere('"appToken".value = :personalInviteToken', { @@ -506,6 +501,12 @@ export class AuthService { }); } + if (email) { + qr.andWhere('"appToken".context->>\'email\' = :email', { + email, + }); + } + return (await qr.getOne()) ?? undefined; } @@ -567,18 +568,40 @@ export class AuthService { workspaceInviteHash?: string; } & ExistingUserOrNewUser & SignInUpBaseParams) { - const hasInvitation = invitation || workspaceInviteHash; + const hasPublicInviteLink = !!workspaceInviteHash; + const hasPersonalInvitation = !!invitation; + const isInvitedToWorkspace = hasPersonalInvitation || hasPublicInviteLink; const isTargetAnExistingWorkspace = !!workspace; const isAnExistingUser = userData.type === 'existingUser'; - if (!hasInvitation && isTargetAnExistingWorkspace && isAnExistingUser) { + if ( + hasPublicInviteLink && + !hasPersonalInvitation && + workspace && + !workspace.isPublicInviteLinkEnabled + ) { + throw new AuthException( + 'Public invite link is disabled for this workspace', + AuthExceptionCode.FORBIDDEN_EXCEPTION, + ); + } + + if ( + !isInvitedToWorkspace && + isTargetAnExistingWorkspace && + isAnExistingUser + ) { return await this.userService.hasUserAccessToWorkspaceOrThrow( userData.existingUser.id, workspace.id, ); } - if (!hasInvitation && isTargetAnExistingWorkspace && !isAnExistingUser) { + if ( + !isInvitedToWorkspace && + isTargetAnExistingWorkspace && + !isAnExistingUser + ) { throw new AuthException( 'User does not have access to this workspace', AuthExceptionCode.FORBIDDEN_EXCEPTION, diff --git a/packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.spec.ts b/packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.spec.ts index 98dd651b2..b4cdba99e 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.spec.ts @@ -67,7 +67,7 @@ describe('SignInUpService', () => { { provide: WorkspaceInvitationService, useValue: { - validateInvitation: jest.fn(), + validatePersonalInvitation: jest.fn(), invalidateWorkspaceInvitation: jest.fn(), }, }, @@ -145,7 +145,7 @@ describe('SignInUpService', () => { }; jest - .spyOn(workspaceInvitationService, 'validateInvitation') + .spyOn(workspaceInvitationService, 'validatePersonalInvitation') .mockResolvedValue({ isValid: true, workspace: params.workspace as Workspace, @@ -163,7 +163,9 @@ describe('SignInUpService', () => { expect(result.workspace).toEqual(params.workspace); expect(result.user).toBeDefined(); - expect(workspaceInvitationService.validateInvitation).toHaveBeenCalledWith({ + expect( + workspaceInvitationService.validatePersonalInvitation, + ).toHaveBeenCalledWith({ workspacePersonalInviteToken: 'invitationToken', email: 'test@example.com', }); 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 41ba1c677..e0237027a 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 @@ -173,7 +173,7 @@ export class SignInUpService { } const invitationValidation = - await this.workspaceInvitationService.validateInvitation({ + await this.workspaceInvitationService.validatePersonalInvitation({ workspacePersonalInviteToken: params.invitation.value, email, }); diff --git a/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.ts b/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.ts index 488c8a2a2..596bab9af 100644 --- a/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.ts +++ b/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.ts @@ -125,10 +125,11 @@ export class UserWorkspaceService extends TypeOrmQueryService { } async addUserToWorkspaceByInviteToken(inviteToken: string, user: User) { - const appToken = await this.workspaceInvitationService.validateInvitation({ - workspacePersonalInviteToken: inviteToken, - email: user.email, - }); + const appToken = + await this.workspaceInvitationService.validatePersonalInvitation({ + workspacePersonalInviteToken: inviteToken, + email: user.email, + }); await this.workspaceInvitationService.invalidateWorkspaceInvitation( appToken.workspace.id, diff --git a/packages/twenty-server/src/engine/core-modules/workspace-invitation/services/workspace-invitation.service.ts b/packages/twenty-server/src/engine/core-modules/workspace-invitation/services/workspace-invitation.service.ts index 8d4594ab9..34d0d6250 100644 --- a/packages/twenty-server/src/engine/core-modules/workspace-invitation/services/workspace-invitation.service.ts +++ b/packages/twenty-server/src/engine/core-modules/workspace-invitation/services/workspace-invitation.service.ts @@ -37,8 +37,6 @@ export class WorkspaceInvitationService { constructor( @InjectRepository(AppToken, 'core') private readonly appTokenRepository: Repository, - @InjectRepository(Workspace, 'core') - private readonly workspaceRepository: Repository, @InjectRepository(UserWorkspace, 'core') private readonly userWorkspaceRepository: Repository, private readonly environmentService: EnvironmentService, @@ -47,32 +45,7 @@ export class WorkspaceInvitationService { private readonly domainManagerService: DomainManagerService, ) {} - // VALIDATIONS METHODS - private async validatePublicInvitation(workspaceInviteHash: string) { - const workspace = await this.workspaceRepository.findOne({ - where: { - inviteHash: workspaceInviteHash, - }, - }); - - if (!workspace) { - throw new AuthException( - 'Workspace not found', - AuthExceptionCode.WORKSPACE_NOT_FOUND, - ); - } - - if (!workspace.isPublicInviteLinkEnabled) { - throw new AuthException( - 'Workspace does not allow public invites', - AuthExceptionCode.FORBIDDEN_EXCEPTION, - ); - } - - return { isValid: true, workspace }; - } - - private async validatePersonalInvitation({ + async validatePersonalInvitation({ workspacePersonalInviteToken, email, }: { @@ -109,32 +82,6 @@ export class WorkspaceInvitationService { } } - async validateInvitation({ - workspacePersonalInviteToken, - workspaceInviteHash, - email, - }: { - workspacePersonalInviteToken?: string; - workspaceInviteHash?: string; - email: string; - }) { - if (workspacePersonalInviteToken) { - return await this.validatePersonalInvitation({ - workspacePersonalInviteToken, - email, - }); - } - - if (workspaceInviteHash) { - return await this.validatePublicInvitation(workspaceInviteHash); - } - - throw new AuthException( - 'Invitation invalid', - AuthExceptionCode.FORBIDDEN_EXCEPTION, - ); - } - async findInvitationByWorkspaceSubdomainAndUserEmail({ subdomain, email,