From 005980c0a18239bfde3a6fb61ca907e27b4315b1 Mon Sep 17 00:00:00 2001 From: Artyom Savchenko Date: Mon, 13 Apr 2026 22:23:18 +0700 Subject: [PATCH] Revert "Hide meeting minutes from guests (#10737)" (#10763) This reverts commit 28b513411dddaba7fbb05c0f6787a5b8264aff21. --- dev/tool/src/index.ts | 24 +-- .../packages/middleware/src/spaceSecurity.ts | 53 +----- .../src/tests/spaceSecurity.test.ts | 172 ------------------ ws-tests/api-tests/src/__tests__/rest.test.ts | 101 ---------- ws-tests/api-tests/src/loveApiRefs.ts | 17 -- ws-tests/prepare.sh | 5 - 6 files changed, 3 insertions(+), 369 deletions(-) delete mode 100644 foundations/server/packages/middleware/src/tests/spaceSecurity.test.ts delete mode 100644 ws-tests/api-tests/src/loveApiRefs.ts diff --git a/dev/tool/src/index.ts b/dev/tool/src/index.ts index d624a32500..56acb52fea 100644 --- a/dev/tool/src/index.ts +++ b/dev/tool/src/index.ts @@ -331,28 +331,10 @@ export function devTool ( // }) // }) - function parseAccountRole (raw: unknown): AccountRole { - const rawRole = typeof raw === 'string' ? raw.trim() : String(raw ?? 'User').trim() - const normalized = rawRole.trim().toLowerCase() - - const match = Object.values(AccountRole).find((v) => v.toLowerCase() === normalized) - if (match !== undefined) return match - - switch (normalized) { - case 'readonly': - return AccountRole.ReadOnlyGuest - case 'docguest': - return AccountRole.DocGuest - default: - throw new Error(`Unknown role: ${rawRole}`) - } - } - program .command('assign-workspace ') .description('assign workspace') - .option('--role ', 'Workspace role (User, Guest, ReadOnlyGuest, DocGuest, Maintainer, Owner, Admin)', 'User') - .action(async (email: string, workspace: string, cmd: { role: string }) => { + .action(async (email: string, workspace: string, cmd) => { await withAccountDatabase(async (db) => { console.log(`assigning user ${email} to ${workspace}...`) try { @@ -361,12 +343,10 @@ export function devTool ( throw new Error(`Workspace ${workspace} not found`) } - const role = cmd.role != null ? parseAccountRole(cmd.role) : AccountRole.User - await assignWorkspace(toolCtx, db, null, getToolToken(), { email, workspaceUuid: ws.uuid, - role + role: AccountRole.User }) } catch (err: any) { console.error(err) diff --git a/foundations/server/packages/middleware/src/spaceSecurity.ts b/foundations/server/packages/middleware/src/spaceSecurity.ts index ff52d11a72..4c8bd4223e 100644 --- a/foundations/server/packages/middleware/src/spaceSecurity.ts +++ b/foundations/server/packages/middleware/src/spaceSecurity.ts @@ -613,54 +613,6 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar return domain === 'tx' ? 'objectSpace' : domain === 'space' ? '_id' : 'space' } - private mergeDocIdRestriction(query: DocumentQuery, allowed: Ref[]): DocumentQuery { - const allowedIds: DocumentQuery['_id'] = { $in: allowed.length === 0 ? [] : allowed } - const prevId = query._id - if (prevId === undefined) { - return { ...query, _id: allowedIds } - } - type WithAnd = DocumentQuery & { $and?: DocumentQuery[] } - const { _id: _drop, $and, ...rest } = query as WithAnd - const andParts: DocumentQuery[] = [...($and ?? []), { _id: prevId }, { _id: allowedIds }] - const merged: DocumentQuery = { ...rest, $and: andParts } - return merged - } - - private async applyGuestCollaboratorReadRestriction( - ctx: MeasureContext, - _class: Ref>, - domain: Domain, - query: DocumentQuery - ): Promise> { - const account = ctx.contextData.account - if ( - ![AccountRole.Guest, AccountRole.ReadOnlyGuest].includes(account.role) || - isSystem(account, ctx) || - domain === DOMAIN_MODEL - ) { - return query - } - - const collabSec = getClassCollaborators(this.context.modelDb, this.context.hierarchy, _class) - if (collabSec?.provideSecurity !== true) { - return query - } - - const rootClass = collabSec.attachedTo - const docClasses = [...this.context.hierarchy.getDescendants(rootClass), rootClass] - const collabs = (await this.provideFindAll( - ctx, - core.class.Collaborator, - { - collaborator: account.uuid, - attachedToClass: { $in: docClasses } - }, - { projection: { attachedTo: 1 }, limit: 10_000 } - )) as Collaborator[] - const allowed = collabs.map((c) => c.attachedTo) as Ref[] - return this.mergeDocIdRestriction(query, allowed) - } - override async findAll( ctx: MeasureContext, _class: Ref>, @@ -727,10 +679,7 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar } } - let queryToUse = !this.skipFindCheck ? newQuery : query - queryToUse = await this.applyGuestCollaboratorReadRestriction(ctx, _class, domain, queryToUse) - - let findResult = await this.provideFindAll(ctx, _class, queryToUse, options) + let findResult = await this.provideFindAll(ctx, _class, !this.skipFindCheck ? newQuery : query, options) if (clientFilterSpaces !== undefined) { const cfs = clientFilterSpaces findResult = toFindResult( diff --git a/foundations/server/packages/middleware/src/tests/spaceSecurity.test.ts b/foundations/server/packages/middleware/src/tests/spaceSecurity.test.ts deleted file mode 100644 index 62e45c76ec..0000000000 --- a/foundations/server/packages/middleware/src/tests/spaceSecurity.test.ts +++ /dev/null @@ -1,172 +0,0 @@ -// -// 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 -// -// 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. -// - -import core, { - AccountRole, - MeasureMetricsContext, - generateId, - toFindResult, - type Account, - type AccountUuid, - type Class, - type ClassCollaborators, - type Collaborator, - type Doc, - type Domain, - type MeasureContext, - type PersonId, - type Ref, - type SessionData -} from '@hcengineering/core' -import type { Middleware, PipelineContext } from '@hcengineering/server-core' -import { SpaceSecurityMiddleware } from '../spaceSecurity' - -const DOC_CLASS = 'test:class:Doc' as Ref> -const DOC_ID = 'test:doc:1' as Ref -const TEST_DOMAIN = 'test-domain' as Domain - -function makeAccount (role: AccountRole): Account { - return { - uuid: generateId() as unknown as AccountUuid, - role, - primarySocialId: 'test-social' as PersonId, - socialIds: ['test-social' as PersonId], - fullSocialIds: [] - } -} - -function makeCtx (account: Account): MeasureContext { - const ctx = new MeasureMetricsContext('test', {}) as MeasureContext - ctx.contextData = { - account, - broadcast: { txes: [], queue: [], sessions: {} }, - socialStringsToUsers: new Map(), - contextCache: new Map() - } as unknown as SessionData - return ctx -} - -function makeMiddleware ( - role: AccountRole, - opts: { provideSecurity: boolean } = { provideSecurity: false } -): { mw: SpaceSecurityMiddleware, account: Account, calls: Array<{ cls: Ref>, query: any }> } { - const account = makeAccount(role) - const calls: Array<{ cls: Ref>, query: any }> = [] - const collabMixin = { - _id: generateId(), - _class: core.class.ClassCollaborators, - space: core.space.Model, - attachedTo: DOC_CLASS, - fields: ['createdBy'], - provideSecurity: opts.provideSecurity, - modifiedOn: Date.now(), - modifiedBy: core.account.System - } as unknown as ClassCollaborators - - // Partial Hierarchy test double — SpaceSecurityMiddleware only needs these methods for this test. - // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- full Hierarchy is not needed here - const hierarchy = { - getDomain: (_class: Ref>) => TEST_DOMAIN, - isDerived: (_class: Ref>, base: Ref>) => - base === core.class.Space && _class === core.class.Space, - getDescendants: (_class: Ref>) => [] as Ref>[], - getAncestors: (_class: Ref>) => [_class] - } as PipelineContext['hierarchy'] - - const modelDb = { - findAllSync: (cl: Ref>, query: { attachedTo?: { $in?: Ref>[] } }) => { - if (cl !== core.class.ClassCollaborators) return [] - const ids = query.attachedTo?.$in ?? [] - return ids.includes(DOC_CLASS) ? [collabMixin] : [] - } - } as unknown as PipelineContext['modelDb'] - - const next: Middleware = { - findAll: async (_ctx: MeasureContext, _class: Ref>, query: any) => { - calls.push({ cls: _class as unknown as Ref>, query: JSON.parse(JSON.stringify(query)) }) - if (_class === (core.class.Space as unknown as Ref>)) { - return toFindResult([]) as any - } - if (_class === (core.class.Collaborator as unknown as Ref>)) { - const collabDoc: Collaborator = { - _id: generateId(), - _class: core.class.Collaborator, - space: core.space.Workspace, - attachedTo: DOC_ID, - attachedToClass: DOC_CLASS, - collection: 'collaborators', - collaborator: account.uuid, - modifiedOn: Date.now(), - modifiedBy: core.account.System - } - return toFindResult([collabDoc]) as any - } - return toFindResult([]) as any - }, - tx: async () => ({}), - groupBy: async () => new Map(), - searchFulltext: async () => ({ docs: [] }) as any, - handleBroadcast: async () => {}, - loadModel: async () => [], - domainRequest: async () => ({ value: undefined }) as any, - closeSession: async () => {}, - close: async () => {} - } - - // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- minimal PipelineContext stub for unit test - const context = { - workspace: { uuid: generateId() as any, url: 'test', dataId: 'test' as any }, - hierarchy, - modelDb, - branding: null as any, - adapterManager: {} as any, - storageAdapter: {} as any, - contextVars: {}, - lastTx: '', - lastHash: '', - broadcastEvent: async () => {} - } as PipelineContext - - const mw = new (SpaceSecurityMiddleware as any)(false, context, next) as SpaceSecurityMiddleware - return { mw, account, calls } -} - -describe('SpaceSecurityMiddleware guest collaborator read restriction', () => { - it('applies collaborator _id filter for guest when provideSecurity is enabled', async () => { - const { mw, account, calls } = makeMiddleware(AccountRole.Guest, { provideSecurity: true }) - const ctx = makeCtx(account) - - await mw.findAll(ctx, DOC_CLASS, { title: 'Meeting minutes' }) - - expect(calls).toHaveLength(3) - expect(calls[1].cls).toBe(core.class.Collaborator) - expect(calls[2].cls).toBe(DOC_CLASS) - expect(calls[2].query).toEqual({ - title: 'Meeting minutes', - _id: { $in: [DOC_ID] } - }) - }) - - it('keeps query unchanged for regular user', async () => { - const { mw, account, calls } = makeMiddleware(AccountRole.User, { provideSecurity: true }) - const ctx = makeCtx(account) - - await mw.findAll(ctx, DOC_CLASS, { title: 'Meeting minutes' }) - - expect(calls).toHaveLength(2) - expect(calls[1].cls).toBe(DOC_CLASS) - expect(calls[1].query).toEqual({ title: 'Meeting minutes' }) - }) -}) diff --git a/ws-tests/api-tests/src/__tests__/rest.test.ts b/ws-tests/api-tests/src/__tests__/rest.test.ts index 10958e6b73..0c7bd1c65e 100644 --- a/ws-tests/api-tests/src/__tests__/rest.test.ts +++ b/ws-tests/api-tests/src/__tests__/rest.test.ts @@ -23,7 +23,6 @@ import { type WorkspaceToken } from '@hcengineering/api-client' import core, { - AccountRole, buildSocialIdString, concatLink, generateId, @@ -46,8 +45,6 @@ import { type AccountClient, getClient as getAccountClient } from '@hcengineerin import chunter from '@hcengineering/chunter' import contact, { ensureEmployee, type SocialIdentityRef, type Person } from '@hcengineering/contact' import { generateToken } from '@hcengineering/server-token' - -import { loveClass, LoveMeetingStatus } from '../loveApiRefs' import WebSocket from 'ws' describe('rest-api-server', () => { @@ -341,104 +338,6 @@ describe('rest-api-server', () => { }) }, 20000) }) - - /** - * Requires a workspace account with AccountRole.Guest (invite / guest link) in the same workspace as `user1`. - * Set API_TESTS_GUEST_EMAIL and API_TESTS_GUEST_PASSWORD to enable. - */ - describe('guest meeting-minutes read', () => { - const guestEmail = 'guest1' - const guestPassword = '1234' - - it('guest findAll does not return meeting minutes without collaborator', async () => { - const userConn = connect() - const rooms = await userConn.findAll(loveClass.Room, {}, { limit: 1 }) - if (rooms.length === 0) { - throw new Error('No love Room in workspace — cannot seed MeetingMinutes') - } - const room = rooms[0] - - const tx = await connectTx() - const title = `api-test-mm-${generateId()}` - const mmId = await tx.createDoc(loveClass.MeetingMinutes, core.space.Workspace, { - title, - description: null, - attachedTo: room._id, - attachedToClass: loveClass.Room, - collection: 'meetings', - status: LoveMeetingStatus.Finished - }) - - try { - const userFound = await userConn.findAll(loveClass.MeetingMinutes, { _id: mmId }) - expect(userFound.length).toBe(1) - - const guestWs = await getWorkspaceToken( - 'http://huly.local:8083', - { - email: guestEmail, - password: guestPassword, - workspace: wsName - }, - serverConfig - ) - expect(guestWs.info.role).toBe(AccountRole.Guest) - - const guestConn = createRestClient(guestWs.endpoint, guestWs.workspaceId, guestWs.token) - const guestFound = await guestConn.findAll(loveClass.MeetingMinutes, { _id: mmId }) - expect(guestFound.length).toBe(0) - } finally { - await tx.removeDoc(loveClass.MeetingMinutes, core.space.Workspace, mmId) - } - }, 60000) - - it('guest findAll returns meeting minutes when guest is a Collaborator', async () => { - const userConn = connect() - const rooms = await userConn.findAll(loveClass.Room, {}, { limit: 1 }) - if (rooms.length === 0) { - throw new Error('No love Room in workspace — cannot seed MeetingMinutes') - } - const room = rooms[0] - - const guestWs = await getWorkspaceToken( - 'http://huly.local:8083', - { - email: guestEmail, - password: guestPassword, - workspace: wsName - }, - serverConfig - ) - expect(guestWs.info.role).toBe(AccountRole.Guest) - - const tx = await connectTx() - const title = `api-test-mm-${generateId()}` - const mmId = await tx.createDoc(loveClass.MeetingMinutes, core.space.Workspace, { - title, - description: null, - attachedTo: room._id, - attachedToClass: loveClass.Room, - collection: 'meetings', - status: LoveMeetingStatus.Finished - }) - - const collabId = await tx.createDoc(core.class.Collaborator, core.space.Workspace, { - attachedTo: mmId, - attachedToClass: loveClass.MeetingMinutes, - collection: 'collaborators', - collaborator: guestWs.info.account - } as any) - - try { - const guestConn = createRestClient(guestWs.endpoint, guestWs.workspaceId, guestWs.token) - const guestFound = await guestConn.findAll(loveClass.MeetingMinutes, { _id: mmId }) - expect(guestFound.length).toBe(1) - } finally { - await tx.removeDoc(core.class.Collaborator, core.space.Workspace, collabId) - await tx.removeDoc(loveClass.MeetingMinutes, core.space.Workspace, mmId) - } - }, 60000) - }) }) async function checkFindPerformance (conn: RestClient): Promise { diff --git a/ws-tests/api-tests/src/loveApiRefs.ts b/ws-tests/api-tests/src/loveApiRefs.ts deleted file mode 100644 index ce6243f56f..0000000000 --- a/ws-tests/api-tests/src/loveApiRefs.ts +++ /dev/null @@ -1,17 +0,0 @@ -// -// Class refs for the love plugin — same id shape the model builder emits (`${pluginId}:class:${name}`). -// Use this in API tests without a dependency on `@hcengineering/love`. -// - -import type { Class, Doc, Ref } from '@hcengineering/core' - -export const loveClass = { - Room: 'love:class:Room' as Ref>, - MeetingMinutes: 'love:class:MeetingMinutes' as Ref> -} - -/** @see `@hcengineering/love` MeetingStatus */ -export enum LoveMeetingStatus { - Active = 0, - Finished = 1 -} diff --git a/ws-tests/prepare.sh b/ws-tests/prepare.sh index 7ba9d2247b..54fcc4bc7c 100755 --- a/ws-tests/prepare.sh +++ b/ws-tests/prepare.sh @@ -32,7 +32,6 @@ echo "Creating user accounts..." ./tool.sh create-account admin -f Super -l Admin -p 1234 ./tool.sh create-account user1 -f John -l Appleseed -p 1234 ./tool.sh create-account user2 -f Kainin -l Dirak -p 1234 -./tool.sh create-account guest1 -f Guest -l One -p 1234 echo "Creating workspace api-tests..." ./tool.sh create-workspace api-tests email:user1 @@ -44,8 +43,4 @@ echo "Assigning user1 to workspaces..." ./tool.sh assign-workspace user1 api-tests ./tool.sh assign-workspace user1 api-tests-cr -echo "Assigning guest1 to api-tests as Guest..." -./tool.sh assign-workspace guest1 api-tests --role Guest -./tool.sh assign-workspace guest1 api-tests-cr --role Guest - rm -rf ./sanity/.auth