diff --git a/e2e/src/api/specs/asset.e2e-spec.ts b/e2e/src/api/specs/asset.e2e-spec.ts index 3eb848c178..97e53d4a75 100644 --- a/e2e/src/api/specs/asset.e2e-spec.ts +++ b/e2e/src/api/specs/asset.e2e-spec.ts @@ -432,20 +432,6 @@ describe('/asset', () => { }); describe('PUT /assets/:id', () => { - it('should require authentication', async () => { - const { status, body } = await request(app).put(`/assets/:${uuidDto.notFound}`); - expect(status).toBe(401); - expect(body).toEqual(errorDto.unauthorized); - }); - - it('should require a valid id', async () => { - const { status, body } = await request(app) - .put(`/assets/${uuidDto.invalid}`) - .set('Authorization', `Bearer ${user1.accessToken}`); - expect(status).toBe(400); - expect(body).toEqual(errorDto.badRequest(['id must be a UUID'])); - }); - it('should require access', async () => { const { status, body } = await request(app) .put(`/assets/${user2Assets[0].id}`) diff --git a/e2e/src/api/specs/auth.e2e-spec.ts b/e2e/src/api/specs/auth.e2e-spec.ts index 1b653a781f..0f407f4ba7 100644 --- a/e2e/src/api/specs/auth.e2e-spec.ts +++ b/e2e/src/api/specs/auth.e2e-spec.ts @@ -19,17 +19,6 @@ describe(`/auth/admin-sign-up`, () => { expect(body).toEqual(signupResponseDto.admin); }); - it('should sign up the admin with a local domain', async () => { - const { status, body } = await request(app) - .post('/auth/admin-sign-up') - .send({ ...signupDto.admin, email: 'admin@local' }); - expect(status).toEqual(201); - expect(body).toEqual({ - ...signupResponseDto.admin, - email: 'admin@local', - }); - }); - it('should not allow a second admin to sign up', async () => { await signUpAdmin({ signUpDto: signupDto.admin }); @@ -57,22 +46,6 @@ describe('/auth/*', () => { expect(body).toEqual(errorDto.incorrectLogin); }); - for (const key of Object.keys(loginDto.admin)) { - it(`should not allow null ${key}`, async () => { - const { status, body } = await request(app) - .post('/auth/login') - .send({ ...loginDto.admin, [key]: null }); - expect(status).toBe(400); - expect(body).toEqual(errorDto.badRequest()); - }); - - it('should reject an invalid email', async () => { - const { status, body } = await request(app).post('/auth/login').send({ email: [], password }); - expect(status).toBe(400); - expect(body).toEqual(errorDto.invalidEmail); - }); - } - it('should accept a correct password', async () => { const { status, body, headers } = await request(app).post('/auth/login').send({ email, password }); expect(status).toBe(201); @@ -127,14 +100,6 @@ describe('/auth/*', () => { }); describe('POST /auth/change-password', () => { - it('should require authentication', async () => { - const { status, body } = await request(app) - .post(`/auth/change-password`) - .send({ password, newPassword: 'Password1234' }); - expect(status).toBe(401); - expect(body).toEqual(errorDto.unauthorized); - }); - it('should require the current password', async () => { const { status, body } = await request(app) .post(`/auth/change-password`) diff --git a/e2e/src/api/specs/download.e2e-spec.ts b/e2e/src/api/specs/download.e2e-spec.ts index 3d3e6c7650..4dcb6934af 100644 --- a/e2e/src/api/specs/download.e2e-spec.ts +++ b/e2e/src/api/specs/download.e2e-spec.ts @@ -1,6 +1,5 @@ import { AssetMediaResponseDto, LoginResponseDto } from '@immich/sdk'; import { readFile, writeFile } from 'node:fs/promises'; -import { errorDto } from 'src/responses'; import { app, tempDir, utils } from 'src/utils'; import request from 'supertest'; import { beforeAll, describe, expect, it } from 'vitest'; @@ -17,15 +16,6 @@ describe('/download', () => { }); describe('POST /download/info', () => { - it('should require authentication', async () => { - const { status, body } = await request(app) - .post(`/download/info`) - .send({ assetIds: [asset1.id] }); - - expect(status).toBe(401); - expect(body).toEqual(errorDto.unauthorized); - }); - it('should download info', async () => { const { status, body } = await request(app) .post('/download/info') @@ -42,15 +32,6 @@ describe('/download', () => { }); describe('POST /download/archive', () => { - it('should require authentication', async () => { - const { status, body } = await request(app) - .post(`/download/archive`) - .send({ assetIds: [asset1.id, asset2.id] }); - - expect(status).toBe(401); - expect(body).toEqual(errorDto.unauthorized); - }); - it('should download an archive', async () => { const { status, body } = await request(app) .post('/download/archive') diff --git a/server/src/controllers/activity.controller.spec.ts b/server/src/controllers/activity.controller.spec.ts index fdf1c34d06..bf2038048f 100644 --- a/server/src/controllers/activity.controller.spec.ts +++ b/server/src/controllers/activity.controller.spec.ts @@ -6,15 +6,15 @@ import { ControllerContext, controllerSetup, mockBaseService } from 'test/utils' describe(ActivityController.name, () => { let ctx: ControllerContext; + const service = mockBaseService(ActivityService); beforeAll(async () => { - ctx = await controllerSetup(ActivityController, [ - { provide: ActivityService, useValue: mockBaseService(ActivityService) }, - ]); + ctx = await controllerSetup(ActivityController, [{ provide: ActivityService, useValue: service }]); return () => ctx.close(); }); beforeEach(() => { + service.resetAllMocks(); ctx.reset(); }); diff --git a/server/src/controllers/album.controller.spec.ts b/server/src/controllers/album.controller.spec.ts index 5aee4dfdd5..9b8a19c129 100644 --- a/server/src/controllers/album.controller.spec.ts +++ b/server/src/controllers/album.controller.spec.ts @@ -6,13 +6,15 @@ import { ControllerContext, controllerSetup, mockBaseService } from 'test/utils' describe(AlbumController.name, () => { let ctx: ControllerContext; + const service = mockBaseService(AlbumService); beforeAll(async () => { - ctx = await controllerSetup(AlbumController, [{ provide: AlbumService, useValue: mockBaseService(AlbumService) }]); + ctx = await controllerSetup(AlbumController, [{ provide: AlbumService, useValue: service }]); return () => ctx.close(); }); beforeEach(() => { + service.resetAllMocks(); ctx.reset(); }); diff --git a/server/src/controllers/api-key.controller.spec.ts b/server/src/controllers/api-key.controller.spec.ts index b481d1e63d..3246eb9b77 100644 --- a/server/src/controllers/api-key.controller.spec.ts +++ b/server/src/controllers/api-key.controller.spec.ts @@ -6,15 +6,15 @@ import { ControllerContext, controllerSetup, mockBaseService } from 'test/utils' describe(APIKeyController.name, () => { let ctx: ControllerContext; + const service = mockBaseService(ApiKeyService); beforeAll(async () => { - ctx = await controllerSetup(APIKeyController, [ - { provide: ApiKeyService, useValue: mockBaseService(ApiKeyService) }, - ]); + ctx = await controllerSetup(APIKeyController, [{ provide: ApiKeyService, useValue: service }]); return () => ctx.close(); }); beforeEach(() => { + service.resetAllMocks(); ctx.reset(); }); diff --git a/server/src/controllers/auth.controller.spec.ts b/server/src/controllers/auth.controller.spec.ts index d8ee5ab27d..0937fc5324 100644 --- a/server/src/controllers/auth.controller.spec.ts +++ b/server/src/controllers/auth.controller.spec.ts @@ -1,4 +1,5 @@ import { AuthController } from 'src/controllers/auth.controller'; +import { LoginResponseDto } from 'src/dtos/auth.dto'; import { AuthService } from 'src/services/auth.service'; import request from 'supertest'; import { errorDto } from 'test/medium/responses'; @@ -14,6 +15,7 @@ describe(AuthController.name, () => { }); beforeEach(() => { + service.resetAllMocks(); ctx.reset(); }); @@ -56,5 +58,88 @@ describe(AuthController.name, () => { expect(status).toEqual(201); expect(service.adminSignUp).toHaveBeenCalledWith(expect.objectContaining({ email: 'admin@immich.cloud' })); }); + + it('should accept an email with a local domain', async () => { + const { status } = await request(ctx.getHttpServer()) + .post('/auth/admin-sign-up') + .send({ name: 'admin', password: 'password', email: 'admin@local' }); + expect(status).toEqual(201); + }); + }); + + describe('POST /auth/login', () => { + it(`should require an email and password`, async () => { + const { status, body } = await request(ctx.getHttpServer()).post('/auth/login').send({ name: 'admin' }); + expect(status).toBe(400); + expect(body).toEqual( + errorDto.badRequest([ + 'email should not be empty', + 'email must be an email', + 'password should not be empty', + 'password must be a string', + ]), + ); + }); + + it(`should not allow null email`, async () => { + const { status, body } = await request(ctx.getHttpServer()) + .post('/auth/login') + .send({ name: 'admin', email: null, password: 'password' }); + expect(status).toBe(400); + expect(body).toEqual(errorDto.badRequest(['email should not be empty', 'email must be an email'])); + }); + + it(`should not allow null password`, async () => { + const { status, body } = await request(ctx.getHttpServer()) + .post('/auth/login') + .send({ name: 'admin', email: 'admin@immich.cloud', password: null }); + expect(status).toBe(400); + expect(body).toEqual(errorDto.badRequest(['password should not be empty', 'password must be a string'])); + }); + + it('should reject an invalid email', async () => { + service.login.mockResolvedValue({ accessToken: 'access-token' } as LoginResponseDto); + + const { status, body } = await request(ctx.getHttpServer()) + .post('/auth/login') + .send({ name: 'admin', email: [], password: 'password' }); + + expect(status).toBe(400); + expect(body).toEqual(errorDto.badRequest(['email must be an email'])); + }); + + it('should transform the email to all lowercase', async () => { + service.login.mockResolvedValue({ accessToken: 'access-token' } as LoginResponseDto); + + const { status } = await request(ctx.getHttpServer()) + .post('/auth/login') + .send({ name: 'admin', email: 'aDmIn@iMmIcH.ApP', password: 'password' }); + + expect(status).toBe(201); + expect(service.login).toHaveBeenCalledWith( + expect.objectContaining({ email: 'admin@immich.app' }), + expect.anything(), + ); + }); + + it('should accept an email with a local domain', async () => { + service.login.mockResolvedValue({ accessToken: 'access-token' } as LoginResponseDto); + + const { status } = await request(ctx.getHttpServer()) + .post('/auth/login') + .send({ name: 'admin', email: 'admin@local', password: 'password' }); + + expect(status).toEqual(201); + expect(service.login).toHaveBeenCalledWith(expect.objectContaining({ email: 'admin@local' }), expect.anything()); + }); + }); + + describe('POST /auth/change-password', () => { + it('should be an authenticated route', async () => { + await request(ctx.getHttpServer()) + .post('/auth/change-password') + .send({ password: 'password', newPassword: 'Password1234' }); + expect(ctx.authenticate).toHaveBeenCalled(); + }); }); }); diff --git a/server/src/controllers/auth.controller.ts b/server/src/controllers/auth.controller.ts index 92fa59f6bf..4ee3c26901 100644 --- a/server/src/controllers/auth.controller.ts +++ b/server/src/controllers/auth.controller.ts @@ -23,8 +23,8 @@ export class AuthController { @Post('login') async login( - @Body() loginCredential: LoginCredentialDto, @Res({ passthrough: true }) res: Response, + @Body() loginCredential: LoginCredentialDto, @GetLoginDetails() loginDetails: LoginDetails, ): Promise { const body = await this.service.login(loginCredential, loginDetails); diff --git a/server/src/controllers/download.controller.spec.ts b/server/src/controllers/download.controller.spec.ts new file mode 100644 index 0000000000..9385c445b5 --- /dev/null +++ b/server/src/controllers/download.controller.spec.ts @@ -0,0 +1,46 @@ +import { DownloadController } from 'src/controllers/download.controller'; +import { DownloadService } from 'src/services/download.service'; +import request from 'supertest'; +import { factory } from 'test/small.factory'; +import { ControllerContext, controllerSetup, mockBaseService } from 'test/utils'; +import { Readable } from 'typeorm/platform/PlatformTools.js'; + +describe(DownloadController.name, () => { + let ctx: ControllerContext; + const service = mockBaseService(DownloadService); + + beforeAll(async () => { + ctx = await controllerSetup(DownloadController, [{ provide: DownloadService, useValue: service }]); + return () => ctx.close(); + }); + + beforeEach(() => { + service.resetAllMocks(); + ctx.reset(); + }); + + describe('POST /download/info', () => { + it('should be an authenticated route', async () => { + await request(ctx.getHttpServer()) + .post('/download/info') + .send({ assetIds: [factory.uuid()] }); + expect(ctx.authenticate).toHaveBeenCalled(); + }); + }); + + describe('POST /download/archive', () => { + it('should be an authenticated route', async () => { + const stream = new Readable({ + read() { + this.push('test'); + this.push(null); + }, + }); + service.downloadArchive.mockResolvedValue({ stream }); + await request(ctx.getHttpServer()) + .post('/download/archive') + .send({ assetIds: [factory.uuid()] }); + expect(ctx.authenticate).toHaveBeenCalled(); + }); + }); +}); diff --git a/server/src/controllers/notification.controller.spec.ts b/server/src/controllers/notification.controller.spec.ts index 4eac7d3451..0dce7d73b5 100644 --- a/server/src/controllers/notification.controller.spec.ts +++ b/server/src/controllers/notification.controller.spec.ts @@ -7,15 +7,15 @@ import { ControllerContext, controllerSetup, mockBaseService } from 'test/utils' describe(NotificationController.name, () => { let ctx: ControllerContext; + const service = mockBaseService(NotificationService); beforeAll(async () => { - ctx = await controllerSetup(NotificationController, [ - { provide: NotificationService, useValue: mockBaseService(NotificationService) }, - ]); + ctx = await controllerSetup(NotificationController, [{ provide: NotificationService, useValue: service }]); return () => ctx.close(); }); beforeEach(() => { + service.resetAllMocks(); ctx.reset(); }); diff --git a/server/src/controllers/search.controller.spec.ts b/server/src/controllers/search.controller.spec.ts index 2388f93e53..03c9625e16 100644 --- a/server/src/controllers/search.controller.spec.ts +++ b/server/src/controllers/search.controller.spec.ts @@ -6,15 +6,15 @@ import { ControllerContext, controllerSetup, mockBaseService } from 'test/utils' describe(SearchController.name, () => { let ctx: ControllerContext; + const service = mockBaseService(SearchService); beforeAll(async () => { - ctx = await controllerSetup(SearchController, [ - { provide: SearchService, useValue: mockBaseService(SearchService) }, - ]); + ctx = await controllerSetup(SearchController, [{ provide: SearchService, useValue: service }]); return () => ctx.close(); }); beforeEach(() => { + service.resetAllMocks(); ctx.reset(); }); diff --git a/server/src/controllers/server.controller.spec.ts b/server/src/controllers/server.controller.spec.ts index 49edee061a..cc373162eb 100644 --- a/server/src/controllers/server.controller.spec.ts +++ b/server/src/controllers/server.controller.spec.ts @@ -6,16 +6,20 @@ import { ControllerContext, controllerSetup, mockBaseService } from 'test/utils' describe(ServerController.name, () => { let ctx: ControllerContext; + const serverService = mockBaseService(ServerService); + const versionService = mockBaseService(VersionService); beforeAll(async () => { ctx = await controllerSetup(ServerController, [ - { provide: ServerService, useValue: mockBaseService(ServerService) }, - { provide: VersionService, useValue: mockBaseService(VersionService) }, + { provide: ServerService, useValue: serverService }, + { provide: VersionService, useValue: versionService }, ]); return () => ctx.close(); }); beforeEach(() => { + serverService.resetAllMocks(); + versionService.resetAllMocks(); ctx.reset(); }); diff --git a/server/src/controllers/user.controller.spec.ts b/server/src/controllers/user.controller.spec.ts index bb3e709e68..19f9e919de 100644 --- a/server/src/controllers/user.controller.spec.ts +++ b/server/src/controllers/user.controller.spec.ts @@ -8,16 +8,18 @@ import { automock, ControllerContext, controllerSetup, mockBaseService } from 't describe(UserController.name, () => { let ctx: ControllerContext; + const service = mockBaseService(UserService); beforeAll(async () => { ctx = await controllerSetup(UserController, [ { provide: LoggingRepository, useValue: automock(LoggingRepository, { strict: false }) }, - { provide: UserService, useValue: mockBaseService(UserService) }, + { provide: UserService, useValue: service }, ]); return () => ctx.close(); }); beforeEach(() => { + service.resetAllMocks(); ctx.reset(); }); diff --git a/server/test/medium/responses.ts b/server/test/medium/responses.ts index 15673ab54b..e9f9daaf76 100644 --- a/server/test/medium/responses.ts +++ b/server/test/medium/responses.ts @@ -66,12 +66,6 @@ export const errorDto = { message: 'The server already has an admin', correlationId: expect.any(String), }, - invalidEmail: { - error: 'Bad Request', - statusCode: 400, - message: ['email must be an email'], - correlationId: expect.any(String), - }, }; export const signupResponseDto = { diff --git a/server/test/utils.ts b/server/test/utils.ts index 1f80c9bbe2..c1459bac84 100644 --- a/server/test/utils.ts +++ b/server/test/utils.ts @@ -82,14 +82,13 @@ export type ControllerContext = { export const controllerSetup = async (controller: ClassConstructor, providers: Provider[]) => { const noopInterceptor = { intercept: (ctx: never, next: CallHandler) => next.handle() }; - const authenticate = vi.fn(); const moduleRef = await Test.createTestingModule({ controllers: [controller], providers: [ { provide: APP_PIPE, useValue: new ValidationPipe({ transform: true, whitelist: true }) }, { provide: APP_GUARD, useClass: AuthGuard }, { provide: LoggingRepository, useValue: LoggingRepository.create() }, - { provide: AuthService, useValue: { authenticate } }, + { provide: AuthService, useValue: { authenticate: vi.fn() } }, ...providers, ], }) @@ -101,6 +100,9 @@ export const controllerSetup = async (controller: ClassConstructor, pro const app = moduleRef.createNestApplication(); await app.init(); + // allow the AuthController to override the AuthService itself + const authenticate = app.get>(AuthService).authenticate as Mock; + return { authenticate, getHttpServer: () => app.getHttpServer(), @@ -113,13 +115,17 @@ export const controllerSetup = async (controller: ClassConstructor, pro }; }; +export type AutoMocked = Mocked & { resetAllMocks: () => void }; + const mockFn = (label: string, { strict }: { strict: boolean }) => { const message = `Called a mock function without a mock implementation (${label})`; - return vitest.fn().mockImplementation(() => { - if (strict) { - assert.fail(message); - } else { - // console.warn(message); + return vitest.fn(() => { + { + if (strict) { + assert.fail(message); + } else { + // console.warn(message); + } } }); }; @@ -134,11 +140,13 @@ export const automock = ( args?: ConstructorParameters>; strict?: boolean; }, -): Mocked => { +): AutoMocked => { const mock: Record = {}; const strict = options?.strict ?? true; const args = options?.args ?? []; + const mocks: Mock[] = []; + const instance = new Dependency(...args); for (const property of Object.getOwnPropertyNames(Dependency.prototype)) { if (property === 'constructor') { @@ -151,7 +159,9 @@ export const automock = ( const target = instance[property as keyof T]; if (typeof target === 'function') { - mock[property] = mockFn(label, { strict }); + const mockImplementation = mockFn(label, { strict }); + mock[property] = mockImplementation; + mocks.push(mockImplementation); continue; } } catch { @@ -159,7 +169,14 @@ export const automock = ( } } - return mock as Mocked; + const result = mock as AutoMocked; + result.resetAllMocks = () => { + for (const mock of mocks) { + mock.mockReset(); + } + }; + + return result; }; export type ServiceOverrides = {