From 92972ac77602a512faa4d73ed0f2a2400aa256f9 Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Wed, 18 Jan 2023 09:40:15 -0500 Subject: [PATCH] refactor(server): api keys (#1339) * refactor: api keys * refactor: test module * chore: tests * chore: fix provider * refactor: test mock repos --- .../src/api-v1/api-key/api-key.module.ts | 16 - server/apps/immich/src/app.module.ts | 6 +- .../api-key.controller.ts | 17 +- server/apps/immich/src/controllers/index.ts | 1 + .../modules/immich-jwt/immich-jwt.module.ts | 3 +- .../immich-jwt/strategies/api-key.strategy.ts | 16 +- server/apps/immich/tsconfig.app.json | 14 +- server/immich-openapi-specs.json | 408 +++++++++--------- .../domain/src/api-key/api-key.repository.ts | 16 + .../src/api-key/api-key.service.spec.ts | 142 ++++++ .../domain/src}/api-key/api-key.service.ts | 31 +- .../src}/api-key/dto/api-key-create.dto.ts | 0 .../src}/api-key/dto/api-key-update.dto.ts | 0 server/libs/domain/src/api-key/dto/index.ts | 2 + server/libs/domain/src/api-key/index.ts | 4 + .../api-key-create-response.dto.ts | 0 .../response-dto}/api-key-response.dto.ts | 0 .../domain/src/api-key/response-dto/index.ts | 2 + .../libs/domain/src/auth/crypto.repository.ts | 7 + server/libs/domain/src/auth/index.ts | 1 + server/libs/domain/src/domain.module.ts | 2 + server/libs/domain/src/index.ts | 1 + .../libs/domain/src/user/user.service.spec.ts | 31 +- .../domain/test/api-key.repository.mock.ts | 12 + .../domain/test/crypto.repository.mock.ts | 9 + server/libs/domain/test/fixtures.ts | 44 ++ server/libs/domain/test/index.ts | 4 + .../libs/domain/test/user.repository.mock.ts | 15 + .../libs/infra/src/auth/crypto.repository.ts | 9 + .../src/db/repository}/api-key.repository.ts | 18 +- server/libs/infra/src/db/repository/index.ts | 1 + server/libs/infra/src/infra.module.ts | 10 +- server/package.json | 8 +- 33 files changed, 538 insertions(+), 312 deletions(-) delete mode 100644 server/apps/immich/src/api-v1/api-key/api-key.module.ts rename server/apps/immich/src/{api-v1/api-key => controllers}/api-key.controller.ts (71%) create mode 100644 server/libs/domain/src/api-key/api-key.repository.ts create mode 100644 server/libs/domain/src/api-key/api-key.service.spec.ts rename server/{apps/immich/src/api-v1 => libs/domain/src}/api-key/api-key.service.ts (70%) rename server/{apps/immich/src/api-v1 => libs/domain/src}/api-key/dto/api-key-create.dto.ts (100%) rename server/{apps/immich/src/api-v1 => libs/domain/src}/api-key/dto/api-key-update.dto.ts (100%) create mode 100644 server/libs/domain/src/api-key/dto/index.ts create mode 100644 server/libs/domain/src/api-key/index.ts rename server/{apps/immich/src/api-v1/api-key/repsonse-dto => libs/domain/src/api-key/response-dto}/api-key-create-response.dto.ts (100%) rename server/{apps/immich/src/api-v1/api-key/repsonse-dto => libs/domain/src/api-key/response-dto}/api-key-response.dto.ts (100%) create mode 100644 server/libs/domain/src/api-key/response-dto/index.ts create mode 100644 server/libs/domain/src/auth/crypto.repository.ts create mode 100644 server/libs/domain/test/api-key.repository.mock.ts create mode 100644 server/libs/domain/test/crypto.repository.mock.ts create mode 100644 server/libs/domain/test/fixtures.ts create mode 100644 server/libs/domain/test/index.ts create mode 100644 server/libs/domain/test/user.repository.mock.ts create mode 100644 server/libs/infra/src/auth/crypto.repository.ts rename server/{apps/immich/src/api-v1/api-key => libs/infra/src/db/repository}/api-key.repository.ts (68%) diff --git a/server/apps/immich/src/api-v1/api-key/api-key.module.ts b/server/apps/immich/src/api-v1/api-key/api-key.module.ts deleted file mode 100644 index 5823358b2c..0000000000 --- a/server/apps/immich/src/api-v1/api-key/api-key.module.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { APIKeyEntity } from '@app/infra'; -import { Module } from '@nestjs/common'; -import { TypeOrmModule } from '@nestjs/typeorm'; -import { APIKeyController } from './api-key.controller'; -import { APIKeyRepository, IKeyRepository } from './api-key.repository'; -import { APIKeyService } from './api-key.service'; - -const KEY_REPOSITORY = { provide: IKeyRepository, useClass: APIKeyRepository }; - -@Module({ - imports: [TypeOrmModule.forFeature([APIKeyEntity])], - controllers: [APIKeyController], - providers: [APIKeyService, KEY_REPOSITORY], - exports: [APIKeyService, KEY_REPOSITORY], -}) -export class APIKeyModule {} diff --git a/server/apps/immich/src/app.module.ts b/server/apps/immich/src/app.module.ts index cea3ef1998..be3a10da43 100644 --- a/server/apps/immich/src/app.module.ts +++ b/server/apps/immich/src/app.module.ts @@ -2,7 +2,6 @@ import { immichAppConfig, immichBullAsyncConfig } from '@app/common/config'; import { MiddlewareConsumer, Module, NestModule } from '@nestjs/common'; import { AssetModule } from './api-v1/asset/asset.module'; import { AuthModule } from './api-v1/auth/auth.module'; -import { APIKeyModule } from './api-v1/api-key/api-key.module'; import { ImmichJwtModule } from './modules/immich-jwt/immich-jwt.module'; import { DeviceInfoModule } from './api-v1/device-info/device-info.module'; import { ConfigModule } from '@nestjs/config'; @@ -22,7 +21,7 @@ import { ImmichConfigModule } from '@app/immich-config'; import { ShareModule } from './api-v1/share/share.module'; import { DomainModule } from '@app/domain'; import { InfraModule } from '@app/infra'; -import { UserController } from './controllers'; +import { APIKeyController, UserController } from './controllers'; @Module({ imports: [ @@ -32,8 +31,6 @@ import { UserController } from './controllers'; imports: [InfraModule], }), - APIKeyModule, - AssetModule, AuthModule, @@ -69,6 +66,7 @@ import { UserController } from './controllers'; controllers: [ // AppController, + APIKeyController, UserController, ], providers: [], diff --git a/server/apps/immich/src/api-v1/api-key/api-key.controller.ts b/server/apps/immich/src/controllers/api-key.controller.ts similarity index 71% rename from server/apps/immich/src/api-v1/api-key/api-key.controller.ts rename to server/apps/immich/src/controllers/api-key.controller.ts index ab2434cd48..178248b7c8 100644 --- a/server/apps/immich/src/api-v1/api-key/api-key.controller.ts +++ b/server/apps/immich/src/controllers/api-key.controller.ts @@ -1,12 +1,15 @@ +import { + APIKeyCreateDto, + APIKeyCreateResponseDto, + APIKeyResponseDto, + APIKeyService, + APIKeyUpdateDto, + AuthUserDto, +} from '@app/domain'; import { Body, Controller, Delete, Get, Param, ParseIntPipe, Post, Put, ValidationPipe } from '@nestjs/common'; import { ApiTags } from '@nestjs/swagger'; -import { AuthUserDto, GetAuthUser } from '../../decorators/auth-user.decorator'; -import { Authenticated } from '../../decorators/authenticated.decorator'; -import { APIKeyService } from './api-key.service'; -import { APIKeyCreateDto } from './dto/api-key-create.dto'; -import { APIKeyUpdateDto } from './dto/api-key-update.dto'; -import { APIKeyCreateResponseDto } from './repsonse-dto/api-key-create-response.dto'; -import { APIKeyResponseDto } from './repsonse-dto/api-key-response.dto'; +import { GetAuthUser } from '../decorators/auth-user.decorator'; +import { Authenticated } from '../decorators/authenticated.decorator'; @ApiTags('API Key') @Controller('api-key') diff --git a/server/apps/immich/src/controllers/index.ts b/server/apps/immich/src/controllers/index.ts index edd3705c44..bac062eb90 100644 --- a/server/apps/immich/src/controllers/index.ts +++ b/server/apps/immich/src/controllers/index.ts @@ -1 +1,2 @@ +export * from './api-key.controller'; export * from './user.controller'; diff --git a/server/apps/immich/src/modules/immich-jwt/immich-jwt.module.ts b/server/apps/immich/src/modules/immich-jwt/immich-jwt.module.ts index 94bd8b772c..a8e60802b4 100644 --- a/server/apps/immich/src/modules/immich-jwt/immich-jwt.module.ts +++ b/server/apps/immich/src/modules/immich-jwt/immich-jwt.module.ts @@ -3,13 +3,12 @@ import { ImmichJwtService } from './immich-jwt.service'; import { JwtModule } from '@nestjs/jwt'; import { jwtConfig } from '../../config/jwt.config'; import { JwtStrategy } from './strategies/jwt.strategy'; -import { APIKeyModule } from '../../api-v1/api-key/api-key.module'; import { APIKeyStrategy } from './strategies/api-key.strategy'; import { ShareModule } from '../../api-v1/share/share.module'; import { PublicShareStrategy } from './strategies/public-share.strategy'; @Module({ - imports: [JwtModule.register(jwtConfig), APIKeyModule, ShareModule], + imports: [JwtModule.register(jwtConfig), ShareModule], providers: [ImmichJwtService, JwtStrategy, APIKeyStrategy, PublicShareStrategy], exports: [ImmichJwtService], }) diff --git a/server/apps/immich/src/modules/immich-jwt/strategies/api-key.strategy.ts b/server/apps/immich/src/modules/immich-jwt/strategies/api-key.strategy.ts index 8a35800072..ad4d15901f 100644 --- a/server/apps/immich/src/modules/immich-jwt/strategies/api-key.strategy.ts +++ b/server/apps/immich/src/modules/immich-jwt/strategies/api-key.strategy.ts @@ -1,8 +1,7 @@ +import { APIKeyService, AuthUserDto } from '@app/domain'; import { Injectable } from '@nestjs/common'; import { PassportStrategy } from '@nestjs/passport'; import { IStrategyOptions, Strategy } from 'passport-http-header-strategy'; -import { APIKeyService } from '../../../api-v1/api-key/api-key.service'; -import { AuthUserDto } from '../../../decorators/auth-user.decorator'; export const API_KEY_STRATEGY = 'api-key'; @@ -16,16 +15,7 @@ export class APIKeyStrategy extends PassportStrategy(Strategy, API_KEY_STRATEGY) super(options); } - async validate(token: string): Promise { - const user = await this.apiKeyService.validate(token); - - const authUser = new AuthUserDto(); - authUser.id = user.id; - authUser.email = user.email; - authUser.isAdmin = user.isAdmin; - authUser.isPublicUser = false; - authUser.isAllowUpload = true; - - return authUser; + validate(token: string): Promise { + return this.apiKeyService.validate(token); } } diff --git a/server/apps/immich/tsconfig.app.json b/server/apps/immich/tsconfig.app.json index 44a4899dda..825e740596 100644 --- a/server/apps/immich/tsconfig.app.json +++ b/server/apps/immich/tsconfig.app.json @@ -4,14 +4,6 @@ "declaration": false, "outDir": "../../dist/apps/immich" }, - "include": [ - "src/**/*", - "../../libs/**/*" - ], - "exclude": [ - "node_modules", - "dist", - "test", - "**/*spec.ts" - ] -} \ No newline at end of file + "include": ["src/**/*"], + "exclude": ["node_modules", "dist", "test", "**/*spec.ts"] +} diff --git a/server/immich-openapi-specs.json b/server/immich-openapi-specs.json index 4de55a4f75..4062a38262 100644 --- a/server/immich-openapi-specs.json +++ b/server/immich-openapi-specs.json @@ -1,6 +1,153 @@ { "openapi": "3.0.0", "paths": { + "/api-key": { + "post": { + "operationId": "createKey", + "description": "", + "parameters": [], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/APIKeyCreateDto" + } + } + } + }, + "responses": { + "201": { + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/APIKeyCreateResponseDto" + } + } + } + } + }, + "tags": [ + "API Key" + ] + }, + "get": { + "operationId": "getKeys", + "description": "", + "parameters": [], + "responses": { + "200": { + "description": "", + "content": { + "application/json": { + "schema": { + "type": "array", + "items": { + "$ref": "#/components/schemas/APIKeyResponseDto" + } + } + } + } + } + }, + "tags": [ + "API Key" + ] + } + }, + "/api-key/{id}": { + "get": { + "operationId": "getKey", + "description": "", + "parameters": [ + { + "name": "id", + "required": true, + "in": "path", + "schema": { + "type": "number" + } + } + ], + "responses": { + "200": { + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/APIKeyResponseDto" + } + } + } + } + }, + "tags": [ + "API Key" + ] + }, + "put": { + "operationId": "updateKey", + "description": "", + "parameters": [ + { + "name": "id", + "required": true, + "in": "path", + "schema": { + "type": "number" + } + } + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/APIKeyUpdateDto" + } + } + } + }, + "responses": { + "200": { + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/APIKeyResponseDto" + } + } + } + } + }, + "tags": [ + "API Key" + ] + }, + "delete": { + "operationId": "deleteKey", + "description": "", + "parameters": [ + { + "name": "id", + "required": true, + "in": "path", + "schema": { + "type": "number" + } + } + ], + "responses": { + "200": { + "description": "" + } + }, + "tags": [ + "API Key" + ] + } + }, "/user": { "get": { "operationId": "getAllUsers", @@ -341,153 +488,6 @@ ] } }, - "/api-key": { - "post": { - "operationId": "createKey", - "description": "", - "parameters": [], - "requestBody": { - "required": true, - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/APIKeyCreateDto" - } - } - } - }, - "responses": { - "201": { - "description": "", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/APIKeyCreateResponseDto" - } - } - } - } - }, - "tags": [ - "API Key" - ] - }, - "get": { - "operationId": "getKeys", - "description": "", - "parameters": [], - "responses": { - "200": { - "description": "", - "content": { - "application/json": { - "schema": { - "type": "array", - "items": { - "$ref": "#/components/schemas/APIKeyResponseDto" - } - } - } - } - } - }, - "tags": [ - "API Key" - ] - } - }, - "/api-key/{id}": { - "get": { - "operationId": "getKey", - "description": "", - "parameters": [ - { - "name": "id", - "required": true, - "in": "path", - "schema": { - "type": "number" - } - } - ], - "responses": { - "200": { - "description": "", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/APIKeyResponseDto" - } - } - } - } - }, - "tags": [ - "API Key" - ] - }, - "put": { - "operationId": "updateKey", - "description": "", - "parameters": [ - { - "name": "id", - "required": true, - "in": "path", - "schema": { - "type": "number" - } - } - ], - "requestBody": { - "required": true, - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/APIKeyUpdateDto" - } - } - } - }, - "responses": { - "200": { - "description": "", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/APIKeyResponseDto" - } - } - } - } - }, - "tags": [ - "API Key" - ] - }, - "delete": { - "operationId": "deleteKey", - "description": "", - "parameters": [ - { - "name": "id", - "required": true, - "in": "path", - "schema": { - "type": "number" - } - } - ], - "responses": { - "200": { - "description": "" - } - }, - "tags": [ - "API Key" - ] - } - }, "/asset/upload": { "post": { "operationId": "uploadFile", @@ -2825,6 +2825,63 @@ } }, "schemas": { + "APIKeyCreateDto": { + "type": "object", + "properties": { + "name": { + "type": "string" + } + } + }, + "APIKeyResponseDto": { + "type": "object", + "properties": { + "id": { + "type": "integer" + }, + "name": { + "type": "string" + }, + "createdAt": { + "type": "string" + }, + "updatedAt": { + "type": "string" + } + }, + "required": [ + "id", + "name", + "createdAt", + "updatedAt" + ] + }, + "APIKeyCreateResponseDto": { + "type": "object", + "properties": { + "secret": { + "type": "string" + }, + "apiKey": { + "$ref": "#/components/schemas/APIKeyResponseDto" + } + }, + "required": [ + "secret", + "apiKey" + ] + }, + "APIKeyUpdateDto": { + "type": "object", + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ] + }, "UserResponseDto": { "type": "object", "properties": { @@ -2969,63 +3026,6 @@ "profileImagePath" ] }, - "APIKeyCreateDto": { - "type": "object", - "properties": { - "name": { - "type": "string" - } - } - }, - "APIKeyResponseDto": { - "type": "object", - "properties": { - "id": { - "type": "integer" - }, - "name": { - "type": "string" - }, - "createdAt": { - "type": "string" - }, - "updatedAt": { - "type": "string" - } - }, - "required": [ - "id", - "name", - "createdAt", - "updatedAt" - ] - }, - "APIKeyCreateResponseDto": { - "type": "object", - "properties": { - "secret": { - "type": "string" - }, - "apiKey": { - "$ref": "#/components/schemas/APIKeyResponseDto" - } - }, - "required": [ - "secret", - "apiKey" - ] - }, - "APIKeyUpdateDto": { - "type": "object", - "properties": { - "name": { - "type": "string" - } - }, - "required": [ - "name" - ] - }, "AssetFileUploadDto": { "type": "object", "properties": { diff --git a/server/libs/domain/src/api-key/api-key.repository.ts b/server/libs/domain/src/api-key/api-key.repository.ts new file mode 100644 index 0000000000..a1dee0887b --- /dev/null +++ b/server/libs/domain/src/api-key/api-key.repository.ts @@ -0,0 +1,16 @@ +import { APIKeyEntity } from '@app/infra'; + +export const IKeyRepository = 'IKeyRepository'; + +export interface IKeyRepository { + create(dto: Partial): Promise; + update(userId: string, id: number, dto: Partial): Promise; + delete(userId: string, id: number): Promise; + /** + * Includes the hashed `key` for verification + * @param id + */ + getKey(id: number): Promise; + getById(userId: string, id: number): Promise; + getByUserId(userId: string): Promise; +} diff --git a/server/libs/domain/src/api-key/api-key.service.spec.ts b/server/libs/domain/src/api-key/api-key.service.spec.ts new file mode 100644 index 0000000000..a99bce5a24 --- /dev/null +++ b/server/libs/domain/src/api-key/api-key.service.spec.ts @@ -0,0 +1,142 @@ +import { APIKeyEntity } from '@app/infra'; +import { BadRequestException, UnauthorizedException } from '@nestjs/common'; +import { authStub, entityStub, newCryptoRepositoryMock, newKeyRepositoryMock } from '../../test'; +import { ICryptoRepository } from '../auth'; +import { IKeyRepository } from './api-key.repository'; +import { APIKeyService } from './api-key.service'; + +const adminKey = Object.freeze({ + id: 1, + name: 'My Key', + key: 'my-api-key (hashed)', + userId: authStub.admin.id, + user: entityStub.admin, +} as APIKeyEntity); + +const token = Buffer.from('1:my-api-key', 'utf8').toString('base64'); + +describe(APIKeyService.name, () => { + let sut: APIKeyService; + let keyMock: jest.Mocked; + let cryptoMock: jest.Mocked; + + beforeEach(async () => { + cryptoMock = newCryptoRepositoryMock(); + keyMock = newKeyRepositoryMock(); + sut = new APIKeyService(cryptoMock, keyMock); + }); + + describe('create', () => { + it('should create a new key', async () => { + keyMock.create.mockResolvedValue(adminKey); + + await sut.create(authStub.admin, { name: 'Test Key' }); + + expect(keyMock.create).toHaveBeenCalledWith({ + key: 'cmFuZG9tLWJ5dGVz (hashed)', + name: 'Test Key', + userId: authStub.admin.id, + }); + expect(cryptoMock.randomBytes).toHaveBeenCalled(); + expect(cryptoMock.hash).toHaveBeenCalled(); + }); + + it('should not require a name', async () => { + keyMock.create.mockResolvedValue(adminKey); + + await sut.create(authStub.admin, {}); + + expect(keyMock.create).toHaveBeenCalledWith({ + key: 'cmFuZG9tLWJ5dGVz (hashed)', + name: 'API Key', + userId: authStub.admin.id, + }); + expect(cryptoMock.randomBytes).toHaveBeenCalled(); + expect(cryptoMock.hash).toHaveBeenCalled(); + }); + }); + + describe('update', () => { + it('should throw an error if the key is not found', async () => { + keyMock.getById.mockResolvedValue(null); + + await expect(sut.update(authStub.admin, 1, { name: 'New Name' })).rejects.toBeInstanceOf(BadRequestException); + + expect(keyMock.update).not.toHaveBeenCalledWith(1); + }); + + it('should update a key', async () => { + keyMock.getById.mockResolvedValue(adminKey); + + await sut.update(authStub.admin, 1, { name: 'New Name' }); + + expect(keyMock.update).toHaveBeenCalledWith(authStub.admin.id, 1, { name: 'New Name' }); + }); + }); + + describe('delete', () => { + it('should throw an error if the key is not found', async () => { + keyMock.getById.mockResolvedValue(null); + + await expect(sut.delete(authStub.admin, 1)).rejects.toBeInstanceOf(BadRequestException); + + expect(keyMock.delete).not.toHaveBeenCalledWith(1); + }); + + it('should delete a key', async () => { + keyMock.getById.mockResolvedValue(adminKey); + + await sut.delete(authStub.admin, 1); + + expect(keyMock.delete).toHaveBeenCalledWith(authStub.admin.id, 1); + }); + }); + + describe('getById', () => { + it('should throw an error if the key is not found', async () => { + keyMock.getById.mockResolvedValue(null); + + await expect(sut.getById(authStub.admin, 1)).rejects.toBeInstanceOf(BadRequestException); + + expect(keyMock.getById).toHaveBeenCalledWith(authStub.admin.id, 1); + }); + + it('should get a key by id', async () => { + keyMock.getById.mockResolvedValue(adminKey); + + await sut.getById(authStub.admin, 1); + + expect(keyMock.getById).toHaveBeenCalledWith(authStub.admin.id, 1); + }); + }); + + describe('getAll', () => { + it('should return all the keys for a user', async () => { + keyMock.getByUserId.mockResolvedValue([adminKey]); + + await expect(sut.getAll(authStub.admin)).resolves.toHaveLength(1); + + expect(keyMock.getByUserId).toHaveBeenCalledWith(authStub.admin.id); + }); + }); + + describe('validate', () => { + it('should throw an error for an invalid id', async () => { + keyMock.getKey.mockResolvedValue(null); + + await expect(sut.validate(token)).rejects.toBeInstanceOf(UnauthorizedException); + + expect(keyMock.getKey).toHaveBeenCalledWith(1); + expect(cryptoMock.compareSync).not.toHaveBeenCalled(); + }); + + it('should validate the token', async () => { + keyMock.getKey.mockResolvedValue(adminKey); + + await expect(sut.validate(token)).resolves.toEqual(authStub.admin); + + expect(keyMock.getKey).toHaveBeenCalledWith(1); + expect(cryptoMock.compareSync).toHaveBeenCalledWith('my-api-key', 'my-api-key (hashed)'); + }); + }); +}); diff --git a/server/apps/immich/src/api-v1/api-key/api-key.service.ts b/server/libs/domain/src/api-key/api-key.service.ts similarity index 70% rename from server/apps/immich/src/api-v1/api-key/api-key.service.ts rename to server/libs/domain/src/api-key/api-key.service.ts index 48f7abc23d..b9ef6ad997 100644 --- a/server/apps/immich/src/api-v1/api-key/api-key.service.ts +++ b/server/libs/domain/src/api-key/api-key.service.ts @@ -1,21 +1,22 @@ import { UserEntity } from '@app/infra'; import { BadRequestException, Inject, Injectable, UnauthorizedException } from '@nestjs/common'; -import { compareSync, hash } from 'bcrypt'; -import { randomBytes } from 'node:crypto'; -import { AuthUserDto } from '../../decorators/auth-user.decorator'; +import { AuthUserDto, ICryptoRepository } from '../auth'; import { IKeyRepository } from './api-key.repository'; import { APIKeyCreateDto } from './dto/api-key-create.dto'; -import { APIKeyCreateResponseDto } from './repsonse-dto/api-key-create-response.dto'; -import { APIKeyResponseDto, mapKey } from './repsonse-dto/api-key-response.dto'; +import { APIKeyCreateResponseDto } from './response-dto/api-key-create-response.dto'; +import { APIKeyResponseDto, mapKey } from './response-dto/api-key-response.dto'; @Injectable() export class APIKeyService { - constructor(@Inject(IKeyRepository) private repository: IKeyRepository) {} + constructor( + @Inject(ICryptoRepository) private crypto: ICryptoRepository, + @Inject(IKeyRepository) private repository: IKeyRepository, + ) {} async create(authUser: AuthUserDto, dto: APIKeyCreateDto): Promise { - const key = randomBytes(24).toString('base64').replace(/\W/g, ''); + const key = this.crypto.randomBytes(24).toString('base64').replace(/\W/g, ''); const entity = await this.repository.create({ - key: await hash(key, 10), + key: await this.crypto.hash(key, 10), name: dto.name || 'API Key', userId: authUser.id, }); @@ -58,14 +59,22 @@ export class APIKeyService { return keys.map(mapKey); } - async validate(token: string): Promise { + async validate(token: string): Promise { const [_id, key] = Buffer.from(token, 'base64').toString('utf8').split(':'); const id = Number(_id); if (id && key) { const entity = await this.repository.getKey(id); - if (entity?.user && entity?.key && compareSync(key, entity.key)) { - return entity.user as UserEntity; + if (entity?.user && entity?.key && this.crypto.compareSync(key, entity.key)) { + const user = entity.user as UserEntity; + + return { + id: user.id, + email: user.email, + isAdmin: user.isAdmin, + isPublicUser: false, + isAllowUpload: true, + }; } } diff --git a/server/apps/immich/src/api-v1/api-key/dto/api-key-create.dto.ts b/server/libs/domain/src/api-key/dto/api-key-create.dto.ts similarity index 100% rename from server/apps/immich/src/api-v1/api-key/dto/api-key-create.dto.ts rename to server/libs/domain/src/api-key/dto/api-key-create.dto.ts diff --git a/server/apps/immich/src/api-v1/api-key/dto/api-key-update.dto.ts b/server/libs/domain/src/api-key/dto/api-key-update.dto.ts similarity index 100% rename from server/apps/immich/src/api-v1/api-key/dto/api-key-update.dto.ts rename to server/libs/domain/src/api-key/dto/api-key-update.dto.ts diff --git a/server/libs/domain/src/api-key/dto/index.ts b/server/libs/domain/src/api-key/dto/index.ts new file mode 100644 index 0000000000..c0deb8fbe6 --- /dev/null +++ b/server/libs/domain/src/api-key/dto/index.ts @@ -0,0 +1,2 @@ +export * from './api-key-create.dto'; +export * from './api-key-update.dto'; diff --git a/server/libs/domain/src/api-key/index.ts b/server/libs/domain/src/api-key/index.ts new file mode 100644 index 0000000000..87f3632f0e --- /dev/null +++ b/server/libs/domain/src/api-key/index.ts @@ -0,0 +1,4 @@ +export * from './api-key.repository'; +export * from './api-key.service'; +export * from './dto'; +export * from './response-dto'; diff --git a/server/apps/immich/src/api-v1/api-key/repsonse-dto/api-key-create-response.dto.ts b/server/libs/domain/src/api-key/response-dto/api-key-create-response.dto.ts similarity index 100% rename from server/apps/immich/src/api-v1/api-key/repsonse-dto/api-key-create-response.dto.ts rename to server/libs/domain/src/api-key/response-dto/api-key-create-response.dto.ts diff --git a/server/apps/immich/src/api-v1/api-key/repsonse-dto/api-key-response.dto.ts b/server/libs/domain/src/api-key/response-dto/api-key-response.dto.ts similarity index 100% rename from server/apps/immich/src/api-v1/api-key/repsonse-dto/api-key-response.dto.ts rename to server/libs/domain/src/api-key/response-dto/api-key-response.dto.ts diff --git a/server/libs/domain/src/api-key/response-dto/index.ts b/server/libs/domain/src/api-key/response-dto/index.ts new file mode 100644 index 0000000000..82cf6a4d9a --- /dev/null +++ b/server/libs/domain/src/api-key/response-dto/index.ts @@ -0,0 +1,2 @@ +export * from './api-key-create-response.dto'; +export * from './api-key-response.dto'; diff --git a/server/libs/domain/src/auth/crypto.repository.ts b/server/libs/domain/src/auth/crypto.repository.ts new file mode 100644 index 0000000000..bce8b221c0 --- /dev/null +++ b/server/libs/domain/src/auth/crypto.repository.ts @@ -0,0 +1,7 @@ +export const ICryptoRepository = 'ICryptoRepository'; + +export interface ICryptoRepository { + randomBytes(size: number): Buffer; + hash(data: string | Buffer, saltOrRounds: string | number): Promise; + compareSync(data: Buffer | string, encrypted: string): boolean; +} diff --git a/server/libs/domain/src/auth/index.ts b/server/libs/domain/src/auth/index.ts index dacfee3d06..bdef02bdaa 100644 --- a/server/libs/domain/src/auth/index.ts +++ b/server/libs/domain/src/auth/index.ts @@ -1 +1,2 @@ +export * from './crypto.repository'; export * from './dto'; diff --git a/server/libs/domain/src/domain.module.ts b/server/libs/domain/src/domain.module.ts index baea8e79f0..56e958fb42 100644 --- a/server/libs/domain/src/domain.module.ts +++ b/server/libs/domain/src/domain.module.ts @@ -1,8 +1,10 @@ import { DynamicModule, Global, Module, ModuleMetadata, Provider } from '@nestjs/common'; +import { APIKeyService } from './api-key'; import { UserService } from './user'; const providers: Provider[] = [ // + APIKeyService, UserService, ]; diff --git a/server/libs/domain/src/index.ts b/server/libs/domain/src/index.ts index 9fda872c19..2d3b6a23b7 100644 --- a/server/libs/domain/src/index.ts +++ b/server/libs/domain/src/index.ts @@ -1,3 +1,4 @@ +export * from './api-key'; export * from './auth'; export * from './domain.module'; export * from './user'; diff --git a/server/libs/domain/src/user/user.service.spec.ts b/server/libs/domain/src/user/user.service.spec.ts index fac5166a1a..115b39e234 100644 --- a/server/libs/domain/src/user/user.service.spec.ts +++ b/server/libs/domain/src/user/user.service.spec.ts @@ -1,10 +1,11 @@ +import { IUserRepository } from '@app/domain'; import { UserEntity } from '@app/infra'; import { BadRequestException, ForbiddenException, NotFoundException } from '@nestjs/common'; -import { AuthUserDto } from '../auth'; -import { IUserRepository } from '@app/domain'; import { when } from 'jest-when'; -import { UserService } from './user.service'; +import { newUserRepositoryMock } from '../../test'; +import { AuthUserDto } from '../auth'; import { UpdateUserDto } from './dto/update-user.dto'; +import { UserService } from './user.service'; const adminUserAuth: AuthUserDto = Object.freeze({ id: 'admin_id', @@ -73,28 +74,18 @@ const adminUserResponse = Object.freeze({ createdAt: '2021-01-01', }); -describe('UserService', () => { +describe(UserService.name, () => { let sut: UserService; let userRepositoryMock: jest.Mocked; - beforeEach(() => { - userRepositoryMock = { - get: jest.fn(), - getAdmin: jest.fn(), - getByEmail: jest.fn(), - getByOAuthId: jest.fn(), - getList: jest.fn(), - create: jest.fn(), - update: jest.fn(), - delete: jest.fn(), - restore: jest.fn(), - }; + beforeEach(async () => { + userRepositoryMock = newUserRepositoryMock(); + sut = new UserService(userRepositoryMock); + when(userRepositoryMock.get).calledWith(adminUser.id).mockResolvedValue(adminUser); when(userRepositoryMock.get).calledWith(adminUser.id, undefined).mockResolvedValue(adminUser); when(userRepositoryMock.get).calledWith(immichUser.id).mockResolvedValue(immichUser); when(userRepositoryMock.get).calledWith(immichUser.id, undefined).mockResolvedValue(immichUser); - - sut = new UserService(userRepositoryMock); }); describe('getAllUsers', () => { @@ -285,9 +276,7 @@ describe('UserService', () => { describe('deleteUser', () => { it('cannot delete admin user', async () => { - const result = sut.deleteUser(adminUserAuth, adminUserAuth.id); - - await expect(result).rejects.toBeInstanceOf(ForbiddenException); + await expect(sut.deleteUser(adminUserAuth, adminUserAuth.id)).rejects.toBeInstanceOf(ForbiddenException); }); it('should require the auth user be an admin', async () => { diff --git a/server/libs/domain/test/api-key.repository.mock.ts b/server/libs/domain/test/api-key.repository.mock.ts new file mode 100644 index 0000000000..dbf8405fc1 --- /dev/null +++ b/server/libs/domain/test/api-key.repository.mock.ts @@ -0,0 +1,12 @@ +import { IKeyRepository } from '../src'; + +export const newKeyRepositoryMock = (): jest.Mocked => { + return { + create: jest.fn(), + update: jest.fn(), + delete: jest.fn(), + getKey: jest.fn(), + getById: jest.fn(), + getByUserId: jest.fn(), + }; +}; diff --git a/server/libs/domain/test/crypto.repository.mock.ts b/server/libs/domain/test/crypto.repository.mock.ts new file mode 100644 index 0000000000..6c747b2087 --- /dev/null +++ b/server/libs/domain/test/crypto.repository.mock.ts @@ -0,0 +1,9 @@ +import { ICryptoRepository } from '../src'; + +export const newCryptoRepositoryMock = (): jest.Mocked => { + return { + randomBytes: jest.fn().mockReturnValue(Buffer.from('random-bytes', 'utf8')), + compareSync: jest.fn().mockReturnValue(true), + hash: jest.fn().mockImplementation((input) => Promise.resolve(`${input} (hashed)`)), + }; +}; diff --git a/server/libs/domain/test/fixtures.ts b/server/libs/domain/test/fixtures.ts new file mode 100644 index 0000000000..278fae59e5 --- /dev/null +++ b/server/libs/domain/test/fixtures.ts @@ -0,0 +1,44 @@ +import { UserEntity } from '@app/infra'; +import { AuthUserDto } from '../src'; + +export const authStub = { + admin: Object.freeze({ + id: 'admin_id', + email: 'admin@test.com', + isAdmin: true, + isPublicUser: false, + isAllowUpload: true, + }), + user1: Object.freeze({ + id: 'immich_id', + email: 'immich@test.com', + isAdmin: false, + isPublicUser: false, + isAllowUpload: true, + }), +}; + +export const entityStub = { + admin: Object.freeze({ + ...authStub.admin, + password: 'admin_password', + firstName: 'admin_first_name', + lastName: 'admin_last_name', + oauthId: '', + shouldChangePassword: false, + profileImagePath: '', + createdAt: '2021-01-01', + tags: [], + }), + user1: Object.freeze({ + ...authStub.user1, + password: 'immich_password', + firstName: 'immich_first_name', + lastName: 'immich_last_name', + oauthId: '', + shouldChangePassword: false, + profileImagePath: '', + createdAt: '2021-01-01', + tags: [], + }), +}; diff --git a/server/libs/domain/test/index.ts b/server/libs/domain/test/index.ts new file mode 100644 index 0000000000..58d289be33 --- /dev/null +++ b/server/libs/domain/test/index.ts @@ -0,0 +1,4 @@ +export * from './api-key.repository.mock'; +export * from './crypto.repository.mock'; +export * from './fixtures'; +export * from './user.repository.mock'; diff --git a/server/libs/domain/test/user.repository.mock.ts b/server/libs/domain/test/user.repository.mock.ts new file mode 100644 index 0000000000..89d04ae263 --- /dev/null +++ b/server/libs/domain/test/user.repository.mock.ts @@ -0,0 +1,15 @@ +import { IUserRepository } from '../src'; + +export const newUserRepositoryMock = (): jest.Mocked => { + return { + get: jest.fn(), + getAdmin: jest.fn(), + getByEmail: jest.fn(), + getByOAuthId: jest.fn(), + getList: jest.fn(), + create: jest.fn(), + update: jest.fn(), + delete: jest.fn(), + restore: jest.fn(), + }; +}; diff --git a/server/libs/infra/src/auth/crypto.repository.ts b/server/libs/infra/src/auth/crypto.repository.ts new file mode 100644 index 0000000000..b628463d89 --- /dev/null +++ b/server/libs/infra/src/auth/crypto.repository.ts @@ -0,0 +1,9 @@ +import { ICryptoRepository } from '@app/domain'; +import { compareSync, hash } from 'bcrypt'; +import { randomBytes } from 'crypto'; + +export const cryptoRepository: ICryptoRepository = { + randomBytes, + hash, + compareSync, +}; diff --git a/server/apps/immich/src/api-v1/api-key/api-key.repository.ts b/server/libs/infra/src/db/repository/api-key.repository.ts similarity index 68% rename from server/apps/immich/src/api-v1/api-key/api-key.repository.ts rename to server/libs/infra/src/db/repository/api-key.repository.ts index 19a1fad067..18ee6e6925 100644 --- a/server/apps/immich/src/api-v1/api-key/api-key.repository.ts +++ b/server/libs/infra/src/db/repository/api-key.repository.ts @@ -1,22 +1,8 @@ -import { APIKeyEntity } from '@app/infra'; +import { IKeyRepository } from '@app/domain'; import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; import { Repository } from 'typeorm'; - -export const IKeyRepository = 'IKeyRepository'; - -export interface IKeyRepository { - create(dto: Partial): Promise; - update(userId: string, id: number, dto: Partial): Promise; - delete(userId: string, id: number): Promise; - /** - * Includes the hashed `key` for verification - * @param id - */ - getKey(id: number): Promise; - getById(userId: string, id: number): Promise; - getByUserId(userId: string): Promise; -} +import { APIKeyEntity } from '../entities'; @Injectable() export class APIKeyRepository implements IKeyRepository { diff --git a/server/libs/infra/src/db/repository/index.ts b/server/libs/infra/src/db/repository/index.ts index 9fb5d34d10..f7e2fb0b21 100644 --- a/server/libs/infra/src/db/repository/index.ts +++ b/server/libs/infra/src/db/repository/index.ts @@ -1 +1,2 @@ +export * from './api-key.repository'; export * from './user.repository'; diff --git a/server/libs/infra/src/infra.module.ts b/server/libs/infra/src/infra.module.ts index af3a4e0cd5..e6e2764041 100644 --- a/server/libs/infra/src/infra.module.ts +++ b/server/libs/infra/src/infra.module.ts @@ -1,11 +1,15 @@ +import { ICryptoRepository, IKeyRepository, IUserRepository } from '@app/domain'; import { databaseConfig, UserEntity } from '@app/infra'; -import { IUserRepository } from '@app/domain'; import { Global, Module, Provider } from '@nestjs/common'; import { TypeOrmModule } from '@nestjs/typeorm'; -import { UserRepository } from './db'; +import { cryptoRepository } from './auth/crypto.repository'; +import { APIKeyEntity, UserRepository } from './db'; +import { APIKeyRepository } from './db/repository'; const providers: Provider[] = [ // + { provide: ICryptoRepository, useValue: cryptoRepository }, + { provide: IKeyRepository, useClass: APIKeyRepository }, { provide: IUserRepository, useClass: UserRepository }, ]; @@ -14,7 +18,7 @@ const providers: Provider[] = [ imports: [ // TypeOrmModule.forRoot(databaseConfig), - TypeOrmModule.forFeature([UserEntity]), + TypeOrmModule.forFeature([APIKeyEntity, UserEntity]), ], providers: [...providers], exports: [...providers], diff --git a/server/package.json b/server/package.json index eaf5838386..d37f865e2e 100644 --- a/server/package.json +++ b/server/package.json @@ -145,10 +145,10 @@ "statements": 20 }, "./libs/domain/": { - "branches": 60, - "functions": 80, - "lines": 80, - "statements": 80 + "branches": 70, + "functions": 85, + "lines": 85, + "statements": 85 } }, "testEnvironment": "node",