refactor: auth, user, and public user endpoints

immich-app · Apr 26, 2024 · c760f24 · c760f24
1 parent 304ee53
commit c760f24
Show file tree

Hide file tree

Showing 54 changed files with 1,529 additions and 1,162 deletions.
diff --git a/e2e/src/api/specs/public-user.e2e-spec.ts b/e2e/src/api/specs/public-user.e2e-spec.ts
@@ -0,0 +1,127 @@
+import { LoginResponseDto, deleteUser, getUserById } from '@immich/sdk';
+import { Socket } from 'socket.io-client';
+import { createUserDto, userDto } from 'src/fixtures';
+import { errorDto } from 'src/responses';
+import { app, asBearerAuth, utils } from 'src/utils';
+import request from 'supertest';
+import { afterAll, beforeAll, describe, expect, it } from 'vitest';
+
+describe('/public-users', () => {
+  let websocket: Socket;
+
+  let admin: LoginResponseDto;
+  let deletedUser: LoginResponseDto;
+  let nonAdmin: LoginResponseDto;
+
+  beforeAll(async () => {
+    await utils.resetDatabase();
+    admin = await utils.adminSetup({ onboarding: false });
+
+    [websocket, deletedUser, nonAdmin] = await Promise.all([
+      utils.connectWebsocket(admin.accessToken),
+      utils.userSetup(admin.accessToken, createUserDto.user1),
+      utils.userSetup(admin.accessToken, createUserDto.user2),
+    ]);
+
+    await deleteUser({ id: deletedUser.userId, deleteUserDto: {} }, { headers: asBearerAuth(admin.accessToken) });
+  });
+
+  afterAll(() => {
+    utils.disconnectWebsocket(websocket);
+  });
+
+  describe('PUT /user', () => {
+    it('should require authentication', async () => {
+      const { status, body } = await request(app).put(`/public-users`);
+      expect(status).toBe(401);
+      expect(body).toEqual(errorDto.unauthorized);
+    });
+
+    for (const key of Object.keys(userDto.admin)) {
+      it(`should not allow null ${key}`, async () => {
+        const { status, body } = await request(app)
+          .put(`/public-users`)
+          .set('Authorization', `Bearer ${admin.accessToken}`)
+          .send({ ...userDto.admin, [key]: null });
+        expect(status).toBe(400);
+        expect(body).toEqual(errorDto.badRequest());
+      });
+    }
+
+    it('should not allow a non-admin to become an admin', async () => {
+      const { status, body } = await request(app)
+        .put(`/public-users`)
+        .send({ isAdmin: true, id: nonAdmin.userId })
+        .set('Authorization', `Bearer ${admin.accessToken}`);
+
+      expect(status).toBe(400);
+      expect(body).toEqual(errorDto.alreadyHasAdmin);
+    });
+
+    it('ignores updates to profileImagePath', async () => {
+      const { status, body } = await request(app)
+        .put(`/public-users`)
+        .send({ id: admin.userId, profileImagePath: 'invalid.jpg' })
+        .set('Authorization', `Bearer ${admin.accessToken}`);
+
+      expect(status).toBe(200);
+      expect(body).toMatchObject({ id: admin.userId, profileImagePath: '' });
+    });
+
+    it('should ignore updates to createdAt, updatedAt and deletedAt', async () => {
+      const before = await getUserById({ id: admin.userId }, { headers: asBearerAuth(admin.accessToken) });
+
+      const { status, body } = await request(app)
+        .put(`/public-users`)
+        .send({
+          id: admin.userId,
+          createdAt: '2023-01-01T00:00:00.000Z',
+          updatedAt: '2023-01-01T00:00:00.000Z',
+          deletedAt: '2023-01-01T00:00:00.000Z',
+        })
+        .set('Authorization', `Bearer ${admin.accessToken}`);
+
+      expect(status).toBe(200);
+      expect(body).toStrictEqual(before);
+    });
+
+    it('should update first and last name', async () => {
+      const before = await getUserById({ id: admin.userId }, { headers: asBearerAuth(admin.accessToken) });
+
+      const { status, body } = await request(app)
+        .put(`/public-users`)
+        .send({
+          id: admin.userId,
+          name: 'Name',
+        })
+        .set('Authorization', `Bearer ${admin.accessToken}`);
+
+      expect(status).toBe(200);
+      expect(body).toEqual({
+        ...before,
+        updatedAt: expect.any(String),
+        name: 'Name',
+      });
+      expect(before.updatedAt).not.toEqual(body.updatedAt);
+    });
+
+    it('should update memories enabled', async () => {
+      const before = await getUserById({ id: admin.userId }, { headers: asBearerAuth(admin.accessToken) });
+      const { status, body } = await request(app)
+        .put(`/public-users`)
+        .send({
+          id: admin.userId,
+          memoriesEnabled: false,
+        })
+        .set('Authorization', `Bearer ${admin.accessToken}`);
+
+      expect(status).toBe(200);
+      expect(body).toMatchObject({
+        ...before,
+        updatedAt: expect.anything(),
+        memoriesEnabled: false,
+      });
+      expect(before.updatedAt).not.toEqual(body.updatedAt);
+    });
+  });
+});
diff --git a/e2e/src/api/specs/user.e2e-spec.ts → e2e/src/api/specs/users.e2e-spec.ts b/e2e/src/api/specs/user.e2e-spec.ts → e2e/src/api/specs/users.e2e-spec.ts
@@ -1,6 +1,6 @@
-import { LoginResponseDto, deleteUser, getUserById } from '@immich/sdk';
+import { LoginResponseDto, deleteUser } from '@immich/sdk';
 import { Socket } from 'socket.io-client';
