From 7b3351c7ab6c3727ac5a4fb5dae1e50006c8afa4 Mon Sep 17 00:00:00 2001 From: Ignat Remizov Date: Mon, 25 May 2026 20:17:50 +0300 Subject: [PATCH] fix(contact): enforce employee contact edit permissions Enforce contact edit boundaries for regular users on both the UI and transaction middleware so users can maintain their own profile details without being able to change other employees' person records or communication channels. Changes: - Reject regular-user CUD transactions that update another employee Person or channels attached to that employee, including nested TxApplyIf payloads. - Allow regular users to edit their own contact details and non-employee contacts, while Maintainers and Owners continue to manage employee records. - Share contact editability logic in contact resources and use it in person editing, channel editors, and channel presenters so the UI hides or disables blocked actions. - Add middleware tests covering own-person edits, blocked cross-employee person/channel edits, and maintainer bypass behavior. Validation: - git diff --check - rushx svelte-check in plugins/contact-resources - targeted Rush tests for @hcengineering/middleware and @hcengineering/contact-resources - targeted Rush build for @hcengineering/middleware and @hcengineering/contact-resources Behavioral effect: Regular users can update their own profile contact details, but attempts to edit another employee's person record or attached email, phone, or GitHub channels are blocked even if sent directly to the server. --- .../middleware/src/guestPermissions.ts | 73 ++++++++- .../src/tests/guestPermissions.test.ts | 139 ++++++++++++++++++ .../src/components/ChannelsEditor.svelte | 31 +++- .../src/components/ChannelsPresenter.svelte | 32 +++- .../src/components/EditPerson.svelte | 16 +- plugins/contact-resources/src/utils.ts | 14 ++ 6 files changed, 286 insertions(+), 19 deletions(-) diff --git a/foundations/server/packages/middleware/src/guestPermissions.ts b/foundations/server/packages/middleware/src/guestPermissions.ts index 077d546a84d..b6a7ba289ea 100644 --- a/foundations/server/packages/middleware/src/guestPermissions.ts +++ b/foundations/server/packages/middleware/src/guestPermissions.ts @@ -24,7 +24,7 @@ import core, { type TxUpdateDoc } from '@hcengineering/core' import platform, { PlatformError, Severity, Status } from '@hcengineering/platform' -import contact, { type Person } from '@hcengineering/contact' +import contact, { type Channel, type Person } from '@hcengineering/contact' /** Cached state loaded from GuestPermissionsSettings configuration document. */ interface GuestPermissionsCache { @@ -122,6 +122,11 @@ export class GuestPermissionsMiddleware extends BaseMiddleware implements Middle async tx (ctx: MeasureContext, txes: Tx[]): Promise { const account = ctx.contextData.account if (hasAccountRole(account, AccountRole.User)) { + if (!hasAccountRole(account, AccountRole.Maintainer)) { + for (const tx of txes) { + await this.processUserTx(ctx, tx) + } + } this.invalidateCacheIfNeeded(txes) return await this.provideTx(ctx, txes) } @@ -160,6 +165,72 @@ export class GuestPermissionsMiddleware extends BaseMiddleware implements Middle } } + private async processUserTx (ctx: MeasureContext, tx: Tx): Promise { + if (tx._class === core.class.TxApplyIf) { + const applyTx = tx as TxApplyIf + for (const t of applyTx.txes) { + await this.processUserTx(ctx, t) + } + return + } + + if (!TxProcessor.isExtendsCUD(tx._class)) return + + const { account } = ctx.contextData + if (await this.isForbiddenUserContactTx(ctx, tx as TxCUD, account)) { + throw new PlatformError(new Status(Severity.ERROR, platform.status.Forbidden, {})) + } + } + + private canUserEditPersonContactDetails (person: Person, account: Account): boolean { + if (person.personUuid === account.uuid) return true + return !this.context.hierarchy.hasMixin(person, contact.mixin.Employee) + } + + private async isForbiddenUserContactTx ( + ctx: MeasureContext, + tx: TxCUD, + account: Account + ): Promise { + const h = this.context.hierarchy + + if (h.isDerived(tx.objectClass, contact.class.Person)) { + if (tx._class === core.class.TxCreateDoc) return false + + 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 (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) + } + + return false + } + + private async getChannelParentPersonId ( + ctx: MeasureContext, + tx: TxCUD + ): Promise | undefined> { + if (tx.attachedToClass === contact.class.Person && tx.attachedTo !== undefined) { + return tx.attachedTo as Ref + } + + if (tx._class === core.class.TxCreateDoc) return undefined + + 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 + + return channel.attachedTo as Ref + } + /** * Returns the covered-class ancestor of the objectClass if one exists in the new permissions model, * or undefined if the class is not covered. diff --git a/foundations/server/packages/middleware/src/tests/guestPermissions.test.ts b/foundations/server/packages/middleware/src/tests/guestPermissions.test.ts index e124aee8cbe..34b4a5d83a1 100644 --- a/foundations/server/packages/middleware/src/tests/guestPermissions.test.ts +++ b/foundations/server/packages/middleware/src/tests/guestPermissions.test.ts @@ -41,6 +41,7 @@ import core, { type Tx, TxFactory } from '@hcengineering/core' +import contact from '@hcengineering/contact' import type { PipelineContext, TxMiddlewareResult } from '@hcengineering/server-core' import { GuestPermissionsMiddleware } from '../guestPermissions' @@ -106,6 +107,28 @@ function makeCreateTx (objectClass: Ref>, objectSpace: Ref): T return factory.createTxCreateDoc(objectClass, objectSpace, {}) } +function patchContactHierarchy (mw: GuestPermissionsMiddleware): void { + ;(mw as any).context.hierarchy.isDerived = (a: any, b: any) => a === b + ;(mw as any).context.hierarchy.hasMixin = (doc: any, mixin: any) => + mixin === contact.mixin.Employee && doc?.employee === true +} + +function makePersonDoc ( + _id: Ref, + personUuid: Account['uuid'], + employee: boolean = true +): Doc { + return { + _id, + _class: contact.class.Person, + space: 'contact:space:Contacts' as Ref, + modifiedOn: Date.now(), + modifiedBy: 'test' as PersonId, + personUuid, + employee + } as any +} + // Helper: buildGuestSettings - simulate the document that loadPermissionsCache would find function makeGuestSettingsDoc (allowedPermissions: Ref[], disabledPermissions?: Ref[]): Doc { return { @@ -157,6 +180,122 @@ 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 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.createTxUpdateDoc( + contact.class.Person as Ref>, + 'core:space:Workspace' as Ref, + otherPersonId, + { name: 'Changed' } as any + ) + + await expect(mw.tx(makeCtx(account), [tx])).rejects.toThrow() + }) + + it('allows a user to update their own employee person', async () => { + const personId = generateId() as Ref + const account = makeAccount(AccountRole.User) + let nextCalled = false + const findAll: FindAllFn = async (_ctx, _class, query: any) => { + if (_class === contact.class.Person && query?._id === personId) { + return [makePersonDoc(personId, account.uuid)] + } + return [] + } + const mw = makeMiddleware(findAll, async () => { + nextCalled = true + return {} + }) + patchContactHierarchy(mw) + + const factory = new TxFactory(account.primarySocialId) + const tx = factory.createTxUpdateDoc( + contact.class.Person as Ref>, + 'core:space:Workspace' as Ref, + personId, + { name: 'Changed' } as any + ) + + await mw.tx(makeCtx(account), [tx]) + expect(nextCalled).toBe(true) + }) + + it('forbids a user from updating channels attached to another employee person', async () => { + const otherPersonId = generateId() as Ref + const channelId = generateId() as Ref + 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)] + } + 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 + ) + + await expect(mw.tx(makeCtx(account), [tx])).rejects.toThrow() + }) + + it('allows maintainers to update another employee person', async () => { + const otherPersonId = generateId() as Ref + const account = makeAccount(AccountRole.Maintainer) + let nextCalled = false + const mw = makeMiddleware( + async () => [], + async () => { + nextCalled = true + return {} + } + ) + patchContactHierarchy(mw) + + const factory = new TxFactory(account.primarySocialId) + const tx = factory.createTxUpdateDoc( + contact.class.Person as Ref>, + 'core:space:Workspace' as Ref, + otherPersonId, + { name: 'Changed' } as any + ) + + await mw.tx(makeCtx(account), [tx]) + expect(nextCalled).toBe(true) + }) + }) + // ─── DocGuest / ReadOnlyGuest are always forbidden ────────────────────────── describe('DocGuest and ReadOnlyGuest', () => { it('DocGuest: throws Forbidden for any tx', async () => { diff --git a/plugins/contact-resources/src/components/ChannelsEditor.svelte b/plugins/contact-resources/src/components/ChannelsEditor.svelte index b6a8ec54b40..892b3c58928 100644 --- a/plugins/contact-resources/src/components/ChannelsEditor.svelte +++ b/plugins/contact-resources/src/components/ChannelsEditor.svelte @@ -18,9 +18,10 @@ import { createQuery, getClient } from '@hcengineering/presentation' import { ButtonKind, ButtonSize, closeTooltip, showPopup } from '@hcengineering/ui' - import { Channel, ChannelProvider } from '@hcengineering/contact' + import { Channel, ChannelProvider, type Person } from '@hcengineering/contact' import { restrictionStore } from '@hcengineering/view-resources' import contact from '../plugin' + import { canEditPersonContactDetails } from '../utils' import ChannelsDropdown from './ChannelsDropdown.svelte' export let attachedTo: Ref @@ -37,6 +38,7 @@ export let restricted: Ref[] = [] let channels: Channel[] = [] + let attachedPerson: Person | undefined = undefined const query = createQuery() $: attachedTo && @@ -50,17 +52,38 @@ } ) + const personQuery = createQuery() + $: if (attachedClass === contact.class.Person) { + personQuery.query( + contact.class.Person, + { + _id: attachedTo as Ref + }, + (res) => { + attachedPerson = res[0] + } + ) + } else { + personQuery.unsubscribe() + attachedPerson = undefined + } + + $: effectiveEditable = + attachedClass === contact.class.Person + ? editable && attachedPerson !== undefined && canEditPersonContactDetails(attachedPerson) + : editable + const client = getClient() async function remove (value: Channel | AttachedData): Promise { - if (!editable) return + if (!effectiveEditable) return if ('_id' in value) { await client.remove(value) } } async function saveHandler (value: Channel | AttachedData): Promise { - if (!editable) return + if (!effectiveEditable) return if ('_id' in value) { await client.update(value, { value: value.value @@ -89,7 +112,7 @@ {size} {length} {integrations} - {editable} + editable={effectiveEditable} {restricted} {shape} {focusIndex} diff --git a/plugins/contact-resources/src/components/ChannelsPresenter.svelte b/plugins/contact-resources/src/components/ChannelsPresenter.svelte index 6814a37c1f4..beaf1e4cfc1 100644 --- a/plugins/contact-resources/src/components/ChannelsPresenter.svelte +++ b/plugins/contact-resources/src/components/ChannelsPresenter.svelte @@ -14,11 +14,15 @@ // limitations under the License. --> {#if value} - + {/if} diff --git a/plugins/contact-resources/src/components/EditPerson.svelte b/plugins/contact-resources/src/components/EditPerson.svelte index 6acf9046e72..84fada6b615 100644 --- a/plugins/contact-resources/src/components/EditPerson.svelte +++ b/plugins/contact-resources/src/components/EditPerson.svelte @@ -14,8 +14,8 @@ // limitations under the License. -->