Fix employee bypass (#11048)

Signed-off-by: Artyom Savchenko <armisav@gmail.com>
This commit is contained in:
Artyom Savchenko
2026-09-29 07:20:35 +02:00
committed by GitHub
parent f4f3d30bf1
commit 488340fc31
2 changed files with 184 additions and 24 deletions
@@ -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<Person> }, { 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<Doc, Doc>).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<SessionData>,
tx: TxCUD<Doc>
): Promise<Ref<Person> | undefined> {
if (tx.attachedToClass === contact.class.Person && tx.attachedTo !== undefined) {
return tx.attachedTo as Ref<Person>
): Promise<Array<Ref<Person>>> {
const result = new Set<Ref<Person>>()
const addPerson = (personId: Ref<Doc> | undefined, personClass: Ref<Class<Doc>> | undefined): void => {
if (
personId !== undefined &&
personClass !== undefined &&
this.context.hierarchy.isDerived(personClass, contact.class.Person)
) {
result.add(personId as Ref<Person>)
}
}
if (tx._class === core.class.TxCreateDoc) return undefined
if (tx._class === core.class.TxCreateDoc) {
const channel = TxProcessor.createDoc2Doc(tx as TxCreateDoc<Channel>)
addPerson(channel.attachedTo, channel.attachedToClass)
return Array.from(result)
}
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
if (channel === undefined) return []
return channel.attachedTo as Ref<Person>
addPerson(channel.attachedTo, channel.attachedToClass)
if (tx._class === core.class.TxUpdateDoc) {
const operations = (tx as TxUpdateDoc<Channel>).operations
addPerson(operations.attachedTo ?? channel.attachedTo, operations.attachedToClass ?? channel.attachedToClass)
}
return Array.from(result)
}
/**
@@ -113,11 +113,7 @@ function patchContactHierarchy (mw: GuestPermissionsMiddleware): void {
mixin === contact.mixin.Employee && doc?.employee === true
}
function makePersonDoc (
_id: Ref<Doc>,
personUuid: Account['uuid'],
employee: boolean = true
): Doc {
function makePersonDoc (_id: Ref<Doc>, 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<Doc>
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<Doc>
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<Doc>
const channelId = generateId() as Ref<Doc>
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<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
)
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<Class<Doc>>,
'contact:space:Contacts' as Ref<Space>,
{
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<Space>,
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<Class<Doc>>,
'core:space:Workspace' as Ref<Space>,
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<Class<Doc>>,
'contact:space:Contacts' as Ref<Space>,
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<Doc>
const otherPersonId = generateId()
const account = makeAccount(AccountRole.Maintainer)
let nextCalled = false
const mw = makeMiddleware(