Vizzuality · alexeh · Aug 2, 2023 · Jul 27, 2023 · Jul 27, 2023 · Aug 2, 2023
diff --git a/api/config/default.json b/api/config/default.json
@@ -14,7 +14,7 @@
   },
   "auth": {
     "requireUserAuth": true,
-    "requireUserAccountActivation": false,
+    "requireUserAccountActivation": true,
     "signUpIsPublic": true,
     "jwt": {
       "expiresIn": "2h",
@@ -25,6 +25,7 @@
       "includeNumerics": false,
       "includeUpperCase": false,
       "includeSpecialCharacters": false,
+      "activationUrl": "/auth/activate-account",
       "resetUrl": "/auth/reset-password"
     }
   },

diff --git a/api/config/test.json b/api/config/test.json
@@ -24,7 +24,7 @@
     ]
   },
   "auth": {
-    "requireUserAccountActivation": false,
+    "requireUserAccountActivation": true,
     "jwt": {
       "expiresIn": "1d",
       "secret": "myVeryBadJWTSecretForTests"

diff --git a/api/src/modules/authentication/authentication.controller.ts b/api/src/modules/authentication/authentication.controller.ts
@@ -2,7 +2,6 @@ import {
   Body,
   Controller,
   Get,
-  Param,
   Post,
   Request,
   UseGuards,
@@ -24,7 +23,9 @@ import {
   AuthenticationService,
 } from 'modules/authentication/authentication.service';
 import { LoginDto } from 'modules/authentication/dto/login.dto';
-import { UserAccountValidationDTO } from 'modules/authentication/dto/user-account.validation.dto';
+import { ResetPasswordDto } from 'modules/authentication/dto/reset-password.dto';
+import { GetUser } from 'decorators/get-user.decorator';
+import { User } from 'modules/users/user.entity';
 
 @Controller('/auth')
 @ApiTags('Authentication')
@@ -51,14 +52,17 @@ export class AuthenticationController {
     return this.authenticationService.login(req.user);
   }
 
-  @Public()
-  @Get('validate-account/:sub/:validationToken')
-  @ApiOperation({ description: 'Confirm an activation token for a new user.' })
+  @Post('validate-account')
+  @ApiOperation({ description: 'Confirm an activation for a new user.' })
   @ApiOkResponse()
-  async confirm(
-    @Param() activationToken: UserAccountValidationDTO,
-  ): Promise<void> {
-    await this.authenticationService.validateActivationToken(activationToken);
+  async validateAccount(
+    @Body(ValidationPipe) resetPasswordDto: ResetPasswordDto,
+    @GetUser() user: User,
+  ): Promise<User> {
+    return await this.authenticationService.validateAccount(
+      resetPasswordDto.password,
+      user,
+    );
   }
 
   /**

diff --git a/api/src/modules/authentication/authentication.module.ts b/api/src/modules/authentication/authentication.module.ts
@@ -1,4 +1,4 @@
-import { forwardRef, Logger, Module } from '@nestjs/common';
+import { forwardRef, Module } from '@nestjs/common';
 import { JwtModule } from '@nestjs/jwt';
 import { PassportModule } from '@nestjs/passport';
 import { TypeOrmModule } from '@nestjs/typeorm';
@@ -14,19 +14,8 @@ import { PasswordValidation } from 'decorators/password-validator.decorator';
 import { AuthorizationModule } from 'modules/authorization/authorization.module';
 import { NotificationsModule } from 'modules/notifications/notifications.module';
 import { AppConfig } from 'utils/app.config';
-
-export const logger: Logger = new Logger('Authentication');
-const getPasswordResetUrl = (): string => {
-  const clientUrl: string = AppConfig.get('client.url');
-  const clientPort: string = AppConfig.get('client.port');
-  const passwordResetUrl: string = AppConfig.get('auth.password.resetUrl');
-  const protocol: string =
-    process.env.NODE_ENV === 'production' ? 'https' : 'http';
-  if (!clientUrl || !clientPort || !passwordResetUrl) {
-    logger.error('Missing client url, port or password reset url');
-  }
-  return `${protocol}://${clientUrl}:${clientPort}${passwordResetUrl}`;
-};
+import { PasswordMailService } from 'modules/authentication/password-mail.service';
+import { getPasswordSettingUrl } from 'modules/authentication/utils/authentication.utils';
 
 @Module({
   imports: [
@@ -46,9 +35,14 @@ const getPasswordResetUrl = (): string => {
     LocalStrategy,
     JwtStrategy,
     PasswordValidation,
+    PasswordMailService,
+    {
+      provide: 'PASSWORD_ACTIVATION_URL',
+      useValue: getPasswordSettingUrl('activation'),
+    },
     {
       provide: 'PASSWORD_RESET_URL',
-      useValue: getPasswordResetUrl(),
+      useValue: getPasswordSettingUrl('reset'),
     },
 
     {

diff --git a/api/src/modules/authentication/authentication.service.ts b/api/src/modules/authentication/authentication.service.ts
@@ -24,8 +24,8 @@ import { AuthorizationService } from 'modules/authorization/authorization.servic
 import { ROLES } from 'modules/authorization/roles/roles.enum';
 import { Role } from 'modules/authorization/roles/role.entity';
 import { CreateUserDTO } from 'modules/users/dto/create.user.dto';
-import { IEmailService } from 'modules/notifications/email/email.service.interface';
 import { AppConfig } from 'utils/app.config';
+import { PasswordMailService } from 'modules/authentication/password-mail.service';
 
 const DEFAULT_USER_NAME: string = 'User';
 
@@ -79,12 +79,11 @@ export class AuthenticationService {
 
   constructor(
     private readonly apiEventsService: ApiEventsService,
-    @Inject('IEmailService') private emailService: IEmailService,
-    @Inject('PASSWORD_RESET_URL') private passwordResetUrl: string,
     @Inject(forwardRef(() => UsersService))
     private readonly usersService: UsersService,
     private readonly jwtService: JwtService,
     private readonly authorizationService: AuthorizationService,
+    private readonly passwordMail: PasswordMailService,
     private userRepository: UserRepository,
   ) {}
 
@@ -166,6 +165,8 @@ export class AuthenticationService {
         sub: newUser.email,
       },
     });
+
+    await this.sendUserActivationEmail(newUser.email);
     /**
      * This is a small aid to help with manual QA :).
      */
@@ -188,6 +189,7 @@ export class AuthenticationService {
       ApiEventsUserData.ActivationTokenGeneratedV1Alpha1,
       'validationToken' | 'sub'
     >,
+    password: string,
   ): Promise<true | never> {
     const invalidOrExpiredActivationTokenMessage: string =
       'Invalid or expired activation token.';
@@ -209,7 +211,14 @@ export class AuthenticationService {
         topic: event.topic,
         kind: API_EVENT_KINDS.user__accountActivationSucceeded__v1alpha1,
       });
-      await this.userRepository.update({ id: event.topic }, { isActive: true });
+
+      const salt: string = await this.authorizationService.generateSalt();
+      const hashedPassword: string =
+        await this.authorizationService.assignPassword(salt, password);
+      await this.userRepository.update(
+        { id: event.topic },
+        { isActive: true, salt, password: hashedPassword },
+      );
       await this.apiEventsService.purgeAll({
         topic: event.topic,
         kind: API_EVENT_KINDS.user__accountActivationTokenGenerated__v1alpha1,
@@ -223,6 +232,20 @@ export class AuthenticationService {
     throw new BadRequestException(invalidOrExpiredActivationTokenMessage);
   }
 
+  async validateAccount(password: string, user: User): Promise<any> {
+    const salt: string = await this.authorizationService.generateSalt();
+    const hashedPassword: string =
+      await this.authorizationService.assignPassword(salt, password);
+    await this.userRepository.update(
+      { id: user.id },
+      { isActive: true, salt, password: hashedPassword },
+    );
+    const updatedUser: User = await this.userRepository.findOneOrFail({
+      where: { id: user.id },
+    });
+    return UsersService.getSanitizedUserMetadata(updatedUser);
+  }
+
   /**
    * Issue a signed JTW token, logging its issuance.
    */
@@ -286,20 +309,6 @@ export class AuthenticationService {
     return this.authorizationService.assignRoles(rolesEnum);
   }
 
-  async sendPasswordRecoveryEmail(user: User): Promise<void> {
-    const payload: Partial<JwtDataPayload> = {
-      sub: user.email,
-    };
-
-    const token: string = this.jwtService.sign(payload);
-    await this.emailService.sendMail({
-      to: user.email,
-      subject: 'Reset password',
-      text: 'Reset password',
-      html: `<a href="${this.passwordResetUrl}/${token}">Reset password</a>`,
-    });
-  }
-
   verifyToken(token: string): JwtDataPayload {
     try {
       return this.jwtService.verify(token);
@@ -317,4 +326,14 @@ export class AuthenticationService {
       },
     );
   }
+
+  async sendPasswordRecoverEmail(email: string): Promise<void> {
+    const token: string = this.signToken(email);
+    return this.passwordMail.sendPasswordRecoveryEmail(email, token);
+  }
+
+  async sendUserActivationEmail(email: string): Promise<void> {
+    const token: string = this.signToken(email);
+    return this.passwordMail.sendUserActivationEmail(email, token);
+  }
 }
diff --git a/api/src/modules/authentication/password-mail.service.ts b/api/src/modules/authentication/password-mail.service.ts
@@ -0,0 +1,30 @@
+import { Inject, Injectable, Logger } from '@nestjs/common';
+import { IEmailService } from 'modules/notifications/email/email.service.interface';
+
+@Injectable()
+export class PasswordMailService {
+  logger: Logger = new Logger(PasswordMailService.name);
+
+  constructor(
+    @Inject('IEmailService') private emailService: IEmailService,
+    @Inject('PASSWORD_RESET_URL') private passwordResetUrl: string,
+  ) {}
+
+  async sendUserActivationEmail(email: string, token: string): Promise<void> {
+    await this.emailService.sendMail({
+      to: email,
+      subject: 'Activate account',
+      text: 'Activate account',
+      html: `<a href="${this.passwordResetUrl}/${token}">Activate account</a>`,
+    });
+  }
+
+  async sendPasswordRecoveryEmail(email: string, token: string): Promise<void> {
+    await this.emailService.sendMail({
+      to: email,
+      subject: 'Reset password',
+      text: 'Reset password',
+      html: `<a href="${this.passwordResetUrl}/${token}">Reset password</a>`,
+    });
+  }
+}
diff --git a/api/src/modules/authentication/utils/authentication.utils.ts b/api/src/modules/authentication/utils/authentication.utils.ts
@@ -0,0 +1,20 @@
+import { Logger } from '@nestjs/common';
+import { AppConfig } from 'utils/app.config';
+
+const logger: Logger = new Logger('Authentication');
+export const getPasswordSettingUrl = (kind: 'activation' | 'reset'): string => {
+  const clientUrl: string = AppConfig.get('client.url');
+  const clientPort: string = AppConfig.get('client.port');
+  const passwordActivationUrl: string = AppConfig.get(
+    'auth.password.activationUrl',
+  );
+  const passwordResetUrl: string = AppConfig.get('auth.password.resetUrl');
+  const protocol: string =
+    process.env.NODE_ENV === 'production' ? 'https' : 'http';
+  if (!clientUrl || !clientPort || !passwordResetUrl) {
+    logger.error('Missing client url, port or password reset url');
+  }
+  return `${protocol}://${clientUrl}:${clientPort}${
+    kind === 'activation' ? passwordActivationUrl : passwordResetUrl
+  }`;
+};
diff --git a/api/src/modules/users/users.service.ts b/api/src/modules/users/users.service.ts
@@ -198,7 +198,7 @@ export class UsersService extends AppBaseService<
     if (!user || !user.isActive) {
       throw new NotFoundException(`No user found with email address ${email}`);
     }
-    return this.authenticationService.sendPasswordRecoveryEmail(user);
+    return this.authenticationService.sendPasswordRecoverEmail(user.email);
   }
 
   async resetPassword(user: User, newPassword: string): Promise<User> {

diff --git a/...ssword-recovery/password-recovery.spec.ts → ...ecovery/password-set-and-recovery.spec.ts b/...ssword-recovery/password-recovery.spec.ts → ...ecovery/password-set-and-recovery.spec.ts
@@ -2,41 +2,38 @@ import ApplicationManager, {
   TestApplication,
 } from '../../utils/application-manager';
 import { DataSource } from 'typeorm';
-import { Test } from '@nestjs/testing';
-import { AppModule } from '../../../src/app.module';
-import {
-  IEmailService,
-  SendMailDTO,
-} from 'modules/notifications/email/email.service.interface';
+
 import { clearTestDataFromDatabase } from '../../utils/database-test-helper';
-import { HttpStatus, Logger } from '@nestjs/common';
+import { HttpStatus } from '@nestjs/common';
 import { createUser } from '../../entity-mocks';
 import { User } from 'modules/users/user.entity';
 import * as request from 'supertest';
 
 import { AuthenticationService } from 'modules/authentication/authentication.service';
+import { Test } from '@nestjs/testing';
+import { AppModule } from '../../../src/app.module';
+import { setupTestUser } from '../../utils/userAuth';
+import { ROLES } from 'modules/authorization/roles/roles.enum';
 
-class MockEmailService implements IEmailService {
-  logger: Logger = new Logger(MockEmailService.name);
-
-  async sendMail(mail: SendMailDTO): Promise<void> {
-    return Promise.resolve();
-  }
-}
+const mockEmailService = {
+  sendMail: jest.fn(),
+};
 
 describe('Password recovery tests (e2e)', () => {
   let testApplication: TestApplication;
   let dataSource: DataSource;
   let authenticationService: AuthenticationService;
+  let jwtToken: string;
 
   beforeAll(async () => {
     testApplication = await ApplicationManager.init(
-      Test.createTestingModule({
+      await Test.createTestingModule({
         imports: [AppModule],
       })
         .overrideProvider('IEmailService')
-        .useClass(MockEmailService),
+        .useValue(mockEmailService),
     );
+    ({ jwtToken } = await setupTestUser(testApplication));
 
     dataSource = testApplication.get<DataSource>(DataSource);
 
@@ -50,6 +47,35 @@ describe('Password recovery tests (e2e)', () => {
     await testApplication.close();
   });
 
+  describe('User account activation (e2e)', () => {
+    test('When a admin user creates a new user, a email should be sent, and the user should be able to create a first password with the token received', async () => {
+      const email = 'activation@mail.com';
+      const firstPassword = 'password';
+      await request(testApplication.getHttpServer())
+        .post('/api/v1/users')
+        .set('Authorization', `Bearer ${jwtToken}`)
+        .send({ email, roles: [ROLES.ADMIN] })
+        .expect(HttpStatus.CREATED);
+
+      expect(mockEmailService.sendMail).toHaveBeenCalledTimes(1);
+
+      const receivedToken = authenticationService.signToken(email);
+
+      await request(testApplication.getHttpServer())
+        .post(`/auth/validate-account`)
+        .set('Authorization', `Bearer ${receivedToken}`)
+        .send({ password: firstPassword })
+        .expect(HttpStatus.CREATED);
+
+      const newLoginUser = await request(testApplication.getHttpServer())
+        .post('/auth/sign-in')
+        .send({ username: email, password: firstPassword })
+        .expect(HttpStatus.CREATED);
+
+      expect(newLoginUser.body.accessToken).toBeDefined();
+    });
+  });
+
   describe('Password recovery request (e2e)', () => {
     test('When I request a password reset, but my user cannot be found, I should see an error message', async () => {
       const nonExistingUser = await request(testApplication.getHttpServer())

diff --git a/api/test/e2e/users/users.spec.ts b/api/test/e2e/users/users.spec.ts
@@ -151,15 +151,15 @@ describe('UsersModule (e2e)', () => {
       expect(res.body.data.attributes.title).toEqual(newUserDto.title);
     });
 
-    test('When a admin creates a user this user should be active by default', async () => {
+    test('When a admin creates a user this user should be inactive by default', async () => {
       await request(testApplication.getHttpServer())
         .post('/api/v1/users')
         .set('Authorization', `Bearer ${adminTestUser.jwtToken}`)
         .send(newUserDto)
         .expect(HttpStatus.CREATED);
 
       const user = await userRepository.findByEmail(newUserDto.email);
-      expect(user?.isActive).toBe(true);
+      expect(user?.isActive).toBe(false);
     });
 
     test('When a user is created but no fname is provided, it should have "User" as fname by default', async () => {