diff --git a/foundations/server/packages/middleware/src/guestPermissions.ts b/foundations/server/packages/middleware/src/guestPermissions.ts index b6a7ba289e..3f3526914b 100644 --- a/foundations/server/packages/middleware/src/guestPermissions.ts +++ b/foundations/server/packages/middleware/src/guestPermissions.ts @@ -20,6 +20,8 @@ import core, { type Tx, type TxApplyIf, type TxCUD, + type TxCreateDoc, + type TxMixin, TxProcessor, type TxUpdateDoc } from '@hcengineering/core' @@ -199,36 +201,62 @@ export class GuestPermissionsMiddleware extends BaseMiddleware implements Middle const persons = await this.findAll(ctx, contact.class.Person, { _id: tx.objectId as Ref }, { limit: 1 }) const person = persons[0] - return person === undefined || !this.canUserEditPersonContactDetails(person, account) + if (person === undefined) return true + if ( + tx._class === core.class.TxMixin && + (tx as TxMixin).mixin === contact.mixin.Employee && + !h.hasMixin(person, contact.mixin.Employee) + ) { + return true + } + return !this.canUserEditPersonContactDetails(person, account) } if (h.isDerived(tx.objectClass, contact.class.Channel)) { - const parentPersonId = await this.getChannelParentPersonId(ctx, tx) - if (parentPersonId === undefined) return false - - const persons = await this.findAll(ctx, contact.class.Person, { _id: parentPersonId }, { limit: 1 }) - const person = persons[0] - return person === undefined || !this.canUserEditPersonContactDetails(person, account) + const parentPersonIds = await this.getChannelParentPersonIds(ctx, tx) + for (const parentPersonId of parentPersonIds) { + const persons = await this.findAll(ctx, contact.class.Person, { _id: parentPersonId }, { limit: 1 }) + const person = persons[0] + if (person === undefined || !this.canUserEditPersonContactDetails(person, account)) return true + } } return false } - private async getChannelParentPersonId ( + private async getChannelParentPersonIds ( ctx: MeasureContext, tx: TxCUD - ): Promise | undefined> { - if (tx.attachedToClass === contact.class.Person && tx.attachedTo !== undefined) { - return tx.attachedTo as Ref + ): Promise>> { + const result = new Set>() + const addPerson = (personId: Ref | undefined, personClass: Ref> | undefined): void => { + if ( + personId !== undefined && + personClass !== undefined && + this.context.hierarchy.isDerived(personClass, contact.class.Person) + ) { + result.add(personId as Ref) + } } - if (tx._class === core.class.TxCreateDoc) return undefined + if (tx._class === core.class.TxCreateDoc) { + const channel = TxProcessor.createDoc2Doc(tx as TxCreateDoc) + addPerson(channel.attachedTo, channel.attachedToClass) + return Array.from(result) + } const channels = await this.findAll(ctx, contact.class.Channel, { _id: tx.objectId as Ref }, { limit: 1 }) const channel = channels[0] - if (channel?.attachedToClass !== contact.class.Person) return undefined + if (channel === undefined) return [] - return channel.attachedTo as Ref + addPerson(channel.attachedTo, channel.attachedToClass) + + if (tx._class === core.class.TxUpdateDoc) { + const operations = (tx as TxUpdateDoc).operations + addPerson(operations.attachedTo ?? channel.attachedTo, operations.attachedToClass ?? channel.attachedToClass) + } + + return Array.from(result) } /** diff --git a/foundations/server/packages/middleware/src/tests/guestPermissions.test.ts b/foundations/server/packages/middleware/src/tests/guestPermissions.test.ts index 34b4a5d83a..25e51a7640 100644 --- a/foundations/server/packages/middleware/src/tests/guestPermissions.test.ts +++ b/foundations/server/packages/middleware/src/tests/guestPermissions.test.ts @@ -113,11 +113,7 @@ function patchContactHierarchy (mw: GuestPermissionsMiddleware): void { mixin === contact.mixin.Employee && doc?.employee === true } -function makePersonDoc ( - _id: Ref, - personUuid: Account['uuid'], - employee: boolean = true -): Doc { +function makePersonDoc (_id: Ref, personUuid: Account['uuid'], employee: boolean = true): Doc { return { _id, _class: contact.class.Person, @@ -183,7 +179,7 @@ describe('GuestPermissionsMiddleware', () => { // ─── User contact mutability ──────────────────────────────────────────────── describe('user contact mutability', () => { it('forbids a user from updating another employee person', async () => { - const otherPersonId = generateId() as Ref + const otherPersonId = generateId() const account = makeAccount(AccountRole.User) const findAll: FindAllFn = async (_ctx, _class, query: any) => { if (_class === contact.class.Person && query?._id === otherPersonId) { @@ -206,7 +202,7 @@ describe('GuestPermissionsMiddleware', () => { }) it('allows a user to update their own employee person', async () => { - const personId = generateId() as Ref + const personId = generateId() const account = makeAccount(AccountRole.User) let nextCalled = false const findAll: FindAllFn = async (_ctx, _class, query: any) => { @@ -234,8 +230,8 @@ describe('GuestPermissionsMiddleware', () => { }) it('forbids a user from updating channels attached to another employee person', async () => { - const otherPersonId = generateId() as Ref - const channelId = generateId() as Ref + const otherPersonId = generateId() + const channelId = generateId() const account = makeAccount(AccountRole.User) const findAll: FindAllFn = async (_ctx, _class, query: any) => { if (_class === contact.class.Person && query?._id === otherPersonId) { @@ -270,8 +266,144 @@ describe('GuestPermissionsMiddleware', () => { await expect(mw.tx(makeCtx(account), [tx])).rejects.toThrow() }) + it('does not trust spoofed channel parent fields', async () => { + const ownPersonId = generateId() + const otherPersonId = generateId() + const channelId = generateId() + const account = makeAccount(AccountRole.User) + const findAll: FindAllFn = async (_ctx, _class, query: any) => { + if (_class === contact.class.Person && query?._id === ownPersonId) { + return [makePersonDoc(ownPersonId, account.uuid)] + } + if (_class === contact.class.Person && query?._id === otherPersonId) { + return [makePersonDoc(otherPersonId, generateId() as any)] + } + if (_class === contact.class.Channel && query?._id === channelId) { + return [ + { + _id: channelId, + _class: contact.class.Channel, + space: 'contact:space:Contacts' as Ref, + modifiedOn: Date.now(), + modifiedBy: 'test' as PersonId, + attachedTo: otherPersonId, + attachedToClass: contact.class.Person + } as any + ] + } + return [] + } + const mw = makeMiddleware(findAll) + patchContactHierarchy(mw) + + const factory = new TxFactory(account.primarySocialId) + const tx = factory.createTxUpdateDoc( + contact.class.Channel as Ref>, + 'core:space:Workspace' as Ref, + channelId, + { value: 'new@example.com' } as any + ) + tx.attachedTo = ownPersonId + tx.attachedToClass = contact.class.Person + + await expect(mw.tx(makeCtx(account), [tx])).rejects.toThrow() + }) + + it('forbids creating a channel attached through attributes to another employee person', async () => { + const otherPersonId = generateId() + const account = makeAccount(AccountRole.User) + const findAll: FindAllFn = async (_ctx, _class, query: any) => { + if (_class === contact.class.Person && query?._id === otherPersonId) { + return [makePersonDoc(otherPersonId, generateId() as any)] + } + return [] + } + const mw = makeMiddleware(findAll) + patchContactHierarchy(mw) + + const factory = new TxFactory(account.primarySocialId) + const tx = factory.createTxCreateDoc( + contact.class.Channel as Ref>, + 'contact:space:Contacts' as Ref, + { + attachedTo: otherPersonId, + attachedToClass: contact.class.Person, + collection: 'channels', + provider: 'contact:channelProvider:Email', + value: 'new@example.com' + } as any + ) + + await expect(mw.tx(makeCtx(account), [tx])).rejects.toThrow() + }) + + it('forbids moving a channel to another employee person', async () => { + const ownPersonId = generateId() + const otherPersonId = generateId() + const channelId = generateId() + const account = makeAccount(AccountRole.User) + const findAll: FindAllFn = async (_ctx, _class, query: any) => { + if (_class === contact.class.Person && query?._id === ownPersonId) { + return [makePersonDoc(ownPersonId, account.uuid)] + } + if (_class === contact.class.Person && query?._id === otherPersonId) { + return [makePersonDoc(otherPersonId, generateId() as any)] + } + if (_class === contact.class.Channel && query?._id === channelId) { + return [ + { + _id: channelId, + _class: contact.class.Channel, + space: 'contact:space:Contacts' as Ref, + modifiedOn: Date.now(), + modifiedBy: 'test' as PersonId, + attachedTo: ownPersonId, + attachedToClass: contact.class.Person + } as any + ] + } + return [] + } + const mw = makeMiddleware(findAll) + patchContactHierarchy(mw) + + const factory = new TxFactory(account.primarySocialId) + const tx = factory.createTxUpdateDoc( + contact.class.Channel as Ref>, + 'core:space:Workspace' as Ref, + channelId, + { attachedTo: otherPersonId, attachedToClass: contact.class.Person } as any + ) + + await expect(mw.tx(makeCtx(account), [tx])).rejects.toThrow() + }) + + it('forbids a user from adding the employee mixin to a person', async () => { + const personId = generateId() + const account = makeAccount(AccountRole.User) + const findAll: FindAllFn = async (_ctx, _class, query: any) => { + if (_class === contact.class.Person && query?._id === personId) { + return [makePersonDoc(personId, generateId() as any, false)] + } + return [] + } + const mw = makeMiddleware(findAll) + patchContactHierarchy(mw) + + const factory = new TxFactory(account.primarySocialId) + const tx = factory.createTxMixin( + personId, + contact.class.Person as Ref>, + 'contact:space:Contacts' as Ref, + contact.mixin.Employee, + { active: true } as any + ) + + await expect(mw.tx(makeCtx(account), [tx])).rejects.toThrow() + }) + it('allows maintainers to update another employee person', async () => { - const otherPersonId = generateId() as Ref + const otherPersonId = generateId() const account = makeAccount(AccountRole.Maintainer) let nextCalled = false const mw = makeMiddleware(