Skip to content

Commit

Permalink
feat(server): Add publicUsers toggle for user search (#14330)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
samholton and alextran1502 authored Nov 26, 2024
1 parent b6ec79c commit 5417e34
Show file tree
Hide file tree
Showing 19 changed files with 93 additions and 11 deletions.
1 change: 1 addition & 0 deletions docs/docs/developer/pr-checklist.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions e2e/src/api/specs/server.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
2 changes: 2 additions & 0 deletions i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
10 changes: 9 additions & 1 deletion mobile/openapi/lib/model/server_config_dto.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 11 additions & 3 deletions mobile/openapi/lib/model/system_config_server_dto.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 9 additions & 1 deletion open-api/immich-openapi-specs.json
Original file line number Diff line number Diff line change
Expand Up @@ -10825,6 +10825,9 @@
"oauthButtonText": {
"type": "string"
},
"publicUsers": {
"type": "boolean"
},
"trashDays": {
"type": "integer"
},
Expand All @@ -10840,6 +10843,7 @@
"mapDarkStyleUrl",
"mapLightStyleUrl",
"oauthButtonText",
"publicUsers",
"trashDays",
"userDeleteDelay"
],
Expand Down Expand Up @@ -12014,11 +12018,15 @@
},
"loginPageMessage": {
"type": "string"
},
"publicUsers": {
"type": "boolean"
}
},
"required": [
"externalDomain",
"loginPageMessage"
"loginPageMessage",
"publicUsers"
],
"type": "object"
},
Expand Down
2 changes: 2 additions & 0 deletions open-api/typescript-sdk/src/fetch-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -928,6 +928,7 @@ export type ServerConfigDto = {
mapDarkStyleUrl: string;
mapLightStyleUrl: string;
oauthButtonText: string;
publicUsers: boolean;
trashDays: number;
userDeleteDelay: number;
};
Expand Down Expand Up @@ -1222,6 +1223,7 @@ export type SystemConfigReverseGeocodingDto = {
export type SystemConfigServerDto = {
externalDomain: string;
loginPageMessage: string;
publicUsers: boolean;
};
export type SystemConfigStorageTemplateDto = {
enabled: boolean;
Expand Down
2 changes: 2 additions & 0 deletions server/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ export interface SystemConfig {
server: {
externalDomain: string;
loginPageMessage: string;
publicUsers: boolean;
};
user: {
deleteDelay: number;
Expand Down Expand Up @@ -296,6 +297,7 @@ export const defaults = Object.freeze<SystemConfig>({
server: {
externalDomain: '',
loginPageMessage: '',
publicUsers: true,
},
notifications: {
smtp: {
Expand Down
4 changes: 2 additions & 2 deletions server/src/controllers/user.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ export class UserController {

@Get()
@Authenticated()
searchUsers(): Promise<UserResponseDto[]> {
return this.service.search();
searchUsers(@Auth() auth: AuthDto): Promise<UserResponseDto[]> {
return this.service.search(auth);
}

@Get('me')
Expand Down
1 change: 1 addition & 0 deletions server/src/dtos/server.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ export class ServerConfigDto {
isInitialized!: boolean;
isOnboarded!: boolean;
externalDomain!: string;
publicUsers!: boolean;
mapDarkStyleUrl!: string;
mapLightStyleUrl!: string;
}
Expand Down
3 changes: 3 additions & 0 deletions server/src/dtos/system-config.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,9 @@ class SystemConfigServerDto {

@IsString()
loginPageMessage!: string;

@IsBoolean()
publicUsers!: boolean;
}

class SystemConfigSmtpTransportDto {
Expand Down
1 change: 1 addition & 0 deletions server/src/services/server.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
});
Expand Down
1 change: 1 addition & 0 deletions server/src/services/server.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down
1 change: 1 addition & 0 deletions server/src/services/system-config.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ const updatedConfig = Object.freeze<SystemConfig>({
server: {
externalDomain: '',
loginPageMessage: '',
publicUsers: true,
},
storageTemplate: {
enabled: false,
Expand Down
27 changes: 25 additions & 2 deletions server/src/services/user.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,39 @@ 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,
}),
]);
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', () => {
Expand Down
10 changes: 8 additions & 2 deletions server/src/services/user.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,14 @@ import { getPreferences, getPreferencesPartial, mergePreferences } from 'src/uti

@Injectable()
export class UserService extends BaseService {
async search(): Promise<UserResponseDto[]> {
const users = await this.userRepository.getList({ withDeleted: false });
async search(auth: AuthDto): Promise<UserResponseDto[]> {
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));
}

Expand Down
5 changes: 5 additions & 0 deletions server/test/fixtures/system-config.stub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,4 +117,9 @@ export const systemConfigStub = {
},
},
},
publicUsersDisabled: {
server: {
publicUsers: false,
},
},
} satisfies Record<string, DeepPartial<SystemConfig>>;
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -44,6 +45,13 @@
isEdited={config.server.loginPageMessage !== savedConfig.server.loginPageMessage}
/>

<SettingSwitch
title={$t('admin.server_public_users')}
subtitle={$t('admin.server_public_users_description')}
{disabled}
bind:checked={config.server.publicUsers}
/>

<div class="ml-4">
<SettingButtonsRow
onReset={(options) => onReset({ ...options, configKeys: ['server'] })}
Expand Down
1 change: 1 addition & 0 deletions web/src/lib/stores/server-config.store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export const serverConfig = writable<ServerConfig>({
externalDomain: '',
mapDarkStyleUrl: '',
mapLightStyleUrl: '',
publicUsers: true,
});

export const retrieveServerConfig = async () => {
Expand Down

0 comments on commit 5417e34

Please sign in to comment.