fix(auth): additional workspace and identity validation in auth flows (#21347)
## What Adds validation across three auth flows so that a session or reset link is consistently scoped to a workspace the authenticated principal belongs to, and to a verified identity. - **Access token** (`jwt.auth.strategy.ts`): when resolving the request's user context, the token's `userWorkspaceId` must belong to the token's `workspaceId` — the same check the application-token path already performs. - **OIDC** (`oidc.auth.strategy.ts`): reject sign-in when the identity provider explicitly reports `email_verified: false`. - **Password reset** (`reset-password.service.ts`): a supplied `workspaceId` is only used when the user is a member of it; otherwise it falls back to the user's own first password-auth-enabled workspace. ## Tests - `jwt.auth.strategy.spec.ts`: rejects an access token whose user workspace belongs to a different workspace than the token; existing mocks updated to carry the cached `workspaceId`. - `oidc.auth.strategy.spec.ts` (new): rejects unverified email; accepts verified and absent-claim cases. - `reset-password.service.spec.ts`: falls back when the supplied `workspaceId` is not one the user belongs to. `tsgo`, `oxlint` and `oxfmt` all clean on the changed files.
This commit is contained in:
+45
@@ -115,6 +115,9 @@ describe('ResetPasswordService', () => {
|
||||
jest
|
||||
.spyOn(userService, 'findUserByEmailOrThrow')
|
||||
.mockResolvedValue(mockUser as UserEntity);
|
||||
jest
|
||||
.spyOn(workspaceRepository, 'findOne')
|
||||
.mockResolvedValue({ id: 'workspace-id' } as WorkspaceEntity);
|
||||
jest.spyOn(appTokenRepository, 'findOne').mockResolvedValue(null);
|
||||
jest
|
||||
.spyOn(appTokenRepository, 'save')
|
||||
@@ -135,6 +138,45 @@ describe('ResetPasswordService', () => {
|
||||
type: AppTokenType.PasswordResetToken,
|
||||
}),
|
||||
);
|
||||
expect(workspaceRepository.findOne).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
where: expect.objectContaining({
|
||||
id: 'workspace-id',
|
||||
isPasswordAuthEnabled: true,
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('falls back to the user own workspace when the provided workspaceId is not one they belong to', async () => {
|
||||
const mockUser = { id: '1', email: 'test@example.com' };
|
||||
|
||||
jest
|
||||
.spyOn(userService, 'findUserByEmailOrThrow')
|
||||
.mockResolvedValue(mockUser as UserEntity);
|
||||
jest
|
||||
.spyOn(workspaceRepository, 'findOne')
|
||||
.mockResolvedValueOnce(null)
|
||||
.mockResolvedValueOnce({
|
||||
id: 'fallback-workspace-id',
|
||||
} as WorkspaceEntity);
|
||||
jest.spyOn(appTokenRepository, 'findOne').mockResolvedValue(null);
|
||||
jest
|
||||
.spyOn(appTokenRepository, 'save')
|
||||
.mockResolvedValue({} as AppTokenEntity);
|
||||
jest.spyOn(twentyConfigService, 'get').mockReturnValue('1h');
|
||||
|
||||
const result = await service.generatePasswordResetToken(
|
||||
'test@example.com',
|
||||
'foreign-workspace-id',
|
||||
);
|
||||
|
||||
expect(result.workspaceId).toBe('fallback-workspace-id');
|
||||
expect(appTokenRepository.save).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
workspaceId: 'fallback-workspace-id',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should resolve workspace when workspaceId is missing', async () => {
|
||||
@@ -205,6 +247,9 @@ describe('ResetPasswordService', () => {
|
||||
jest
|
||||
.spyOn(userService, 'findUserByEmailOrThrow')
|
||||
.mockResolvedValue(mockUser as UserEntity);
|
||||
jest
|
||||
.spyOn(workspaceRepository, 'findOne')
|
||||
.mockResolvedValue({ id: 'workspace-id' } as WorkspaceEntity);
|
||||
jest
|
||||
.spyOn(appTokenRepository, 'findOne')
|
||||
.mockResolvedValue(mockExistingToken as AppTokenEntity);
|
||||
|
||||
+29
-3
@@ -62,9 +62,10 @@ export class ResetPasswordService {
|
||||
}),
|
||||
);
|
||||
|
||||
const targetWorkspaceId =
|
||||
workspaceId ??
|
||||
(await this.findFirstPasswordAuthEnabledWorkspaceIdOrThrow(user.id));
|
||||
const targetWorkspaceId = await this.resolveTargetWorkspaceId(
|
||||
user.id,
|
||||
workspaceId,
|
||||
);
|
||||
|
||||
const expiresIn = this.twentyConfigService.get(
|
||||
'PASSWORD_RESET_TOKEN_EXPIRES_IN',
|
||||
@@ -124,6 +125,31 @@ export class ResetPasswordService {
|
||||
};
|
||||
}
|
||||
|
||||
private async resolveTargetWorkspaceId(
|
||||
userId: string,
|
||||
workspaceId?: string,
|
||||
): Promise<string> {
|
||||
if (!isDefined(workspaceId)) {
|
||||
return this.findFirstPasswordAuthEnabledWorkspaceIdOrThrow(userId);
|
||||
}
|
||||
|
||||
const requestedWorkspace = await this.workspaceRepository.findOne({
|
||||
where: {
|
||||
id: workspaceId,
|
||||
isPasswordAuthEnabled: true,
|
||||
workspaceUsers: {
|
||||
user: {
|
||||
id: userId,
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
return isDefined(requestedWorkspace)
|
||||
? requestedWorkspace.id
|
||||
: this.findFirstPasswordAuthEnabledWorkspaceIdOrThrow(userId);
|
||||
}
|
||||
|
||||
async sendEmailPasswordResetLink({
|
||||
resetToken,
|
||||
email,
|
||||
|
||||
+61
@@ -332,6 +332,61 @@ describe('JwtAuthStrategy', () => {
|
||||
expect(user.user?.lastName).toBe('lastNameDefault');
|
||||
expect(user.userWorkspaceId).toBe(validUserWorkspaceId);
|
||||
});
|
||||
|
||||
it('should reject when the user workspace belongs to a different workspace than the token', async () => {
|
||||
const validUserId = 'valid-user-id';
|
||||
const validUserWorkspaceId = randomUUID();
|
||||
const tokenWorkspaceId = randomUUID();
|
||||
const otherWorkspaceId = randomUUID();
|
||||
|
||||
const payload = {
|
||||
sub: validUserId,
|
||||
type: JwtTokenTypeEnum.ACCESS,
|
||||
userWorkspaceId: validUserWorkspaceId,
|
||||
workspaceId: tokenWorkspaceId,
|
||||
};
|
||||
|
||||
const mockWorkspace = new WorkspaceEntity();
|
||||
|
||||
mockWorkspace.id = tokenWorkspaceId;
|
||||
workspaceStore[tokenWorkspaceId] = mockWorkspace;
|
||||
userStore[validUserId] = { id: validUserId, lastName: 'lastNameDefault' };
|
||||
|
||||
coreEntityCacheService.get.mockImplementation(
|
||||
async (keyName: string, entityId: string) => {
|
||||
if (keyName === 'workspaceEntity') {
|
||||
return workspaceStore[entityId] ?? null;
|
||||
}
|
||||
|
||||
if (keyName === 'user') {
|
||||
return userStore[entityId] ?? null;
|
||||
}
|
||||
|
||||
if (keyName === 'userWorkspaceEntity') {
|
||||
return {
|
||||
id: validUserWorkspaceId,
|
||||
workspaceId: otherWorkspaceId,
|
||||
user: { id: validUserId },
|
||||
workspace: { id: otherWorkspaceId },
|
||||
};
|
||||
}
|
||||
|
||||
return null;
|
||||
},
|
||||
);
|
||||
|
||||
strategy = createStrategy();
|
||||
|
||||
await expect(strategy.validate(payload as JwtPayload)).rejects.toThrow(
|
||||
new AuthException(
|
||||
'User or user workspace not found',
|
||||
expect.any(String),
|
||||
{
|
||||
userFriendlyMessage: msg`User does not have access to this workspace`,
|
||||
},
|
||||
),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('APPLICATION_ACCESS token validation', () => {
|
||||
@@ -516,6 +571,7 @@ describe('JwtAuthStrategy', () => {
|
||||
if (keyName === 'userWorkspaceEntity') {
|
||||
return {
|
||||
id: validUserWorkspaceId,
|
||||
workspaceId: validWorkspaceId,
|
||||
user: mockUser,
|
||||
workspace: mockWorkspace,
|
||||
};
|
||||
@@ -583,6 +639,7 @@ describe('JwtAuthStrategy', () => {
|
||||
if (keyName === 'userWorkspaceEntity') {
|
||||
return {
|
||||
id: validUserWorkspaceId,
|
||||
workspaceId: validWorkspaceId,
|
||||
user: mockUser,
|
||||
workspace: mockWorkspace,
|
||||
};
|
||||
@@ -644,6 +701,7 @@ describe('JwtAuthStrategy', () => {
|
||||
if (keyName === 'userWorkspaceEntity') {
|
||||
return {
|
||||
id: validUserWorkspaceId,
|
||||
workspaceId: validWorkspaceId,
|
||||
user: mockUser,
|
||||
workspace: mockWorkspace,
|
||||
};
|
||||
@@ -718,6 +776,7 @@ describe('JwtAuthStrategy', () => {
|
||||
if (keyName === 'userWorkspaceEntity') {
|
||||
return {
|
||||
id: validUserWorkspaceId,
|
||||
workspaceId: validWorkspaceId,
|
||||
user: mockUser,
|
||||
workspace: mockWorkspace,
|
||||
};
|
||||
@@ -826,6 +885,7 @@ describe('JwtAuthStrategy', () => {
|
||||
if (keyName === 'userWorkspaceEntity') {
|
||||
return {
|
||||
id: validUserWorkspaceId,
|
||||
workspaceId: validWorkspaceId,
|
||||
user: mockUser,
|
||||
workspace: mockWorkspace,
|
||||
};
|
||||
@@ -906,6 +966,7 @@ describe('JwtAuthStrategy', () => {
|
||||
if (keyName === 'userWorkspaceEntity') {
|
||||
return {
|
||||
id: validUserWorkspaceId,
|
||||
workspaceId: validWorkspaceId,
|
||||
user: mockUser,
|
||||
workspace: mockWorkspace,
|
||||
};
|
||||
|
||||
@@ -145,6 +145,7 @@ export class JwtAuthStrategy extends PassportStrategy(Strategy, 'jwt') {
|
||||
const userContext = await this.resolveUserContext({
|
||||
userId,
|
||||
userWorkspaceId: payload.userWorkspaceId,
|
||||
expectedWorkspaceId: workspace.id,
|
||||
});
|
||||
|
||||
assertIsDefinedOrThrow(
|
||||
|
||||
Reference in New Issue
Block a user