mirror of
https://github.com/hcengineering/platform.git
synced 2026-09-30 13:35:04 +02:00
fix(contact): enforce employee contact edit permissions (#10890)
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.
This commit is contained in:
@@ -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<SessionData>, txes: Tx[]): Promise<TxMiddlewareResult> {
|
||||
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<SessionData>, tx: Tx): Promise<void> {
|
||||
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<Doc>, 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<SessionData>,
|
||||
tx: TxCUD<Doc>,
|
||||
account: Account
|
||||
): Promise<boolean> {
|
||||
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<Person> }, { 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<SessionData>,
|
||||
tx: TxCUD<Doc>
|
||||
): Promise<Ref<Person> | undefined> {
|
||||
if (tx.attachedToClass === contact.class.Person && tx.attachedTo !== undefined) {
|
||||
return tx.attachedTo as Ref<Person>
|
||||
}
|
||||
|
||||
if (tx._class === core.class.TxCreateDoc) return undefined
|
||||
|
||||
const channels = await this.findAll(ctx, contact.class.Channel, { _id: tx.objectId as Ref<Channel> }, { limit: 1 })
|
||||
const channel = channels[0]
|
||||
if (channel?.attachedToClass !== contact.class.Person) return undefined
|
||||
|
||||
return channel.attachedTo as Ref<Person>
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the covered-class ancestor of the objectClass if one exists in the new permissions model,
|
||||
* or undefined if the class is not covered.
|
||||
|
||||
@@ -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<Class<Doc>>, objectSpace: Ref<Space>): 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<Doc>,
|
||||
personUuid: Account['uuid'],
|
||||
employee: boolean = true
|
||||
): Doc {
|
||||
return {
|
||||
_id,
|
||||
_class: contact.class.Person,
|
||||
space: 'contact:space:Contacts' as Ref<Space>,
|
||||
modifiedOn: Date.now(),
|
||||
modifiedBy: 'test' as PersonId,
|
||||
personUuid,
|
||||
employee
|
||||
} as any
|
||||
}
|
||||
|
||||
// Helper: buildGuestSettings - simulate the document that loadPermissionsCache would find
|
||||
function makeGuestSettingsDoc (allowedPermissions: Ref<Doc>[], disabledPermissions?: Ref<Doc>[]): 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<Doc>
|
||||
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<Class<Doc>>,
|
||||
'core:space:Workspace' as Ref<Space>,
|
||||
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<Doc>
|
||||
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<Class<Doc>>,
|
||||
'core:space:Workspace' as Ref<Space>,
|
||||
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<Doc>
|
||||
const channelId = generateId() as Ref<Doc>
|
||||
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<Space>,
|
||||
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<Class<Doc>>,
|
||||
'core:space:Workspace' as Ref<Space>,
|
||||
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<Doc>
|
||||
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<Class<Doc>>,
|
||||
'core:space:Workspace' as Ref<Space>,
|
||||
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 () => {
|
||||
|
||||
@@ -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<Doc>
|
||||
@@ -37,6 +38,7 @@
|
||||
export let restricted: Ref<ChannelProvider>[] = []
|
||||
|
||||
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<Person>
|
||||
},
|
||||
(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<Channel>): Promise<void> {
|
||||
if (!editable) return
|
||||
if (!effectiveEditable) return
|
||||
if ('_id' in value) {
|
||||
await client.remove(value)
|
||||
}
|
||||
}
|
||||
|
||||
async function saveHandler (value: Channel | AttachedData<Channel>): Promise<void> {
|
||||
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}
|
||||
|
||||
@@ -14,11 +14,15 @@
|
||||
// limitations under the License.
|
||||
-->
|
||||
<script lang="ts">
|
||||
import type { Channel } from '@hcengineering/contact'
|
||||
import type { Channel, Person } from '@hcengineering/contact'
|
||||
import type { Ref } from '@hcengineering/core'
|
||||
import { getResource } from '@hcengineering/platform'
|
||||
import { createQuery } from '@hcengineering/presentation'
|
||||
import type { ButtonKind, ButtonSize } from '@hcengineering/ui'
|
||||
import { showPopup } from '@hcengineering/ui'
|
||||
import { ViewAction } from '@hcengineering/view'
|
||||
import contact from '../plugin'
|
||||
import { canEditPersonContactDetails } from '../utils'
|
||||
import ChannelsDropdown from './ChannelsDropdown.svelte'
|
||||
|
||||
export let value: Channel[] | Channel | null
|
||||
@@ -29,6 +33,30 @@
|
||||
export let length: 'tiny' | 'short' | 'full' = 'short'
|
||||
export let shape: 'circle' | undefined = 'circle'
|
||||
|
||||
let attachedPerson: Person | undefined = undefined
|
||||
const personQuery = createQuery()
|
||||
|
||||
$: channel = Array.isArray(value) ? value[0] : value
|
||||
$: if (channel?.attachedToClass === contact.class.Person) {
|
||||
personQuery.query(
|
||||
contact.class.Person,
|
||||
{
|
||||
_id: channel.attachedTo as Ref<Person>
|
||||
},
|
||||
(res) => {
|
||||
attachedPerson = res[0]
|
||||
}
|
||||
)
|
||||
} else {
|
||||
personQuery.unsubscribe()
|
||||
attachedPerson = undefined
|
||||
}
|
||||
|
||||
$: effectiveEditable =
|
||||
channel?.attachedToClass === contact.class.Person
|
||||
? editable === true && attachedPerson !== undefined && canEditPersonContactDetails(attachedPerson)
|
||||
: editable
|
||||
|
||||
async function _open (ev: CustomEvent): Promise<void> {
|
||||
if (ev.detail.presenter !== undefined && Array.isArray(value)) {
|
||||
showPopup(ev.detail.presenter, { channel: ev.detail.channel }, 'float')
|
||||
@@ -44,5 +72,5 @@
|
||||
</script>
|
||||
|
||||
{#if value}
|
||||
<ChannelsDropdown bind:value {length} {kind} {size} {shape} {editable} on:open={_open} />
|
||||
<ChannelsDropdown bind:value {length} {kind} {size} {shape} editable={effectiveEditable} on:open={_open} />
|
||||
{/if}
|
||||
|
||||
@@ -14,8 +14,8 @@
|
||||
// limitations under the License.
|
||||
-->
|
||||
<script lang="ts">
|
||||
import { Channel, Person, combineName, getCurrentEmployee, getFirstName, getLastName } from '@hcengineering/contact'
|
||||
import { AccountRole, Ref, getCurrentAccount, hasAccountRole, type AccountUuid } from '@hcengineering/core'
|
||||
import { Channel, Person, combineName, getFirstName, getLastName } from '@hcengineering/contact'
|
||||
import { Ref, getCurrentAccount, type AccountUuid } from '@hcengineering/core'
|
||||
import { AttributeEditor, createQuery, getClient, hasResource } from '@hcengineering/presentation'
|
||||
import type { PersonRating } from '@hcengineering/rating'
|
||||
import ratingPlugin from '@hcengineering/rating'
|
||||
@@ -27,24 +27,16 @@
|
||||
import ChannelsDropdown from './ChannelsDropdown.svelte'
|
||||
import ChannelsEditor from './ChannelsEditor.svelte'
|
||||
import EditableAvatar from './EditableAvatar.svelte'
|
||||
import { canEditPersonContactDetails } from '../utils'
|
||||
|
||||
export let object: Person
|
||||
export let readonly: boolean = false
|
||||
export let channels: Channel[] | undefined = undefined
|
||||
|
||||
const client = getClient()
|
||||
const h = client.getHierarchy()
|
||||
|
||||
const account = getCurrentAccount()
|
||||
const me = getCurrentEmployee()
|
||||
$: owner = me === object._id
|
||||
|
||||
function isEditable (owner: boolean, object: Person): boolean {
|
||||
if (owner) return true
|
||||
if (!h.hasMixin(object, contact.mixin.Employee)) return true
|
||||
return hasAccountRole(account, AccountRole.Maintainer)
|
||||
}
|
||||
$: editable = !readonly && isEditable(owner, object)
|
||||
$: editable = !readonly && canEditPersonContactDetails(object)
|
||||
|
||||
let avatarEditor: EditableAvatar
|
||||
|
||||
|
||||
@@ -53,6 +53,7 @@ import {
|
||||
type SocialIdentity
|
||||
} from '@hcengineering/contact'
|
||||
import core, {
|
||||
AccountRole,
|
||||
type AccountUuid,
|
||||
type AggregateValue,
|
||||
type Class,
|
||||
@@ -60,6 +61,7 @@ import core, {
|
||||
type Doc,
|
||||
type DocumentQuery,
|
||||
getCurrentAccount,
|
||||
hasAccountRole,
|
||||
type Hierarchy,
|
||||
type IdMap,
|
||||
notEmpty,
|
||||
@@ -110,6 +112,18 @@ export function formatDate (dueDateMs: Timestamp): string {
|
||||
})
|
||||
}
|
||||
|
||||
export function canEditPersonContactDetails (person: Person): boolean {
|
||||
const currentEmployee = getCurrentEmployee()
|
||||
if (currentEmployee === person._id) return true
|
||||
|
||||
const account = getCurrentAccount()
|
||||
if (person.personUuid === account.uuid) return true
|
||||
|
||||
if (!getClient().getHierarchy().hasMixin(person, contact.mixin.Employee)) return true
|
||||
|
||||
return hasAccountRole(account, AccountRole.Maintainer)
|
||||
}
|
||||
|
||||
export async function employeeSort (client: TxOperations, value: Array<Ref<Employee>>): Promise<Array<Ref<Employee>>> {
|
||||
const h = client.getHierarchy()
|
||||
return value.sort((a, b) => {
|
||||
|
||||
Reference in New Issue
Block a user