forked from twentyhq/twenty
-
Notifications
You must be signed in to change notification settings - Fork 3
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Feat/migrate password reset token to app token table (twentyhq#5051)
# This PR - Fix twentyhq#5021 - Migrates `passwordResetToken` and `passwordResetTokenExpiresAt` fields from `core.users` to `core.appToken` - Marks those fields as `deprecated` so we can remove them later if we are happy with the transition -- I took this decision on my own, @FellipeMTX let me know what you think about it, we can also remove them straight away if you think it's better - Fixed the `database:migration` script from the `twenty-server` to: ```json "database:migrate": { "executor": "nx:run-commands", "dependsOn": ["build"], // added this line "options": { "cwd": "packages/twenty-server", "commands": [ "nx typeorm -- migration:run -d src/database/typeorm/metadata/metadata.datasource", "nx typeorm -- migration:run -d src/database/typeorm/core/core.datasource" ], "parallel": false } }, ``` The migration script wasn't running because the builds were not executed - [x] Added unit tests for the token.service file's changes Looking forward to hearing feedback from you cc: @charlesBochet --------- Co-authored-by: Weiko <[email protected]>
- Loading branch information
Showing
6 changed files
with
248 additions
and
40 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,21 @@ | ||
| import { Test, TestingModule } from '@nestjs/testing'; | ||
| import { JwtService } from '@nestjs/jwt'; | ||
| import { getRepositoryToken } from '@nestjs/typeorm'; | ||
| import { | ||
| BadRequestException, | ||
| InternalServerErrorException, | ||
| NotFoundException, | ||
| } from '@nestjs/common'; | ||
|
|
||
| import crypto from 'crypto'; | ||
|
|
||
| import { IsNull, MoreThan, Repository } from 'typeorm'; | ||
|
|
||
| import { EnvironmentService } from 'src/engine/integrations/environment/environment.service'; | ||
| import { AppToken } from 'src/engine/core-modules/app-token/app-token.entity'; | ||
| import { | ||
| AppToken, | ||
| AppTokenType, | ||
| } from 'src/engine/core-modules/app-token/app-token.entity'; | ||
| import { User } from 'src/engine/core-modules/user/user.entity'; | ||
| import { JwtAuthStrategy } from 'src/engine/core-modules/auth/strategies/jwt.auth.strategy'; | ||
| import { EmailService } from 'src/engine/integrations/email/email.service'; | ||
|
|
@@ -13,34 +25,48 @@ import { TokenService } from './token.service'; | |
|
|
||
| describe('TokenService', () => { | ||
| let service: TokenService; | ||
| let environmentService: EnvironmentService; | ||
| let userRepository: Repository<User>; | ||
| let appTokenRepository: Repository<AppToken>; | ||
|
|
||
| beforeEach(async () => { | ||
| const module: TestingModule = await Test.createTestingModule({ | ||
| providers: [ | ||
| TokenService, | ||
| { | ||
| provide: JwtService, | ||
| useValue: {}, | ||
| useValue: { | ||
| sign: jest.fn().mockReturnValue('mock-jwt-token'), | ||
| }, | ||
| }, | ||
| { | ||
| provide: JwtAuthStrategy, | ||
| useValue: {}, | ||
| }, | ||
| { | ||
| provide: EnvironmentService, | ||
| useValue: {}, | ||
| useValue: { | ||
| get: jest.fn().mockReturnValue('some-value'), | ||
| }, | ||
| }, | ||
| { | ||
| provide: EmailService, | ||
| useValue: {}, | ||
| useValue: { | ||
| send: jest.fn(), | ||
| }, | ||
| }, | ||
| { | ||
| provide: getRepositoryToken(User, 'core'), | ||
| useValue: {}, | ||
| useValue: { | ||
| findOneBy: jest.fn(), | ||
| }, | ||
| }, | ||
| { | ||
| provide: getRepositoryToken(AppToken, 'core'), | ||
| useValue: {}, | ||
| useValue: { | ||
| findOne: jest.fn(), | ||
| save: jest.fn(), | ||
| }, | ||
| }, | ||
| { | ||
| provide: getRepositoryToken(Workspace, 'core'), | ||
|
|
@@ -50,9 +76,167 @@ describe('TokenService', () => { | |
| }).compile(); | ||
|
|
||
| service = module.get<TokenService>(TokenService); | ||
| environmentService = module.get<EnvironmentService>(EnvironmentService); | ||
| userRepository = module.get(getRepositoryToken(User, 'core')); | ||
| appTokenRepository = module.get(getRepositoryToken(AppToken, 'core')); | ||
| }); | ||
|
|
||
| it('should be defined', () => { | ||
| expect(service).toBeDefined(); | ||
| }); | ||
|
|
||
| describe('generatePasswordResetToken', () => { | ||
| it('should generate a new password reset token when no existing token is found', async () => { | ||
| const mockUser = { id: '1', email: '[email protected]' } as User; | ||
| const expiresIn = '3600000'; // 1 hour in ms | ||
|
|
||
| jest.spyOn(userRepository, 'findOneBy').mockResolvedValue(mockUser); | ||
| jest.spyOn(appTokenRepository, 'findOne').mockResolvedValue(null); | ||
| jest.spyOn(environmentService, 'get').mockReturnValue(expiresIn); | ||
| jest | ||
| .spyOn(appTokenRepository, 'save') | ||
| .mockImplementation(async (token) => token as AppToken); | ||
|
|
||
| const result = await service.generatePasswordResetToken(mockUser.email); | ||
|
|
||
| expect(userRepository.findOneBy).toHaveBeenCalledWith({ | ||
| email: mockUser.email, | ||
| }); | ||
| expect(appTokenRepository.findOne).toHaveBeenCalled(); | ||
| expect(appTokenRepository.save).toHaveBeenCalled(); | ||
| expect(result.passwordResetToken).toBeDefined(); | ||
| expect(result.passwordResetTokenExpiresAt).toBeDefined(); | ||
| }); | ||
|
|
||
| it('should throw BadRequestException if an existing valid token is found', async () => { | ||
| const mockUser = { id: '1', email: '[email protected]' } as User; | ||
| const mockToken = { | ||
| userId: '1', | ||
| type: AppTokenType.PasswordResetToken, | ||
| expiresAt: new Date(Date.now() + 10000), // expires 10 seconds in the future | ||
| } as AppToken; | ||
|
|
||
| jest.spyOn(userRepository, 'findOneBy').mockResolvedValue(mockUser); | ||
| jest.spyOn(appTokenRepository, 'findOne').mockResolvedValue(mockToken); | ||
| jest.spyOn(environmentService, 'get').mockReturnValue('3600000'); | ||
|
|
||
| await expect( | ||
| service.generatePasswordResetToken(mockUser.email), | ||
| ).rejects.toThrow(BadRequestException); | ||
| }); | ||
|
|
||
| it('should throw NotFoundException if no user is found', async () => { | ||
| jest.spyOn(userRepository, 'findOneBy').mockResolvedValue(null); | ||
|
|
||
| await expect( | ||
| service.generatePasswordResetToken('[email protected]'), | ||
| ).rejects.toThrow(NotFoundException); | ||
| }); | ||
|
|
||
| it('should throw InternalServerErrorException if environment variable is not found', async () => { | ||
| const mockUser = { id: '1', email: '[email protected]' } as User; | ||
|
|
||
| jest.spyOn(userRepository, 'findOneBy').mockResolvedValue(mockUser); | ||
| jest.spyOn(environmentService, 'get').mockReturnValue(''); // No environment variable set | ||
|
|
||
| await expect( | ||
| service.generatePasswordResetToken(mockUser.email), | ||
| ).rejects.toThrow(InternalServerErrorException); | ||
| }); | ||
| }); | ||
|
|
||
| describe('validatePasswordResetToken', () => { | ||
| it('should return user data for a valid and active token', async () => { | ||
| const resetToken = 'valid-reset-token'; | ||
| const hashedToken = crypto | ||
| .createHash('sha256') | ||
| .update(resetToken) | ||
| .digest('hex'); | ||
| const mockToken = { | ||
| userId: '1', | ||
| value: hashedToken, | ||
| type: AppTokenType.PasswordResetToken, | ||
| expiresAt: new Date(Date.now() + 10000), // Valid future date | ||
| }; | ||
| const mockUser = { id: '1', email: '[email protected]' }; | ||
|
|
||
| jest | ||
| .spyOn(appTokenRepository, 'findOne') | ||
| .mockResolvedValue(mockToken as AppToken); | ||
| jest | ||
| .spyOn(userRepository, 'findOneBy') | ||
| .mockResolvedValue(mockUser as User); | ||
|
|
||
| const result = await service.validatePasswordResetToken(resetToken); | ||
|
|
||
| expect(appTokenRepository.findOne).toHaveBeenCalledWith({ | ||
| where: { | ||
| value: hashedToken, | ||
| type: AppTokenType.PasswordResetToken, | ||
| expiresAt: MoreThan(new Date()), | ||
| revokedAt: IsNull(), | ||
| }, | ||
| }); | ||
| expect(userRepository.findOneBy).toHaveBeenCalledWith({ | ||
| id: mockToken.userId, | ||
| }); | ||
| expect(result).toEqual({ id: mockUser.id, email: mockUser.email }); | ||
| }); | ||
|
|
||
| it('should throw NotFoundException if token is invalid or expired', async () => { | ||
| const resetToken = 'invalid-reset-token'; | ||
|
|
||
| jest.spyOn(appTokenRepository, 'findOne').mockResolvedValue(null); | ||
|
|
||
| await expect( | ||
| service.validatePasswordResetToken(resetToken), | ||
| ).rejects.toThrow(NotFoundException); | ||
| }); | ||
|
|
||
| it('should throw NotFoundException if user does not exist for a valid token', async () => { | ||
| const resetToken = 'orphan-token'; | ||
| const hashedToken = crypto | ||
| .createHash('sha256') | ||
| .update(resetToken) | ||
| .digest('hex'); | ||
| const mockToken = { | ||
| userId: 'nonexistent-user', | ||
| value: hashedToken, | ||
| type: AppTokenType.PasswordResetToken, | ||
| expiresAt: new Date(Date.now() + 10000), // Valid future date | ||
| revokedAt: null, | ||
| }; | ||
|
|
||
| jest | ||
| .spyOn(appTokenRepository, 'findOne') | ||
| .mockResolvedValue(mockToken as AppToken); | ||
| jest.spyOn(userRepository, 'findOneBy').mockResolvedValue(null); | ||
|
|
||
| await expect( | ||
| service.validatePasswordResetToken(resetToken), | ||
| ).rejects.toThrow(NotFoundException); | ||
| }); | ||
|
|
||
| it('should throw NotFoundException if token is revoked', async () => { | ||
| const resetToken = 'revoked-token'; | ||
| const hashedToken = crypto | ||
| .createHash('sha256') | ||
| .update(resetToken) | ||
| .digest('hex'); | ||
| const mockToken = { | ||
| userId: '1', | ||
| value: hashedToken, | ||
| type: AppTokenType.PasswordResetToken, | ||
| expiresAt: new Date(Date.now() + 10000), | ||
| revokedAt: new Date(), | ||
| }; | ||
|
|
||
| jest | ||
| .spyOn(appTokenRepository, 'findOne') | ||
| .mockResolvedValue(mockToken as AppToken); | ||
| await expect( | ||
| service.validatePasswordResetToken(resetToken), | ||
| ).rejects.toThrow(NotFoundException); | ||
| }); | ||
| }); | ||
| }); | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.