From 5417e34fb6dc90455ab9df68b497e7f3aa46b38b Mon Sep 17 00:00:00 2001 From: Sam Holton Date: Tue, 26 Nov 2024 10:51:01 -0500 Subject: [PATCH] feat(server): Add publicUsers toggle for user search (#14330) * feat(server): Add publicUsers toggle for user search * tests * docs: add check:typescript for web PR checklist * return auth.user when publicUsers is false - app testing --------- Co-authored-by: Alex --- docs/docs/developer/pr-checklist.md | 1 + e2e/src/api/specs/server.e2e-spec.ts | 1 + i18n/en.json | 2 ++ .../openapi/lib/model/server_config_dto.dart | Bin 5417 -> 5696 bytes .../lib/model/system_config_server_dto.dart | Bin 3366 -> 3645 bytes open-api/immich-openapi-specs.json | 10 ++++++- open-api/typescript-sdk/src/fetch-client.ts | 2 ++ server/src/config.ts | 2 ++ server/src/controllers/user.controller.ts | 4 +-- server/src/dtos/server.dto.ts | 1 + server/src/dtos/system-config.dto.ts | 3 ++ server/src/services/server.service.spec.ts | 1 + server/src/services/server.service.ts | 1 + .../services/system-config.service.spec.ts | 1 + server/src/services/user.service.spec.ts | 27 ++++++++++++++++-- server/src/services/user.service.ts | 10 +++++-- server/test/fixtures/system-config.stub.ts | 5 ++++ .../settings/server/server-settings.svelte | 8 ++++++ web/src/lib/stores/server-config.store.ts | 1 + 19 files changed, 73 insertions(+), 7 deletions(-) diff --git a/docs/docs/developer/pr-checklist.md b/docs/docs/developer/pr-checklist.md index 601569497..58581e669 100644 --- a/docs/docs/developer/pr-checklist.md +++ b/docs/docs/developer/pr-checklist.md @@ -11,6 +11,7 @@ When contributing code through a pull request, please check the following: - [ ] `npm run lint` (linting via ESLint) - [ ] `npm run format` (formatting via Prettier) - [ ] `npm run check:svelte` (Type checking via SvelteKit) +- [ ] `npm run check:typescript` (check typescript) - [ ] `npm test` (unit tests) ## Documentation diff --git a/e2e/src/api/specs/server.e2e-spec.ts b/e2e/src/api/specs/server.e2e-spec.ts index 4bff4b3de..c89280f57 100644 --- a/e2e/src/api/specs/server.e2e-spec.ts +++ b/e2e/src/api/specs/server.e2e-spec.ts @@ -133,6 +133,7 @@ describe('/server', () => { userDeleteDelay: 7, isInitialized: true, externalDomain: '', + publicUsers: true, isOnboarded: false, mapDarkStyleUrl: 'https://tiles.immich.cloud/v1/style/dark.json', mapLightStyleUrl: 'https://tiles.immich.cloud/v1/style/light.json', diff --git a/i18n/en.json b/i18n/en.json index 91bbb6cda..9224597fe 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -224,6 +224,8 @@ "send_welcome_email": "Send welcome email", "server_external_domain_settings": "External domain", "server_external_domain_settings_description": "Domain for public shared links, including http(s)://", + "server_public_users": "Public Users", + "server_public_users_description": "All users (name and email) are listed when adding a user to shared albums. When disabled, the user list will only be available to admin users.", "server_settings": "Server Settings", "server_settings_description": "Manage server settings", "server_welcome_message": "Welcome message", diff --git a/mobile/openapi/lib/model/server_config_dto.dart b/mobile/openapi/lib/model/server_config_dto.dart index bd5c2405e29d34d6165f766159bc7044ddfca853..01c82af4d98804d92fd2db980590b3f9e907fed6 100644 GIT binary patch delta 215 zcmZ3fbwFoBJrj39X;MyRa%gdCQSoF(CXvbYOs@r!^7C^PkR&#zGCyXNLK0B0wMCJ) zVr^$cHfi&EwnRn=Bn`GINSw`l9GpzZ`qg62avpNS)0kfkB2LiL%3hx863J>@OlNA;# Dt632K diff --git a/mobile/openapi/lib/model/system_config_server_dto.dart b/mobile/openapi/lib/model/system_config_server_dto.dart index b1b92c9515d5b9f92c75aaa7fbc14d8acf91150e..8099292dd052bc8732363b39f466d229d8fabf5c 100644 GIT binary patch delta 294 zcmZ1`wO3}tbw+6gg`(8L(#)dN6orzE%woNQ(xjZssQ0>+az1B;X11Amiu1i!Oo1wI0^({ server: { externalDomain: '', loginPageMessage: '', + publicUsers: true, }, notifications: { smtp: { diff --git a/server/src/controllers/user.controller.ts b/server/src/controllers/user.controller.ts index 10076098d..15bb1913d 100644 --- a/server/src/controllers/user.controller.ts +++ b/server/src/controllers/user.controller.ts @@ -39,8 +39,8 @@ export class UserController { @Get() @Authenticated() - searchUsers(): Promise { - return this.service.search(); + searchUsers(@Auth() auth: AuthDto): Promise { + return this.service.search(auth); } @Get('me') diff --git a/server/src/dtos/server.dto.ts b/server/src/dtos/server.dto.ts index cbabfa7ae..e1f94dbaa 100644 --- a/server/src/dtos/server.dto.ts +++ b/server/src/dtos/server.dto.ts @@ -144,6 +144,7 @@ export class ServerConfigDto { isInitialized!: boolean; isOnboarded!: boolean; externalDomain!: string; + publicUsers!: boolean; mapDarkStyleUrl!: string; mapLightStyleUrl!: string; } diff --git a/server/src/dtos/system-config.dto.ts b/server/src/dtos/system-config.dto.ts index 7a2e0632b..8d79fecb2 100644 --- a/server/src/dtos/system-config.dto.ts +++ b/server/src/dtos/system-config.dto.ts @@ -404,6 +404,9 @@ class SystemConfigServerDto { @IsString() loginPageMessage!: string; + + @IsBoolean() + publicUsers!: boolean; } class SystemConfigSmtpTransportDto { diff --git a/server/src/services/server.service.spec.ts b/server/src/services/server.service.spec.ts index 475d1d619..3f7fafceb 100644 --- a/server/src/services/server.service.spec.ts +++ b/server/src/services/server.service.spec.ts @@ -169,6 +169,7 @@ describe(ServerService.name, () => { isInitialized: undefined, isOnboarded: false, externalDomain: '', + publicUsers: true, mapDarkStyleUrl: 'https://tiles.immich.cloud/v1/style/dark.json', mapLightStyleUrl: 'https://tiles.immich.cloud/v1/style/light.json', }); diff --git a/server/src/services/server.service.ts b/server/src/services/server.service.ts index 7df322a84..e9dd908a7 100644 --- a/server/src/services/server.service.ts +++ b/server/src/services/server.service.ts @@ -110,6 +110,7 @@ export class ServerService extends BaseService { isInitialized, isOnboarded: onboarding?.isOnboarded || false, externalDomain: config.server.externalDomain, + publicUsers: config.server.publicUsers, mapDarkStyleUrl: config.map.darkStyle, mapLightStyleUrl: config.map.lightStyle, }; diff --git a/server/src/services/system-config.service.spec.ts b/server/src/services/system-config.service.spec.ts index f9ee42eb0..4d5a29e8a 100644 --- a/server/src/services/system-config.service.spec.ts +++ b/server/src/services/system-config.service.spec.ts @@ -133,6 +133,7 @@ const updatedConfig = Object.freeze({ server: { externalDomain: '', loginPageMessage: '', + publicUsers: true, }, storageTemplate: { enabled: false, diff --git a/server/src/services/user.service.spec.ts b/server/src/services/user.service.spec.ts index 767d8d895..08b663046 100644 --- a/server/src/services/user.service.spec.ts +++ b/server/src/services/user.service.spec.ts @@ -38,9 +38,9 @@ describe(UserService.name, () => { }); describe('getAll', () => { - it('should get all users', async () => { + it('admin should get all users', async () => { userMock.getList.mockResolvedValue([userStub.admin]); - await expect(sut.search()).resolves.toEqual([ + await expect(sut.search(authStub.admin)).resolves.toEqual([ expect.objectContaining({ id: authStub.admin.user.id, email: authStub.admin.user.email, @@ -48,6 +48,29 @@ describe(UserService.name, () => { ]); expect(userMock.getList).toHaveBeenCalledWith({ withDeleted: false }); }); + + it('non-admin should get all users when publicUsers enabled', async () => { + userMock.getList.mockResolvedValue([userStub.user1]); + await expect(sut.search(authStub.user1)).resolves.toEqual([ + expect.objectContaining({ + id: authStub.user1.user.id, + email: authStub.user1.user.email, + }), + ]); + expect(userMock.getList).toHaveBeenCalledWith({ withDeleted: false }); + }); + + it('non-admin user should only receive itself when publicUsers is disabled', async () => { + userMock.getList.mockResolvedValue([userStub.user1]); + systemMock.get.mockResolvedValue(systemConfigStub.publicUsersDisabled); + await expect(sut.search(authStub.user1)).resolves.toEqual([ + expect.objectContaining({ + id: authStub.user1.user.id, + email: authStub.user1.user.email, + }), + ]); + expect(userMock.getList).not.toHaveBeenCalledWith({ withDeleted: false }); + }); }); describe('get', () => { diff --git a/server/src/services/user.service.ts b/server/src/services/user.service.ts index 926482fb9..f4ae42b5e 100644 --- a/server/src/services/user.service.ts +++ b/server/src/services/user.service.ts @@ -19,8 +19,14 @@ import { getPreferences, getPreferencesPartial, mergePreferences } from 'src/uti @Injectable() export class UserService extends BaseService { - async search(): Promise { - const users = await this.userRepository.getList({ withDeleted: false }); + async search(auth: AuthDto): Promise { + const config = await this.getConfig({ withCache: false }); + + let users: UserEntity[] = [auth.user]; + if (auth.user.isAdmin || config.server.publicUsers) { + users = await this.userRepository.getList({ withDeleted: false }); + } + return users.map((user) => mapUser(user)); } diff --git a/server/test/fixtures/system-config.stub.ts b/server/test/fixtures/system-config.stub.ts index 10a0de77b..ed8cc8694 100644 --- a/server/test/fixtures/system-config.stub.ts +++ b/server/test/fixtures/system-config.stub.ts @@ -117,4 +117,9 @@ export const systemConfigStub = { }, }, }, + publicUsersDisabled: { + server: { + publicUsers: false, + }, + }, } satisfies Record>; diff --git a/web/src/lib/components/admin-page/settings/server/server-settings.svelte b/web/src/lib/components/admin-page/settings/server/server-settings.svelte index 14d5624c5..b9134d1e5 100644 --- a/web/src/lib/components/admin-page/settings/server/server-settings.svelte +++ b/web/src/lib/components/admin-page/settings/server/server-settings.svelte @@ -5,6 +5,7 @@ import type { SettingsResetEvent, SettingsSaveEvent } from '../admin-settings'; import SettingInputField from '$lib/components/shared-components/settings/setting-input-field.svelte'; import SettingButtonsRow from '$lib/components/shared-components/settings/setting-buttons-row.svelte'; + import SettingSwitch from '$lib/components/shared-components/settings/setting-switch.svelte'; import { t } from 'svelte-i18n'; import { SettingInputFieldType } from '$lib/constants'; @@ -44,6 +45,13 @@ isEdited={config.server.loginPageMessage !== savedConfig.server.loginPageMessage} /> + +
onReset({ ...options, configKeys: ['server'] })} diff --git a/web/src/lib/stores/server-config.store.ts b/web/src/lib/stores/server-config.store.ts index 358765fe0..254db7194 100644 --- a/web/src/lib/stores/server-config.store.ts +++ b/web/src/lib/stores/server-config.store.ts @@ -34,6 +34,7 @@ export const serverConfig = writable({ externalDomain: '', mapDarkStyleUrl: '', mapLightStyleUrl: '', + publicUsers: true, }); export const retrieveServerConfig = async () => {