fix(server): auth strategies (#1459)

* fix(server): auth strategies

* chore: tests

server/apps/immich/src/modules/immich-auth/strategies/api-key.strategy.ts

@@ -15,7 +15,7 @@ export class APIKeyStrategy extends PassportStrategy(Strategy, API_KEY_STRATEGY)
     super(options);
   }
 
-  validate(token: string): Promise<AuthUserDto> {
+  validate(token: string): Promise<AuthUserDto | null> {
     return this.apiKeyService.validate(token);
   }
 }

server/apps/immich/src/modules/immich-auth/strategies/public-share.strategy.ts

@@ -16,7 +16,7 @@ export class PublicShareStrategy extends PassportStrategy(Strategy, PUBLIC_SHARE
     super(options);
   }
 
-  async validate(key: string): Promise<AuthUserDto> {
+  validate(key: string): Promise<AuthUserDto | null> {
     return this.shareService.validate(key);
   }
 }

server/apps/immich/src/modules/immich-auth/strategies/user-auth.strategy.ts

@@ -1,24 +1,18 @@
-import { Injectable, UnauthorizedException } from '@nestjs/common';
+import { AuthService, AuthUserDto } from '@app/domain';
+import { Injectable } from '@nestjs/common';
 import { PassportStrategy } from '@nestjs/passport';
-import { AuthService, AuthUserDto, UserService } from '@app/domain';
-import { Strategy } from 'passport-custom';
 import { Request } from 'express';
