diff --git a/common/config/rush/pnpm-lock.yaml b/common/config/rush/pnpm-lock.yaml index 57a469ad1fa..e8383843775 100644 --- a/common/config/rush/pnpm-lock.yaml +++ b/common/config/rush/pnpm-lock.yaml @@ -18056,6 +18056,9 @@ importers: '@hcengineering/text': specifier: workspace:^0.7.19 version: link:../../foundations/core/packages/text + '@hcengineering/text-core': + specifier: workspace:^0.7.19 + version: link:../../foundations/core/packages/text-core '@hcengineering/text-editor': specifier: workspace:^0.7.0 version: link:../text-editor @@ -61713,7 +61716,7 @@ snapshots: node-loader@2.0.0(webpack@5.102.1): dependencies: loader-utils: 2.0.4 - webpack: 5.102.1 + webpack: 5.102.1(esbuild@0.25.12)(webpack-cli@5.1.4) node-localstorage@2.2.1: dependencies: diff --git a/foundations/core/packages/core/src/__tests__/collaborators.test.ts b/foundations/core/packages/core/src/__tests__/collaborators.test.ts index 3e69795765a..5b372a85c6b 100644 --- a/foundations/core/packages/core/src/__tests__/collaborators.test.ts +++ b/foundations/core/packages/core/src/__tests__/collaborators.test.ts @@ -13,11 +13,11 @@ // limitations under the License. // -import { getClassCollaborators } from '../collaborators' +import { getClassCollaborators, resolveMentionGrantTarget } from '../collaborators' import { ModelDb } from '../memdb' import { Hierarchy } from '../hierarchy' import core from '../component' -import type { Class, ClassCollaborators, Doc, Ref } from '../classes' +import type { AttachedDoc, Class, ClassCollaborators, Doc, Ref } from '../classes' describe('collaborators', () => { let model: ModelDb @@ -198,4 +198,109 @@ describe('collaborators', () => { expect(result).toBe(collab2) }) }) + + describe('resolveMentionGrantTarget', () => { + const ISSUE = 'tracker:class:Issue' as Ref> + const THREAD = 'chunter:class:ThreadMessage' as Ref> + const MSG = 'chunter:class:ChatMessage' as Ref> + + // Builds a findAll injector like the server/client pass in. `opted` is the + // set of _class refs whose ClassCollaborators is provideSecurity+mentionsGrantAccess. + // `docs` maps _id -> Doc for the attachedTo parent lookups. + function makeFindAll ( + opted: Set, + docs: Record + ): jest.Mock, [Ref>, any, any?]> { + return jest.fn(async (cls: Ref>, q: any, _options?: any) => { + if (cls === core.class.ClassCollaborators) { + const target = q.attachedTo as string + return opted.has(target) ? [{ provideSecurity: true, mentionsGrantAccess: true }] : [] + } + const doc = docs[q._id as string] + return doc != null ? [doc] : [] + }) + } + + it('returns the start doc when it is itself opted in', async () => { + const start: Doc = { _id: 'i1' as Ref, _class: ISSUE } as unknown as Doc + const findAll = makeFindAll(new Set([ISSUE]), {}) + + const result = await resolveMentionGrantTarget(start, findAll) + + expect(result).toBe(start) + }) + + it('walks the attachedTo chain to the first opted-in ancestor', async () => { + const issue: Doc = { _id: 'i1' as Ref, _class: ISSUE } as unknown as Doc + const thread = { + _id: 'tm1' as Ref, + _class: THREAD, + attachedTo: 'i1' as Ref, + attachedToClass: ISSUE + } as unknown as AttachedDoc + + // ThreadMessage + ChatMessage are NOT opted in; only the Issue ancestor is. + const findAll = makeFindAll(new Set([ISSUE]), { i1: issue }) + + const result = await resolveMentionGrantTarget(thread, findAll) + + expect(result).toBe(issue) + }) + + it('returns null when nothing in the chain is opted in', async () => { + const parent = { _id: 'p1' as Ref, _class: MSG } as unknown as Doc + const child = { + _id: 'c1' as Ref, + _class: THREAD, + attachedTo: 'p1' as Ref, + attachedToClass: MSG + } as unknown as AttachedDoc + + const findAll = makeFindAll(new Set(), { p1: parent }) + + const result = await resolveMentionGrantTarget(child, findAll) + + expect(result).toBeNull() + }) + + it('returns null on an attachedTo cycle (depth cap 8)', async () => { + // Self-referential doc: attachedTo points back at itself, never opted in. + const node = { + _id: 'x' as Ref, + _class: THREAD, + attachedTo: 'x' as Ref, + attachedToClass: THREAD + } as unknown as AttachedDoc + const findAll = makeFindAll(new Set(), { x: node as unknown as Doc }) + + const result = await resolveMentionGrantTarget(node, findAll) + + expect(result).toBeNull() + // Depth cap 8: exactly 8 ClassCollaborators probes (one per loop turn). + const ccProbes = findAll.mock.calls.filter((c) => c[0] === core.class.ClassCollaborators) + expect(ccProbes.length).toBe(8) + }) + + it('passes { limit: 1 } as the third arg on every findAll lookup (fix guard)', async () => { + const issue: Doc = { _id: 'i1' as Ref, _class: ISSUE } as unknown as Doc + const thread = { + _id: 'tm1' as Ref, + _class: THREAD, + attachedTo: 'i1' as Ref, + attachedToClass: ISSUE + } as unknown as AttachedDoc + const findAll = makeFindAll(new Set([ISSUE]), { i1: issue }) + + await resolveMentionGrantTarget(thread, findAll) + + // Both the ClassCollaborators probe and the parent lookup must be bounded. + expect(findAll).toHaveBeenCalled() + for (const call of findAll.mock.calls) { + expect(call[2]).toEqual({ limit: 1 }) + } + // Sanity: at least one ClassCollaborators probe and one parent lookup ran. + expect(findAll.mock.calls.some((c) => c[0] === core.class.ClassCollaborators)).toBe(true) + expect(findAll.mock.calls.some((c) => c[0] === ISSUE)).toBe(true) + }) + }) }) diff --git a/foundations/core/packages/core/src/classes.ts b/foundations/core/packages/core/src/classes.ts index b9c62466d90..07a9baa37d0 100644 --- a/foundations/core/packages/core/src/classes.ts +++ b/foundations/core/packages/core/src/classes.ts @@ -1010,8 +1010,15 @@ export interface ClassCollaborators extends Doc { attachedTo: Ref> allFields?: boolean // for all (PersonId | Ref | PersonId[] | Ref[]) attributes fields: (keyof T)[] // PersonId | Ref | PersonId[] | Ref[] - provideSecurity?: boolean // If true, will provide security for collaborators + // If true, Collaborator status grants read visibility on the doc, + // bypassing space-membership. Writes are governed by the class's + // TxAccessLevel and any pre-commit middleware (see GuestPermissions). + provideSecurity?: boolean provideAttachedSecurity?: boolean // If true, will provide security for collaborators of attached doc + // If true, @-mentions in chat/activity messages on this doc auto-create + // Collaborator records (mention = explicit, disclosed grant). Has no + // effect unless provideSecurity is also true. + mentionsGrantAccess?: boolean } export interface Collaborator extends AttachedDoc { diff --git a/foundations/core/packages/core/src/collaborators.ts b/foundations/core/packages/core/src/collaborators.ts index 1574a2c1401..1c516803783 100644 --- a/foundations/core/packages/core/src/collaborators.ts +++ b/foundations/core/packages/core/src/collaborators.ts @@ -13,7 +13,17 @@ // limitations under the License. // -import core, { Class, ClassCollaborators, Doc, Hierarchy, ModelDb, Ref } from '.' +import core, { + AttachedDoc, + Class, + ClassCollaborators, + Doc, + DocumentQuery, + FindOptions, + Hierarchy, + ModelDb, + Ref +} from '.' export function getClassCollaborators ( model: ModelDb, @@ -35,3 +45,51 @@ export function getClassCollaborators ( } } } + +/** + * Walk a Doc's attachedTo chain to find the nearest ancestor (including the + * Doc itself) whose ClassCollaborators has BOTH provideSecurity===true AND + * mentionsGrantAccess===true. Returns that ancestor Doc as the grant target, + * or null if no such class is reached within the depth cap. + * + * Used by both the chunter mention-trigger (server) and the warning popup + * (client) so the disclosure UX matches the actual server-side grant. The + * helper is isomorphic via the findAll dependency injection — server passes + * `(cls, q, o) => control.findAll(control.ctx, cls, q, o)`, client passes + * `(cls, q, o) => getClient().findAll(cls, q, o)`. + * + * The ClassCollaborators lookup is exact-class (not inherited via ancestors) + * — adequate for tracker.class.Issue and avoids surprising matches on + * abstract base classes like AttachedDoc. If future opt-in classes need + * inherited semantics, switch to `getClassCollaborators(model, hierarchy, _class)` + * here (requires plumbing ModelDb + Hierarchy through the dependency + * injection — kept out for now to keep the helper isomorphic without + * the ModelDb tax on the client). + * + * Depth cap (8) defends against pathological attachedTo cycles. + */ +export async function resolveMentionGrantTarget ( + start: Doc, + findAll: (cls: Ref>, q: DocumentQuery, options?: FindOptions) => Promise +): Promise { + let cur: Doc | undefined = start + for (let i = 0; i < 8 && cur != null; i++) { + const ccQuery: DocumentQuery> = { + attachedTo: cur._class + } + const cc = (await findAll(core.class.ClassCollaborators, ccQuery, { limit: 1 }))[0] + if (cc?.provideSecurity === true && cc.mentionsGrantAccess === true) { + return cur + } + const attached = cur as AttachedDoc + if (attached.attachedTo == null || attached.attachedToClass == null) { + return null + } + const parentQuery: DocumentQuery = { + _id: attached.attachedTo + } + const parent = (await findAll(attached.attachedToClass, parentQuery, { limit: 1 }))[0] + cur = parent + } + return null +} diff --git a/foundations/core/packages/text-core/src/markup/__tests__/reference.test.ts b/foundations/core/packages/text-core/src/markup/__tests__/reference.test.ts new file mode 100644 index 00000000000..167ac76e1cf --- /dev/null +++ b/foundations/core/packages/text-core/src/markup/__tests__/reference.test.ts @@ -0,0 +1,74 @@ +import { extractReferences } from '../reference' +import { MarkupNodeType, type MarkupNode } from '../model' + +function refNode (id: string, label: string, objectclass: string, grantsAccess?: 'true' | 'false'): MarkupNode { + return { + type: MarkupNodeType.reference, + attrs: { id, label, objectclass, ...(grantsAccess !== undefined ? { grantsAccess } : {}) } + } +} + +function doc (...children: MarkupNode[]): MarkupNode { + return { type: MarkupNodeType.doc, content: children } +} + +describe('extractReferences grantsAccess', () => { + test('surfaces grantsAccess from a reference node', () => { + const refs = extractReferences(doc(refNode('p1', 'Alice', 'contact:class:Person', 'true'))) + expect(refs).toHaveLength(1) + expect(refs[0].objectId).toBe('p1') + expect(refs[0].grantsAccess).toBe('true') + }) + + test('grantsAccess undefined when attr absent (default = grant)', () => { + const refs = extractReferences(doc(refNode('p1', 'Alice', 'contact:class:Person'))) + expect(refs[0].grantsAccess).toBeUndefined() + }) + + test('any-wins: false then true => true', () => { + const refs = extractReferences( + doc( + refNode('p1', 'Alice', 'contact:class:Person', 'false'), + refNode('p1', 'Alice', 'contact:class:Person', 'true') + ) + ) + expect(refs).toHaveLength(1) + expect(refs[0].grantsAccess).toBe('true') + }) + + test('any-wins: true then false => true', () => { + const refs = extractReferences( + doc( + refNode('p1', 'Alice', 'contact:class:Person', 'true'), + refNode('p1', 'Alice', 'contact:class:Person', 'false') + ) + ) + expect(refs[0].grantsAccess).toBe('true') + }) + + test('any-wins: false then undefined => not false (undefined grants)', () => { + const refs = extractReferences( + doc(refNode('p1', 'Alice', 'contact:class:Person', 'false'), refNode('p1', 'Alice', 'contact:class:Person')) + ) + expect(refs[0].grantsAccess).not.toBe('false') + }) + + test('all explicitly false => false', () => { + const refs = extractReferences( + doc( + refNode('p1', 'Alice', 'contact:class:Person', 'false'), + refNode('p1', 'Alice', 'contact:class:Person', 'false') + ) + ) + expect(refs[0].grantsAccess).toBe('false') + }) + + test('different persons kept separate', () => { + const refs = extractReferences( + doc(refNode('p1', 'Alice', 'contact:class:Person', 'true'), refNode('p2', 'Bob', 'contact:class:Person', 'false')) + ) + expect(refs).toHaveLength(2) + expect(refs.find((r) => r.objectId === 'p1')?.grantsAccess).toBe('true') + expect(refs.find((r) => r.objectId === 'p2')?.grantsAccess).toBe('false') + }) +}) diff --git a/foundations/core/packages/text-core/src/markup/model.ts b/foundations/core/packages/text-core/src/markup/model.ts index 9b275c14f0c..cc5b581914a 100644 --- a/foundations/core/packages/text-core/src/markup/model.ts +++ b/foundations/core/packages/text-core/src/markup/model.ts @@ -93,5 +93,11 @@ export interface LinkMark extends MarkupMark { /** @public */ export interface ReferenceMarkupNode extends MarkupNode { type: MarkupNodeType.reference - attrs: { id: string, label: string, objectclass: string } + /** + * grantsAccess (V3): when 'false', the chunter mention-grants trigger skips + * Collaborator creation for this mention. undefined means "grant" (default, + * preserves pre-V3 behaviour). Stored as a string because markup attrs + * serialize through HTML where boolean coercion is unreliable. + */ + attrs: { id: string, label: string, objectclass: string, grantsAccess?: 'true' | 'false' } } diff --git a/foundations/core/packages/text-core/src/markup/reference.ts b/foundations/core/packages/text-core/src/markup/reference.ts index a9e4c64261a..cccf9f35025 100644 --- a/foundations/core/packages/text-core/src/markup/reference.ts +++ b/foundations/core/packages/text-core/src/markup/reference.ts @@ -24,6 +24,9 @@ export interface Reference { objectId: Ref objectClass: Ref> parentNode: MarkupNode | null + // V3: undefined = grant (default); 'false' = explicit deny. Merged any-wins + // across duplicate references to the same object (see extractReferences). + grantsAccess?: 'true' | 'false' } /** @@ -37,9 +40,19 @@ export function extractReferences (content: MarkupNode): Array { const reference = node as ReferenceMarkupNode const objectId = reference.attrs.id as Ref const objectClass = reference.attrs.objectclass as Ref> + const grantsAccess = reference.attrs.grantsAccess const e = result.find((e) => e.objectId === objectId && e.objectClass === objectClass) if (e === undefined) { - result.push({ objectId, objectClass, parentNode: parent ?? node }) + result.push({ objectId, objectClass, parentNode: parent ?? node, grantsAccess }) + } else { + // any-wins: a person is denied only if EVERY reference is explicitly + // 'false'. Upgrade away from 'false' as soon as a non-'false' (grant + // or default) reference appears; upgrade default->'true' cosmetically. + if (e.grantsAccess === 'false' && grantsAccess !== 'false') { + e.grantsAccess = grantsAccess + } else if (e.grantsAccess === undefined && grantsAccess === 'true') { + e.grantsAccess = 'true' + } } } return true diff --git a/foundations/core/packages/text/src/nodes/reference.ts b/foundations/core/packages/text/src/nodes/reference.ts index 8a9bbf685ab..02fa7240df5 100644 --- a/foundations/core/packages/text/src/nodes/reference.ts +++ b/foundations/core/packages/text/src/nodes/reference.ts @@ -22,6 +22,7 @@ export interface ReferenceNodeProps { id: Ref objectclass: Ref> label: string + grantsAccess?: 'true' | 'false' fixed?: boolean } @@ -44,6 +45,17 @@ export const ReferenceNode = Node.create({ id: getDataAttribute('id'), objectclass: getDataAttribute('objectclass'), label: getDataAttribute('label'), + grantsAccess: { + // V3: keep the deny flag across edit round-trips. parseHTML normalises + // a missing attribute to `undefined` rather than `null` so the runtime + // value matches the public type `'true' | 'false' | undefined` declared + // on ReferenceMarkupNode.attrs — a `null` would break strict + // `=== undefined` checks downstream. + default: undefined, + parseHTML: (element) => element.getAttribute('data-grants-access') ?? undefined, + renderHTML: (attributes) => + attributes.grantsAccess == null ? null : { 'data-grants-access': attributes.grantsAccess } + }, fixed: { default: null, parseHTML: (element) => element.getAttribute('data-fixed') === 'true', @@ -90,6 +102,11 @@ export const ReferenceNode = Node.create({ 'data-id': node.attrs.id, 'data-objectclass': node.attrs.objectclass, 'data-label': node.attrs.label, + // V3: belt-and-suspenders — explicitly emit data-grants-access here + // (in addition to the per-attribute renderHTML in addAttributes) so + // the deny flag is rendered even if the attribute-renderer merge + // path is bypassed by a downstream override. + ...(node.attrs.grantsAccess != null ? { 'data-grants-access': node.attrs.grantsAccess } : {}), class: 'antiMention' }, this.options.HTMLAttributes, @@ -110,10 +127,13 @@ function getAttrs (el: HTMLSpanElement): Attrs | false { return false } + const grantsAccess = el.dataset.grantsAccess + return { id, label, objectclass, + ...(grantsAccess !== undefined ? { grantsAccess } : {}), fixed: fixed || null } } diff --git a/foundations/server/packages/core/src/adapter.ts b/foundations/server/packages/core/src/adapter.ts index 89776f06fbf..0c27a3c0c31 100644 --- a/foundations/server/packages/core/src/adapter.ts +++ b/foundations/server/packages/core/src/adapter.ts @@ -70,6 +70,18 @@ export interface RawFindIterator { * @public */ export interface DbAdapter extends LowLevelStorage { + // Backend capability flag. When true, this adapter enforces collaborator-grant + // read security at the storage layer (e.g. the Postgres addSecurity collab + // OR-branch that joins the Collaborator domain into the visibility check). + // + // The space-security middleware only drops the space filter for collab-read + // bypasses (Guests reading docs they collaborate on, Collaborator self-reads, + // collab-only space listing) when the adapter serving the domain sets this to + // true. Absent/false is treated as fail-closed: the middleware keeps the normal + // space filter, so a backend without an equivalent clause (e.g. Mongo, which has + // no adapter-side space security) cannot leak docs across spaces. + supportsCollaboratorSecurity?: boolean + init?: ( ctx: MeasureContext, contextVars: Record, diff --git a/foundations/server/packages/middleware/src/guestPermissions.ts b/foundations/server/packages/middleware/src/guestPermissions.ts index 077d546a84d..506bd8a30ce 100644 --- a/foundations/server/packages/middleware/src/guestPermissions.ts +++ b/foundations/server/packages/middleware/src/guestPermissions.ts @@ -10,6 +10,7 @@ import core, { type Class, type Doc, type ClassPermission, + getClassCollaborators, type Permission, hasAccountRole, type MeasureContext, @@ -157,9 +158,57 @@ export class GuestPermissionsMiddleware extends BaseMiddleware implements Middle } else if (cudTx.space !== core.space.DerivedTx && (await this.isForbiddenTx(ctx, cudTx, account))) { throw new PlatformError(new Status(Severity.ERROR, platform.status.Forbidden, {})) } + if (await this.isForbiddenCollabOnlyGuestFieldUpdate(ctx, cudTx, account)) { + throw new PlatformError(new Status(Severity.ERROR, platform.status.Forbidden, {})) + } } } + /** + * Class-agnostic veto for field updates on docs whose class has opted into + * mention-grants-access. The opt-in is the pair (provideSecurity: true, + * mentionsGrantAccess: true) on the class's ClassCollaborators model entry. + * + * For such classes, a guest-tier account that obtained read visibility ONLY + * through Collaborator status (i.e. is NOT in the doc's space.members) must + * not be able to modify the doc's fields via TxUpdateDoc NOR delete it via + * TxRemoveDoc (L-RM). Comments via chunter.class.ChatMessage createAccessLevel + * still pass through. + * + * Space-member guests retain their current behavior — they pass through + * this check untouched and their normal access rules continue to apply. + * User+ accounts always pass through. + * + * If the class has not opted in, this veto is a no-op (returns false). + */ + private async isForbiddenCollabOnlyGuestFieldUpdate ( + ctx: MeasureContext, + cudTx: TxCUD, + account: Account + ): Promise { + // L-RM: veto covers both field-updates AND removes by collab-only guests. + if (cudTx._class !== core.class.TxUpdateDoc && cudTx._class !== core.class.TxRemoveDoc) return false + + const isGuest = + account.role === AccountRole.Guest || + account.role === AccountRole.DocGuest || + account.role === AccountRole.ReadOnlyGuest + if (!isGuest) return false + + const classCollab = getClassCollaborators(this.context.modelDb, this.context.hierarchy, cudTx.objectClass) + if (classCollab?.provideSecurity !== true) return false + if (classCollab.mentionsGrantAccess !== true) return false + + const space = (await this.findAll(ctx, core.class.Space, { _id: cudTx.objectSpace }))[0] + // L-GP: fail-closed — if the doc's space cannot be resolved we cannot prove + // the caller is a space member, so the collab-only-guest veto must FORBID + // (previously returned false = fail-open = mutation allowed). + if (space === undefined) return true + if (space.members?.includes(account.uuid)) return false + + return true + } + /** * 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/spaceSecurity.ts b/foundations/server/packages/middleware/src/spaceSecurity.ts index 4c8bd4223e1..2fafe0203b9 100644 --- a/foundations/server/packages/middleware/src/spaceSecurity.ts +++ b/foundations/server/packages/middleware/src/spaceSecurity.ts @@ -613,6 +613,27 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar return domain === 'tx' ? 'objectSpace' : domain === 'space' ? '_id' : 'space' } + // H5 (Option A, fail-closed): the collab-read bypasses in findAll drop the space + // filter and delegate the restriction to the adapter's collab OR-branch, which + // only exists in the Postgres adapter (see postgres/src/storage.ts addSecurity). + // On a backend without an equivalent clause (e.g. Mongo) dropping the space + // filter would leak docs across spaces. So the bypasses may only fire when the + // adapter serving this domain declares supportsCollaboratorSecurity. Unknown or + // unsupported backend => false => keep the normal space filter (no leak; the + // cross-space collab feature is simply inactive there). + private isCollabBackendSupported (domain: Domain): boolean { + const mgr = this.context.adapterManager + if (mgr === undefined) { + return false + } + try { + const adapter = mgr.getAdapterByName(mgr.getAdapterName(domain), false) + return adapter?.supportsCollaboratorSecurity === true + } catch { + return false + } + } + override async findAll( ctx: MeasureContext, _class: Ref>, @@ -630,7 +651,46 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar let clientFilterSpaces: Set> | undefined - if (!isSystem(account, ctx) && account.role !== AccountRole.DocGuest && domain !== DOMAIN_MODEL) { + // When a class opts into collaborator-grants-read security AND the caller is a Guest/ReadOnlyGuest, + // we deliberately skip the middleware-level space filter. The Postgres adapter's `collabRes` + // OR-branch (see postgres/src/storage.ts, getSecurityClause) joins Collaborator records into the + // visibility check, so Guests can read individual docs they were added to as Collaborator even + // when they are not members of the owning Space. Filtering by space here would strip those docs + // before the adapter ever sees the query. + const collabSec = + domain !== DOMAIN_MODEL ? getClassCollaborators(this.context.modelDb, this.context.hierarchy, _class) : undefined + // H5 (fail-closed): a collab-read bypass drops the space filter and relies on + // the adapter's collab OR-branch to re-restrict visibility. That branch only + // exists on backends that declare supportsCollaboratorSecurity (Postgres / + // CockroachDB). On any other backend the bypass would leak across spaces, so we + // gate all three bypasses behind this single check. Unknown backend => false => + // the normal space filter stays in place. + const collabBackendSupported = domain !== DOMAIN_MODEL && this.isCollabBackendSupported(domain) + const collabReadBypass = + collabBackendSupported && + (collabSec?.provideSecurity === true || collabSec?.provideAttachedSecurity === true) && + [AccountRole.Guest, AccountRole.ReadOnlyGuest].includes(account.role) + // Self-Collaborator visibility: let the Postgres adapter's self-collab OR-branch + // (storage.ts, addSecurity) fire for any non-System caller when reading the + // Collaborator class itself. Required for queries like the tracker "Subscribed" + // tab `{collaborator: self, attachedToClass: Issue}`, which must surface the + // user's own subscriptions even on docs in non-member spaces. + const selfCollabBypass = collabBackendSupported && this.context.hierarchy.isDerived(_class, core.class.Collaborator) + // Containing-Space visibility for collab-only Guests: let the Postgres adapter's + // space-collab OR-branch surface Spaces that host docs the caller is a + // Collaborator on. Required so the project/space nav tree can list projects + // where the user is collab-only (no member status). + const spaceCollabBypass = + collabBackendSupported && isSpace && [AccountRole.Guest, AccountRole.ReadOnlyGuest].includes(account.role) + + if ( + !isSystem(account, ctx) && + account.role !== AccountRole.DocGuest && + domain !== DOMAIN_MODEL && + !collabReadBypass && + !selfCollabBypass && + !spaceCollabBypass + ) { if (!isOwner(account, ctx) || !isSpace || !showArchived) { if (newQuery[field] !== undefined) { const res = await this.mergeQuery(ctx, account, newQuery[field], domain, isSpace, showArchived) diff --git a/foundations/server/packages/middleware/src/tests/collabSpaceSecurity.test.ts b/foundations/server/packages/middleware/src/tests/collabSpaceSecurity.test.ts new file mode 100644 index 00000000000..7c8d86ae4b9 --- /dev/null +++ b/foundations/server/packages/middleware/src/tests/collabSpaceSecurity.test.ts @@ -0,0 +1,254 @@ +// +// Copyright © 2025 Hardcore Engineering Inc. +// +// Licensed under the Eclipse Public License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. You may +// obtain a copy of the License at https://www.eclipse.org/legal/epl-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// +// See the License for the specific language governing permissions and +// limitations under the License. +// + +/** + * H5 - Collab-read bypass must be backend-guarded (fail-closed). + * + * The three collab-read bypasses in SpaceSecurityMiddleware.findAll drop the + * space filter and delegate the restriction to the adapter's collab OR-branch, + * which only exists in the Postgres adapter (postgres/src/storage.ts addSecurity). + * On a backend without an equivalent clause (Mongo has none) dropping the space + * filter would leak docs across spaces. + * + * These tests drive findAll with a mocked pipeline and capture the query that + * leaves the middleware towards the adapter: + * - backend supports collab security -> bypass fires -> space filter dropped + * - backend does NOT (Mongo/unknown) -> bypass skipped -> space filter kept + */ + +import core, { + AccountRole, + generateId, + MeasureMetricsContext, + toFindResult, + type Account, + type Class, + type Doc, + type Domain, + type MeasureContext, + type PersonId, + type Ref, + type SessionData, + type Space +} from '@hcengineering/core' +import type { PipelineContext } from '@hcengineering/server-core' +import { SpaceSecurityMiddleware } from '../spaceSecurity' + +const ISSUE_CLASS = 'test:class:Issue' as Ref> +const ISSUE_DOMAIN = 'test-issue' as Domain +const SPACE_CLASS = core.class.Space +const COLLAB_CLASS = core.class.Collaborator + +const S1 = 'test:space:S1' as Ref // guest IS a member +const S2 = 'test:space:S2' as Ref // guest is NOT a member (collab-only) + +function makeGuest (): Account { + return { + uuid: generateId() as any, + role: AccountRole.Guest, + primarySocialId: 'test' as PersonId, + socialIds: ['test' as PersonId], + fullSocialIds: [] + } +} + +function makeCtx (account: Account): MeasureContext { + const ctx = new MeasureMetricsContext('test', {}) as MeasureContext + ctx.contextData = { + account, + socialStringsToUsers: new Map(), + broadcast: { txes: [], queue: [], sessions: {} } + } as any + return ctx +} + +interface Captured { + query?: any +} + +function admittedSpaces (filterValue: any): Array> { + if (filterValue === undefined) return [] + if (typeof filterValue === 'object' && Array.isArray(filterValue.$in)) return filterValue.$in + return [filterValue] +} + +/** + * Build a SpaceSecurityMiddleware wired to a mocked pipeline. + * + * backendSupportsCollab: whether the adapter serving the domain declares + * supportsCollaboratorSecurity (true = Postgres-like, false = Mongo-like). + * noAdapterManager: simulate a pipeline without an adapter manager. + * provideSecurity: whether the queried class opts into collab read security. + * derivedFromSpace / derivedFromCollaborator: how `_class` reports isDerived. + */ +function makeMiddleware (opts: { + backendSupportsCollab?: boolean + noAdapterManager?: boolean + provideSecurity?: boolean + derivedFromSpace?: boolean + derivedFromCollaborator?: boolean +}): { mw: SpaceSecurityMiddleware, captured: Captured } { + const captured: Captured = {} + + const collabConfig = opts.provideSecurity === true ? [{ attachedTo: ISSUE_CLASS, provideSecurity: true } as any] : [] + + const hierarchy = { + getDomain: (_class: Ref>): Domain => { + if (_class === SPACE_CLASS) return 'space' as Domain + if (_class === COLLAB_CLASS) return 'collaborator' as Domain + return ISSUE_DOMAIN + }, + isDerived: (a: Ref>, b: Ref>): boolean => { + if (b === core.class.Space) return opts.derivedFromSpace === true + if (b === core.class.Collaborator) return opts.derivedFromCollaborator === true + return a === b + }, + getAncestors: (_class: Ref>): Array>> => [_class] + } + + const modelDb = { + findAllSync: (_class: Ref>): any[] => { + if (_class === core.class.ClassCollaborators) return collabConfig + return [] + } + } + + const adapterManager = + opts.noAdapterManager === true + ? undefined + : { + getAdapterName: (_domain: Domain): string => 'default', + getAdapterByName: (_name: string): any => ({ + supportsCollaboratorSecurity: opts.backendSupportsCollab === true ? true : undefined + }) + } + + const context = { + workspace: { uuid: 'test-workspace' as any, url: 'test', dataId: 'test' as any }, + hierarchy, + modelDb, + branding: null, + adapterManager, + contextVars: {} + } as unknown as PipelineContext + + const next = { + findAll: async (_ctx: MeasureContext, _class: Ref>, query: any) => { + captured.query = query + return toFindResult([]) + }, + // Domain spaces for the queried domain: S1 and S2 both host docs. + groupBy: async () => + new Map, number>([ + [S1, 1], + [S2, 1] + ]) + } + + const mw = new (SpaceSecurityMiddleware as any)(false, context, next) + // Skip async init (would call next.findAll for Space); we seed state directly. + mw.wasInit = true + + return { mw, captured } +} + +describe('SpaceSecurityMiddleware - collab-read backend guard (H5)', () => { + describe('collabReadBypass (Guest reading a collab-secured Doc class)', () => { + it('Mongo/unknown backend: does NOT drop the space filter (fail-closed, no cross-space leak)', async () => { + const { mw, captured } = makeMiddleware({ backendSupportsCollab: false, provideSecurity: true }) + const account = makeGuest() + ;(mw as any).allowedSpaces = { [account.uuid]: [S1] } // member of S1 only + + await mw.findAll(makeCtx(account), ISSUE_CLASS, {}) + + // Space filter must be present and must NOT admit S2 (the collab-only space). + expect(captured.query.space).toBeDefined() + const admitted = admittedSpaces(captured.query.space) + expect(admitted).not.toContain(S2) + expect(admitted).toContain(S1) + }) + + it('Postgres backend: drops the space filter so the adapter collab-branch can widen visibility', async () => { + const { mw, captured } = makeMiddleware({ backendSupportsCollab: true, provideSecurity: true }) + const account = makeGuest() + ;(mw as any).allowedSpaces = { [account.uuid]: [S1] } + + await mw.findAll(makeCtx(account), ISSUE_CLASS, {}) + + // Bypass fired: no middleware-level space filter (adapter enforces collab security). + expect(captured.query.space).toBeUndefined() + }) + }) + + describe('selfCollabBypass (reading the Collaborator class)', () => { + it('Mongo/unknown backend: keeps the space filter', async () => { + const { mw, captured } = makeMiddleware({ backendSupportsCollab: false, derivedFromCollaborator: true }) + const account = makeGuest() + ;(mw as any).allowedSpaces = { [account.uuid]: [S1] } + + await mw.findAll(makeCtx(account), COLLAB_CLASS, {}) + + expect(captured.query.space).toBeDefined() + expect(admittedSpaces(captured.query.space)).not.toContain(S2) + }) + + it('Postgres backend: drops the space filter', async () => { + const { mw, captured } = makeMiddleware({ backendSupportsCollab: true, derivedFromCollaborator: true }) + const account = makeGuest() + ;(mw as any).allowedSpaces = { [account.uuid]: [S1] } + + await mw.findAll(makeCtx(account), COLLAB_CLASS, {}) + + expect(captured.query.space).toBeUndefined() + }) + }) + + describe('spaceCollabBypass (Guest listing Spaces)', () => { + it('Mongo/unknown backend: keeps the _id space filter', async () => { + const { mw, captured } = makeMiddleware({ backendSupportsCollab: false, derivedFromSpace: true }) + const account = makeGuest() + ;(mw as any).allowedSpaces = { [account.uuid]: [S1] } + + await mw.findAll(makeCtx(account), SPACE_CLASS, {}) + + // For the space domain the filter field is `_id`. + expect(captured.query._id).toBeDefined() + expect(admittedSpaces(captured.query._id)).not.toContain(S2) + }) + + it('Postgres backend: drops the _id space filter', async () => { + const { mw, captured } = makeMiddleware({ backendSupportsCollab: true, derivedFromSpace: true }) + const account = makeGuest() + ;(mw as any).allowedSpaces = { [account.uuid]: [S1] } + + await mw.findAll(makeCtx(account), SPACE_CLASS, {}) + + expect(captured.query._id).toBeUndefined() + }) + }) + + describe('fail-closed when no adapterManager is available', () => { + it('keeps the space filter even for a collab-secured class', async () => { + const { mw, captured } = makeMiddleware({ noAdapterManager: true, provideSecurity: true }) + const account = makeGuest() + ;(mw as any).allowedSpaces = { [account.uuid]: [S1] } + + await mw.findAll(makeCtx(account), ISSUE_CLASS, {}) + + expect(captured.query.space).toBeDefined() + expect(admittedSpaces(captured.query.space)).not.toContain(S2) + }) + }) +}) diff --git a/foundations/server/packages/middleware/src/tests/guestPermissions.test.ts b/foundations/server/packages/middleware/src/tests/guestPermissions.test.ts index e124aee8cbe..50464ddd8ff 100644 --- a/foundations/server/packages/middleware/src/tests/guestPermissions.test.ts +++ b/foundations/server/packages/middleware/src/tests/guestPermissions.test.ts @@ -192,6 +192,7 @@ describe('GuestPermissionsMiddleware', () => { return a === b } ;(mw as any).context.hierarchy.classHierarchyMixin = () => undefined + ;(mw as any).context.hierarchy.getAncestors = () => [] } it('allows create for covered class in any space (TxAccessLevel is irrelevant)', async () => { @@ -276,6 +277,7 @@ describe('GuestPermissionsMiddleware', () => { if (b === core.class.Space) return false return a === b } + ;(mw as any).context.hierarchy.getAncestors = () => [] const tx = makeCreateTx(UNCOVERED_CLASS, ALLOWED_SPACE) const ctx = makeCtx(makeAccount(AccountRole.Guest)) @@ -310,6 +312,7 @@ describe('GuestPermissionsMiddleware', () => { if (b === core.class.Space) return false return a === b } + ;(mw as any).context.hierarchy.getAncestors = () => [] const tx = makeCreateTx(COVERED_CLASS, ALLOWED_SPACE) const ctx = makeCtx(makeAccount(AccountRole.Guest)) @@ -337,6 +340,7 @@ describe('GuestPermissionsMiddleware', () => { if (b === core.class.Space) return false return a === b } + ;(mw as any).context.hierarchy.getAncestors = () => [] const tx = makeCreateTx(COVERED_CLASS, FORBIDDEN_SPACE) const ctx = makeCtx(makeAccount(AccountRole.Guest)) @@ -364,6 +368,7 @@ describe('GuestPermissionsMiddleware', () => { if (b === core.class.Space) return false return a === b } + ;(mw as any).context.hierarchy.getAncestors = () => [] } it('allows guest to update document created by same account', async () => { @@ -468,6 +473,7 @@ describe('GuestPermissionsMiddleware', () => { return a === b } ;(mw as any).context.hierarchy.classHierarchyMixin = () => undefined + ;(mw as any).context.hierarchy.getAncestors = () => [] // First tx as guest should load cache const userCtx = makeCtx(makeAccount(AccountRole.User)) @@ -488,4 +494,66 @@ describe('GuestPermissionsMiddleware', () => { expect((mw as any).permissionsCache).toBeUndefined() }) }) + + // ─── collab-only guest veto: fail-closed (L-GP) + covers remove (L-RM) ──────── + describe('collab-only guest veto (mention-grants opt-in)', () => { + const ISSUE_CLASS = 'test:class:Issue' as Ref> + const ISSUE_SPACE = 'test:space:Issue' as Ref + + // Build a middleware whose model opts ISSUE_CLASS into mention-grants + // (provideSecurity + mentionsGrantAccess). `space` controls what findAll + // returns for the doc's space (undefined => unresolvable). + function makeCollabMw (space: Space | undefined): GuestPermissionsMiddleware { + const mw = makeMiddleware(async (_ctx, _class) => (space !== undefined ? [space] : [])) + ;(mw as any).context.hierarchy.getAncestors = (id: any) => [id] + ;(mw as any).context.hierarchy.isDerived = (a: any, b: any) => a === b + ;(mw as any).context.hierarchy.classHierarchyMixin = () => undefined + ;(mw as any).context.modelDb = { + findAllSync: (_class: any, _q: any) => [ + { attachedTo: ISSUE_CLASS, provideSecurity: true, mentionsGrantAccess: true } + ] + } + return mw + } + + function makeSpace (members: any[]): Space { + return { _id: ISSUE_SPACE, members } as any + } + + it('L-GP: fail-closed — unresolvable space forbids a guest field update', async () => { + const mw = makeCollabMw(undefined) + const guest = makeAccount(AccountRole.Guest) + const factory = new TxFactory('test:account:System' as PersonId) + const tx = factory.createTxUpdateDoc(ISSUE_CLASS, ISSUE_SPACE, generateId() as any, {}) + const forbidden = await (mw as any).isForbiddenCollabOnlyGuestFieldUpdate(makeCtx(guest), tx, guest) + expect(forbidden).toBe(true) + }) + + it('L-RM: collab-only guest TxRemoveDoc on opted-in doc is forbidden', async () => { + const guest = makeAccount(AccountRole.Guest) + const mw = makeCollabMw(makeSpace([])) // guest is NOT a space member + const factory = new TxFactory('test:account:System' as PersonId) + const tx = factory.createTxRemoveDoc(ISSUE_CLASS, ISSUE_SPACE, generateId() as any) + const forbidden = await (mw as any).isForbiddenCollabOnlyGuestFieldUpdate(makeCtx(guest), tx, guest) + expect(forbidden).toBe(true) + }) + + it('space-member guest is NOT vetoed (update stays allowed)', async () => { + const guest = makeAccount(AccountRole.Guest) + const mw = makeCollabMw(makeSpace([guest.uuid])) + const factory = new TxFactory('test:account:System' as PersonId) + const tx = factory.createTxUpdateDoc(ISSUE_CLASS, ISSUE_SPACE, generateId() as any, {}) + const forbidden = await (mw as any).isForbiddenCollabOnlyGuestFieldUpdate(makeCtx(guest), tx, guest) + expect(forbidden).toBe(false) + }) + + it('non-guest (User) TxRemoveDoc passes the veto', async () => { + const user = makeAccount(AccountRole.User) + const mw = makeCollabMw(makeSpace([])) + const factory = new TxFactory('test:account:System' as PersonId) + const tx = factory.createTxRemoveDoc(ISSUE_CLASS, ISSUE_SPACE, generateId() as any) + const forbidden = await (mw as any).isForbiddenCollabOnlyGuestFieldUpdate(makeCtx(user), tx, user) + expect(forbidden).toBe(false) + }) + }) }) diff --git a/foundations/server/packages/postgres/src/storage.ts b/foundations/server/packages/postgres/src/storage.ts index c2fec296227..be24d59098d 100644 --- a/foundations/server/packages/postgres/src/storage.ts +++ b/foundations/server/packages/postgres/src/storage.ts @@ -210,6 +210,11 @@ class ValuesVariables { } abstract class PostgresAdapterBase implements DbAdapter { + // Postgres enforces collaborator-grant read security via addSecurity's collab + // OR-branch (see getSecurityClause below), so the space-security middleware may + // safely drop the space filter for collab-read bypasses on this backend. + readonly supportsCollaboratorSecurity = true + protected readonly _helper: DBCollectionHelper protected readonly tableFields = new Map() @@ -647,6 +652,24 @@ abstract class PostgresAdapterBase implements DbAdapter { collabRes += ` OR EXISTS (SELECT 1 FROM ${translateDomain(DOMAIN_COLLABORATOR)} collab_sec WHERE collab_sec."workspaceId" = ${vars.add(this.workspaceId, '::uuid')} AND collab_sec."attachedTo" = ${domain}."attachedTo" AND collab_sec.collaborator = '${acc.uuid}')` } } + // Self-Collaborator visibility: any non-Admin/non-System caller can always read + // their own Collaborator records, regardless of space membership. Without this, + // a Guest who is a Collaborator on a doc in a project they are not a member of + // could never enumerate their own subscriptions (e.g. the tracker "Subscribed" + // tab queries Collaborator by `{collaborator: self}`). + if (domain === DOMAIN_COLLABORATOR) { + collabRes += ` OR ${domain}.collaborator = '${acc.uuid}'` + } + // Containing-Space visibility for collab-only Guests: surface Spaces that + // host docs the caller is a Collaborator on. Required for the project/space + // nav tree to list such projects (and for any code resolving the doc's + // parent space to succeed). The Collaborator record's `space` field always + // mirrors the parent doc's `space`, so existence of any such record naming + // the caller is sufficient evidence that the Space contains something they + // can see. + if (domain === DOMAIN_SPACE && [AccountRole.Guest, AccountRole.ReadOnlyGuest].includes(acc.role)) { + collabRes += ` OR EXISTS (SELECT 1 FROM ${translateDomain(DOMAIN_COLLABORATOR)} space_collab WHERE space_collab."workspaceId" = ${vars.add(this.workspaceId, '::uuid')} AND space_collab.space = ${domain}._id AND space_collab.collaborator = '${acc.uuid}')` + } return `AND (${res}${collabRes})` } } diff --git a/models/core/src/core.ts b/models/core/src/core.ts index d6f4588a905..310a17f3dce 100644 --- a/models/core/src/core.ts +++ b/models/core/src/core.ts @@ -438,6 +438,7 @@ export class TClassCollaborators extends TDoc implements ClassCollaborators fields!: (keyof Doc)[] provideSecurity?: boolean provideAttachedSecurity?: boolean + mentionsGrantAccess?: boolean } @Model(core.class.Collaborator, core.class.Doc, DOMAIN_COLLABORATOR) diff --git a/models/tracker/src/index.ts b/models/tracker/src/index.ts index baabea1381a..1296c8c1f17 100644 --- a/models/tracker/src/index.ts +++ b/models/tracker/src/index.ts @@ -523,7 +523,17 @@ export function createModel (builder: Builder): void { builder.createDoc>(core.class.ClassCollaborators, core.space.Model, { attachedTo: tracker.class.Issue, - fields: ['createdBy', 'assignee'] + fields: ['createdBy', 'assignee'], + // Collaborator status grants read visibility on the issue, bypassing + // project-space membership. Used so a user @-mentioned on an issue + // they are not a project member of can actually see and comment on it. + provideSecurity: true, + // @-mentions in chat/activity messages on an issue auto-create a + // Collaborator record (server-plugins/chunter-resources). Combined + // with provideSecurity above, the mentioned user gets explicit, + // disclosed read+comment access. Field writes remain blocked for + // collab-only guests by the GuestPermissions middleware veto. + mentionsGrantAccess: true }) builder.mixin(tracker.class.Issue, core.class.Class, setting.mixin.Editable, { diff --git a/plugins/chunter-resources/package.json b/plugins/chunter-resources/package.json index 05123872f19..ff2c420eb37 100644 --- a/plugins/chunter-resources/package.json +++ b/plugins/chunter-resources/package.json @@ -57,6 +57,7 @@ "@hcengineering/preference": "workspace:^0.7.0", "@hcengineering/presentation": "workspace:^0.7.0", "@hcengineering/text": "workspace:^0.7.19", + "@hcengineering/text-core": "workspace:^0.7.19", "@hcengineering/ui": "workspace:^0.7.0", "@hcengineering/view": "workspace:^0.7.0", "@hcengineering/view-resources": "workspace:^0.7.0", diff --git a/plugins/chunter-resources/src/__tests__/mentionGrants.test.ts b/plugins/chunter-resources/src/__tests__/mentionGrants.test.ts new file mode 100644 index 00000000000..bbf5f5eab9f --- /dev/null +++ b/plugins/chunter-resources/src/__tests__/mentionGrants.test.ts @@ -0,0 +1,50 @@ +import { markupToJSON, jsonToMarkup, type MarkupNode, MarkupNodeType } from '@hcengineering/text-core' +import { applyMentionGrantChoices } from '../mentionGrants' + +function refNode (id: string): MarkupNode { + return { type: MarkupNodeType.reference, attrs: { id, label: id, objectclass: 'contact:class:Person' } } +} +function docMarkup (...refs: MarkupNode[]): string { + return jsonToMarkup({ type: MarkupNodeType.doc, content: refs }) +} +function grantsOf (markup: string, id: string): Array<'true' | 'false' | undefined> { + const node = markupToJSON(markup) + const out: Array<'true' | 'false' | undefined> = [] + const walk = (n: MarkupNode): void => { + if (n.type === MarkupNodeType.reference && n.attrs?.id === id) { + out.push(n.attrs.grantsAccess as 'true' | 'false' | undefined) + } + n.content?.forEach(walk) + } + walk(node) + return out +} + +describe('applyMentionGrantChoices', () => { + test('deselect sets grantsAccess=false on the person reference', () => { + const result = applyMentionGrantChoices(docMarkup(refNode('p1')), new Map([['p1', false]])) + expect(grantsOf(result, 'p1')).toEqual(['false']) + }) + + test('select sets grantsAccess=true explicitly', () => { + const result = applyMentionGrantChoices(docMarkup(refNode('p1')), new Map([['p1', true]])) + expect(grantsOf(result, 'p1')).toEqual(['true']) + }) + + test('deselect rewrites EVERY reference to that person', () => { + const result = applyMentionGrantChoices(docMarkup(refNode('p1'), refNode('p1')), new Map([['p1', false]])) + expect(grantsOf(result, 'p1')).toEqual(['false', 'false']) + }) + + test('persons absent from the choice map are left unchanged', () => { + const result = applyMentionGrantChoices(docMarkup(refNode('p1'), refNode('p2')), new Map([['p1', false]])) + expect(grantsOf(result, 'p1')).toEqual(['false']) + expect(grantsOf(result, 'p2')).toEqual([undefined]) + }) + + test('empty choice map is a no-op', () => { + const markup = docMarkup(refNode('p1')) + const result = applyMentionGrantChoices(markup, new Map()) + expect(grantsOf(result, 'p1')).toEqual([undefined]) + }) +}) diff --git a/plugins/chunter-resources/src/components/chat-message/ChatMessageInput.svelte b/plugins/chunter-resources/src/components/chat-message/ChatMessageInput.svelte index 4f9c51ed2c4..328ebf56d54 100644 --- a/plugins/chunter-resources/src/components/chat-message/ChatMessageInput.svelte +++ b/plugins/chunter-resources/src/components/chat-message/ChatMessageInput.svelte @@ -17,16 +17,30 @@ import { Analytics } from '@hcengineering/analytics' import { AttachmentRefInput } from '@hcengineering/attachment-resources' import chunter, { ChatMessage, ChunterEvents, ThreadMessage } from '@hcengineering/chunter' - import { Class, Doc, generateId, getCurrentAccount, Ref, type CommitResult } from '@hcengineering/core' + import contact, { type Person } from '@hcengineering/contact' + import core, { + type AccountUuid, + Class, + Doc, + generateId, + getCurrentAccount, + Ref, + resolveMentionGrantTarget, + type Space, + type CommitResult + } from '@hcengineering/core' import { createQuery, DraftController, draftsStore, getClient } from '@hcengineering/presentation' - import { EmptyMarkup, isEmptyMarkup } from '@hcengineering/text' + import { EmptyMarkup, isEmptyMarkup, markupToJSON } from '@hcengineering/text' + import { extractReferences } from '@hcengineering/text-core' import { createEventDispatcher } from 'svelte' import { getObjectId } from '@hcengineering/view-resources' - import { ThrottledCaller } from '@hcengineering/ui' + import { showPopup, ThrottledCaller } from '@hcengineering/ui' import { getSpace, editingMessageStore } from '@hcengineering/activity-resources' import { getChannelSpace } from '../../utils' + import { applyMentionGrantChoices } from '../../mentionGrants' import ChannelTypingInfo from '../ChannelTypingInfo.svelte' + import MentionGrantConfirm from './MentionGrantConfirm.svelte' export let object: Doc export let chatMessage: ChatMessage | undefined = undefined @@ -126,47 +140,123 @@ currentMessage.attachments = attachments } - async function handleCreate (event: CustomEvent, _id: Ref): Promise { + /** + * Disclosure UX for the mention-grants-access flow. Resolves the set of NEW + * grantees (mentioned Persons not already in the grant-target space) and asks + * the actor to confirm/deselect each. Returns: + * - null => cancel (do not send) + * - Map => send; map is personId -> grant choice (true/false). Empty map + * when there is nothing to disclose (send unchanged). + * The actual access grant happens server-side via the chunter trigger; this + * dialog only discloses + lets the actor opt specific people out. + */ + async function resolveMentionGrantChoices (markup: string): Promise | null> { + if (markup === undefined || markup === '' || isEmptyMarkup(markup)) return new Map() + let node try { + node = markupToJSON(markup) + } catch { + return new Map() + } + const references = extractReferences(node) + const mentionedPersonIds = references + .filter(({ objectClass }) => hierarchy.isDerived(objectClass, contact.class.Person)) + .filter(({ grantsAccess }) => grantsAccess !== 'false') // V3c: already-denied refs need no disclosure + .map(({ objectId }) => objectId as Ref) + if (mentionedPersonIds.length === 0) return new Map() + + const grantTarget = await resolveMentionGrantTarget(object, (cls, q, o) => client.findAll(cls, q, o)) + if (grantTarget == null) return new Map() + + const space = (await client.findAll(core.class.Space, { _id: grantTarget.space }))[0] + if (space === undefined) return new Map() + const members = new Set(space.members ?? []) + + const persons = await client.findAll(contact.class.Person, { _id: { $in: mentionedPersonIds } }) + const newGrantees = persons.filter((p) => p.personUuid != null && !members.has(p.personUuid as AccountUuid)) + if (newGrantees.length === 0) return new Map() + + const targetName: string = (grantTarget as any).name ?? (grantTarget as any).title ?? grantTarget._id + const spaceName: string = (space as any).name ?? space._id + const grantees = newGrantees.map((p) => ({ id: p._id, name: p.name ?? String(p.personUuid) })) + + return await new Promise | null>((resolve) => { + showPopup(MentionGrantConfirm, { grantees, targetName, spaceName }, undefined, (res?: Map) => { + resolve(res instanceof Map ? res : null) + }) + }) + } + + // Runs the mention-grants disclosure for an outgoing/edited message and + // rewrites event.detail.message with the actor's per-grantee choices. + // Returns false if the actor cancelled (caller must abort the send). + async function prepareMentionGrantChoices (event: CustomEvent): Promise { + const markup = event.detail?.message + const choices = await resolveMentionGrantChoices(typeof markup === 'string' ? markup : '') + if (choices === null) return false // actor cancelled + if (choices.size > 0 && typeof markup === 'string') { + event.detail.message = applyMentionGrantChoices(markup, choices) + } + return true + } + + // Returns true if the message was sent, false if the actor cancelled the + // grant dialog or the send failed. The caller (onMessage) must only clear + // the draft/input on a true result, otherwise a cancelled grant dialog + // would lose the unsent comment. + async function handleCreate (event: CustomEvent, _id: Ref): Promise { + try { + if (!(await prepareMentionGrantChoices(event))) return false + const res = await createMessage(event, _id, `chunter.create.${_class} ${object._class}`) console.log(`create.${_class} measure`, res.serverTime, res.time) const objectId = await getObjectId(object, client.getHierarchy()) Analytics.handleEvent(ChunterEvents.MessageCreated, { ok: res.result, objectId, objectClass: object._class }) + return true } catch (err: any) { const objectId = await getObjectId(object, client.getHierarchy()) Analytics.handleEvent(ChunterEvents.MessageCreated, { ok: false, objectId, objectClass: object._class }) Analytics.handleError(err) + return false } } - async function handleEdit (event: CustomEvent): Promise { + async function handleEdit (event: CustomEvent): Promise { try { + if (!(await prepareMentionGrantChoices(event))) return false + await editMessage(event) const objectId = await getObjectId(object, client.getHierarchy()) Analytics.handleEvent(ChunterEvents.MessageEdited, { ok: true, objectId, objectClass: object._class }) + return true } catch (err: any) { const objectId = await getObjectId(object, client.getHierarchy()) Analytics.handleEvent(ChunterEvents.MessageEdited, { ok: false, objectId, objectClass: object._class }) Analytics.handleError(err) + return false } } async function onMessage (event: CustomEvent): Promise { - draftController.remove() - inputRef.removeDraft(false) + loading = true + let ok = false if (chatMessage !== undefined) { - loading = true - await handleEdit(event) + ok = await handleEdit(event) } else { - void handleCreate(event, _id) + ok = await handleCreate(event, _id) void deleteTypingInfo() } - // Remove draft from Local Storage - clear() - dispatch('submit', false) + // Only clear the input / drop the draft after a successful send or edit. + // A cancelled grant dialog (ok === false) must preserve the unsent comment. + if (ok) { + draftController.remove() + inputRef.removeDraft(false) + clear() + dispatch('submit', false) + } loading = false } diff --git a/plugins/chunter-resources/src/components/chat-message/MentionGrantConfirm.svelte b/plugins/chunter-resources/src/components/chat-message/MentionGrantConfirm.svelte new file mode 100644 index 00000000000..fa5e29a8fd1 --- /dev/null +++ b/plugins/chunter-resources/src/components/chat-message/MentionGrantConfirm.svelte @@ -0,0 +1,68 @@ + + + +
+
+
+
+
+ {#each rows as row (row.id)} +
+ + {row.name} +
+ {/each} +
+
+
+ + + diff --git a/plugins/chunter-resources/src/mentionGrants.ts b/plugins/chunter-resources/src/mentionGrants.ts new file mode 100644 index 00000000000..e8056e3661c --- /dev/null +++ b/plugins/chunter-resources/src/mentionGrants.ts @@ -0,0 +1,50 @@ +// +// Copyright © 2026 Hardcore Engineering Inc. +// +// Licensed under the Eclipse Public License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. You may +// obtain a copy of the License at https://www.eclipse.org/legal/epl-2.0 +// + +import { markupToJSON, jsonToMarkup, type MarkupNode, MarkupNodeType } from '@hcengineering/text-core' +import { type Markup } from '@hcengineering/core' + +/** + * Rewrite the grantsAccess attribute on mention references in a message's + * markup according to per-person choices made in the send-time disclosure. + * + * - choice === false -> set grantsAccess='false' on EVERY reference to that + * person. Setting all of them is required because extractReferences merges + * any-wins: a single remaining undefined reference would re-grant access. + * - choice === true -> set grantsAccess='true' explicitly. + * - person not in the map -> left unchanged (e.g. existing space members, + * whose mentions stay undefined and grant-by-default as before). + * + * Pure function: returns a new markup string; does not mutate its input + * string (it parses, mutates the parsed tree, and re-serializes). + */ +export function applyMentionGrantChoices (markup: Markup, choices: Map): Markup { + if (choices.size === 0) return markup + let node: MarkupNode + try { + node = markupToJSON(markup) + } catch { + return markup + } + + const walk = (n: MarkupNode): void => { + // Use `!= null` to guard against both `undefined` AND `null` — a defensive + // attrs == null on a reference node would otherwise throw on n.attrs.id. + if (n.type === MarkupNodeType.reference && n.attrs != null) { + const id = n.attrs.id as string + const choice = choices.get(id) + if (choice !== undefined) { + n.attrs.grantsAccess = choice ? 'true' : 'false' + } + } + n.content?.forEach(walk) + } + walk(node) + + return jsonToMarkup(node) +} diff --git a/plugins/tracker-assets/lang/de.json b/plugins/tracker-assets/lang/de.json index d1e698c9c9a..130d8a89ee3 100644 --- a/plugins/tracker-assets/lang/de.json +++ b/plugins/tracker-assets/lang/de.json @@ -293,7 +293,8 @@ "UnsetParentIssue": "Übergeordnete Aufgabe entfernen", "ForbidCreateProjectPermission": "Projekterstellung verbieten", "ForbidCreateProjectPermissionDescription": "Verbietet Benutzern das Erstellen neuer Projekte", - "AllowCreatingIssues": "Erstellen von Aufgaben erlauben" + "AllowCreatingIssues": "Erstellen von Aufgaben erlauben", + "SharedWithYouTooltip": "Du hast nur Zugriff auf einzelne Issues; du bist kein Mitglied dieses Projekts" }, "status": {} } diff --git a/plugins/tracker-assets/lang/en.json b/plugins/tracker-assets/lang/en.json index 3d856a20031..a4b92987f92 100644 --- a/plugins/tracker-assets/lang/en.json +++ b/plugins/tracker-assets/lang/en.json @@ -293,7 +293,8 @@ "UnsetParentIssue": "Unset parent issue", "ForbidCreateProjectPermission": "Forbid create project", "ForbidCreateProjectPermissionDescription": "Forbid users creating new projects", - "AllowCreatingIssues": "Allow creating issues" + "AllowCreatingIssues": "Allow creating issues", + "SharedWithYouTooltip": "You have access to specific issues only; you are not a member of this project" }, "status": {} } diff --git a/plugins/tracker-assets/lang/ru.json b/plugins/tracker-assets/lang/ru.json index 7ccd4cb5ccd..7761662a737 100644 --- a/plugins/tracker-assets/lang/ru.json +++ b/plugins/tracker-assets/lang/ru.json @@ -293,7 +293,8 @@ "UnsetParentIssue": "Снять родительскую задачу", "ForbidCreateProjectPermission": "Запретить создание проекта", "ForbidCreateProjectPermissionDescription": "Запрещает пользователям создавать новые проекты", - "AllowCreatingIssues": "Разрешить создание задач" + "AllowCreatingIssues": "Разрешить создание задач", + "SharedWithYouTooltip": "У вас есть доступ только к отдельным задачам; вы не являетесь участником этого проекта" }, "status": {} } diff --git a/plugins/tracker-resources/src/components/issues/edit/EditIssue.svelte b/plugins/tracker-resources/src/components/issues/edit/EditIssue.svelte index bf7de0a5db4..2633a1d21ec 100644 --- a/plugins/tracker-resources/src/components/issues/edit/EditIssue.svelte +++ b/plugins/tracker-resources/src/components/issues/edit/EditIssue.svelte @@ -47,7 +47,7 @@ import { createEventDispatcher, onDestroy } from 'svelte' import { generateIssueShortLink, getIssueIdByIdentifier } from '../../../issues' - import { canEditIssue } from '../../../utils' + import { canCommentOnIssue, canEditIssueFields } from '../../../utils' import tracker from '../../../plugin' import IssueStatusActivity from '../IssueStatusActivity.svelte' import ControlPanel from './ControlPanel.svelte' @@ -73,16 +73,29 @@ let descriptionBox: AttachmentStyleBoxCollabEditor let showAllMixins: boolean + // Two independent gates: + // effectiveReadonly — Issue field editors (title, status, dates, ...). + // Driven by canEditIssueFields(). Guests are blocked. + // canComment — Comment composer at the bottom of the panel. + // Driven by canCommentOnIssue(). Guests who are + // Creator OR Collaborator on the issue are allowed. let effectiveReadonly = true + let canComment = false $: if (issue !== undefined) { const currentIssue = issue - void canEditIssue(currentIssue).then((canEdit) => { + void canEditIssueFields(currentIssue).then((canEdit) => { if (issue === currentIssue) { effectiveReadonly = readonly || !canEdit } }) + void canCommentOnIssue(currentIssue).then((v) => { + if (issue === currentIssue) { + canComment = !readonly && v + } + }) } else { effectiveReadonly = readonly + canComment = false } const inboxClient = InboxNotificationsClientImpl.getClient() @@ -211,7 +224,7 @@ {#if specials} - getActions(space)} - {forciblyСollapsed} - > - {#each specials as special} - - - - {/each} - - - {#if visible} - {@const item = specials.find((sp) => sp.id === currentSpecial && currentSpace === space._id)} - {#if item} - - + {#if isCollabOnlyProject} +
+ getActions(space)} + {forciblyСollapsed} + > + {#each specials as special} + + + {/each} + + + {#if visible} + {@const item = specials.find((sp) => sp.id === currentSpecial && currentSpace === space._id)} + {#if item} + + + + {/if} + {/if} + + +
+ {:else} + getActions(space)} + {forciblyСollapsed} + > + {#each specials as special} + + + + {/each} + + + {#if visible} + {@const item = specials.find((sp) => sp.id === currentSpecial && currentSpace === space._id)} + {#if item} + + + + {/if} {/if} - {/if} - - +
+
+ {/if} {/if} diff --git a/plugins/tracker-resources/src/index.ts b/plugins/tracker-resources/src/index.ts index 071a13ac73f..0f0a7cf1b72 100644 --- a/plugins/tracker-resources/src/index.ts +++ b/plugins/tracker-resources/src/index.ts @@ -14,7 +14,7 @@ // import { Analytics } from '@hcengineering/analytics' -import { +import core, { type Attribute, type Class, type Client, @@ -517,7 +517,19 @@ export default async (): Promise => ({ ) => await getAllStates(query, onUpdate, queryId, attr, false), GetVisibleFilters: getVisibleFilters, IssueChatTitleProvider: getIssueChatTitle, - IsProjectJoined: async (project: Project) => project.members.includes(getCurrentAccount().uuid), + IsProjectJoined: async (project: Project) => { + const accountUuid = getCurrentAccount().uuid + if (project.members.includes(accountUuid)) return true + // Collab-only access: surface projects in the nav tree when the user is a + // Collaborator on any doc inside this project, even without space-membership. + // Backend security (postgres collabRes OR-branch + middleware bypass) limits + // what the user can actually open within the project. + const collab = await getClient().findOne(core.class.Collaborator, { + collaborator: accountUuid, + space: project._id + }) + return collab !== undefined + }, GetIssueStatusCategories: getIssueStatusCategories, SetComponentStore: setStore, ComponentFilterFunction: filterComponents, diff --git a/plugins/tracker-resources/src/plugin.ts b/plugins/tracker-resources/src/plugin.ts index eed088c59d5..dadcd1249b6 100644 --- a/plugins/tracker-resources/src/plugin.ts +++ b/plugins/tracker-resources/src/plugin.ts @@ -307,7 +307,8 @@ export default mergeIds(trackerId, tracker, { UnsetParent: '' as IntlString, PreviousAssigned: '' as IntlString, EditRelatedTargets: '' as IntlString, - RelatedIssueTargetDescription: '' as IntlString + RelatedIssueTargetDescription: '' as IntlString, + SharedWithYouTooltip: '' as IntlString }, component: { NopeComponent: '' as AnyComponent, diff --git a/plugins/tracker-resources/src/utils.ts b/plugins/tracker-resources/src/utils.ts index c0afa1ed7bc..ef149701f6e 100644 --- a/plugins/tracker-resources/src/utils.ts +++ b/plugins/tracker-resources/src/utils.ts @@ -277,8 +277,21 @@ export async function moveIssuesToAnotherMilestone ( } } -export async function canEditIssue (issue?: Issue | WithLookup): Promise { - const client = getClient() +/** + * True when the current user may edit Issue fields (title, description, + * status, dates, labels, assignee, dependencies, etc). + * + * Guest-tier accounts are blocked unconditionally — even when listed as + * Collaborator on the issue. Their Collaborator status grants read + + * comment via {@link canCommentOnIssue}, but not field-edit. This matches + * the server-side veto in the GuestPermissions middleware so the UI + * doesnt show editors that would fail on submit. + * + * Project-member guests (e.g. a guest who is a member of their own + * project) are also blocked here; if you need to allow them to + * edit issues they own, promote them to AccountRole.User+. + */ +export async function canEditIssueFields (issue?: Issue | WithLookup): Promise { if (issue === undefined) return false const account = getCurrentAccount() @@ -286,14 +299,33 @@ export async function canEditIssue (issue?: Issue | WithLookup): Promise< account.role === AccountRole.Guest || account.role === AccountRole.DocGuest || account.role === AccountRole.ReadOnlyGuest + return !isGuest +} + +/** + * True when the current user may post comments on the Issue. + * + * - User+ accounts always pass. + * - ReadOnlyGuest is always blocked. + * - Other guests pass if they are the Issues creator OR they are listed + * as Collaborator on the issue (auto-added via @-mention by the + * chunter trigger when mentionsGrantAccess is set on the class, or + * added manually via the Collaborators editor in the panel). + */ +export async function canCommentOnIssue (issue?: Issue | WithLookup): Promise { + if (issue === undefined) return false + const account = getCurrentAccount() + if (account.role === AccountRole.ReadOnlyGuest) return false + + const isGuest = account.role === AccountRole.Guest || account.role === AccountRole.DocGuest if (!isGuest) return true const isCreator = issue.createdBy !== undefined && Array.isArray(account.socialIds) && account.socialIds.includes(issue.createdBy) - if (isCreator) return true + const client = getClient() const collaborator = await client.findOne(core.class.Collaborator, { attachedTo: issue._id, collaborator: account.uuid @@ -301,6 +333,13 @@ export async function canEditIssue (issue?: Issue | WithLookup): Promise< return collaborator !== undefined } +/** + * Backwards-compat alias for callers that have not yet migrated to + * {@link canEditIssueFields}. New code should use the explicit split. + * Tag for removal in a follow-up sweep across tracker-resources. + */ +export const canEditIssue = canEditIssueFields + export function getTimeReportDate (type: TimeReportDayType): number { const date = new Date(Date.now()) diff --git a/plugins/workbench-resources/src/components/Navigator.svelte b/plugins/workbench-resources/src/components/Navigator.svelte index ca09ae04e37..21aab9d8cc2 100644 --- a/plugins/workbench-resources/src/components/Navigator.svelte +++ b/plugins/workbench-resources/src/components/Navigator.svelte @@ -13,7 +13,7 @@ // limitations under the License. -->