From f0bcbd1c82505a1d269aeddde7ac18af43e9b46f Mon Sep 17 00:00:00 2001 From: Denis Bykhov Date: Thu, 2 Mar 2023 17:45:55 +0600 Subject: [PATCH] Disable owner space security (#2705) Signed-off-by: Denis Bykhov --- models/core/src/index.ts | 17 ++++++++-- models/core/src/migration.ts | 21 +----------- server/middleware/src/private.ts | 6 ++-- server/middleware/src/spaceSecurity.ts | 44 ++++++++++++++------------ server/middleware/src/utils.ts | 21 ++++++++++-- 5 files changed, 60 insertions(+), 49 deletions(-) diff --git a/models/core/src/index.ts b/models/core/src/index.ts index 87fc5937ee..4289458204 100644 --- a/models/core/src/index.ts +++ b/models/core/src/index.ts @@ -13,10 +13,10 @@ // limitations under the License. // +import { AccountRole } from '@hcengineering/core' import { Builder } from '@hcengineering/model' import core from './component' import { - TFullTextSearchContext, TArrOf, TAttachedDoc, TAttribute, @@ -30,6 +30,8 @@ import { TEnum, TEnumOf, TFulltextData, + TFullTextSearchContext, + TIndexStageState, TInterface, TMixin, TObj, @@ -46,8 +48,7 @@ import { TTypeRelatedDocument, TTypeString, TTypeTimestamp, - TVersion, - TIndexStageState + TVersion } from './core' import { TAccount, TSpace } from './security' import { TUserStatus } from './transient' @@ -105,4 +106,14 @@ export function createModel (builder: Builder): void { TConfiguration, TConfigurationElement ) + + builder.createDoc( + core.class.Account, + core.space.Model, + { + email: 'anticrm@hc.engineering', + role: AccountRole.Owner + }, + core.account.System + ) } diff --git a/models/core/src/migration.ts b/models/core/src/migration.ts index 1ac344c955..01d2fe246d 100644 --- a/models/core/src/migration.ts +++ b/models/core/src/migration.ts @@ -13,28 +13,9 @@ // limitations under the License. // -import core, { AccountRole, Client, TxOperations } from '@hcengineering/core' import { MigrateOperation, MigrationClient, MigrationUpgradeClient } from '@hcengineering/model' export const coreOperation: MigrateOperation = { async migrate (client: MigrationClient): Promise {}, - async upgrade (client: MigrationUpgradeClient): Promise { - await createSystemAccount(client) - } -} - -async function createSystemAccount (client: Client): Promise { - const current = await client.findOne(core.class.Account, { _id: core.account.System }) - if (current === undefined) { - const txop = new TxOperations(client, core.account.System) - await txop.createDoc( - core.class.Account, - core.space.Model, - { - email: 'anticrm@hc.engineering', - role: AccountRole.Owner - }, - core.account.System - ) - } + async upgrade (client: MigrationUpgradeClient): Promise {} } diff --git a/server/middleware/src/private.ts b/server/middleware/src/private.ts index 1eea7a8600..a52ec33c3b 100644 --- a/server/middleware/src/private.ts +++ b/server/middleware/src/private.ts @@ -53,7 +53,7 @@ export class PrivateMiddleware extends BaseMiddleware implements Middleware { const txCUD = tx as TxCUD const domain = this.storage.hierarchy.getDomain(txCUD.objectClass) if (this.targetDomains.includes(domain)) { - const account = await getUser(this.storage, ctx) + const account = (await getUser(this.storage, ctx))._id if (account !== tx.modifiedBy && account !== core.account.System) { throw new PlatformError(new Status(Severity.ERROR, platform.status.Forbidden, {})) } @@ -74,7 +74,7 @@ export class PrivateMiddleware extends BaseMiddleware implements Middleware { const domain = this.storage.hierarchy.getDomain(_class) if (this.targetDomains.includes(domain)) { const account = await getUser(this.storage, ctx) - if (account !== core.account.System) { + if (account._id !== core.account.System) { newQuery = { ...query, modifiedBy: account @@ -95,7 +95,7 @@ export class PrivateMiddleware extends BaseMiddleware implements Middleware { async isAvailable (ctx: SessionContext, doc: Doc): Promise { const domain = this.storage.hierarchy.getDomain(doc._class) if (!this.targetDomains.includes(domain)) return true - const account = await getUser(this.storage, ctx) + const account = (await getUser(this.storage, ctx))._id return doc.modifiedBy === account || account === core.account.System } diff --git a/server/middleware/src/spaceSecurity.ts b/server/middleware/src/spaceSecurity.ts index 732075c17a..c2b92b85c5 100644 --- a/server/middleware/src/spaceSecurity.ts +++ b/server/middleware/src/spaceSecurity.ts @@ -39,7 +39,7 @@ import core, { import platform, { PlatformError, Severity, Status } from '@hcengineering/platform' import { Middleware, SessionContext, TxMiddlewareResult } from '@hcengineering/server-core' import { BaseMiddleware } from './base' -import { getUser, mergeTargets } from './utils' +import { getUser, isOwner, mergeTargets } from './utils' /** * @public @@ -241,8 +241,8 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar const space = this.privateSpaces[tx.objectSpace] if (space !== undefined) { const account = await getUser(this.storage, ctx) - if (account !== core.account.System) { - const allowed = this.allowedSpaces[account] + if (!isOwner(account)) { + const allowed = this.allowedSpaces[account._id] if (allowed === undefined || !allowed.includes(isSpace ? (cudTx.objectId as Ref) : tx.objectSpace)) { throw new PlatformError(new Status(Severity.ERROR, platform.status.Forbidden, {})) } @@ -255,22 +255,21 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar return [res[0], res[1], mergeTargets(targets, res[2])] } - private async getAllAllowedSpaces (ctx: SessionContext): Promise[]> { + private async getAllAllowedSpaces (account: Account): Promise[]> { let userSpaces: Ref[] = [] try { - const account = await getUser(this.storage, ctx) - userSpaces = this.allowedSpaces[account] ?? [] - return [...userSpaces, account as string as Ref, ...this.publicSpaces, ...this.systemSpaces] + userSpaces = this.allowedSpaces[account._id] ?? [] + return [...userSpaces, account._id as string as Ref, ...this.publicSpaces, ...this.systemSpaces] } catch { return [...this.publicSpaces, ...this.systemSpaces] } } private async mergeQuery( - ctx: SessionContext, + account: Account, query: ObjQueryType ): Promise> { - const spaces = await this.getAllAllowedSpaces(ctx) + const spaces = await this.getAllAllowedSpaces(account) if (typeof query === 'string') { if (!spaces.includes(query)) { throw new PlatformError(new Status(Severity.ERROR, platform.status.Forbidden, {})) @@ -290,17 +289,22 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar options?: FindOptions ): Promise> { const newQuery = query - if (query.space !== undefined) { - newQuery.space = await this.mergeQuery(ctx, query.space) - } else { - const spaces = await this.getAllAllowedSpaces(ctx) - newQuery.space = { $in: spaces } + const account = await getUser(this.storage, ctx) + if (!isOwner(account)) { + if (query.space !== undefined) { + newQuery.space = await this.mergeQuery(account, query.space) + } else { + const spaces = await this.getAllAllowedSpaces(account) + newQuery.space = { $in: spaces } + } } const findResult = await this.provideFindAll(ctx, _class, newQuery, options) - if (options?.lookup !== undefined) { - for (const object of findResult) { - if (object.$lookup !== undefined) { - await this.filterLookup(ctx, object.$lookup) + if (!isOwner(account)) { + if (options?.lookup !== undefined) { + for (const object of findResult) { + if (object.$lookup !== undefined) { + await this.filterLookup(ctx, object.$lookup) + } } } } @@ -310,8 +314,8 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar async isUnavailable (ctx: SessionContext, space: Ref): Promise { if (this.privateSpaces[space] === undefined) return false const account = await getUser(this.storage, ctx) - if (account === core.account.System) return false - return !this.allowedSpaces[account]?.includes(space) + if (isOwner(account)) return false + return !this.allowedSpaces[account._id]?.includes(space) } async filterLookup(ctx: SessionContext, lookup: LookupData): Promise { diff --git a/server/middleware/src/utils.ts b/server/middleware/src/utils.ts index 6a59fc3666..1d08b088df 100644 --- a/server/middleware/src/utils.ts +++ b/server/middleware/src/utils.ts @@ -13,7 +13,7 @@ // limitations under the License. // -import core, { Account, Ref, ServerStorage } from '@hcengineering/core' +import core, { Account, AccountRole, ServerStorage } from '@hcengineering/core' import platform, { PlatformError, Severity, Status } from '@hcengineering/platform' import { SessionContext } from '@hcengineering/server-core' @@ -29,13 +29,28 @@ export function mergeTargets (current: string[] | undefined, prev: string[] | un return res } -export async function getUser (storage: ServerStorage, ctx: SessionContext): Promise> { +export async function getUser (storage: ServerStorage, ctx: SessionContext): Promise { if (ctx.userEmail === undefined) { throw new PlatformError(new Status(Severity.ERROR, platform.status.Forbidden, {})) } const account = (await storage.modelDb.findAll(core.class.Account, { email: ctx.userEmail }))[0] if (account === undefined) { + if (ctx.userEmail === 'anticrm@hc.engineering') { + return { + _id: core.account.System, + _class: core.class.Account, + role: AccountRole.Owner, + email: 'anticrm@hc.engineering', + space: core.space.Model, + modifiedBy: core.account.System, + modifiedOn: 0 + } + } throw new PlatformError(new Status(Severity.ERROR, platform.status.Forbidden, {})) } - return account._id + return account +} + +export function isOwner (account: Account): boolean { + return account.role === AccountRole.Owner || account._id === core.account.System }