From 633f5bb947ea3fd15763d36af48b7f6ec3a811ab Mon Sep 17 00:00:00 2001 From: Marton Horvath <17813215+horvathmarton@users.noreply.github.com> Date: Sat, 15 Jul 2023 14:34:32 +0200 Subject: [PATCH 1/5] Decouple googleLogin function from Request object --- packages/altair-api/src/auth/auth.controller.ts | 4 ++-- packages/altair-api/src/auth/auth.service.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/altair-api/src/auth/auth.controller.ts b/packages/altair-api/src/auth/auth.controller.ts index 9de423293d..fa105977f3 100644 --- a/packages/altair-api/src/auth/auth.controller.ts +++ b/packages/altair-api/src/auth/auth.controller.ts @@ -28,7 +28,7 @@ export class AuthController { @Get('google/callback') @UseGuards(GoogleOAuthGuard) googleSigninCallback(@Req() req: Request, @Res() res: Response) { - const result = this.authService.googleLogin(req); + const result = this.authService.googleLogin(req.user); if (req.query.state && typeof req.query.state === 'string') { try { const origin = new URL(req.query.state); @@ -45,7 +45,7 @@ export class AuthController { @Get('me') @UseGuards(JwtAuthGuard) getUserProfile(@Req() req: Request) { - return this.authService.googleLogin(req); + return this.authService.googleLogin(req.user); } @Get('slt') diff --git a/packages/altair-api/src/auth/auth.service.ts b/packages/altair-api/src/auth/auth.service.ts index 184514f235..5803c65002 100644 --- a/packages/altair-api/src/auth/auth.service.ts +++ b/packages/altair-api/src/auth/auth.service.ts @@ -42,12 +42,12 @@ export class AuthService { return this.getLoginResponse(user); } - googleLogin(req: Request) { - if (!req.user) { + googleLogin(user: User) { + if (!user) { throw new BadRequestException('No user from google'); } - return this.getLoginResponse(req.user); + return this.getLoginResponse(user); } getUserCredential(providerUserId: string, provider: IdentityProvider) { From 124ca40af3279af84b60d10431c9bbd0c54e44d7 Mon Sep 17 00:00:00 2001 From: Marton Horvath <17813215+horvathmarton@users.noreply.github.com> Date: Sat, 15 Jul 2023 15:18:32 +0200 Subject: [PATCH 2/5] Only include the source folder for the coverage report --- packages/altair-api/jest.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/altair-api/jest.config.js b/packages/altair-api/jest.config.js index 9893a32f9c..321200c7fc 100644 --- a/packages/altair-api/jest.config.js +++ b/packages/altair-api/jest.config.js @@ -9,7 +9,7 @@ module.exports = { transform: { '^.+\\.(t|j)s$': 'ts-jest', }, - collectCoverageFrom: ['**/*.(t|j)s'], + collectCoverageFrom: ['src/**/*.(t|j)s'], coverageDirectory: '../coverage', testEnvironment: 'node', setupFilesAfterEnv: ['./custom-matchers.ts'], From 243d74d472125504137fe14625f8a3a6cf570d81 Mon Sep 17 00:00:00 2001 From: Marton Horvath <17813215+horvathmarton@users.noreply.github.com> Date: Sat, 15 Jul 2023 17:19:57 +0200 Subject: [PATCH 3/5] Add unit tests for user service --- .../altair-api/src/auth/auth.service.spec.ts | 223 ++++++++++++++++++ 1 file changed, 223 insertions(+) diff --git a/packages/altair-api/src/auth/auth.service.spec.ts b/packages/altair-api/src/auth/auth.service.spec.ts index 671050d304..1e224dc49e 100644 --- a/packages/altair-api/src/auth/auth.service.spec.ts +++ b/packages/altair-api/src/auth/auth.service.spec.ts @@ -4,9 +4,19 @@ import { Test, TestingModule } from '@nestjs/testing'; import { PrismaService } from 'nestjs-prisma'; import { AuthService } from './auth.service'; import { PasswordService } from './password/password.service'; +import { mockUser } from './mocks/prisma-service.mock'; +import { ChangePasswordInput } from './models/change-password.input'; +import { UnauthorizedException } from '@nestjs/common'; describe('AuthService', () => { let service: AuthService; + let prismaService: PrismaService; + let passwordService: PasswordService; + let jwtService: JwtService; + + const passwordMock = '123456'; + const tokenMock = + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c'; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -20,9 +30,222 @@ describe('AuthService', () => { }).compile(); service = module.get(AuthService); + prismaService = module.get(PrismaService); + passwordService = module.get(PasswordService); + jwtService = module.get(JwtService); }); it('should be defined', () => { expect(service).toBeDefined(); }); + + describe('passwordLogin', () => { + it(`should return a user object on successful login`, async () => { + // GIVEN + const userMock = mockUser(); + jest + .spyOn(prismaService.user, 'findUnique') + .mockResolvedValueOnce(userMock); + jest + .spyOn(passwordService, 'validatePassword') + .mockResolvedValueOnce(true); + jest.spyOn(jwtService, 'sign').mockReturnValue(tokenMock); + + // WHEN + const user = await service.passwordLogin(userMock.email, passwordMock); + + // THEN + expect(user).toBeUser(); + expect(user.email).toBe(userMock.email); + }); + + it(`should throw an error if the user doesn't exist`, () => { + // GIVEN + const emailMock = mockUser().email; + jest.spyOn(prismaService.user, 'findUnique').mockResolvedValueOnce(null); + + // THEN + expect(service.passwordLogin(emailMock, passwordMock)).rejects.toThrow( + `No user found for email: ${emailMock}` + ); + }); + + it(`should throw an error if the provided password is invalid`, () => { + // GIVEN + const userMock = mockUser(); + jest + .spyOn(prismaService.user, 'findUnique') + .mockResolvedValueOnce(userMock); + jest + .spyOn(passwordService, 'validatePassword') + .mockResolvedValueOnce(false); + + // THEN + expect( + service.passwordLogin(userMock.email, passwordMock) + ).rejects.toThrow(`Invalid password`); + }); + }); + + describe('googleLogin', () => { + it(`should return a user object on successful login`, async () => { + // GIVEN + const userMock = mockUser(); + jest.spyOn(jwtService, 'sign').mockReturnValue(tokenMock); + + // WHEN + const user = await service.googleLogin(userMock); + + // THEN + expect(user).toBeUser(); + expect(user.email).toBe(userMock.email); + }); + + it(`should throw an error if the user object is missing from the request`, () => { + // THEN + expect(() => service.googleLogin(undefined)).toThrow( + 'No user from google' + ); + }); + }); + + describe('getUserFromToken', () => { + it(`should return a user object`, async () => { + // GIVEN + const userMock = mockUser(); + jest.spyOn(jwtService, 'decode').mockReturnValueOnce({ + userId: 'e23b7b34-8996-45d3-9097-b14b8f451dbd', + }); + jest + .spyOn(prismaService.user, 'findUnique') + .mockResolvedValueOnce(userMock); + + // WHEN + const user = await service.getUserFromToken(tokenMock); + + // THEN + expect(user).toBeUser(); + }); + + it(`should throw an error if the token is invalid`, () => { + // GIVEN + jest + .spyOn(jwtService, 'decode') + .mockReturnValueOnce(`Couldn't decode token.`); + + // THEN + expect(() => service.getUserFromToken(tokenMock)).toThrow( + 'Invalid JWT token' + ); + }); + }); + + describe('changePassword', () => { + let changePasswordInputMock: ChangePasswordInput; + + beforeAll(() => { + changePasswordInputMock = { + oldPassword: passwordMock, + newPassword: passwordMock, + } as ChangePasswordInput; + }); + + it(`should return a user object on successful password change`, async () => { + // GIVEN + const userMock = mockUser(); + jest + .spyOn(passwordService, 'validatePassword') + .mockResolvedValueOnce(true); + jest + .spyOn(passwordService, 'hashPassword') + .mockResolvedValue( + '6bfbaac71e1cb8876538505e5c8d9a6653eef56c592cc9691e9a70a427680136' + ); + jest.spyOn(prismaService.user, 'update').mockResolvedValueOnce(userMock); + + // WHEN + const user = await service.changePassword( + userMock.id, + passwordMock, + changePasswordInputMock + ); + + // THEN + expect(user).toBeUser(); + }); + + it(`should throw an error if the user password is invalid`, () => { + // GIVEN + jest + .spyOn(passwordService, 'validatePassword') + .mockResolvedValueOnce(false); + + // THEN + expect( + service.changePassword( + mockUser().id, + passwordMock, + changePasswordInputMock + ) + ).rejects.toThrow('Invalid password'); + }); + }); + + describe('getLoginResponse', () => { + it(`should return a user object with tokens`, async () => { + // GIVEN + jest.spyOn(jwtService, 'sign').mockReturnValue(tokenMock); + + // WHEN + const response = await service.getLoginResponse(mockUser()); + + // THEN + expect(response).toBeUser(); + expect(response.tokens.accessToken).toEqual(expect.any(String)); + expect(response.tokens.refreshToken).toEqual(expect.any(String)); + }); + }); + + describe('generateTokens', () => { + it(`should return authentication tokens`, async () => { + // GIVEN + jest.spyOn(jwtService, 'sign').mockReturnValue(tokenMock); + + // WHEN + const tokens = await service.generateTokens({ userId: mockUser().id }); + + // THEN + expect(tokens.accessToken).toEqual(expect.any(String)); + expect(tokens.refreshToken).toEqual(expect.any(String)); + }); + }); + + describe('refreshToken', () => { + it(`should return authentication tokens on successful refresh`, () => { + // GIVEN + jest + .spyOn(jwtService, 'verify') + .mockReturnValueOnce({ userId: mockUser().id }); + jest.spyOn(jwtService, 'sign').mockReturnValue(tokenMock); + + // WHEN + const tokens = service.refreshToken(tokenMock); + + // THEN + expect(tokens.accessToken).toEqual(expect.any(String)); + expect(tokens.refreshToken).toEqual(expect.any(String)); + }); + + it(`should throw an error if the token input token is invalid`, () => { + // GIVEN + jest.spyOn(jwtService, 'verify').mockImplementationOnce(() => { + throw new Error('Invalid token'); + }); + + // THEN + expect(() => service.refreshToken(tokenMock)).toThrow( + UnauthorizedException + ); + }); + }); }); From 3c4eb64983ee1c56a6cfd6a7add937c068649a6b Mon Sep 17 00:00:00 2001 From: Marton Horvath <17813215+horvathmarton@users.noreply.github.com> Date: Sat, 15 Jul 2023 17:20:27 +0200 Subject: [PATCH 4/5] Exclude untestable files from coverage report --- packages/altair-api/jest.config.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/altair-api/jest.config.js b/packages/altair-api/jest.config.js index 321200c7fc..003548ef96 100644 --- a/packages/altair-api/jest.config.js +++ b/packages/altair-api/jest.config.js @@ -9,7 +9,15 @@ module.exports = { transform: { '^.+\\.(t|j)s$': 'ts-jest', }, - collectCoverageFrom: ['src/**/*.(t|j)s'], + collectCoverageFrom: [ + 'src/**/*.(t|j)s', + '!src/**/mocks/**', + '!src/**/config.ts', + '!src/**/*.dto.ts', + '!src/**/*.input.ts', + '!src/**/*.module.ts', + '!src/**/*.strategy.ts', + ], coverageDirectory: '../coverage', testEnvironment: 'node', setupFilesAfterEnv: ['./custom-matchers.ts'], From ece39b39f742403e9733f3661d4c716bb19fd90a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rton=20Horv=C3=A1th?= Date: Fri, 21 Jul 2023 07:53:45 +0200 Subject: [PATCH 5/5] Revert ignoring data holder files in the coverage report Co-authored-by: Samuel --- packages/altair-api/jest.config.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/altair-api/jest.config.js b/packages/altair-api/jest.config.js index 003548ef96..a87b942d64 100644 --- a/packages/altair-api/jest.config.js +++ b/packages/altair-api/jest.config.js @@ -12,11 +12,6 @@ module.exports = { collectCoverageFrom: [ 'src/**/*.(t|j)s', '!src/**/mocks/**', - '!src/**/config.ts', - '!src/**/*.dto.ts', - '!src/**/*.input.ts', - '!src/**/*.module.ts', - '!src/**/*.strategy.ts', ], coverageDirectory: '../coverage', testEnvironment: 'node',