+import { Strategy } from 'passport-custom';
 
 export const AUTH_COOKIE_STRATEGY = 'auth-cookie';
 
 @Injectable()
 export class UserAuthStrategy extends PassportStrategy(Strategy, AUTH_COOKIE_STRATEGY) {
-  constructor(private userService: UserService, private authService: AuthService) {
+  constructor(private authService: AuthService) {
     super();
   }
 
-  async validate(request: Request): Promise<AuthUserDto> {
-    const authUser = await this.authService.validate(request.headers);
-
-    if (!authUser) {
-      throw new UnauthorizedException('Incorrect token provided');
-    }
-
-    return authUser;
+  validate(request: Request): Promise<AuthUserDto | null> {
+    return this.authService.validate(request.headers);
   }
 }

server/libs/domain/src/api-key/api-key.service.spec.ts

@@ -1,5 +1,5 @@
 import { APIKeyEntity } from '@app/infra/db/entities';
-import { BadRequestException, UnauthorizedException } from '@nestjs/common';
+import { BadRequestException } from '@nestjs/common';
 import { authStub, userEntityStub, newCryptoRepositoryMock, newKeyRepositoryMock } from '../../test';
 import { ICryptoRepository } from '../auth';
 import { IKeyRepository } from './api-key.repository';
@@ -124,7 +124,7 @@ describe(APIKeyService.name, () => {
     it('should throw an error for an invalid id', async () => {
       keyMock.getKey.mockResolvedValue(null);
 
-      await expect(sut.validate(token)).rejects.toBeInstanceOf(UnauthorizedException);
+      await expect(sut.validate(token)).resolves.toBeNull();
 
       expect(keyMock.getKey).toHaveBeenCalledWith('bXktYXBpLWtleQ== (hashed)');
     });

server/libs/domain/src/api-key/api-key.service.ts

@@ -1,4 +1,4 @@
-import { BadRequestException, Inject, Injectable, UnauthorizedException } from '@nestjs/common';
+import { BadRequestException, Inject, Injectable } from '@nestjs/common';
 import { AuthUserDto, ICryptoRepository } from '../auth';
 import { IKeyRepository } from './api-key.repository';
 import { APIKeyCreateDto } from './dto/api-key-create.dto';
@@ -56,7 +56,7 @@ export class APIKeyService {
     return keys.map(mapKey);
   }
 
-  async validate(token: string): Promise<AuthUserDto> {
+  async validate(token: string): Promise<AuthUserDto | null> {
     const hashedToken = this.crypto.hashSha256(token);
     const keyEntity = await this.repository.getKey(hashedToken);
     if (keyEntity?.user) {
@@ -71,6 +71,6 @@ export class APIKeyService {
       };
     }
 
-    throw new UnauthorizedException('Invalid API Key');
+    return null;
   }
 }

server/libs/domain/src/auth/auth.service.spec.ts

@@ -229,7 +229,7 @@ describe('AuthService', () => {
   describe('validate - api request', () => {
     it('should throw if no user is found', async () => {
       userMock.get.mockResolvedValue(null);
-      await expect(sut.validate({ email: 'a', userId: 'test' })).rejects.toBeInstanceOf(UnauthorizedException);
+      await expect(sut.validate({ email: 'a', userId: 'test' })).resolves.toBeNull();
     });
 
     it('should return an auth dto', async () => {

server/libs/domain/src/auth/auth.service.ts

@@ -115,10 +115,10 @@ export class AuthService {
     }
   }
 
-  public async validate(headers: IncomingHttpHeaders): Promise<AuthUserDto> {
+  public async validate(headers: IncomingHttpHeaders): Promise<AuthUserDto | null> {
     const tokenValue = this.extractTokenFromHeader(headers);
     if (!tokenValue) {
-      throw new UnauthorizedException('No access token provided in request');
+      return null;
     }
 
     const hashedToken = this.cryptoRepository.hashSha256(tokenValue);
@@ -133,7 +133,7 @@ export class AuthService {
       };
     }
 
-    throw new UnauthorizedException('Invalid access token provided');
+    return null;
   }
 
   extractTokenFromHeader(headers: IncomingHttpHeaders) {

server/libs/domain/src/share/share.service.spec.ts

@@ -1,4 +1,4 @@
-import { BadRequestException, ForbiddenException, UnauthorizedException } from '@nestjs/common';
+import { BadRequestException, ForbiddenException } from '@nestjs/common';
 import {
   authStub,
   userEntityStub,
@@ -34,18 +34,18 @@ describe(ShareService.name, () => {
   describe('validate', () => {
     it('should not accept a non-existant key', async () => {
       shareMock.getByKey.mockResolvedValue(null);
-      await expect(sut.validate('key')).rejects.toBeInstanceOf(UnauthorizedException);
+      await expect(sut.validate('key')).resolves.toBeNull();
     });
 
     it('should not accept an expired key', async () => {
       shareMock.getByKey.mockResolvedValue(sharedLinkStub.expired);
-      await expect(sut.validate('key')).rejects.toBeInstanceOf(UnauthorizedException);
+      await expect(sut.validate('key')).resolves.toBeNull();
     });
 
     it('should not accept a key without a user', async () => {
       shareMock.getByKey.mockResolvedValue(sharedLinkStub.expired);
       userMock.get.mockResolvedValue(null);
-      await expect(sut.validate('key')).rejects.toBeInstanceOf(UnauthorizedException);
+      await expect(sut.validate('key')).resolves.toBeNull();
     });
 
     it('should accept a valid key', async () => {

server/libs/domain/src/share/share.service.ts

@@ -1,11 +1,4 @@
-import {
-  BadRequestException,
-  ForbiddenException,
-  Inject,
-  Injectable,
-  Logger,
-  UnauthorizedException,
-} from '@nestjs/common';
+import { BadRequestException, ForbiddenException, Inject, Injectable, Logger } from '@nestjs/common';
 import { AuthUserDto, ICryptoRepository } from '../auth';
 import { IUserRepository, UserCore } from '../user';
 import { EditSharedLinkDto } from './dto';
@@ -28,7 +21,7 @@ export class ShareService {
     this.userCore = new UserCore(userRepository, cryptoRepository);
   }
 
-  async validate(key: string): Promise<AuthUserDto> {
+  async validate(key: string): Promise<AuthUserDto | null> {
     const link = await this.shareCore.getByKey(key);
     if (link) {
       if (!link.expiresAt || new Date(link.expiresAt) > new Date()) {
@@ -47,7 +40,7 @@ export class ShareService {
         }
       }
     }
-    throw new UnauthorizedException();
+    return null;
   }
 
   async getAll(authUser: AuthUserDto): Promise<SharedLinkResponseDto[]> {