From e5219f1f311a04850007993ef36d24b26491e1cf Mon Sep 17 00:00:00 2001 From: nosajthenitram Date: Wed, 11 Jun 2025 21:11:13 -0500 Subject: [PATCH] feat(web): Added admin user config to user settings (#15380) * feat(web): Added admin user config to user settings * feat (web) - cleaned up the files and added tests * feat (web) - added missing files * feat (web) - updated per review comments * feat (web) - e2e admin command test failures --- e2e/src/api/specs/user-admin.e2e-spec.ts | 11 ++- .../specs/immich-admin.e2e-spec.ts | 38 ++++++++ e2e/src/web/specs/user-admin.e2e-spec.ts | 89 ++++++++++++++++++ i18n/en.json | 1 + .../lib/model/user_admin_create_dto.dart | Bin 6032 -> 6695 bytes .../lib/model/user_admin_update_dto.dart | Bin 6989 -> 7652 bytes open-api/immich-openapi-specs.json | 6 ++ open-api/typescript-sdk/src/fetch-client.ts | 2 + server/src/commands/grant-admin.ts | 67 +++++++++++++ server/src/commands/index.ts | 4 + server/src/dtos/user.dto.ts | 8 ++ server/src/services/cli.service.ts | 18 ++++ .../src/services/user-admin.service.spec.ts | 10 +- server/src/services/user-admin.service.ts | 10 +- web/src/lib/modals/UserEditModal.svelte | 26 +++-- 15 files changed, 272 insertions(+), 18 deletions(-) create mode 100644 e2e/src/web/specs/user-admin.e2e-spec.ts create mode 100644 server/src/commands/grant-admin.ts diff --git a/e2e/src/api/specs/user-admin.e2e-spec.ts b/e2e/src/api/specs/user-admin.e2e-spec.ts index 1fbee84c3..b0696dcad 100644 --- a/e2e/src/api/specs/user-admin.e2e-spec.ts +++ b/e2e/src/api/specs/user-admin.e2e-spec.ts @@ -118,7 +118,7 @@ describe('/admin/users', () => { }); } - it('should ignore `isAdmin`', async () => { + it('should accept `isAdmin`', async () => { const { status, body } = await request(app) .post(`/admin/users`) .send({ @@ -130,7 +130,7 @@ describe('/admin/users', () => { .set('Authorization', `Bearer ${admin.accessToken}`); expect(body).toMatchObject({ email: 'user5@immich.cloud', - isAdmin: false, + isAdmin: true, shouldChangePassword: true, }); expect(status).toBe(201); @@ -163,14 +163,15 @@ describe('/admin/users', () => { }); } - it('should not allow a non-admin to become an admin', async () => { + it('should allow a non-admin to become an admin', async () => { + const user = await utils.userSetup(admin.accessToken, createUserDto.create('admin2')); const { status, body } = await request(app) - .put(`/admin/users/${nonAdmin.userId}`) + .put(`/admin/users/${user.userId}`) .send({ isAdmin: true }) .set('Authorization', `Bearer ${admin.accessToken}`); expect(status).toBe(200); - expect(body).toMatchObject({ isAdmin: false }); + expect(body).toMatchObject({ isAdmin: true }); }); it('ignores updates to profileImagePath', async () => { diff --git a/e2e/src/immich-admin/specs/immich-admin.e2e-spec.ts b/e2e/src/immich-admin/specs/immich-admin.e2e-spec.ts index cf0558883..24699cda3 100644 --- a/e2e/src/immich-admin/specs/immich-admin.e2e-spec.ts +++ b/e2e/src/immich-admin/specs/immich-admin.e2e-spec.ts @@ -7,6 +7,44 @@ describe(`immich-admin`, () => { await utils.adminSetup(); }); + describe('revoke-admin', () => { + it('should revoke admin privileges from a user', async () => { + const { child, promise } = immichAdmin(['revoke-admin']); + + let data = ''; + child.stdout.on('data', (chunk) => { + data += chunk; + if (data.includes('Please enter the user email:')) { + child.stdin.end('admin@immich.cloud\n'); + } + }); + + const { stdout, exitCode } = await promise; + expect(exitCode).toBe(0); + + expect(stdout).toContain('Admin access has been revoked from'); + }); + }); + + describe('grant-admin', () => { + it('should grant admin privileges to a user', async () => { + const { child, promise } = immichAdmin(['grant-admin']); + + let data = ''; + child.stdout.on('data', (chunk) => { + data += chunk; + if (data.includes('Please enter the user email:')) { + child.stdin.end('admin@immich.cloud\n'); + } + }); + + const { stdout, exitCode } = await promise; + expect(exitCode).toBe(0); + + expect(stdout).toContain('Admin access has been granted to'); + }); + }); + describe('list-users', () => { it('should list the admin user', async () => { const { stdout, exitCode } = await immichAdmin(['list-users']).promise; diff --git a/e2e/src/web/specs/user-admin.e2e-spec.ts b/e2e/src/web/specs/user-admin.e2e-spec.ts new file mode 100644 index 000000000..3d64e47ae --- /dev/null +++ b/e2e/src/web/specs/user-admin.e2e-spec.ts @@ -0,0 +1,89 @@ +import { getUserAdmin } from '@immich/sdk'; +import { expect, test } from '@playwright/test'; +import { asBearerAuth, utils } from 'src/utils'; + +test.describe('User Administration', () => { + test.beforeAll(() => { + utils.initSdk(); + }); + + test.beforeEach(async () => { + await utils.resetDatabase(); + }); + + test('validate admin/users link', async ({ context, page }) => { + const admin = await utils.adminSetup(); + await utils.setAuthCookies(context, admin.accessToken); + + // Navigate to user management page and verify title and header + await page.goto(`/admin/users`); + await expect(page).toHaveTitle(/User Management/); + await expect(page.getByText('User Management')).toBeVisible(); + }); + + test('create user', async ({ context, page }) => { + const admin = await utils.adminSetup(); + await utils.setAuthCookies(context, admin.accessToken); + + // Create a new user + await page.goto('/admin/users'); + await page.getByRole('button', { name: 'Create user' }).click(); + await page.getByLabel('Email').fill('user@immich.cloud'); + await page.getByLabel('Password', { exact: true }).fill('password'); + await page.getByLabel('Confirm Password').fill('password'); + await page.getByLabel('Name').fill('Immich User'); + await page.getByRole('button', { name: 'Create', exact: true }).click(); + + // Verify the user exists in the user list + await page.getByRole('row', { name: 'user@immich.cloud' }); + }); + + test('promote to admin', async ({ context, page }) => { + const admin = await utils.adminSetup(); + await utils.setAuthCookies(context, admin.accessToken); + + const user = await utils.userSetup(admin.accessToken, { + name: 'Admin 2', + email: 'admin2@immich.cloud', + password: 'password', + }); + + expect(user.isAdmin).toBe(false); + + await page.goto(`/admin/users/${user.userId}`); + + await page.getByRole('button', { name: 'Edit user' }).click(); + await expect(page.getByLabel('Admin User')).not.toBeChecked(); + await page.getByText('Admin User').click(); + await expect(page.getByLabel('Admin User')).toBeChecked(); + await page.getByRole('button', { name: 'Confirm' }).click(); + + const updated = await getUserAdmin({ id: user.userId }, { headers: asBearerAuth(admin.accessToken) }); + expect(updated.isAdmin).toBe(true); + }); + + test('revoke admin access', async ({ context, page }) => { + const admin = await utils.adminSetup(); + await utils.setAuthCookies(context, admin.accessToken); + + const user = await utils.userSetup(admin.accessToken, { + name: 'Admin 2', + email: 'admin2@immich.cloud', + password: 'password', + isAdmin: true, + }); + + expect(user.isAdmin).toBe(true); + + await page.goto(`/admin/users/${user.userId}`); + + await page.getByRole('button', { name: 'Edit user' }).click(); + await expect(page.getByLabel('Admin User')).toBeChecked(); + await page.getByText('Admin User').click(); + await expect(page.getByLabel('Admin User')).not.toBeChecked(); + await page.getByRole('button', { name: 'Confirm' }).click(); + + const updated = await getUserAdmin({ id: user.userId }, { headers: asBearerAuth(admin.accessToken) }); + expect(updated.isAdmin).toBe(false); + }); +}); diff --git a/i18n/en.json b/i18n/en.json index 86de3808b..2735393d3 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -34,6 +34,7 @@ "added_to_favorites_count": "Added {count, number} to favorites", "admin": { "add_exclusion_pattern_description": "Add exclusion patterns. Globbing using *, **, and ? is supported. To ignore all files in any directory named \"Raw\", use \"**/Raw/**\". To ignore all files ending in \".tif\", use \"**/*.tif\". To ignore an absolute path, use \"/path/to/ignore/**\".", + "admin_user": "Admin User", "asset_offline_description": "This external library asset is no longer found on disk and has been moved to trash. If the file was moved within the library, check your timeline for the new corresponding asset. To restore this asset, please ensure that the file path below can be accessed by Immich and scan the library.", "authentication_settings": "Authentication Settings", "authentication_settings_description": "Manage password, OAuth, and other authentication settings", diff --git a/mobile/openapi/lib/model/user_admin_create_dto.dart b/mobile/openapi/lib/model/user_admin_create_dto.dart index 1477c82ca1ec3cdc5666fcb007ce67290aa670df..8c8b70fbce0fff571ef422c12f1791b6af2590df 100644 GIT binary patch delta 218 zcmbQBzuaWQPDY-RjLc%a%wors+|0bmn;9=}p2n2S$PN|Q+{h%vA_V0q*xD+<#9P@W zFv=l`=auHalIHc&K8K?B9&0A5K6s0#I11zQCa i$svlzSZuTtHqxo)e~1bMsC9?d$*+o=Qdl delta 41 zcmV+^0M`GfG>|W_xdD^Q0nM`q1PuhUGzfwLv*rnY0kd`snE|tk4YvfddJ?S%O}GzP diff --git a/mobile/openapi/lib/model/user_admin_update_dto.dart b/mobile/openapi/lib/model/user_admin_update_dto.dart index ee5c006840f488a6975426a827ffc02d97604622..9605552d201e8f22335d0187be5adb8c1d3a4e8b 100644 GIT binary patch delta 234 zcmX?W_QZO_Rz~*BV#k!+%)HI37;Bk%lk)R(>=j@_uh^0pH>u?Rs$6l`tblG>ch z8D){g^Gb7a6zmlY6s+J%_H*rL6o4wRRe_mW!}ASp>*RV~NeQTgdaQ!2LP { + return function ask(): Promise { + return inquirer.ask<{ email: string }>('prompt-email', {}).then(({ email }: { email: string }) => email); + }; +}; + +@Command({ + name: 'grant-admin', + description: 'Grant admin privileges to a user (by email)', +}) +export class GrantAdminCommand extends CommandRunner { + constructor( + private service: CliService, + private inquirer: InquirerService, + ) { + super(); + } + + async run(): Promise { + try { + const email = await prompt(this.inquirer)(); + await this.service.grantAdminAccess(email); + console.debug('Admin access has been granted to', email); + } catch (error) { + console.error(error); + console.error('Unable to grant admin access to user'); + } + } +} + +@Command({ + name: 'revoke-admin', + description: 'Revoke admin privileges from a user (by email)', +}) +export class RevokeAdminCommand extends CommandRunner { + constructor( + private service: CliService, + private inquirer: InquirerService, + ) { + super(); + } + + async run(): Promise { + try { + const email = await prompt(this.inquirer)(); + await this.service.revokeAdminAccess(email); + console.debug('Admin access has been revoked from', email); + } catch (error) { + console.error(error); + console.error('Unable to revoke admin access from user'); + } + } +} + +@QuestionSet({ name: 'prompt-email' }) +export class PromptEmailQuestion { + @Question({ + message: 'Please enter the user email: ', + name: 'email', + }) + parseEmail(value: string) { + return value; + } +} diff --git a/server/src/commands/index.ts b/server/src/commands/index.ts index 59846628b..ce085f6e3 100644 --- a/server/src/commands/index.ts +++ b/server/src/commands/index.ts @@ -1,3 +1,4 @@ +import { GrantAdminCommand, PromptEmailQuestion, RevokeAdminCommand } from 'src/commands/grant-admin'; import { ListUsersCommand } from 'src/commands/list-users.command'; import { DisableOAuthLogin, EnableOAuthLogin } from 'src/commands/oauth-login'; import { DisablePasswordLoginCommand, EnablePasswordLoginCommand } from 'src/commands/password-login'; @@ -7,10 +8,13 @@ import { VersionCommand } from 'src/commands/version.command'; export const commands = [ ResetAdminPasswordCommand, PromptPasswordQuestions, + PromptEmailQuestion, EnablePasswordLoginCommand, DisablePasswordLoginCommand, EnableOAuthLogin, DisableOAuthLogin, ListUsersCommand, VersionCommand, + GrantAdminCommand, + RevokeAdminCommand, ]; diff --git a/server/src/dtos/user.dto.ts b/server/src/dtos/user.dto.ts index 9d43e53f8..ed08f7534 100644 --- a/server/src/dtos/user.dto.ts +++ b/server/src/dtos/user.dto.ts @@ -106,6 +106,10 @@ export class UserAdminCreateDto { @Optional() @IsBoolean() notify?: boolean; + + @Optional() + @IsBoolean() + isAdmin?: boolean; } export class UserAdminUpdateDto { @@ -145,6 +149,10 @@ export class UserAdminUpdateDto { @Min(0) @ApiProperty({ type: 'integer', format: 'int64' }) quotaSizeInBytes?: number | null; + + @Optional() + @IsBoolean() + isAdmin?: boolean; } export class UserAdminDeleteDto { diff --git a/server/src/services/cli.service.ts b/server/src/services/cli.service.ts index f6173c69f..021a5240f 100644 --- a/server/src/services/cli.service.ts +++ b/server/src/services/cli.service.ts @@ -37,6 +37,24 @@ export class CliService extends BaseService { await this.updateConfig(config); } + async grantAdminAccess(email: string): Promise { + const user = await this.userRepository.getByEmail(email); + if (!user) { + throw new Error('User does not exist'); + } + + await this.userRepository.update(user.id, { isAdmin: true }); + } + + async revokeAdminAccess(email: string): Promise { + const user = await this.userRepository.getByEmail(email); + if (!user) { + throw new Error('User does not exist'); + } + + await this.userRepository.update(user.id, { isAdmin: false }); + } + async disableOAuthLogin(): Promise { const config = await this.getConfig({ withCache: false }); config.oauth.enabled = false; diff --git a/server/src/services/user-admin.service.spec.ts b/server/src/services/user-admin.service.spec.ts index 3e613bc48..85cbb8238 100644 --- a/server/src/services/user-admin.service.spec.ts +++ b/server/src/services/user-admin.service.spec.ts @@ -4,6 +4,7 @@ import { JobName, UserStatus } from 'src/enum'; import { UserAdminService } from 'src/services/user-admin.service'; import { authStub } from 'test/fixtures/auth.stub'; import { userStub } from 'test/fixtures/user.stub'; +import { factory } from 'test/small.factory'; import { newTestService, ServiceMocks } from 'test/utils'; import { describe } from 'vitest'; @@ -116,7 +117,7 @@ describe(UserAdminService.name, () => { it('should throw error if user could not be found', async () => { mocks.user.get.mockResolvedValue(void 0); - await expect(sut.delete(authStub.admin, userStub.admin.id, {})).rejects.toThrowError(BadRequestException); + await expect(sut.delete(authStub.admin, 'not-found', {})).rejects.toThrowError(BadRequestException); expect(mocks.user.delete).not.toHaveBeenCalled(); }); @@ -124,8 +125,11 @@ describe(UserAdminService.name, () => { await expect(sut.delete(authStub.admin, userStub.admin.id, {})).rejects.toBeInstanceOf(ForbiddenException); }); - it('should require the auth user be an admin', async () => { - await expect(sut.delete(authStub.user1, authStub.admin.user.id, {})).rejects.toBeInstanceOf(ForbiddenException); + it('should not allow deleting own account', async () => { + const user = factory.userAdmin({ isAdmin: false }); + const auth = factory.auth({ user }); + mocks.user.get.mockResolvedValue(user); + await expect(sut.delete(auth, user.id, {})).rejects.toBeInstanceOf(ForbiddenException); expect(mocks.user.delete).not.toHaveBeenCalled(); }); diff --git a/server/src/services/user-admin.service.ts b/server/src/services/user-admin.service.ts index dcd415174..332496a95 100644 --- a/server/src/services/user-admin.service.ts +++ b/server/src/services/user-admin.service.ts @@ -52,6 +52,10 @@ export class UserAdminService extends BaseService { async update(auth: AuthDto, id: string, dto: UserAdminUpdateDto): Promise { const user = await this.findOrFail(id, {}); + if (dto.isAdmin !== undefined && dto.isAdmin !== auth.user.isAdmin && auth.user.id === id) { + throw new BadRequestException('Admin status can only be changed by another admin'); + } + if (dto.quotaSizeInBytes && user.quotaSizeInBytes !== dto.quotaSizeInBytes) { await this.userRepository.syncUsage(id); } @@ -89,9 +93,9 @@ export class UserAdminService extends BaseService { async delete(auth: AuthDto, id: string, dto: UserAdminDeleteDto): Promise { const { force } = dto; - const { isAdmin } = await this.findOrFail(id, {}); - if (isAdmin) { - throw new ForbiddenException('Cannot delete admin user'); + await this.findOrFail(id, {}); + if (auth.user.id === id) { + throw new ForbiddenException('Cannot delete your own account'); } await this.albumRepository.softDeleteAll(id); diff --git a/web/src/lib/modals/UserEditModal.svelte b/web/src/lib/modals/UserEditModal.svelte index 0bb018721..aa0d6c3e8 100644 --- a/web/src/lib/modals/UserEditModal.svelte +++ b/web/src/lib/modals/UserEditModal.svelte @@ -1,10 +1,11 @@