diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index c81a3dbba1..3817d1e0fc 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -25547,7 +25547,7 @@ "PersonUsersCreateDto": { "properties": { "personIds": { - "description": "Person IDs", + "description": "Person IDs, defaults to every person owned by the user", "items": { "format": "uuid", "pattern": "^([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[1-8][0-9a-fA-F]{3}-[89abAB][0-9a-fA-F]{3}-[0-9a-fA-F]{12}|00000000-0000-0000-0000-000000000000|ffffffff-ffff-ffff-ffff-ffffffffffff)$", @@ -25570,7 +25570,6 @@ } }, "required": [ - "personIds", "role", "sharedWithIds" ], diff --git a/packages/sdk/src/fetch-client.ts b/packages/sdk/src/fetch-client.ts index 1b3e029b91..b4af32e609 100644 --- a/packages/sdk/src/fetch-client.ts +++ b/packages/sdk/src/fetch-client.ts @@ -2067,8 +2067,8 @@ export type PersonUsersResponseDto = { sharedWithId: string; }[]; export type PersonUsersCreateDto = { - /** Person IDs */ - personIds: string[]; + /** Person IDs, defaults to every person owned by the user */ + personIds?: string[]; /** Role that should be applied */ role: PersonUserRole; /** User IDs that should be given access to the person */ diff --git a/server/src/dtos/person.dto.ts b/server/src/dtos/person.dto.ts index b2de5bc468..931290b2c5 100644 --- a/server/src/dtos/person.dto.ts +++ b/server/src/dtos/person.dto.ts @@ -222,7 +222,7 @@ const PersonUsersSearchSchema = z const PersonUsersCreateSchema = z .object({ - personIds: uniqueIds.describe('Person IDs'), + personIds: uniqueIds.optional().describe('Person IDs, defaults to every person owned by the user'), sharedWithIds: uniqueIds.describe('User IDs that should be given access to the person'), role: PersonUserRoleSchema.describe('Role that should be applied'), }) diff --git a/server/src/queries/person.user.repository.sql b/server/src/queries/person.user.repository.sql index 3c0162889e..3c99359dfc 100644 --- a/server/src/queries/person.user.repository.sql +++ b/server/src/queries/person.user.repository.sql @@ -66,6 +66,33 @@ set returning * +-- PersonUserRepository.createAllForOwner +insert into + "person_user" ( + "personGroupId", + "sharedById", + "sharedWithId", + "role" + ) +select + "person"."personGroupId" as "personGroupId", + "person"."ownerId" as "sharedById", + "shared"."sharedWithId" as "sharedWithId", + $1::person_user_role_enum as "role" +from + "person" + cross join ( + select + unnest($2::uuid[]) as "sharedWithId" + ) as "shared" +where + "person"."ownerId" = $3 +on conflict ("personGroupId", "sharedById", "sharedWithId") do update +set + "role" = "excluded"."role" +returning + * + -- PersonUserRepository.deleteAll delete from "person_user" where diff --git a/server/src/repositories/person-user.repository.ts b/server/src/repositories/person-user.repository.ts index 730f721c43..6ca011e1c6 100644 --- a/server/src/repositories/person-user.repository.ts +++ b/server/src/repositories/person-user.repository.ts @@ -9,6 +9,8 @@ import { SharingDirection } from 'src/enum.js'; import { DB } from 'src/schema/index.js'; import { PersonUserTable } from 'src/schema/tables/person-user.table.js'; +type CreateAllForOwnerOptions = { ownerId: string; sharedWithIds: string[]; role: PersonUserRole }; + @Injectable() export class PersonUserRepository { constructor(@InjectKysely() private db: Kysely) {} @@ -81,6 +83,41 @@ export class PersonUserRepository { .execute(); } + @GenerateSql({ params: [{ ownerId: DummyValue.UUID, sharedWithIds: [DummyValue.UUID], role: PersonUserRole.Admin }] }) + createAllForOwner({ ownerId, sharedWithIds, role }: CreateAllForOwnerOptions) { + if (sharedWithIds.length === 0) { + return []; + } + + const shared = sql<{ sharedWithId: string }>`( + select + unnest(${sharedWithIds}::uuid[]) as "sharedWithId" + )`.as('shared'); + + return this.db + .insertInto('person_user') + .columns(['personGroupId', 'sharedById', 'sharedWithId', 'role']) + .expression((eb) => + eb + .selectFrom('person') + .crossJoin(shared) + .select(({ ref }) => [ + ref('person.personGroupId').as('personGroupId'), + ref('person.ownerId').as('sharedById'), + ref('shared.sharedWithId').as('sharedWithId'), + sql`${role}::person_user_role_enum`.as('role'), + ]) + .where('person.ownerId', '=', ownerId), + ) + .onConflict((oc) => + oc + .columns(['personGroupId', 'sharedById', 'sharedWithId']) + .doUpdateSet((eb) => ({ role: eb.ref('excluded.role') })), + ) + .returningAll() + .execute(); + } + @GenerateSql({ params: [DummyValue.UUID, [{ personId: DummyValue.UUID, sharedWithId: DummyValue.UUID }]] }) async deleteAll(sharedById: string, dto: PersonUsersDeleteDto) { if (dto.length === 0) { diff --git a/server/src/services/person.service.spec.ts b/server/src/services/person.service.spec.ts index f44e7f9b6a..d19a53e390 100644 --- a/server/src/services/person.service.spec.ts +++ b/server/src/services/person.service.spec.ts @@ -1441,6 +1441,80 @@ describe(PersonService.name, () => { }); }); + describe('addUsersToPeople', () => { + it('should reject sharing a person with yourself', async () => { + const auth = AuthFactory.create(); + + await expect( + sut.addUsersToPeople(auth, { + personIds: [newUuid()], + sharedWithIds: [auth.user.id], + role: PersonUserRole.Read, + }), + ).rejects.toBeInstanceOf(BadRequestException); + + expect(mocks.personUser.createAll).not.toHaveBeenCalled(); + }); + + it('should share the given people', async () => { + const auth = AuthFactory.create(); + const user = UserFactory.create({ id: auth.user.id }); + const sharedWith = UserFactory.create({ clusterGroupId: user.clusterGroupId }); + const personId = newUuid(); + + mocks.access.person.checkAccess.mockResolvedValue(new Set([{ personGroupId: personId, ownerId: auth.user.id }])); + mocks.user.get.mockResolvedValue(user); + mocks.clusterGroup.getUsers.mockResolvedValue([user, sharedWith]); + + await sut.addUsersToPeople(auth, { + personIds: [personId], + sharedWithIds: [sharedWith.id], + role: PersonUserRole.Read, + }); + + expect(mocks.personUser.createAllForOwner).not.toHaveBeenCalled(); + expect(mocks.personUser.createAll).toHaveBeenCalledWith([ + { personGroupId: personId, sharedById: auth.user.id, sharedWithId: sharedWith.id, role: PersonUserRole.Read }, + ]); + }); + + it('should share every person owned by the user when personIds is omitted', async () => { + const auth = AuthFactory.create(); + const user = UserFactory.create({ id: auth.user.id }); + const sharedWith = UserFactory.create({ clusterGroupId: user.clusterGroupId }); + + mocks.user.get.mockResolvedValue(user); + mocks.clusterGroup.getUsers.mockResolvedValue([user, sharedWith]); + + await sut.addUsersToPeople(auth, { sharedWithIds: [sharedWith.id], role: PersonUserRole.Write }); + + expect(mocks.access.person.checkAccess).not.toHaveBeenCalled(); + expect(mocks.personUser.createAll).not.toHaveBeenCalled(); + expect(mocks.personUser.createAllForOwner).toHaveBeenCalledWith({ + ownerId: auth.user.id, + sharedWithIds: [sharedWith.id], + role: PersonUserRole.Write, + }); + }); + + it('should reject users outside the cluster group', async () => { + const auth = AuthFactory.create(); + const user = UserFactory.create({ id: auth.user.id }); + const outsider = UserFactory.create(); + const personId = newUuid(); + + mocks.access.person.checkAccess.mockResolvedValue(new Set([{ personGroupId: personId, ownerId: auth.user.id }])); + mocks.user.get.mockResolvedValue(user); + mocks.clusterGroup.getUsers.mockResolvedValue([user]); + + await expect( + sut.addUsersToPeople(auth, { personIds: [personId], sharedWithIds: [outsider.id], role: PersonUserRole.Read }), + ).rejects.toBeInstanceOf(BadRequestException); + + expect(mocks.personUser.createAll).not.toHaveBeenCalled(); + }); + }); + describe('mapFace', () => { it('should map a face', () => { const user = UserFactory.create(); diff --git a/server/src/services/person.service.ts b/server/src/services/person.service.ts index d4d6b73fd4..dbf9d747f3 100644 --- a/server/src/services/person.service.ts +++ b/server/src/services/person.service.ts @@ -833,11 +833,13 @@ export class PersonService extends BaseService { throw new BadRequestException('Cannot share a person with yourself'); } - await this.requirePersonAccess({ - auth, - permission: Permission.PersonUpdate, - ids: dto.personIds.map((item) => ({ personGroupId: item, ownerId: auth.user.id })), - }); + if (dto.personIds) { + await this.requirePersonAccess({ + auth, + permission: Permission.PersonUpdate, + ids: dto.personIds.map((item) => ({ personGroupId: item, ownerId: auth.user.id })), + }); + } const user = await findOrFail(() => this.userRepository.get(auth.user.id, {}), 'User'); const clusterGroupUsers = await this.clusterGroupRepository.getUsers({ @@ -846,20 +848,30 @@ export class PersonService extends BaseService { }); const clusterGroupUserIds = new Set(clusterGroupUsers.map(({ id }) => id)); - const items: Insertable[] = []; - const sharedById = auth.user.id; - for (const sharedWithId of dto.sharedWithIds) { if (!clusterGroupUserIds.has(sharedWithId)) { throw new BadRequestException('All users must be in the same cluster group'); } - - for (const personGroupId of dto.personIds) { - items.push({ personGroupId, sharedById, sharedWithId, role: dto.role }); - } } - await this.personUserRepository.createAll(items); + const sharedById = auth.user.id; + + if (dto.personIds) { + const items: Insertable[] = []; + for (const sharedWithId of dto.sharedWithIds) { + for (const personGroupId of dto.personIds) { + items.push({ personGroupId, sharedById, sharedWithId, role: dto.role }); + } + } + + await this.personUserRepository.createAll(items); + } else { + await this.personUserRepository.createAllForOwner({ + ownerId: sharedById, + sharedWithIds: dto.sharedWithIds, + role: dto.role, + }); + } } async removeUsersFromPeople(auth: AuthDto, dto: PersonUsersDeleteDto) {