-import { createUserDto, userDto } from 'src/fixtures';
+import { createUserDto } from 'src/fixtures';
 import { errorDto } from 'src/responses';
 import { app, asBearerAuth, utils } from 'src/utils';
 import request from 'supertest';
@@ -13,18 +13,16 @@ describe('/user', () => {
   let deletedUser: LoginResponseDto;
   let userToDelete: LoginResponseDto;
   let userToHardDelete: LoginResponseDto;
-  let nonAdmin: LoginResponseDto;
 
   beforeAll(async () => {
     await utils.resetDatabase();
     admin = await utils.adminSetup({ onboarding: false });
 
-    [websocket, deletedUser, nonAdmin, userToDelete, userToHardDelete] = await Promise.all([
+    [websocket, deletedUser, userToDelete, userToHardDelete] = await Promise.all([
       utils.connectWebsocket(admin.accessToken),
       utils.userSetup(admin.accessToken, createUserDto.user1),
       utils.userSetup(admin.accessToken, createUserDto.user2),
       utils.userSetup(admin.accessToken, createUserDto.user3),
-      utils.userSetup(admin.accessToken, createUserDto.user4),
     ]);
 
     await deleteUser({ id: deletedUser.userId, deleteUserDto: {} }, { headers: asBearerAuth(admin.accessToken) });
@@ -34,74 +32,71 @@ describe('/user', () => {
     utils.disconnectWebsocket(websocket);
   });
 
-  describe('GET /user', () => {
+  describe('GET /users', () => {
     it('should require authentication', async () => {
-      const { status, body } = await request(app).get('/user');
+      const { status, body } = await request(app).get('/users');
       expect(status).toBe(401);
       expect(body).toEqual(errorDto.unauthorized);
     });
 
     it('should get users', async () => {
-      const { status, body } = await request(app).get('/user').set('Authorization', `Bearer ${admin.accessToken}`);
+      const { status, body } = await request(app).get('/users').set('Authorization', `Bearer ${admin.accessToken}`);
       expect(status).toEqual(200);
-      expect(body).toHaveLength(5);
+      expect(body).toHaveLength(4);
       expect(body).toEqual(
         expect.arrayContaining([
           expect.objectContaining({ email: 'admin@immich.cloud' }),
           expect.objectContaining({ email: 'user1@immich.cloud' }),
           expect.objectContaining({ email: 'user2@immich.cloud' }),
           expect.objectContaining({ email: 'user3@immich.cloud' }),
-          expect.objectContaining({ email: 'user4@immich.cloud' }),
         ]),
       );
     });
 
     it('should hide deleted users', async () => {
       const { status, body } = await request(app)
-        .get(`/user`)
+        .get(`/users`)
         .query({ isAll: true })
         .set('Authorization', `Bearer ${admin.accessToken}`);
       expect(status).toBe(200);
-      expect(body).toHaveLength(4);
+      expect(body).toHaveLength(3);
       expect(body).toEqual(
         expect.arrayContaining([
           expect.objectContaining({ email: 'admin@immich.cloud' }),
           expect.objectContaining({ email: 'user2@immich.cloud' }),
           expect.objectContaining({ email: 'user3@immich.cloud' }),
-          expect.objectContaining({ email: 'user4@immich.cloud' }),
         ]),
       );
     });
 
     it('should include deleted users', async () => {
       const { status, body } = await request(app)
-        .get(`/user`)
+        .get(`/users`)
         .query({ isAll: false })
         .set('Authorization', `Bearer ${admin.accessToken}`);
 
       expect(status).toBe(200);
-      expect(body).toHaveLength(5);
+      expect(body).toHaveLength(4);
       expect(body).toEqual(
         expect.arrayContaining([
           expect.objectContaining({ email: 'admin@immich.cloud' }),
           expect.objectContaining({ email: 'user1@immich.cloud' }),
           expect.objectContaining({ email: 'user2@immich.cloud' }),
           expect.objectContaining({ email: 'user3@immich.cloud' }),
-          expect.objectContaining({ email: 'user4@immich.cloud' }),
         ]),
       );
     });
   });
 
-  describe('GET /user/info/:id', () => {
+  describe('GET /users/:id', () => {
     it('should require authentication', async () => {
-      const { status } = await request(app).get(`/user/info/${admin.userId}`);
+      const { status } = await request(app).get(`/users/${admin.userId}`);
       expect(status).toEqual(401);
     });
 
     it('should get the user info', async () => {
       const { status, body } = await request(app)
-        .get(`/user/info/${admin.userId}`)
+        .get(`/users/${admin.userId}`)
         .set('Authorization', `Bearer ${admin.accessToken}`);
       expect(status).toBe(200);
       expect(body).toMatchObject({
@@ -182,16 +177,16 @@ describe('/user', () => {
     });
   });
 
-  describe('DELETE /user/:id', () => {
+  describe('DELETE /users/:id', () => {
     it('should require authentication', async () => {
-      const { status, body } = await request(app).delete(`/user/${userToDelete.userId}`);
+      const { status, body } = await request(app).delete(`/users/${userToDelete.userId}`);
       expect(status).toBe(401);
       expect(body).toEqual(errorDto.unauthorized);
     });
 
     it('should delete user', async () => {
       const { status, body } = await request(app)
-        .delete(`/user/${userToDelete.userId}`)
+        .delete(`/users/${userToDelete.userId}`)
         .set('Authorization', `Bearer ${admin.accessToken}`);
 
       expect(status).toBe(200);
@@ -204,7 +199,7 @@ describe('/user', () => {
 
     it('should hard delete user', async () => {
       const { status, body } = await request(app)
-        .delete(`/user/${userToHardDelete.userId}`)
+        .delete(`/users/${userToHardDelete.userId}`)
         .send({ force: true })
         .set('Authorization', `Bearer ${admin.accessToken}`);
 
@@ -218,99 +213,4 @@ describe('/user', () => {
       await utils.waitForWebsocketEvent({ event: 'userDelete', id: userToHardDelete.userId, timeout: 5000 });
     });
   });
-
-  describe('PUT /user', () => {
-    it('should require authentication', async () => {
-      const { status, body } = await request(app).put(`/public-users`);
-      expect(status).toBe(401);
-      expect(body).toEqual(errorDto.unauthorized);
-    });
-
-    for (const key of Object.keys(userDto.admin)) {
-      it(`should not allow null ${key}`, async () => {
-        const { status, body } = await request(app)
-          .put(`/public-users`)
-          .set('Authorization', `Bearer ${admin.accessToken}`)
-          .send({ ...userDto.admin, [key]: null });
-        expect(status).toBe(400);
-        expect(body).toEqual(errorDto.badRequest());
-      });
-    }
-
-    it('should not allow a non-admin to become an admin', async () => {
-      const { status, body } = await request(app)
-        .put(`/public-users`)
-        .send({ isAdmin: true, id: nonAdmin.userId })
-        .set('Authorization', `Bearer ${admin.accessToken}`);
-
-      expect(status).toBe(400);
-      expect(body).toEqual(errorDto.alreadyHasAdmin);
-    });
-
-    it('ignores updates to profileImagePath', async () => {
-      const { status, body } = await request(app)
-        .put(`/public-users`)
-        .send({ id: admin.userId, profileImagePath: 'invalid.jpg' })
-        .set('Authorization', `Bearer ${admin.accessToken}`);
-
-      expect(status).toBe(200);
-      expect(body).toMatchObject({ id: admin.userId, profileImagePath: '' });
-    });
-
-    it('should ignore updates to createdAt, updatedAt and deletedAt', async () => {
-      const before = await getUserById({ id: admin.userId }, { headers: asBearerAuth(admin.accessToken) });
-
-      const { status, body } = await request(app)
-        .put(`/public-users`)
-        .send({
-          id: admin.userId,
-          createdAt: '2023-01-01T00:00:00.000Z',
-          updatedAt: '2023-01-01T00:00:00.000Z',
-          deletedAt: '2023-01-01T00:00:00.000Z',
-        })
-        .set('Authorization', `Bearer ${admin.accessToken}`);
-
-      expect(status).toBe(200);
-      expect(body).toStrictEqual(before);
-    });
-
-    it('should update first and last name', async () => {
-      const before = await getUserById({ id: admin.userId }, { headers: asBearerAuth(admin.accessToken) });
-
-      const { status, body } = await request(app)
-        .put(`/public-users`)
-        .send({
-          id: admin.userId,
-          name: 'Name',
-        })
-        .set('Authorization', `Bearer ${admin.accessToken}`);
-
-      expect(status).toBe(200);
-      expect(body).toEqual({
-        ...before,
-        updatedAt: expect.any(String),
-        name: 'Name',
-      });
-      expect(before.updatedAt).not.toEqual(body.updatedAt);
-    });
-
-    it('should update memories enabled', async () => {
-      const before = await getUserById({ id: admin.userId }, { headers: asBearerAuth(admin.accessToken) });
-      const { status, body } = await request(app)
-        .put(`/public-users`)
-        .send({
-          id: admin.userId,
-          memoriesEnabled: false,
-        })
-        .set('Authorization', `Bearer ${admin.accessToken}`);
-
-      expect(status).toBe(200);
-      expect(body).toMatchObject({
-        ...before,
-        updatedAt: expect.anything(),
-        memoriesEnabled: false,
-      });
-      expect(before.updatedAt).not.toEqual(body.updatedAt);
-    });
-  });
 });
diff --git a/mobile/lib/modules/login/providers/authentication.provider.dart b/mobile/lib/modules/login/providers/authentication.provider.dart
@@ -138,6 +138,7 @@ class AuthenticationNotifier extends StateNotifier<AuthenticationState> {
 
   Future<bool> changePassword(String newPassword) async {
     try {
+      // TODO: use authApi to change password
       await _apiService.userApi.updateUser(
         UpdateUserDto(
           id: state.userId,

diff --git a/mobile/lib/shared/ui/user_avatar.dart b/mobile/lib/shared/ui/user_avatar.dart
@@ -6,7 +6,7 @@ import 'package:immich_mobile/shared/models/user.dart';
 
 Widget userAvatar(BuildContext context, User u, {double? radius}) {
   final url =
-      "${Store.get(StoreKey.serverEndpoint)}/user/profile-image/${u.id}";
+      "${Store.get(StoreKey.serverEndpoint)}/public-users/${u.id}/profile-image";
   final nameFirstLetter = u.name.isNotEmpty ? u.name[0] : "";
   return CircleAvatar(
     radius: radius,

diff --git a/mobile/lib/shared/ui/user_circle_avatar.dart b/mobile/lib/shared/ui/user_circle_avatar.dart
@@ -24,7 +24,7 @@ class UserCircleAvatar extends ConsumerWidget {
   Widget build(BuildContext context, WidgetRef ref) {
     bool isDarkTheme = Theme.of(context).brightness == Brightness.dark;
     final profileImageUrl =
-        '${Store.get(StoreKey.serverEndpoint)}/user/profile-image/${user.id}?d=${Random().nextInt(1024)}';
+        '${Store.get(StoreKey.serverEndpoint)}/public-users/${user.id}/profile-image?d=${Random().nextInt(1024)}';
 
     final textIcon = Text(
       user.name[0].toUpperCase(),