diff --git a/common/config/rush/pnpm-lock.yaml b/common/config/rush/pnpm-lock.yaml index 5437a5f45b7..d34c2e02052 100644 --- a/common/config/rush/pnpm-lock.yaml +++ b/common/config/rush/pnpm-lock.yaml @@ -18050,6 +18050,9 @@ importers: '@hcengineering/text': specifier: workspace:^0.7.19 version: link:../../foundations/core/packages/text + '@hcengineering/text-core': + specifier: workspace:^0.7.0 + version: link:../../foundations/core/packages/text-core '@hcengineering/text-editor': specifier: workspace:^0.7.0 version: link:../text-editor diff --git a/foundations/core/packages/core/src/classes.ts b/foundations/core/packages/core/src/classes.ts index 0cd0c5080be..b8ab76903f7 100644 --- a/foundations/core/packages/core/src/classes.ts +++ b/foundations/core/packages/core/src/classes.ts @@ -1008,8 +1008,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..775bf116792 100644 --- a/foundations/core/packages/core/src/collaborators.ts +++ b/foundations/core/packages/core/src/collaborators.ts @@ -13,7 +13,16 @@ // limitations under the License. // -import core, { Class, ClassCollaborators, Doc, Hierarchy, ModelDb, Ref } from '.' +import core, { + AttachedDoc, + Class, + ClassCollaborators, + Doc, + DocumentQuery, + Hierarchy, + ModelDb, + Ref +} from '.' export function getClassCollaborators ( model: ModelDb, @@ -35,3 +44,49 @@ 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) => control.findAll(control.ctx, cls, q)`, client passes + * `(cls, q) => getClient().findAll(cls, q)`. + * + * 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) => Promise +): Promise { + let cur: Doc | undefined = start + for (let i = 0; i < 8 && cur != null; i++) { + const cc = (await findAll(core.class.ClassCollaborators, { + attachedTo: cur._class + } as DocumentQuery>))[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 parent = (await findAll(attached.attachedToClass, { + _id: attached.attachedTo + } as DocumentQuery))[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..21479d166c2 --- /dev/null +++ b/foundations/core/packages/text-core/src/markup/__tests__/reference.test.ts @@ -0,0 +1,80 @@ +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 613a51eca10..9bd547081d8 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' } export interface ReferenceOptions { @@ -42,7 +43,18 @@ export const ReferenceNode = Node.create({ return { id: getDataAttribute('id'), objectclass: getDataAttribute('objectclass'), - label: getDataAttribute('label') + 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 } + } } }, @@ -77,6 +89,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, @@ -96,9 +113,12 @@ function getAttrs (el: HTMLSpanElement): Attrs | false { return false } + const grantsAccess = el.dataset.grantsAccess + return { id, label, - objectclass + objectclass, + ...(grantsAccess !== undefined ? { grantsAccess } : {}) } } diff --git a/foundations/server/packages/middleware/src/guestPermissions.ts b/foundations/server/packages/middleware/src/guestPermissions.ts index 077d546a84d..31415c5b786 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,56 @@ 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. 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 { + if (cudTx._class !== core.class.TxUpdateDoc) 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] + if (space === undefined) return false + if (space.members?.includes(account.uuid) === true) 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..2676b6e1590 100644 --- a/foundations/server/packages/middleware/src/spaceSecurity.ts +++ b/foundations/server/packages/middleware/src/spaceSecurity.ts @@ -630,7 +630,40 @@ 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 + const collabReadBypass = + (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 = 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 = + 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/guestPermissions.test.ts b/foundations/server/packages/middleware/src/tests/guestPermissions.test.ts index e124aee8cbe..2d9c09cb45e 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)) diff --git a/foundations/server/packages/postgres/src/storage.ts b/foundations/server/packages/postgres/src/storage.ts index c2fec296227..4ada23e60df 100644 --- a/foundations/server/packages/postgres/src/storage.ts +++ b/foundations/server/packages/postgres/src/storage.ts @@ -647,6 +647,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 f0feb131665..0628d01ca5e 100644 --- a/models/core/src/core.ts +++ b/models/core/src/core.ts @@ -437,6 +437,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 44d1b97b506..fe5230340b3 100644 --- a/models/tracker/src/index.ts +++ b/models/tracker/src/index.ts @@ -521,7 +521,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..64a71cfcf4a 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.0", "@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..a7238d3a82f 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,128 @@ 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) => client.findAll(cls, q)) + 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..bed99407fee --- /dev/null +++ b/plugins/chunter-resources/src/components/chat-message/MentionGrantConfirm.svelte @@ -0,0 +1,72 @@ + + + +
+
+
+
+
+ {#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 99fe946c9d4..c34a62280b5 100644 --- a/plugins/tracker-assets/lang/de.json +++ b/plugins/tracker-assets/lang/de.json @@ -290,7 +290,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 4345dc5a0aa..09ca69d8ea8 100644 --- a/plugins/tracker-assets/lang/en.json +++ b/plugins/tracker-assets/lang/en.json @@ -290,7 +290,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-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..08f6765007c 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 his own + * Ostrowo 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,35 @@ 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 +335,14 @@ 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..34925c73a6e 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. -->