From e69cde8ec060de415bd1c7e6dfba72dd2bcda508 Mon Sep 17 00:00:00 2001 From: Mnigos Date: Tue, 16 Jan 2024 10:41:51 +0100 Subject: [PATCH] refactor(modules/auth): move save user login to `authService` --- src/modules/auth/auth.controller.spec.ts | 106 ++--------- src/modules/auth/auth.controller.ts | 53 +----- src/modules/auth/auth.service.spec.ts | 175 +++++++----------- src/modules/auth/auth.service.ts | 103 ++++------- .../auth/types/authorize-params.type.ts | 5 + src/modules/auth/types/index.ts | 5 +- 6 files changed, 132 insertions(+), 315 deletions(-) create mode 100644 src/modules/auth/types/authorize-params.type.ts diff --git a/src/modules/auth/auth.controller.spec.ts b/src/modules/auth/auth.controller.spec.ts index 4e83a349..6cd01b9c 100644 --- a/src/modules/auth/auth.controller.spec.ts +++ b/src/modules/auth/auth.controller.spec.ts @@ -4,27 +4,23 @@ import { Test, TestingModule } from '@nestjs/testing' import { AuthController } from './auth.controller' import { SecretData } from './dtos' +import { AuthService } from './auth.service' +import { AuthorizeParams } from './types' import { accessToken, accessTokenMock, - profileMock, refreshToken, userMock, } from '@common/mocks' -import { ProfilesService } from '@modules/profiles' -import { UsersRepository } from '@modules/users' import { SpotifyAuthService } from '@modules/spotify/auth' -import { SpotifyUsersService } from '@modules/spotify/users' describe('AuthController', () => { const redirectUrl = 'http://test.com' let authController: AuthController let spotifyAuthService: SpotifyAuthService - let spotifyUsersService: SpotifyUsersService - let profilesService: ProfilesService - let usersRepository: UsersRepository + let authService: AuthService beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -44,22 +40,9 @@ describe('AuthController', () => { }, }, { - provide: ProfilesService, + provide: AuthService, useValue: { - create: vi.fn(), - }, - }, - { - provide: UsersRepository, - useValue: { - createUser: vi.fn(), - findOneByProfileId: vi.fn(), - }, - }, - { - provide: SpotifyUsersService, - useValue: { - profile: vi.fn(), + saveUser: vi.fn(), }, }, ], @@ -67,9 +50,7 @@ describe('AuthController', () => { authController = module.get(AuthController) spotifyAuthService = module.get(SpotifyAuthService) - profilesService = module.get(ProfilesService) - usersRepository = module.get(UsersRepository) - spotifyUsersService = module.get(SpotifyUsersService) + authService = module.get(AuthService) }) test('should be defined', () => { @@ -85,83 +66,28 @@ describe('AuthController', () => { describe('callback', () => { const code = 'code' + const authorizeParams: AuthorizeParams = { + accessToken, + refreshToken, + id: userMock.id, + } test('callback should return valid redirect path', async () => { const tokenSpy = vi .spyOn(spotifyAuthService, 'token') .mockResolvedValue(accessTokenMock) - const profileSpy = vi - .spyOn(spotifyUsersService, 'profile') - .mockResolvedValue(profileMock) - const findOneByProfileIdSpy = vi - .spyOn(usersRepository, 'findOneByProfileId') - .mockResolvedValue(userMock) + const saveUserSpy = vi + .spyOn(authService, 'saveUser') + .mockResolvedValue(authorizeParams) expect(await authController.callback(code)).toEqual({ url: `${redirectUrl}/api/authorize?${new URLSearchParams({ - accessToken, - refreshToken, - id: userMock.id, + ...authorizeParams, }).toString()}`, statusCode: HttpStatus.PERMANENT_REDIRECT, }) expect(tokenSpy).toHaveBeenCalledWith({ code }) - expect(profileSpy).toHaveBeenCalledWith(accessTokenMock) - expect(findOneByProfileIdSpy).toHaveBeenCalledWith(profileMock.id) - }) - - test('should find profile by id', async () => { - vi.spyOn(spotifyAuthService, 'token').mockResolvedValue(accessTokenMock) - vi.spyOn(spotifyUsersService, 'profile').mockResolvedValue(profileMock) - - const findUserByProfileId = vi - .spyOn(usersRepository, 'findOneByProfileId') - .mockResolvedValue(userMock) - const createSpy = vi.spyOn(profilesService, 'create') - const createUserSpy = vi.spyOn(usersRepository, 'createUser') - - expect(await authController.callback(code)).toEqual({ - url: `${redirectUrl}/api/authorize?${new URLSearchParams({ - accessToken, - refreshToken, - id: userMock.id, - }).toString()}`, - statusCode: HttpStatus.PERMANENT_REDIRECT, - }) - expect(findUserByProfileId).toHaveBeenCalledWith(profileMock.id) - expect(createSpy).not.toHaveBeenCalled() - expect(createUserSpy).not.toHaveBeenCalled() - }) - - test('should create profile and user', async () => { - vi.spyOn(spotifyAuthService, 'token').mockResolvedValue(accessTokenMock) - vi.spyOn(spotifyUsersService, 'profile').mockResolvedValue(profileMock) - - const findUserByProfileId = vi.spyOn( - usersRepository, - 'findOneByProfileId' - ) - const createSpy = vi - .spyOn(profilesService, 'create') - .mockResolvedValue(profileMock) - const createUserSpy = vi - .spyOn(usersRepository, 'createUser') - .mockResolvedValue(userMock) - - expect(await authController.callback(code)).toEqual({ - url: `${redirectUrl}/api/authorize?${new URLSearchParams({ - accessToken, - refreshToken, - id: userMock.id, - }).toString()}`, - statusCode: HttpStatus.PERMANENT_REDIRECT, - }) - expect(findUserByProfileId).toHaveBeenCalledWith(profileMock.id) - expect(createSpy).toHaveBeenCalledWith(profileMock) - expect(createUserSpy).toHaveBeenCalledWith({ - profile: profileMock, - refreshToken, - }) + expect(saveUserSpy).toHaveBeenCalledWith(accessTokenMock) }) }) diff --git a/src/modules/auth/auth.controller.ts b/src/modules/auth/auth.controller.ts index ed6f5e42..de576541 100644 --- a/src/modules/auth/auth.controller.ts +++ b/src/modules/auth/auth.controller.ts @@ -3,11 +3,9 @@ import { Controller, Get, HttpStatus, - Inject, Post, Query, Redirect, - forwardRef, } from '@nestjs/common' import { ConfigService } from '@nestjs/config' import { @@ -21,12 +19,10 @@ import { import { spotifyAuthorizationScopes } from './config' import { RedirectResponse } from './types' import { RefreshToken, SecretData } from './dtos' +import { AuthService } from './auth.service' import { SpotifyAuthService } from '@modules/spotify/auth' import { Environment } from '@config/environment' -import { UsersRepository } from '@modules/users' -import { ProfilesService } from '@modules/profiles' -import { SpotifyUsersService } from '@modules/spotify/users' import { adaptSecretData } from '@common/adapters' const { @@ -41,11 +37,8 @@ const { export class AuthController { constructor( private readonly configService: ConfigService, - @Inject(forwardRef(() => ProfilesService)) - private readonly profilesService: ProfilesService, - private readonly usersRepository: UsersRepository, - private readonly spotifyAuthService: SpotifyAuthService, - private readonly spotifyUsersService: SpotifyUsersService + private readonly authService: AuthService, + private readonly spotifyAuthService: SpotifyAuthService ) {} @Get('login') @@ -76,40 +69,14 @@ export class AuthController { const token = await this.spotifyAuthService.token({ code, }) - const spotifyProfile = await this.spotifyUsersService.profile(token) - const foundUser = await this.usersRepository.findOneByProfileId( - spotifyProfile.id - ) - - const { access_token: accessToken, refresh_token: refreshToken } = token - - if (refreshToken) { - let id: string - - if (foundUser) { - id = foundUser.id - } else { - const profile = await this.profilesService.create(spotifyProfile) - - const { id: createdUserId } = await this.usersRepository.createUser({ - profile, - refreshToken, - }) - - id = createdUserId - } - - return { - url: `${this.configService.get( - CLIENT_CALLBACK_URL - )}/api/authorize?${new URLSearchParams({ - accessToken, - refreshToken, - id, - }).toString()}`, - statusCode: HttpStatus.PERMANENT_REDIRECT, - } + return { + url: `${this.configService.get( + CLIENT_CALLBACK_URL + )}/api/authorize?${new URLSearchParams({ + ...(await this.authService.saveUser(token)), + }).toString()}`, + statusCode: HttpStatus.PERMANENT_REDIRECT, } } diff --git a/src/modules/auth/auth.service.spec.ts b/src/modules/auth/auth.service.spec.ts index db236aff..96c83075 100644 --- a/src/modules/auth/auth.service.spec.ts +++ b/src/modules/auth/auth.service.spec.ts @@ -1,144 +1,105 @@ -import { URLSearchParams } from 'node:url' - -import { HttpService } from '@nestjs/axios' -import { ConfigService } from '@nestjs/config' -import { JwtService } from '@nestjs/jwt' -import { Test, TestingModule } from '@nestjs/testing' -import { Profile } from 'passport-spotify' -import { of } from 'rxjs' +import { Test } from '@nestjs/testing' import { AuthService } from './auth.service' -import { spotifyProfileMock, profileMock } from '@common/mocks' +import { accessToken, refreshToken } from '@common/mocks' +import { profileMock, userMock, accessTokenMock } from '@common/mocks' +import { UsersRepository } from '@modules/users' +import { ProfilesService } from '@modules/profiles' +import { SpotifyUsersService } from '@modules/spotify/users' describe('AuthService', () => { let authService: AuthService - let jwtService: JwtService - let httpService: HttpService - let configService: ConfigService + let usersRepository: UsersRepository + let profilesService: ProfilesService + let spotifyUsersService: SpotifyUsersService beforeEach(async () => { - const module: TestingModule = await Test.createTestingModule({ + const module = await Test.createTestingModule({ providers: [ AuthService, - { - provide: JwtService, + provide: UsersRepository, useValue: { - sign: vi.fn(), + findOneByProfileId: vi.fn(), + createUser: vi.fn(), }, }, { - provide: HttpService, + provide: ProfilesService, useValue: { - post: vi.fn(), - get: vi.fn(), + create: vi.fn(), }, }, { - provide: ConfigService, + provide: SpotifyUsersService, useValue: { - get: vi.fn(), + profile: vi.fn(), }, }, ], }).compile() - authService = module.get(AuthService) - jwtService = module.get(JwtService) - httpService = module.get(HttpService) - configService = module.get(ConfigService) + authService = module.get(AuthService) + usersRepository = module.get(UsersRepository) + profilesService = module.get(ProfilesService) + spotifyUsersService = module.get(SpotifyUsersService) }) test('should be defined', () => { expect(authService).toBeDefined() }) - test('should login', () => { - const profile: Profile = { - provider: 'spotify', - id: '123', - displayName: 'John Doe', - username: 'john.doe', - photos: ['example'], - profileUrl: 'example.com', - country: 'US', - followers: 0, - product: 'premium', - _raw: 'raw', - _json: {}, - } - - jwtService.sign = vi.fn().mockReturnValue('token') - - expect(authService.login(profile)).toBe('token') - }) - - describe('token', () => { - test('should refresh token', async () => { - configService.get = vi.fn().mockReturnValue('value') - - const response = { - data: { - access_token: 'token', - expires_in: 3600, - }, - } - - const expectedResponse = { - accessToken: 'token', - expiresIn: 3600, - } - - httpService.post = vi - .fn() - .mockImplementation((_url, parameters: URLSearchParams) => { - if (parameters.get('grant_type') === 'refresh_token') - return of(response) - }) - - expect(await authService.token({ refreshToken: 'refresh' })).toEqual( - expectedResponse + describe('saveUser', () => { + test('should create new user', async () => { + const profileSpy = vi + .spyOn(spotifyUsersService, 'profile') + .mockResolvedValue(profileMock) + const findOneByProfileIdSpy = vi.spyOn( + usersRepository, + 'findOneByProfileId' ) + const createSpy = vi + .spyOn(profilesService, 'create') + .mockResolvedValue(profileMock) + const createUserSpy = vi + .spyOn(usersRepository, 'createUser') + .mockResolvedValue(userMock) + + expect(await authService.saveUser(accessTokenMock)).toEqual({ + accessToken, + refreshToken, + id: userMock.id, + }) + expect(profileSpy).toHaveBeenCalledWith(accessTokenMock) + expect(findOneByProfileIdSpy).toHaveBeenCalledWith(profileMock.id) + expect(createSpy).toHaveBeenCalledWith(profileMock) + expect(createUserSpy).toHaveBeenCalledWith({ + profile: profileMock, + refreshToken, + }) }) - test('should authorize and get tokens', async () => { - configService.get = vi.fn().mockReturnValue('value') - - const response = { - data: { - access_token: 'token', - refresh_token: 'refresh', - expires_in: 3600, - }, - } - - const expectedResponse = { - accessToken: 'token', - refreshToken: 'refresh', - expiresIn: 3600, - } - - httpService.post = vi - .fn() - .mockImplementation((_url, parameters: URLSearchParams) => { - if (parameters.get('grant_type') === 'authorization_code') - return of(response) - }) - - expect(await authService.token({ code: 'code' })).toEqual( - expectedResponse - ) + test('should return existing user', async () => { + const profileSpy = vi + .spyOn(spotifyUsersService, 'profile') + .mockResolvedValue(profileMock) + const findOneByProfileIdSpy = vi + .spyOn(usersRepository, 'findOneByProfileId') + .mockResolvedValue(userMock) + const createSpy = vi.spyOn(profilesService, 'create') + + const createUserSpy = vi.spyOn(usersRepository, 'createUser') + + expect(await authService.saveUser(accessTokenMock)).toEqual({ + accessToken, + refreshToken, + id: userMock.id, + }) + expect(profileSpy).toHaveBeenCalledWith(accessTokenMock) + expect(findOneByProfileIdSpy).toHaveBeenCalledWith(profileMock.id) + expect(createSpy).not.toHaveBeenCalled() + expect(createUserSpy).not.toHaveBeenCalled() }) }) - - test('should return profile', async () => { - const response = { - data: spotifyProfileMock, - } - - httpService.get = vi.fn().mockReturnValue(of(response)) - - expect(await authService.profile('token')).toEqual(profileMock) - }) }) diff --git a/src/modules/auth/auth.service.ts b/src/modules/auth/auth.service.ts index 3e7945e5..6c8e56f6 100644 --- a/src/modules/auth/auth.service.ts +++ b/src/modules/auth/auth.service.ts @@ -1,89 +1,48 @@ -import { HttpService } from '@nestjs/axios' import { Injectable } from '@nestjs/common' -import { ConfigService } from '@nestjs/config' -import { JwtService } from '@nestjs/jwt' -import { Profile as PassportSpotifyProfile } from 'passport-spotify' -import { map, catchError, firstValueFrom } from 'rxjs' +import { AccessToken } from '@spotify/web-api-ts-sdk' -import { SecretData } from './dtos' -import { TokenOptions } from './types' +import { AuthorizeParams } from './types' -import { Environment } from '@config/environment' -import { SpotifyToken, Profile, SpotifyProfile } from '@common/types/spotify' -import { applyAuthorizationHeader, catchSpotifyError } from '@common/utils' -import { adaptProfile, adaptSecretData } from '@common/adapters' +import { UsersRepository } from '@modules/users' +import { ProfilesService } from '@modules/profiles' +import { SpotifyUsersService } from '@modules/spotify/users' @Injectable() export class AuthService { constructor( - private readonly jwtService: JwtService, - private readonly httpService: HttpService, - private readonly configService: ConfigService + private readonly usersRepository: UsersRepository, + private readonly profilesService: ProfilesService, + private readonly spotifyUsersService: SpotifyUsersService ) {} - login({ id, username }: PassportSpotifyProfile) { - const payload = { - name: username, - sub: id, - } - - return this.jwtService.sign(payload) - } + async saveUser(token: AccessToken): Promise { + const spotifyProfile = await this.spotifyUsersService.profile(token) - token({ refreshToken, code }: TokenOptions): Promise { - const url = `${this.configService.get( - Environment.SPOTIFY_ACCOUNTS_URL - )}/api/token` - const cliendId = this.configService.get( - Environment.SPOTIFY_CLIENT_ID - ) - const clientSecret = this.configService.get( - Environment.SPOTIFY_CLIENT_SECRET - ) - const callbackUrl = this.configService.get( - Environment.SPOTIFY_CALLBACK_URL + const foundUser = await this.usersRepository.findOneByProfileId( + spotifyProfile.id ) - const bufferedCredentials = Buffer.from( - `${cliendId}:${clientSecret}` - ).toString('base64') - const params = new URLSearchParams() + const { access_token: accessToken, refresh_token: refreshToken } = token - if (refreshToken) { - params.append('refresh_token', refreshToken) - params.append('grant_type', 'refresh_token') - } - if (code) { - params.append('code', code) - params.append('grant_type', 'authorization_code') - callbackUrl && params.append('redirect_uri', callbackUrl) - } + let id: string - return firstValueFrom( - this.httpService - .post(url, params, { - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - Authorization: `Basic ${bufferedCredentials}`, - }, - }) - .pipe( - map(response => response.data), - map(adaptSecretData), - catchError(catchSpotifyError) - ) - ) - } + if (foundUser) { + id = foundUser.id + } else { + const profile = await this.profilesService.create(spotifyProfile) - profile(accessToken: string): Promise { - return firstValueFrom( - this.httpService - .get('/me', applyAuthorizationHeader(accessToken)) - .pipe( - map(response => response.data), - map(adaptProfile), - catchError(catchSpotifyError) - ) - ) + const createdUser = await this.usersRepository.createUser({ + profile, + refreshToken, + }) + + id = createdUser.id + } + + return { + accessToken, + refreshToken, + id, + } } } diff --git a/src/modules/auth/types/authorize-params.type.ts b/src/modules/auth/types/authorize-params.type.ts new file mode 100644 index 00000000..a495a50e --- /dev/null +++ b/src/modules/auth/types/authorize-params.type.ts @@ -0,0 +1,5 @@ +export interface AuthorizeParams { + refreshToken: string + accessToken: string + id: string +} diff --git a/src/modules/auth/types/index.ts b/src/modules/auth/types/index.ts index d3b323f5..afbb7c99 100644 --- a/src/modules/auth/types/index.ts +++ b/src/modules/auth/types/index.ts @@ -1,3 +1,2 @@ -export * from './jwt-payload' -export * from './redirect-response' -export * from './token-options' +export * from './authorize-params.type' +export * from './redirect-response.type'