From 708ef5ee6feffb9795abb25b79fefe1004635522 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 +- api/test/integration/auth/create-user.spec.ts | 2 +- api/test/integration/auth/sign-up.spec.ts | 23 +++++++------ 10 files changed, 77 insertions(+), 26 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..e1f7d1e1 --- /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 { UnauthorizedException } 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 UnauthorizedException(); + } + 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/integration/auth/create-user.spec.ts b/api/test/integration/auth/create-user.spec.ts index 2443d652..2f96f993 100644 --- a/api/test/integration/auth/create-user.spec.ts +++ b/api/test/integration/auth/create-user.spec.ts @@ -72,7 +72,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..77ce069b 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,13 @@ 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 +46,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); + }); });