Skip to content

Commit

Permalink
refactor(invitation): streamline invitation validation logic (#9699)
Browse files Browse the repository at this point in the history 
Replaced `validateInvitation` with `validatePersonalInvitation` across
services for consistent and specific validation handling. Removed
outdated public invitation validation and improved error handling for
workspace invitations. Updated tests to align with the refactored logic
and added checks for edge cases.
  • Loading branch information
@AMoreaux
AMoreaux authored Jan 20, 2025
1 parent 2c8954a commit 59cb420
Show file tree
Hide file tree
Showing 10 changed files with 223 additions and 196 deletions.
12 changes: 8 additions & 4 deletions packages/twenty-server/src/engine/core-modules/auth/auth.resolver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
13 changes: 8 additions & 5 deletions packages/twenty-server/src/engine/core-modules/auth/controllers/google-auth.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down
13 changes: 8 additions & 5 deletions packages/twenty-server/src/engine/core-modules/auth/controllers/microsoft-auth.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down
11 changes: 7 additions & 4 deletions packages/twenty-server/src/engine/core-modules/auth/controllers/sso-auth.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
235 changes: 138 additions & 97 deletions packages/twenty-server/src/engine/core-modules/auth/services/auth.service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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();
Expand Down Expand Up @@ -112,7 +116,8 @@ describe('AuthService', () => {
useValue: {
getOneWorkspaceInvitation:
workspaceInvitationGetOneWorkspaceInvitationMock,
validateInvitation: workspaceInvitationValidateInvitationMock,
validatePersonalInvitation:
workspaceInvitationValidatePersonalInvitationMock,
},
},
{
Expand Down Expand Up @@ -193,7 +198,7 @@ describe('AuthService', () => {
userWorkspaceServiceCheckUserWorkspaceExistsMock.mockReturnValueOnce(false);

workspaceInvitationGetOneWorkspaceInvitationMock.mockReturnValueOnce({});
workspaceInvitationValidateInvitationMock.mockReturnValueOnce({});
workspaceInvitationValidatePersonalInvitationMock.mockReturnValueOnce({});
userWorkspaceAddUserToWorkspaceMock.mockReturnValueOnce({});

const response = await service.challenge(
Expand All @@ -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();

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'));
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 expect(
service.checkAccessForSignIn({
await service.checkAccessForSignIn({
userData: {
type: 'existingUser',
existingUser: {
Expand All @@ -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);
});
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,
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({
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'));

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');

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,
),
);

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();
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: 'existingUser',
existingUser: {
id: 'user-id',
},
} as ExistingUserOrNewUser['userData'],
invitation: undefined,
workspaceInviteHash: undefined,
workspace: undefined,
await service.checkAccessForSignIn({
userData: {
type: 'newUser',
newUserPayload: {},
} as ExistingUserOrNewUser['userData'],
invitation: undefined,
workspaceInviteHash: undefined,
workspace: undefined,
});

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();

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: undefined,
workspaceInviteHash: undefined,
workspace: undefined,
});

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 target a workspace with invitation', 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: 'workspaceInviteHash',
workspace: {} as Workspace,
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 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 () => {
Expand Down
Loading

0 comments on commit 59cb420

Please sign in to comment.