refactor(server): auth delete device (#4720)

* refactor(server): auth delete device

* fix: person e2e

server/src/domain/access/access.core.ts

@@ -21,6 +21,8 @@ export enum Permission {
   ALBUM_SHARE = 'album.share',
   ALBUM_DOWNLOAD = 'album.download',
 
+  AUTH_DEVICE_DELETE = 'authDevice.delete',
+
   ARCHIVE_READ = 'archive.read',
 
   TIMELINE_READ = 'timeline.read',
@@ -196,6 +198,9 @@ export class AccessCore {
       case Permission.ARCHIVE_READ:
         return authUser.id === id;
 
+      case Permission.AUTH_DEVICE_DELETE:
+        return this.repository.authDevice.hasOwnerAccess(authUser.id, id);
+
       case Permission.TIMELINE_READ:
         return authUser.id === id || (await this.repository.timeline.hasPartnerAccess(authUser.id, id));
 

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

@@ -1,9 +1,11 @@
 import { UserEntity } from '@app/infra/entities';
 import { BadRequestException, UnauthorizedException } from '@nestjs/common';
 import {
+  IAccessRepositoryMock,
   authStub,
   keyStub,
   loginResponseStub,
+  newAccessRepositoryMock,
   newCryptoRepositoryMock,
   newKeyRepositoryMock,
   newLibraryRepositoryMock,
@@ -52,6 +54,7 @@ const fixtures = {
 
 describe('AuthService', () => {
   let sut: AuthService;
+  let accessMock: jest.Mocked<IAccessRepositoryMock>;
   let cryptoMock: jest.Mocked<ICryptoRepository>;
   let userMock: jest.Mocked<IUserRepository>;
   let libraryMock: jest.Mocked<ILibraryRepository>;
@@ -84,6 +87,7 @@ describe('AuthService', () => {
       }),
     } as any);
 
+    accessMock = newAccessRepositoryMock();
     cryptoMock = newCryptoRepositoryMock();
     userMock = newUserRepositoryMock();
     libraryMock = newLibraryRepositoryMock();
@@ -92,7 +96,7 @@ describe('AuthService', () => {
     shareMock = newSharedLinkRepositoryMock();
     keyMock = newKeyRepositoryMock();
 
-    sut = new AuthService(cryptoMock, configMock, userMock, userTokenMock, libraryMock, shareMock, keyMock);
+    sut = new AuthService(accessMock, cryptoMock, configMock, libraryMock, userMock, userTokenMock, shareMock, keyMock);
   });
 
   it('should be defined', () => {
@@ -218,7 +222,7 @@ describe('AuthService', () => {
         redirectUri: '/auth/login?autoLaunch=0',
       });
 
-      expect(userTokenMock.delete).toHaveBeenCalledWith('123', 'token123');
+      expect(userTokenMock.delete).toHaveBeenCalledWith('token123');
     });
 
     it('should return the default redirect if auth type is OAUTH but oauth is not enabled', async () => {
@@ -384,16 +388,19 @@ describe('AuthService', () => {
       await sut.logoutDevices(authStub.user1);
 
       expect(userTokenMock.getAll).toHaveBeenCalledWith(authStub.user1.id);
-      expect(userTokenMock.delete).toHaveBeenCalledWith(authStub.user1.id, 'not_active');
+      expect(userTokenMock.delete).toHaveBeenCalledWith('not_active');
-      expect(userTokenMock.delete).not.toHaveBeenCalledWith(authStub.user1.id, 'token-id');
+      expect(userTokenMock.delete).not.toHaveBeenCalledWith('token-id');
     });
   });
 
   describe('logoutDevice', () => {
     it('should logout the device', async () => {
+      accessMock.authDevice.hasOwnerAccess.mockResolvedValue(true);
+
       await sut.logoutDevice(authStub.user1, 'token-1');
 
-      expect(userTokenMock.delete).toHaveBeenCalledWith(authStub.user1.id, 'token-1');
+      expect(accessMock.authDevice.hasOwnerAccess).toHaveBeenCalledWith(authStub.user1.id, 'token-1');
+      expect(userTokenMock.delete).toHaveBeenCalledWith('token-1');
     });
   });
 

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

@@ -11,7 +11,9 @@ import cookieParser from 'cookie';
 import { IncomingHttpHeaders } from 'http';
 import { DateTime } from 'luxon';
 import { ClientMetadata, Issuer, UserinfoResponse, custom, generators } from 'openid-client';
+import { AccessCore, Permission } from '../access';
 import {
+  IAccessRepository,
   ICryptoRepository,
   IKeyRepository,
   ILibraryRepository,
@@ -61,19 +63,22 @@ interface OAuthProfile extends UserinfoResponse {
 
 @Injectable()
 export class AuthService {
-  private userCore: UserCore;
+  private access: AccessCore;
   private configCore: SystemConfigCore;
   private logger = new Logger(AuthService.name);
+  private userCore: UserCore;
 
   constructor(
+    @Inject(IAccessRepository) accessRepository: IAccessRepository,
     @Inject(ICryptoRepository) private cryptoRepository: ICryptoRepository,
     @Inject(ISystemConfigRepository) configRepository: ISystemConfigRepository,
+    @Inject(ILibraryRepository) libraryRepository: ILibraryRepository,
     @Inject(IUserRepository) userRepository: IUserRepository,
     @Inject(IUserTokenRepository) private userTokenRepository: IUserTokenRepository,
-    @Inject(ILibraryRepository) libraryRepository: ILibraryRepository,
     @Inject(ISharedLinkRepository) private sharedLinkRepository: ISharedLinkRepository,
     @Inject(IKeyRepository) private keyRepository: IKeyRepository,
   ) {
+    this.access = AccessCore.create(accessRepository);
     this.configCore = SystemConfigCore.create(configRepository);
     this.userCore = UserCore.create(cryptoRepository, libraryRepository, userRepository);
 
@@ -104,7 +109,7 @@ export class AuthService {
 
   async logout(authUser: AuthUserDto, authType: AuthType): Promise<LogoutResponseDto> {
     if (authUser.accessTokenId) {
-      await this.userTokenRepository.delete(authUser.id, authUser.accessTokenId);
+      await this.userTokenRepository.delete(authUser.accessTokenId);
     }
 
     return {
@@ -175,8 +180,9 @@ export class AuthService {
     return userTokens.map((userToken) => mapUserToken(userToken, authUser.accessTokenId));
   }
 
-  async logoutDevice(authUser: AuthUserDto, deviceId: string): Promise<void> {
+  async logoutDevice(authUser: AuthUserDto, id: string): Promise<void> {
-    await this.userTokenRepository.delete(authUser.id, deviceId);
+    await this.access.requirePermission(authUser, Permission.AUTH_DEVICE_DELETE, id);
+    await this.userTokenRepository.delete(id);
   }
 
   async logoutDevices(authUser: AuthUserDto): Promise<void> {
@@ -185,7 +191,7 @@ export class AuthService {
       if (device.id === authUser.accessTokenId) {
         continue;
       }
-      await this.userTokenRepository.delete(authUser.id, device.id);
+      await this.userTokenRepository.delete(device.id);
     }
   }
 

server/src/domain/repositories/access.repository.ts

@@ -8,6 +8,10 @@ export interface IAccessRepository {
     hasSharedLinkAccess(sharedLinkId: string, assetId: string): Promise<boolean>;
   };
 
+  authDevice: {
+    hasOwnerAccess(userId: string, deviceId: string): Promise<boolean>;
+  };
+
   album: {
     hasOwnerAccess(userId: string, albumId: string): Promise<boolean>;
     hasSharedAlbumAccess(userId: string, albumId: string): Promise<boolean>;

server/src/domain/repositories/user-token.repository.ts

@@ -5,7 +5,7 @@ export const IUserTokenRepository = 'IUserTokenRepository';
 export interface IUserTokenRepository {
   create(dto: Partial<UserTokenEntity>): Promise<UserTokenEntity>;
   save(dto: Partial<UserTokenEntity>): Promise<UserTokenEntity>;
-  delete(userId: string, id: string): Promise<void>;
+  delete(id: string): Promise<void>;
   getByToken(token: string): Promise<UserTokenEntity | null>;
   getAll(userId: string): Promise<UserTokenEntity[]>;
 }

server/src/infra/repositories/access.repository.ts

@@ -1,16 +1,25 @@
 import { IAccessRepository } from '@app/domain';
 import { InjectRepository } from '@nestjs/typeorm';
 import { Repository } from 'typeorm';
-import { AlbumEntity, AssetEntity, LibraryEntity, PartnerEntity, PersonEntity, SharedLinkEntity } from '../entities';
+import {
+  AlbumEntity,
+  AssetEntity,
+  LibraryEntity,
+  PartnerEntity,
+  PersonEntity,
+  SharedLinkEntity,
+  UserTokenEntity,
+} from '../entities';
 
 export class AccessRepository implements IAccessRepository {
   constructor(
     @InjectRepository(AssetEntity) private assetRepository: Repository<AssetEntity>,
     @InjectRepository(AlbumEntity) private albumRepository: Repository<AlbumEntity>,
+    @InjectRepository(LibraryEntity) private libraryRepository: Repository<LibraryEntity>,
     @InjectRepository(PartnerEntity) private partnerRepository: Repository<PartnerEntity>,
     @InjectRepository(PersonEntity) private personRepository: Repository<PersonEntity>,
     @InjectRepository(SharedLinkEntity) private sharedLinkRepository: Repository<SharedLinkEntity>,
-    @InjectRepository(LibraryEntity) private libraryRepository: Repository<LibraryEntity>,
+    @InjectRepository(UserTokenEntity) private tokenRepository: Repository<UserTokenEntity>,
   ) {}
 
   library = {
@@ -148,6 +157,17 @@ export class AccessRepository implements IAccessRepository {
     },
   };
 
+  authDevice = {
+    hasOwnerAccess: (userId: string, deviceId: string): Promise<boolean> => {
+      return this.tokenRepository.exist({
+        where: {
+          userId,
+          id: deviceId,
+        },
+      });
+    },
+  };
+
   album = {
     hasOwnerAccess: (userId: string, albumId: string): Promise<boolean> => {
       return this.albumRepository.exist({

server/src/infra/repositories/user-token.repository.ts

@@ -35,7 +35,7 @@ export class UserTokenRepository implements IUserTokenRepository {
     return this.repository.save(userToken);
   }
 
-  async delete(userId: string, id: string): Promise<void> {
+  async delete(id: string): Promise<void> {
-    await this.repository.delete({ userId, id });
+    await this.repository.delete({ id });
   }
 }

server/test/e2e/auth.e2e-spec.ts

@@ -189,6 +189,14 @@ describe(`${AuthController.name} (e2e)`, () => {
       expect(body).toEqual(errorStub.unauthorized);
     });
 
+    it('should throw an error for a non-existent device id', async () => {
+      const { status, body } = await request(server)
+        .delete(`/auth/devices/${uuidStub.notFound}`)
+        .set('Authorization', `Bearer ${accessToken}`);
+      expect(status).toBe(400);
+      expect(body).toEqual(errorStub.badRequest('Not found or no authDevice.delete access'));
+    });
+
     it('should logout a device', async () => {
       const [device] = await api.authApi.getAuthDevices(server, accessToken);
       const { status } = await request(server)

server/test/e2e/person.e2e-spec.ts

@@ -139,10 +139,10 @@ describe(`${PersonController.name}`, () => {
 
     it('should not accept invalid birth dates', async () => {
       for (const { birthDate, response } of [
-        { birthDate: false, response: ['id must be a UUID'] },
+        { birthDate: false, response: 'Not found or no person.write access' },
         { birthDate: 'false', response: ['birthDate must be a Date instance'] },
-        { birthDate: '123567', response: ['id must be a UUID'] },
+        { birthDate: '123567', response: 'Not found or no person.write access' },
-        { birthDate: 123456, response: ['id must be a UUID'] },
+        { birthDate: 123567, response: 'Not found or no person.write access' },
       ]) {
         const { status, body } = await request(server)
           .put(`/person/${uuidStub.notFound}`)

server/test/fixtures/uuid.stub.ts

@@ -1,4 +1,5 @@
 export const uuidStub = {
   invalid: 'invalid-uuid',
-  notFound: '00000000-0000-0000-0000-000000000000',
+  // valid uuid v4
+  notFound: '00000000-0000-4000-a000-000000000000',
 };

server/test/repositories/access.repository.mock.ts

@@ -3,6 +3,7 @@ import { AccessCore, IAccessRepository } from '@app/domain';
 export interface IAccessRepositoryMock {
   asset: jest.Mocked<IAccessRepository['asset']>;
   album: jest.Mocked<IAccessRepository['album']>;
+  authDevice: jest.Mocked<IAccessRepository['authDevice']>;
   library: jest.Mocked<IAccessRepository['library']>;
   timeline: jest.Mocked<IAccessRepository['timeline']>;
   person: jest.Mocked<IAccessRepository['person']>;
@@ -27,6 +28,10 @@ export const newAccessRepositoryMock = (reset = true): IAccessRepositoryMock =>
       hasSharedLinkAccess: jest.fn(),
     },
 
+    authDevice: {
+      hasOwnerAccess: jest.fn(),
+    },
+
     library: {
       hasOwnerAccess: jest.fn(),
       hasPartnerAccess: jest.fn(),