Skip to content

Commit 4399588

Browse files
wuzihao051119alextran1502
authored and
savely-krasovsky
committed
feat(web): granular api access controls (immich-app#18179)
* feat: api access control * feat(web): granular api access controls * fix test * fix e2e test * fix: lint * pr feedback * merge main + new design * finalize styling --------- Co-authored-by: Alex <[email protected]>
1 parent f88e56f commit 4399588

File tree

12 files changed

+311
-37
lines changed

12 files changed

+311
-37
lines changed

‎e2e/src/api/specs/api-key.e2e-spec.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ describe('/api-keys', () => {
143143
const { apiKey } = await create(user.accessToken, [Permission.All]);
144144
const { status, body } = await request(app)
145145
.put(`/api-keys/${apiKey.id}`)
146-
.send({ name: 'new name' })
146+
.send({ name: 'new name', permissions: [Permission.All] })
147147
.set('Authorization', `Bearer ${admin.accessToken}`);
148148
expect(status).toBe(400);
149149
expect(body).toEqual(errorDto.badRequest('API Key not found'));
@@ -153,13 +153,16 @@ describe('/api-keys', () => {
153153
const { apiKey } = await create(user.accessToken, [Permission.All]);
154154
const { status, body } = await request(app)
155155
.put(`/api-keys/${apiKey.id}`)
156-
.send({ name: 'new name' })
156+
.send({
157+
name: 'new name',
158+
permissions: [Permission.ActivityCreate, Permission.ActivityRead, Permission.ActivityUpdate],
159+
})
157160
.set('Authorization', `Bearer ${user.accessToken}`);
158161
expect(status).toBe(200);
159162
expect(body).toEqual({
160163
id: expect.any(String),
161164
name: 'new name',
162-
permissions: [Permission.All],
165+
permissions: [Permission.ActivityCreate, Permission.ActivityRead, Permission.ActivityUpdate],
163166
createdAt: expect.any(String),
164167
updatedAt: expect.any(String),
165168
});

‎i18n/en.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1381,6 +1381,8 @@
13811381
"permanently_delete_assets_prompt": "Are you sure you want to permanently delete {count, plural, one {this asset?} other {these <b>#</b> assets?}} This will also remove {count, plural, one {it from its} other {them from their}} album(s).",
13821382
"permanently_deleted_asset": "Permanently deleted asset",
13831383
"permanently_deleted_assets_count": "Permanently deleted {count, plural, one {# asset} other {# assets}}",
1384+
"permission": "Permission",
1385+
"permission_empty": "Your permission shouldn't be empty",
13841386
"permission_onboarding_back": "Back",
13851387
"permission_onboarding_continue_anyway": "Continue anyway",
13861388
"permission_onboarding_get_started": "Get started",

‎mobile/openapi/lib/model/api_key_update_dto.dart‎

Lines changed: 11 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎open-api/immich-openapi-specs.json‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8294,10 +8294,18 @@
82948294
"properties": {
82958295
"name": {
82968296
"type": "string"
8297+
},
8298+
"permissions": {
8299+
"items": {
8300+
"$ref": "#/components/schemas/Permission"
8301+
},
8302+
"minItems": 1,
8303+
"type": "array"
82978304
}
82988305
},
82998306
"required": [
8300-
"name"
8307+
"name",
8308+
"permissions"
83018309
],
83028310
"type": "object"
83038311
},

‎open-api/typescript-sdk/src/fetch-client.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,7 @@ export type ApiKeyCreateResponseDto = {
408408
};
409409
export type ApiKeyUpdateDto = {
410410
name: string;
411+
permissions: Permission[];
411412
};
412413
export type AssetBulkDeleteDto = {
413414
force?: boolean;

‎server/src/controllers/api-key.controller.spec.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { APIKeyController } from 'src/controllers/api-key.controller';
2+
import { Permission } from 'src/enum';
23
import { ApiKeyService } from 'src/services/api-key.service';
34
import request from 'supertest';
45
import { factory } from 'test/small.factory';
@@ -52,7 +53,9 @@ describe(APIKeyController.name, () => {
5253
});
5354

5455
it('should require a valid uuid', async () => {
55-
const { status, body } = await request(ctx.getHttpServer()).put(`/api-keys/123`).send({ name: 'new name' });
56+
const { status, body } = await request(ctx.getHttpServer())
57+
.put(`/api-keys/123`)
58+
.send({ name: 'new name', permissions: [Permission.ALL] });
5659
expect(status).toBe(400);
5760
expect(body).toEqual(factory.responses.badRequest(['id must be a UUID']));
5861
});

‎server/src/dtos/api-key.dto.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ export class APIKeyUpdateDto {
1818
@IsString()
1919
@IsNotEmpty()
2020
name!: string;
21+
22+
@IsEnum(Permission, { each: true })
23+
@ApiProperty({ enum: Permission, enumName: 'Permission', isArray: true })
24+
@ArrayMinSize(1)
25+
permissions!: Permission[];
2126
}
2227

2328
export class APIKeyCreateResponseDto {

‎server/src/services/api-key.service.spec.ts‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,9 @@ describe(ApiKeyService.name, () => {
6969

7070
mocks.apiKey.getById.mockResolvedValue(void 0);
7171

72-
await expect(sut.update(auth, id, { name: 'New Name' })).rejects.toBeInstanceOf(BadRequestException);
72+
await expect(sut.update(auth, id, { name: 'New Name', permissions: [Permission.ALL] })).rejects.toBeInstanceOf(
73+
BadRequestException,
74+
);
7375

7476
expect(mocks.apiKey.update).not.toHaveBeenCalledWith(id);
7577
});
@@ -82,9 +84,28 @@ describe(ApiKeyService.name, () => {
8284
mocks.apiKey.getById.mockResolvedValue(apiKey);
8385
mocks.apiKey.update.mockResolvedValue(apiKey);
8486

85-
await sut.update(auth, apiKey.id, { name: newName });
87+
await sut.update(auth, apiKey.id, { name: newName, permissions: [Permission.ALL] });
88+
89+
expect(mocks.apiKey.update).toHaveBeenCalledWith(auth.user.id, apiKey.id, {
90+
name: newName,
91+
permissions: [Permission.ALL],
92+
});
93+
});
8694

87-
expect(mocks.apiKey.update).toHaveBeenCalledWith(auth.user.id, apiKey.id, { name: newName });
95+
it('should update permissions', async () => {
96+
const auth = factory.auth();
97+
const apiKey = factory.apiKey({ userId: auth.user.id });
98+
const newPermissions = [Permission.ACTIVITY_CREATE, Permission.ACTIVITY_READ, Permission.ACTIVITY_UPDATE];
99+
100+
mocks.apiKey.getById.mockResolvedValue(apiKey);
101+
mocks.apiKey.update.mockResolvedValue(apiKey);
102+
103+
await sut.update(auth, apiKey.id, { name: apiKey.name, permissions: newPermissions });
104+
105+
expect(mocks.apiKey.update).toHaveBeenCalledWith(auth.user.id, apiKey.id, {
106+
name: apiKey.name,
107+
permissions: newPermissions,
108+
});
88109
});
89110
});
90111

‎server/src/services/api-key.service.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export class ApiKeyService extends BaseService {
3232
throw new BadRequestException('API Key not found');
3333
}
3434

35-
const key = await this.apiKeyRepository.update(auth.user.id, id, { name: dto.name });
35+
const key = await this.apiKeyRepository.update(auth.user.id, id, { name: dto.name, permissions: dto.permissions });
3636

3737
return this.map(key);
3838
}

‎web/src/lib/components/user-settings-page/user-api-key-grid.svelte‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
<script lang="ts">
2+
import { Permission } from '@immich/sdk';
3+
import { Checkbox, Label } from '@immich/ui';
4+
5+
interface Props {
6+
title: string;
7+
subItems: Permission[];
8+
selectedItems: Permission[];
9+
handleSelectItems: (permissions: Permission[]) => void;
10+
handleDeselectItems: (permissions: Permission[]) => void;
11+
}
12+
13+
let { title, subItems, selectedItems, handleSelectItems, handleDeselectItems }: Props = $props();
14+
15+
let selectAllSubItems = $derived(subItems.filter((item) => selectedItems.includes(item)).length === subItems.length);
16+
17+
const handleSelectAllSubItems = () => {
18+
if (selectAllSubItems) {
19+
handleDeselectItems(subItems);
20+
} else {
21+
handleSelectItems(subItems);
22+
}
23+
};
24+
25+
const handleToggleItem = (permission: Permission) => {
26+
if (selectedItems.includes(permission)) {
27+
handleDeselectItems([permission]);
28+
} else {
29+
handleSelectItems([permission]);
30+
}
31+
};
32+
</script>
33+
34+
<div class="mx-4 my-2 border bg-subtle dark:bg-black/30 dark:border-black p-4 rounded-2xl">
35+
<div class="flex items-center gap-2">
36+
<Checkbox
37+
id="permission-{title}"
38+
size="tiny"
39+
checked={selectAllSubItems}
40+
onCheckedChange={handleSelectAllSubItems}
41+
/>
42+
<Label label={title} for={title} class="font-mono text-primary text-lg" />
43+
</div>
44+
<div class="mx-6 mt-3 grid grid-cols-3 gap-2">
45+
{#each subItems as item (item)}
46+
<div class="flex items-center gap-2">
47+
<Checkbox
48+
id="permission-{item}"
49+
size="tiny"
50+
checked={selectedItems.includes(item)}
51+
onCheckedChange={() => handleToggleItem(item)}
52+
/>
53+
<Label label={item} for={item} class="text-sm font-mono" />
54+
</div>
55+
{/each}
56+
</div>
57+
</div>

0 commit comments

Comments
 (0)