refactor(invitation): streamline invitation validation logic (#9699)

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.
This commit is contained in:
Antoine Moreaux
2025-01-20 12:25:08 +01:00
committed by GitHub
parent 2c8954a44d
commit 59cb420d82
10 changed files with 222 additions and 195 deletions

View File

@ -176,10 +176,14 @@ export class AuthResolver {
workspaceId: signUpInput.workspaceId, workspaceId: signUpInput.workspaceId,
}); });
const invitation = await this.authService.findInvitationForSignInUp({ const invitation =
currentWorkspace, currentWorkspace && signUpInput.workspacePersonalInviteToken
workspacePersonalInviteToken: signUpInput.workspacePersonalInviteToken, ? await this.authService.findInvitationForSignInUp({
}); currentWorkspace,
workspacePersonalInviteToken:
signUpInput.workspacePersonalInviteToken,
})
: undefined;
const existingUser = await this.userRepository.findOne({ const existingUser = await this.userRepository.findOne({
where: { where: {

View File

@ -66,11 +66,14 @@ export class GoogleAuthController {
}); });
try { try {
const invitation = await this.authService.findInvitationForSignInUp({ const invitation =
currentWorkspace, currentWorkspace && workspacePersonalInviteToken && email
workspacePersonalInviteToken, ? await this.authService.findInvitationForSignInUp({
email, currentWorkspace,
}); workspacePersonalInviteToken,
email,
})
: undefined;
const existingUser = await this.userRepository.findOne({ const existingUser = await this.userRepository.findOne({
where: { email }, where: { email },

View File

@ -64,11 +64,14 @@ export class MicrosoftAuthController {
}); });
try { try {
const invitation = await this.authService.findInvitationForSignInUp({ const invitation =
currentWorkspace, currentWorkspace && workspacePersonalInviteToken && email
workspacePersonalInviteToken, ? await this.authService.findInvitationForSignInUp({
email, currentWorkspace,
}); workspacePersonalInviteToken,
email,
})
: undefined;
const existingUser = await this.userRepository.findOne({ const existingUser = await this.userRepository.findOne({
where: { email }, where: { email },

View File

@ -153,10 +153,13 @@ export class SSOAuthController {
); );
} }
const invitation = await this.authService.findInvitationForSignInUp({ const invitation =
currentWorkspace: identityProvider.workspace, payload.email && identityProvider.workspace
email: payload.email, ? await this.authService.findInvitationForSignInUp({
}); currentWorkspace: identityProvider.workspace,
email: payload.email,
})
: undefined;
const existingUser = await this.userRepository.findOne({ const existingUser = await this.userRepository.findOne({
where: { where: {

View File

@ -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 { SocialSsoService } from 'src/engine/core-modules/auth/services/social-sso.service';
import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
import { ExistingUserOrNewUser } from 'src/engine/core-modules/auth/types/signInUp.type'; 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'; import { AuthService } from './auth.service';
@ -29,7 +33,7 @@ const UserWorkspaceFindOneByMock = jest.fn();
const userWorkspaceServiceCheckUserWorkspaceExistsMock = jest.fn(); const userWorkspaceServiceCheckUserWorkspaceExistsMock = jest.fn();
const workspaceInvitationGetOneWorkspaceInvitationMock = jest.fn(); const workspaceInvitationGetOneWorkspaceInvitationMock = jest.fn();
const workspaceInvitationValidateInvitationMock = jest.fn(); const workspaceInvitationValidatePersonalInvitationMock = jest.fn();
const userWorkspaceAddUserToWorkspaceMock = jest.fn(); const userWorkspaceAddUserToWorkspaceMock = jest.fn();
const environmentServiceGetMock = jest.fn(); const environmentServiceGetMock = jest.fn();
@ -112,7 +116,8 @@ describe('AuthService', () => {
useValue: { useValue: {
getOneWorkspaceInvitation: getOneWorkspaceInvitation:
workspaceInvitationGetOneWorkspaceInvitationMock, workspaceInvitationGetOneWorkspaceInvitationMock,
validateInvitation: workspaceInvitationValidateInvitationMock, validatePersonalInvitation:
workspaceInvitationValidatePersonalInvitationMock,
}, },
}, },
{ {
@ -193,7 +198,7 @@ describe('AuthService', () => {
userWorkspaceServiceCheckUserWorkspaceExistsMock.mockReturnValueOnce(false); userWorkspaceServiceCheckUserWorkspaceExistsMock.mockReturnValueOnce(false);
workspaceInvitationGetOneWorkspaceInvitationMock.mockReturnValueOnce({}); workspaceInvitationGetOneWorkspaceInvitationMock.mockReturnValueOnce({});
workspaceInvitationValidateInvitationMock.mockReturnValueOnce({}); workspaceInvitationValidatePersonalInvitationMock.mockReturnValueOnce({});
userWorkspaceAddUserToWorkspaceMock.mockReturnValueOnce({}); userWorkspaceAddUserToWorkspaceMock.mockReturnValueOnce({});
const response = await service.challenge( const response = await service.challenge(
@ -216,40 +221,20 @@ describe('AuthService', () => {
expect( expect(
workspaceInvitationGetOneWorkspaceInvitationMock, workspaceInvitationGetOneWorkspaceInvitationMock,
).toHaveBeenCalledTimes(1); ).toHaveBeenCalledTimes(1);
expect(workspaceInvitationValidateInvitationMock).toHaveBeenCalledTimes(1); expect(
workspaceInvitationValidatePersonalInvitationMock,
).toHaveBeenCalledTimes(1);
expect(userWorkspaceAddUserToWorkspaceMock).toHaveBeenCalledTimes(1); expect(userWorkspaceAddUserToWorkspaceMock).toHaveBeenCalledTimes(1);
expect(UserFindOneMock).toHaveBeenCalledTimes(1); expect(UserFindOneMock).toHaveBeenCalledTimes(1);
}); });
it('checkAccessForSignIn - allow signin for existing user who target a workspace with right access', async () => { describe('checkAccessForSignIn', () => {
const spy = jest it('checkAccessForSignIn - allow signin for existing user who target a workspace with right access', async () => {
.spyOn(userService, 'hasUserAccessToWorkspaceOrThrow') const spy = jest
.mockResolvedValue(); .spyOn(userService, 'hasUserAccessToWorkspaceOrThrow')
.mockResolvedValue();
await service.checkAccessForSignIn({ 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({
userData: { userData: {
type: 'existingUser', type: 'existingUser',
existingUser: { existingUser: {
@ -260,87 +245,143 @@ describe('AuthService', () => {
workspaceInviteHash: undefined, workspaceInviteHash: undefined,
workspace: { workspace: {
id: 'workspace-id', id: 'workspace-id',
isPublicInviteLinkEnabled: true,
} as Workspace, } 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,
}); });
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 () => { await expect(
const spy = jest service.checkAccessForSignIn({
.spyOn(userService, 'hasUserAccessToWorkspaceOrThrow') userData: {
.mockResolvedValue(); 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({ expect(spy).toHaveBeenCalledTimes(1);
userData: {
type: 'existingUser',
existingUser: {
id: 'user-id',
},
} as ExistingUserOrNewUser['userData'],
invitation: undefined,
workspaceInviteHash: undefined,
workspace: undefined,
}); });
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 () => { await expect(
const spy = jest service.checkAccessForSignIn({
.spyOn(userService, 'hasUserAccessToWorkspaceOrThrow') userData: {
.mockResolvedValue(); 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({ expect(spy).toHaveBeenCalledTimes(0);
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 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 () => { await service.checkAccessForSignIn({
const spy = jest userData: {
.spyOn(userService, 'hasUserAccessToWorkspaceOrThrow') type: 'newUser',
.mockResolvedValue(); newUserPayload: {},
} as ExistingUserOrNewUser['userData'],
invitation: undefined,
workspaceInviteHash: undefined,
workspace: undefined,
});
await service.checkAccessForSignIn({ expect(spy).toHaveBeenCalledTimes(0);
userData: {
type: 'newUser',
newUserPayload: {},
} as ExistingUserOrNewUser['userData'],
invitation: undefined,
workspaceInviteHash: 'workspaceInviteHash',
workspace: {} as Workspace,
}); });
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 () => { it('findWorkspaceForSignInUp - signup password auth', async () => {

View File

@ -98,7 +98,7 @@ export class AuthService {
); );
if (invitation) { if (invitation) {
await this.workspaceInvitationService.validateInvitation({ await this.workspaceInvitationService.validatePersonalInvitation({
workspacePersonalInviteToken: invitation.value, workspacePersonalInviteToken: invitation.value,
email: user.email, email: user.email,
}); });
@ -480,25 +480,20 @@ export class AuthService {
workspacePersonalInviteToken, workspacePersonalInviteToken,
email, email,
}: { }: {
currentWorkspace?: Workspace | null; currentWorkspace: Workspace | null;
workspacePersonalInviteToken?: string; workspacePersonalInviteToken?: string;
email?: string; email?: string;
}) { }) {
if (!currentWorkspace) return undefined; if (!currentWorkspace || !workspacePersonalInviteToken) return undefined;
const qr = this.appTokenRepository.createQueryBuilder('appToken'); const qr = this.appTokenRepository
.createQueryBuilder('appToken')
qr.where('"appToken"."workspaceId" = :workspaceId', { .where('"appToken"."workspaceId" = :workspaceId', {
workspaceId: currentWorkspace.id, workspaceId: currentWorkspace.id,
}).andWhere('"appToken".type = :type', { })
type: AppTokenType.InvitationToken, .andWhere('"appToken".type = :type', {
}); type: AppTokenType.InvitationToken,
if (email) {
qr.andWhere('"appToken".context->>\'email\' = :email', {
email,
}); });
}
if (workspacePersonalInviteToken) { if (workspacePersonalInviteToken) {
qr.andWhere('"appToken".value = :personalInviteToken', { 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; return (await qr.getOne()) ?? undefined;
} }
@ -567,18 +568,40 @@ export class AuthService {
workspaceInviteHash?: string; workspaceInviteHash?: string;
} & ExistingUserOrNewUser & } & ExistingUserOrNewUser &
SignInUpBaseParams) { SignInUpBaseParams) {
const hasInvitation = invitation || workspaceInviteHash; const hasPublicInviteLink = !!workspaceInviteHash;
const hasPersonalInvitation = !!invitation;
const isInvitedToWorkspace = hasPersonalInvitation || hasPublicInviteLink;
const isTargetAnExistingWorkspace = !!workspace; const isTargetAnExistingWorkspace = !!workspace;
const isAnExistingUser = userData.type === 'existingUser'; 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( return await this.userService.hasUserAccessToWorkspaceOrThrow(
userData.existingUser.id, userData.existingUser.id,
workspace.id, workspace.id,
); );
} }
if (!hasInvitation && isTargetAnExistingWorkspace && !isAnExistingUser) { if (
!isInvitedToWorkspace &&
isTargetAnExistingWorkspace &&
!isAnExistingUser
) {
throw new AuthException( throw new AuthException(
'User does not have access to this workspace', 'User does not have access to this workspace',
AuthExceptionCode.FORBIDDEN_EXCEPTION, AuthExceptionCode.FORBIDDEN_EXCEPTION,

View File

@ -67,7 +67,7 @@ describe('SignInUpService', () => {
{ {
provide: WorkspaceInvitationService, provide: WorkspaceInvitationService,
useValue: { useValue: {
validateInvitation: jest.fn(), validatePersonalInvitation: jest.fn(),
invalidateWorkspaceInvitation: jest.fn(), invalidateWorkspaceInvitation: jest.fn(),
}, },
}, },
@ -145,7 +145,7 @@ describe('SignInUpService', () => {
}; };
jest jest
.spyOn(workspaceInvitationService, 'validateInvitation') .spyOn(workspaceInvitationService, 'validatePersonalInvitation')
.mockResolvedValue({ .mockResolvedValue({
isValid: true, isValid: true,
workspace: params.workspace as Workspace, workspace: params.workspace as Workspace,
@ -163,7 +163,9 @@ describe('SignInUpService', () => {
expect(result.workspace).toEqual(params.workspace); expect(result.workspace).toEqual(params.workspace);
expect(result.user).toBeDefined(); expect(result.user).toBeDefined();
expect(workspaceInvitationService.validateInvitation).toHaveBeenCalledWith({ expect(
workspaceInvitationService.validatePersonalInvitation,
).toHaveBeenCalledWith({
workspacePersonalInviteToken: 'invitationToken', workspacePersonalInviteToken: 'invitationToken',
email: 'test@example.com', email: 'test@example.com',
}); });

View File

@ -173,7 +173,7 @@ export class SignInUpService {
} }
const invitationValidation = const invitationValidation =
await this.workspaceInvitationService.validateInvitation({ await this.workspaceInvitationService.validatePersonalInvitation({
workspacePersonalInviteToken: params.invitation.value, workspacePersonalInviteToken: params.invitation.value,
email, email,
}); });

View File

@ -125,10 +125,11 @@ export class UserWorkspaceService extends TypeOrmQueryService<UserWorkspace> {
} }
async addUserToWorkspaceByInviteToken(inviteToken: string, user: User) { async addUserToWorkspaceByInviteToken(inviteToken: string, user: User) {
const appToken = await this.workspaceInvitationService.validateInvitation({ const appToken =
workspacePersonalInviteToken: inviteToken, await this.workspaceInvitationService.validatePersonalInvitation({
email: user.email, workspacePersonalInviteToken: inviteToken,
}); email: user.email,
});
await this.workspaceInvitationService.invalidateWorkspaceInvitation( await this.workspaceInvitationService.invalidateWorkspaceInvitation(
appToken.workspace.id, appToken.workspace.id,

View File

@ -37,8 +37,6 @@ export class WorkspaceInvitationService {
constructor( constructor(
@InjectRepository(AppToken, 'core') @InjectRepository(AppToken, 'core')
private readonly appTokenRepository: Repository<AppToken>, private readonly appTokenRepository: Repository<AppToken>,
@InjectRepository(Workspace, 'core')
private readonly workspaceRepository: Repository<Workspace>,
@InjectRepository(UserWorkspace, 'core') @InjectRepository(UserWorkspace, 'core')
private readonly userWorkspaceRepository: Repository<UserWorkspace>, private readonly userWorkspaceRepository: Repository<UserWorkspace>,
private readonly environmentService: EnvironmentService, private readonly environmentService: EnvironmentService,
@ -47,32 +45,7 @@ export class WorkspaceInvitationService {
private readonly domainManagerService: DomainManagerService, private readonly domainManagerService: DomainManagerService,
) {} ) {}
// VALIDATIONS METHODS async validatePersonalInvitation({
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({
workspacePersonalInviteToken, workspacePersonalInviteToken,
email, 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({ async findInvitationByWorkspaceSubdomainAndUserEmail({
subdomain, subdomain,
email, email,