Reset password #262
Reset password #262
Conversation
…into 79-change-password
There was a problem hiding this comment.
Très bonne PR, juste une chose, est-ce que le onboarding du premier user change avec cette PR? Parce que tu as supprimé le first component, qui servait à créer le premier user ever en cas de déploiment prod, et j'ai pas vu de code qui crée un user initial. Possible que je l'aie manqué though. Je le vois pas dans les variables de devspace en tous cas.
| */ | ||
| @Get('setup') | ||
| async getIsSetup(): Promise<any> { | ||
| return { isSetup: await this.authService.isAuthenticationSetup() }; | ||
| } | ||
|
|
||
| private async loginCore(req: any) { |
There was a problem hiding this comment.
I think the type here is something like ExpressRequest or something
There was a problem hiding this comment.
import { Request } from 'express';
// [...]
private async loginCore(req: Request) {| refresh = r.body.refresh_token; | ||
| }); | ||
|
|
||
| it('Should connect as the magic link user (POST /auth/login-magic-link)', async () => { |
There was a problem hiding this comment.
| it('Should connect as the magic link user (POST /auth/login-magic-link)', async () => { | |
| it('Should not connect as the magic link user (POST /auth/login-magic-link)', async () => { |
| expect(authenticatedUser).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('Should return access token when magic token does not exist', async () => { |
There was a problem hiding this comment.
| it('Should return access token when magic token does not exist', async () => { | |
| it('Should not return access token when magic token does not exist', async () => { |
| expect(token).toBeNull(); | ||
| }); | ||
|
|
||
| it('Should return access token when magic token is expired', async () => { |
There was a problem hiding this comment.
| it('Should return access token when magic token is expired', async () => { | |
| it('Should not return access token when magic token is expired', async () => { |
| async requestPasswordReset( | ||
| @Request() request: any, | ||
| @Body() dto: ResetPasswordRequestDto, | ||
| ) { | ||
| if (!dto.email) throw new BadRequestException(); | ||
|
|
||
| await this.usersService.createPasswordResetRequest(dto.email); | ||
| } | ||
| } |
There was a problem hiding this comment.
I would not await the call to create a password reset.
await this.usersService.createPasswordResetRequest(dto.email);It is a common time based user enumeration technique. The reply time would be longer for a valid user than for an invalid user, since the backend sends an email. Not awaiting the call would make for a more constant response time.
| private replaceValues(template: string, context: unknown) { | ||
| return compile(template)(context); | ||
| } |
There was a problem hiding this comment.
De ce que j'ai lu, handle bars HTML encode par défaut pour toi tant que tu as mis les deux accolades et pas trois. C'est great! 💯
| export class NullEmailService implements EmailService { | ||
| public sendResetPassword( | ||
| context: ResetPasswordContext, | ||
| recipients: EmailRecipient[], | ||
| ): Promise<void> { | ||
| return Promise.resolve(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Vraiment bien pensé de pouvoir fonctionner même sans email service 💡
| } | ||
| } | ||
|
|
||
| private filterRecipients(recipients: EmailRecipient[]) { |
| export const EMAIL_RECIPIENTS_FILTER_LIST_MODE = | ||
| process.env.JM_ENVIRONMENT === JM_ENVIRONMENTS.prod | ||
| ? 'block-list' | ||
| : 'allow-list'; |
There was a problem hiding this comment.
On devrait surement faire une page de doc qui explique ces différentes valeurs configurables
| path: 'first', | ||
| component: FirstComponent, | ||
| path: 'reset', | ||
| loadComponent: () => | ||
| import('./modules/auth/reset/reset-password.component').then((c) => c.ResetPasswordComponent), |
There was a problem hiding this comment.
Le component first servait à créer le premier user ever lors du déploiment, est-ce que la façon de créer le premier user a changé?
There was a problem hiding this comment.
À moins que j'ai manqué la création du premier user de l'app dans le code, je pense que ce component est possiblement encore nécessaire?
What's new?
This branch implements the reset password feature.
TODO