From 768b5e7fb211e885fc25a1f8a4044a9c5041153d Mon Sep 17 00:00:00 2001 From: alexeh Date: Thu, 26 Sep 2024 08:14:25 +0200 Subject: [PATCH] refactor --- api/src/modules/auth/auth.module.ts | 7 +++- .../modules/auth/authentication.controller.ts | 14 ++++---- .../modules/auth/authentication.service.ts | 4 +++ .../request-password-recovery.command.ts | 3 ++ .../request-password-recovery.handler.ts | 32 +++++++++++++++++++ api/src/modules/auth/services/auth.mailer.ts | 7 ++-- api/src/modules/auth/services/jwt.manager.ts | 8 ++--- api/src/modules/users/users.service.ts | 3 +- .../password-recovery-send-email.feature | 2 +- .../password-recovery-reset-email.steps.ts | 3 ++ api/test/integration/auth/create-user.spec.ts | 9 +++--- api/test/integration/auth/sign-up.spec.ts | 24 +++++++------- api/test/utils/mocks/entity-mocks.ts | 1 + api/test/utils/test-manager.ts | 5 ++- 14 files changed, 90 insertions(+), 32 deletions(-) create mode 100644 api/src/modules/auth/commands/request-password-recovery.command.ts create mode 100644 api/src/modules/auth/commands/request-password-recovery.handler.ts diff --git a/api/src/modules/auth/auth.module.ts b/api/src/modules/auth/auth.module.ts index 95745c4e..5e13f1e2 100644 --- a/api/src/modules/auth/auth.module.ts +++ b/api/src/modules/auth/auth.module.ts @@ -4,11 +4,16 @@ import { AuthMailer } from '@api/modules/auth/services/auth.mailer'; import { NotificationsModule } from '@api/modules/notifications/notifications.module'; import { AuthenticationController } from '@api/modules/auth/authentication.controller'; import { AuthenticationModule } from '@api/modules/auth/authentication.module'; +import { RequestPasswordRecoveryHandler } from '@api/modules/auth/commands/request-password-recovery.handler'; @Module({ imports: [AuthenticationModule, NotificationsModule], controllers: [AuthenticationController], - providers: [PasswordRecoveryService, AuthMailer], + providers: [ + PasswordRecoveryService, + AuthMailer, + RequestPasswordRecoveryHandler, + ], exports: [AuthenticationModule, AuthMailer], }) export class AuthModule {} diff --git a/api/src/modules/auth/authentication.controller.ts b/api/src/modules/auth/authentication.controller.ts index 7ede1c78..edd5d546 100644 --- a/api/src/modules/auth/authentication.controller.ts +++ b/api/src/modules/auth/authentication.controller.ts @@ -19,6 +19,8 @@ import { authContract } from '@shared/contracts/auth.contract'; import { AuthenticationService } from '@api/modules/auth/authentication.service'; import { JwtAuthGuard } from '@api/modules/auth/guards/jwt-auth.guard'; import { SignUp } from '@api/modules/auth/strategies/sign-up.strategy'; +import { CommandBus } from '@nestjs/cqrs'; +import { RequestPasswordRecoveryCommand } from '@api/modules/auth/commands/request-password-recovery.command'; @Controller() @UseInterceptors(ClassSerializerInterceptor) @@ -26,6 +28,7 @@ export class AuthenticationController { constructor( private authService: AuthenticationService, private readonly passwordRecovery: PasswordRecoveryService, + private readonly commandBus: CommandBus, ) {} @Public() @@ -59,12 +62,9 @@ export class AuthenticationController { return tsRestHandler( authContract.resetPassword, async ({ body: { password } }) => { - const userWithAccessToken = await this.passwordRecovery.resetPassword( - user, - password, - ); + await this.authService.updatePassword(user, password); return { - body: userWithAccessToken, + body: null, status: 201, }; }, @@ -78,7 +78,9 @@ export class AuthenticationController { return tsRestHandler( authContract.requestPasswordRecovery, async ({ body: { email } }) => { - await this.passwordRecovery.requestPasswordRecovery(email, origin); + await this.commandBus.execute( + new RequestPasswordRecoveryCommand(email), + ); return { body: null, status: HttpStatus.CREATED, diff --git a/api/src/modules/auth/authentication.service.ts b/api/src/modules/auth/authentication.service.ts index 4073dabc..b10d84a4 100644 --- a/api/src/modules/auth/authentication.service.ts +++ b/api/src/modules/auth/authentication.service.ts @@ -66,4 +66,8 @@ export class AuthenticationService { } throw new UnauthorizedException(); } + + async updatePassword(user: User, newPassword: string): Promise { + await this.usersService.updatePassword(user, newPassword); + } } diff --git a/api/src/modules/auth/commands/request-password-recovery.command.ts b/api/src/modules/auth/commands/request-password-recovery.command.ts new file mode 100644 index 00000000..32ad4cab --- /dev/null +++ b/api/src/modules/auth/commands/request-password-recovery.command.ts @@ -0,0 +1,3 @@ +export class RequestPasswordRecoveryCommand { + constructor(public readonly email: string) {} +} diff --git a/api/src/modules/auth/commands/request-password-recovery.handler.ts b/api/src/modules/auth/commands/request-password-recovery.handler.ts new file mode 100644 index 00000000..46776d84 --- /dev/null +++ b/api/src/modules/auth/commands/request-password-recovery.handler.ts @@ -0,0 +1,32 @@ +import { CommandHandler, EventBus, ICommandHandler } from '@nestjs/cqrs'; +import { AuthMailer } from '@api/modules/auth/services/auth.mailer'; +import { RequestPasswordRecoveryCommand } from '@api/modules/auth/commands/request-password-recovery.command'; +import { UsersService } from '@api/modules/users/users.service'; +import { PasswordRecoveryRequestedEvent } from '@api/modules/events/user-events/password-recovery-requested.event'; +import { NotFoundException } from '@nestjs/common'; + +@CommandHandler(RequestPasswordRecoveryCommand) +export class RequestPasswordRecoveryHandler + implements ICommandHandler +{ + constructor( + private readonly users: UsersService, + private readonly authMailer: AuthMailer, + private readonly eventBus: EventBus, + ) {} + + async execute(command: RequestPasswordRecoveryCommand): Promise { + const { email } = command; + const user = await this.users.findByEmail(email); + if (!user) { + this.eventBus.publish(new PasswordRecoveryRequestedEvent(email, null)); + throw new NotFoundException(`Email ${email} not found`); + } + await this.authMailer.sendPasswordRecoveryEmail({ + user, + // TODO: Origin must come from env vars + origin: 'http://localhost:3000', + }); + this.eventBus.publish(new PasswordRecoveryRequestedEvent(email, user.id)); + } +} diff --git a/api/src/modules/auth/services/auth.mailer.ts b/api/src/modules/auth/services/auth.mailer.ts index a646f98c..43a413fa 100644 --- a/api/src/modules/auth/services/auth.mailer.ts +++ b/api/src/modules/auth/services/auth.mailer.ts @@ -51,12 +51,13 @@ export class AuthMailer { user: User; defaultPassword: string; }) { - const { emailConfirmationToken, expiresIn } = - await this.jwt.signEmailConfirmationToken(welcomeEmailDto.user.id); + const { signUpToken, expiresIn } = await this.jwt.signSignUpToken( + welcomeEmailDto.user.id, + ); // TODO: We need to know the URL to confirm the email, we could rely on origin but we would need to pass it through a lot of code. // probably better to have a config value for this. - const resetPasswordUrl = `TODO/auth/sign-up/${emailConfirmationToken}`; + const resetPasswordUrl = `TODO/auth/sign-up/${signUpToken}`; const htmlContent: string = `

Dear User,

diff --git a/api/src/modules/auth/services/jwt.manager.ts b/api/src/modules/auth/services/jwt.manager.ts index a49b7a1e..6f556a6c 100644 --- a/api/src/modules/auth/services/jwt.manager.ts +++ b/api/src/modules/auth/services/jwt.manager.ts @@ -53,15 +53,15 @@ export class JwtManager { }; } - async signEmailConfirmationToken( + async signSignUpToken( userId: string, - ): Promise<{ emailConfirmationToken: string; expiresIn: string }> { - const { token: emailConfirmationToken, expiresIn } = await this.sign( + ): Promise<{ signUpToken: string; expiresIn: string }> { + const { token: signUpToken, expiresIn } = await this.sign( userId, TOKEN_TYPE_ENUM.SIGN_UP, ); return { - emailConfirmationToken, + signUpToken: signUpToken, expiresIn, }; } diff --git a/api/src/modules/users/users.service.ts b/api/src/modules/users/users.service.ts index f8cf45b0..d064149a 100644 --- a/api/src/modules/users/users.service.ts +++ b/api/src/modules/users/users.service.ts @@ -2,6 +2,7 @@ import { ConflictException, Injectable } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; import { User } from '@shared/entities/users/user.entity'; import { Repository } from 'typeorm'; +import * as bcrypt from 'bcrypt'; @Injectable() export class UsersService { @@ -26,7 +27,7 @@ export class UsersService { } async updatePassword(user: User, newPassword: string) { - user.password = newPassword; + user.password = await bcrypt.hash(newPassword, 10); return this.repo.save(user); } diff --git a/api/test/e2e/features/password-recovery-send-email.feature b/api/test/e2e/features/password-recovery-send-email.feature index 00a7f99c..1ec3d7b9 100644 --- a/api/test/e2e/features/password-recovery-send-email.feature +++ b/api/test/e2e/features/password-recovery-send-email.feature @@ -8,5 +8,5 @@ Feature: Password Recovery Scenario: No email should be sent if the user is not found When the user requests password recovery with an invalid email - Then the user should receive a 201 status code + Then the user should receive a 404 status code And no email should be sent diff --git a/api/test/e2e/steps/password-recovery-reset-email.steps.ts b/api/test/e2e/steps/password-recovery-reset-email.steps.ts index becd0c44..f1254b42 100644 --- a/api/test/e2e/steps/password-recovery-reset-email.steps.ts +++ b/api/test/e2e/steps/password-recovery-reset-email.steps.ts @@ -28,6 +28,9 @@ describe('Reset Password', () => { resetPasswordSecret = apiConfigService.getJWTConfigByType( TOKEN_TYPE_ENUM.RESET_PASSWORD, ).secret; + }); + + afterEach(async () => { await testManager.clearDatabase(); }); diff --git a/api/test/integration/auth/create-user.spec.ts b/api/test/integration/auth/create-user.spec.ts index 2443d652..79e6a029 100644 --- a/api/test/integration/auth/create-user.spec.ts +++ b/api/test/integration/auth/create-user.spec.ts @@ -37,9 +37,10 @@ describe('Create Users', () => { // Given a user exists with valid credentials // But the user has the role partner - const user = await testManager - .mocks() - .createUser({ role: ROLES.PARTNER, email: 'random@test.com' }); + const user = await testManager.mocks().createUser({ + role: ROLES.PARTNER, + email: 'random@test.com', + }); const { jwtToken } = await testManager.logUserIn(user); // When the user creates a new user @@ -72,7 +73,7 @@ describe('Create Users', () => { ); }); - test('An Admin registers a new user', async () => { + test('An Admin registers a new user ', async () => { // Given a admin user exists with valid credentials // beforeAll const newUser = { diff --git a/api/test/integration/auth/sign-up.spec.ts b/api/test/integration/auth/sign-up.spec.ts index 5f809d9c..bf46c5c5 100644 --- a/api/test/integration/auth/sign-up.spec.ts +++ b/api/test/integration/auth/sign-up.spec.ts @@ -1,22 +1,20 @@ import { TestManager } from '../../utils/test-manager'; import { HttpStatus } from '@nestjs/common'; import { ApiConfigService } from '@api/modules/config/app-config.service'; -import { JwtService } from '@nestjs/jwt'; import { TOKEN_TYPE_ENUM } from '@shared/schemas/auth/token-type.schema'; import { authContract } from '@shared/contracts/auth.contract'; import { ROLES } from '@api/modules/auth/roles.enum'; +import { JwtManager } from '@api/modules/auth/services/jwt.manager'; //create-user.feature describe('Create Users', () => { let testManager: TestManager; - let apiConfig: ApiConfigService; - let jwtService: JwtService; + let jwtManager: JwtManager; beforeAll(async () => { testManager = await TestManager.createTestManager(); - apiConfig = testManager.getModule(ApiConfigService); - jwtService = testManager.getModule(JwtService); + jwtManager = testManager.getModule(JwtManager); }); afterEach(async () => { @@ -30,17 +28,12 @@ describe('Create Users', () => { test('A sign-up token should not be valid if the user bound to that token has already been activated', async () => { // Given a user exists with valid credentials // But the user has the role partner - const user = await testManager.mocks().createUser({ role: ROLES.PARTNER, email: 'random@test.com', - isActive: true, }); - const { secret, expiresIn } = apiConfig.getJWTConfigByType( - TOKEN_TYPE_ENUM.SIGN_UP, - ); - const token = jwtService.sign({ id: user.id }, { secret, expiresIn }); + const token = jwtManager.signSignUpToken(user.id); // When the user creates a new user @@ -52,4 +45,13 @@ describe('Create Users', () => { expect(response.status).toBe(HttpStatus.UNAUTHORIZED); }); + + test('Sign up should fail if the current password is incorrect', async () => { + const user = await testManager.mocks().createUser({ + role: ROLES.PARTNER, + email: 'random@test.com', + isActive: true, + }); + const token = await jwtManager.signSignUpToken(user.id); + }); }); diff --git a/api/test/utils/mocks/entity-mocks.ts b/api/test/utils/mocks/entity-mocks.ts index 7748b856..ca050b05 100644 --- a/api/test/utils/mocks/entity-mocks.ts +++ b/api/test/utils/mocks/entity-mocks.ts @@ -12,6 +12,7 @@ export const createUser = async ( email: 'test@user.com', ...additionalData, password: await hash(usedPassword, salt), + isActive: true, }; await dataSource.getRepository(User).save(user); diff --git a/api/test/utils/test-manager.ts b/api/test/utils/test-manager.ts index aa7d49f3..ced8e63f 100644 --- a/api/test/utils/test-manager.ts +++ b/api/test/utils/test-manager.ts @@ -73,7 +73,10 @@ export class TestManager { } async setUpTestUser() { - const user = await createUser(this.getDataSource(), { role: ROLES.ADMIN }); + const user = await createUser(this.getDataSource(), { + role: ROLES.ADMIN, + isActive: true, + }); return logUserIn(this, user); }