From b6b5fd1759c7d205dd8d497d7809ec6e5ef7f6cb Mon Sep 17 00:00:00 2001 From: Antoine Moreaux Date: Fri, 17 Jan 2025 14:59:54 +0100 Subject: [PATCH] refactor(auth): remove redundant workspace lookup logic (#9716) Removed unnecessary workspace lookup in `findWorkspaceForSignInUp` when using password-based auth. Updated tests to validate the refactored behavior and ensure no regressions in workspace resolution for different auth scenarios. --- .../auth/services/auth.service.spec.ts | 104 +++++++++++++++++- .../auth/services/auth.service.ts | 8 -- 2 files changed, 102 insertions(+), 10 deletions(-) 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 613964c22..36d8ed949 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 @@ -3,6 +3,7 @@ import { getRepositoryToken } from '@nestjs/typeorm'; import { expect, jest } from '@jest/globals'; import bcrypt from 'bcrypt'; +import { Repository } from 'typeorm'; import { AppToken } from 'src/engine/core-modules/app-token/app-token.entity'; import { SignInUpService } from 'src/engine/core-modules/auth/services/sign-in-up.service'; @@ -36,6 +37,8 @@ const environmentServiceGetMock = jest.fn(); describe('AuthService', () => { let service: AuthService; let userService: UserService; + let workspaceRepository: Repository; + let socialSsoService: SocialSsoService; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -43,7 +46,9 @@ describe('AuthService', () => { AuthService, { provide: getRepositoryToken(Workspace, 'core'), - useValue: {}, + useValue: { + findOneBy: jest.fn(), + }, }, { provide: getRepositoryToken(User, 'core'), @@ -112,13 +117,19 @@ describe('AuthService', () => { }, { provide: SocialSsoService, - useValue: {}, + useValue: { + findWorkspaceFromWorkspaceIdOrAuthProvider: jest.fn(), + }, }, ], }).compile(); service = module.get(AuthService); userService = module.get(UserService); + socialSsoService = module.get(SocialSsoService); + workspaceRepository = module.get>( + getRepositoryToken(Workspace, 'core'), + ); }); beforeEach(() => { @@ -331,4 +342,93 @@ describe('AuthService', () => { expect(spy).toHaveBeenCalledTimes(0); }); + + it('findWorkspaceForSignInUp - signup password auth', async () => { + const spyWorkspaceRepository = jest.spyOn(workspaceRepository, 'findOneBy'); + const spySocialSsoService = jest.spyOn( + socialSsoService, + 'findWorkspaceFromWorkspaceIdOrAuthProvider', + ); + + const result = await service.findWorkspaceForSignInUp({ + authProvider: 'password', + workspaceId: 'workspaceId', + }); + + expect(result).toBeUndefined(); + expect(spyWorkspaceRepository).toHaveBeenCalledTimes(0); + expect(spySocialSsoService).toHaveBeenCalledTimes(0); + }); + it('findWorkspaceForSignInUp - signup password auth with workspaceInviteHash', async () => { + const spyWorkspaceRepository = jest + .spyOn(workspaceRepository, 'findOneBy') + .mockResolvedValue({} as Workspace); + const spySocialSsoService = jest.spyOn( + socialSsoService, + 'findWorkspaceFromWorkspaceIdOrAuthProvider', + ); + + const result = await service.findWorkspaceForSignInUp({ + authProvider: 'password', + workspaceId: 'workspaceId', + workspaceInviteHash: 'workspaceInviteHash', + }); + + expect(result).toBeDefined(); + expect(spyWorkspaceRepository).toHaveBeenCalledTimes(1); + expect(spySocialSsoService).toHaveBeenCalledTimes(0); + }); + it('findWorkspaceForSignInUp - signup social sso auth with workspaceInviteHash', async () => { + const spyWorkspaceRepository = jest + .spyOn(workspaceRepository, 'findOneBy') + .mockResolvedValue({} as Workspace); + const spySocialSsoService = jest.spyOn( + socialSsoService, + 'findWorkspaceFromWorkspaceIdOrAuthProvider', + ); + + const result = await service.findWorkspaceForSignInUp({ + authProvider: 'password', + workspaceId: 'workspaceId', + workspaceInviteHash: 'workspaceInviteHash', + }); + + expect(result).toBeDefined(); + expect(spyWorkspaceRepository).toHaveBeenCalledTimes(1); + expect(spySocialSsoService).toHaveBeenCalledTimes(0); + }); + it('findWorkspaceForSignInUp - signup social sso auth', async () => { + const spyWorkspaceRepository = jest.spyOn(workspaceRepository, 'findOneBy'); + + const spySocialSsoService = jest + .spyOn(socialSsoService, 'findWorkspaceFromWorkspaceIdOrAuthProvider') + .mockResolvedValue({} as Workspace); + + const result = await service.findWorkspaceForSignInUp({ + authProvider: 'google', + workspaceId: 'workspaceId', + email: 'email', + }); + + expect(result).toBeDefined(); + expect(spyWorkspaceRepository).toHaveBeenCalledTimes(0); + expect(spySocialSsoService).toHaveBeenCalledTimes(1); + }); + it('findWorkspaceForSignInUp - sso auth', async () => { + const spyWorkspaceRepository = jest.spyOn(workspaceRepository, 'findOneBy'); + + const spySocialSsoService = jest + .spyOn(socialSsoService, 'findWorkspaceFromWorkspaceIdOrAuthProvider') + .mockResolvedValue({} as Workspace); + + const result = await service.findWorkspaceForSignInUp({ + authProvider: 'sso', + workspaceId: 'workspaceId', + email: 'email', + }); + + expect(result).toBeDefined(); + expect(spyWorkspaceRepository).toHaveBeenCalledTimes(0); + expect(spySocialSsoService).toHaveBeenCalledTimes(1); + }); }); 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 0b77ff339..8a1e6ed4d 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 @@ -541,14 +541,6 @@ export class AuthService { ); } - if (params.authProvider === 'password' && params.workspaceId) { - return ( - (await this.workspaceRepository.findOneBy({ - id: params.workspaceId, - })) ?? undefined - ); - } - return undefined; }