From d8e742aa0d8d7a5d7cb83f8d84a7a59b508c5343 Mon Sep 17 00:00:00 2001 From: Dmitry S Date: Thu, 14 Jan 2021 11:10:42 -0500 Subject: [PATCH] (core) Add getAclResources method for making all tables/columns available when editing ACL rules Summary: The goal is that those who can edit ACL rules can create or change rules for any resource, even if the rules block their own ability to see the resource. Test Plan: Added a browser test, and a server test for who can call the new method. Reviewers: paulfitz Reviewed By: paulfitz Differential Revision: https://phab.getgrist.com/D2703 --- app/client/aclui/ACLColumnList.ts | 8 +++++--- app/client/aclui/AccessRules.ts | 26 ++++++++++++-------------- app/client/components/DocComm.ts | 1 + app/common/ActiveDocAPI.ts | 7 +++++++ app/server/lib/ActiveDoc.ts | 26 ++++++++++++++++++++++++-- app/server/lib/DocWorker.ts | 1 + 6 files changed, 50 insertions(+), 19 deletions(-) diff --git a/app/client/aclui/ACLColumnList.ts b/app/client/aclui/ACLColumnList.ts index b1c986cb..1b7492bf 100644 --- a/app/client/aclui/ACLColumnList.ts +++ b/app/client/aclui/ACLColumnList.ts @@ -3,7 +3,7 @@ * add a new column, and allows removing already-added columns. */ import {aclSelect, cssSelect} from 'app/client/aclui/ACLSelect'; -import {colors} from 'app/client/ui2018/cssVars'; +import {colors, testId} from 'app/client/ui2018/cssVars'; import {icon} from 'app/client/ui2018/icons'; import {Computed, dom, Observable, styled} from 'grainjs'; @@ -53,8 +53,10 @@ export function aclColumnList(colIds: Observable, validColIds: string[ cssColItem( cssColId(colId), cssColItemIcon(icon('CrossSmall'), - dom.on('click', () => removeColId(colId)) - ) + dom.on('click', () => removeColId(colId)), + testId('acl-col-remove'), + ), + testId('acl-column'), ) ), cssNewColItem( diff --git a/app/client/aclui/AccessRules.ts b/app/client/aclui/AccessRules.ts index 092d5777..c4612a92 100644 --- a/app/client/aclui/AccessRules.ts +++ b/app/client/aclui/AccessRules.ts @@ -6,7 +6,6 @@ import {aclFormulaEditor} from 'app/client/aclui/ACLFormulaEditor'; import {aclSelect} from 'app/client/aclui/ACLSelect'; import {PermissionKey, permissionsWidget} from 'app/client/aclui/PermissionsWidget'; import {GristDoc} from 'app/client/components/GristDoc'; -import {createObsArray} from 'app/client/lib/koArrayWrap'; import {reportError, UserError} from 'app/client/models/errors'; import {TableData} from 'app/client/models/TableData'; import {shadowScroll} from 'app/client/ui/shadowScroll'; @@ -66,9 +65,6 @@ export class AccessRules extends Disposable { // The default rule set for the document (for "*:*"). private _docDefaultRuleSet = Observable.create(this, null); - // Array of all tableIds in the document (for adding new per-table rules). - private _allTableIds = createObsArray(this, this._gristDoc.docModel.allTableIds); - // Array of all UserAttribute rules. private _userAttrRules = this.autoDispose(obsArray()); @@ -82,6 +78,9 @@ export class AccessRules extends Disposable { // Error or warning message to show next to Save/Reset buttons if non-empty. private _errorMessage = Observable.create(this, ''); + // Map of tableId to the list of columns for all tables in the document. + private _aclResources: {[tableId: string]: string[]} = {}; + constructor(private _gristDoc: GristDoc) { super(); this._ruleStatus = Computed.create(this, (use) => { @@ -141,7 +140,7 @@ export class AccessRules extends Disposable { } } - public get allTableIds() { return this._allTableIds; } + public get allTableIds() { return Object.keys(this._aclResources).sort(); } public get userAttrRules() { return this._userAttrRules; } public get userAttrChoices() { return this._userAttrChoices; } @@ -152,6 +151,7 @@ export class AccessRules extends Disposable { this._errorMessage.set(''); const rules = this._ruleCollection; await rules.update(this._gristDoc.docData, {log: console}); + this._aclResources = await this._gristDoc.docComm.getAclResources(); this._tableRules.set( rules.getAllTableIds().map(tableId => TableRules.create(this._tableRules, tableId, this, rules.getAllColumnRuleSets(tableId), rules.getTableDefaultRuleSet(tableId))) @@ -279,8 +279,8 @@ export class AccessRules extends Disposable { ), bigBasicButton('Add Table Rules', cssDropdownIcon('Dropdown'), {style: 'margin-left: auto'}, - menu(() => [ - dom.forEach(this._allTableIds, (tableId) => + menu(() => + this.allTableIds.map((tableId) => // Add the table on a timeout, to avoid disabling the clicked menu item // synchronously, which prevents the menu from closing on click. menuItemAsync(() => this._addTableRules(tableId), @@ -288,7 +288,7 @@ export class AccessRules extends Disposable { dom.cls('disabled', (use) => use(this._tableRules).some(t => t.tableId === tableId)), ) ), - ]), + ), ), bigBasicButton('Add User Attributes', dom.on('click', () => this._addUserAttributes())), ), @@ -365,10 +365,10 @@ export class AccessRules extends Disposable { // Returns '' if valid, or an error string if not. Exempt colIds will not trigger an error. public checkTableColumns(tableId: string, colIds?: string[], exemptColIds?: string[]): string { if (!tableId) { return ''; } - const table = this._gristDoc.docData.getTable(tableId); - if (!table) { return `Invalid table: ${tableId}`; } + const tableColIds = this._aclResources[tableId]; + if (!tableColIds) { return `Invalid table: ${tableId}`; } if (colIds) { - const validColIds = new Set([...table.getColIds(), ...exemptColIds || []]); + const validColIds = new Set([...tableColIds, ...exemptColIds || []]); const invalidColIds = colIds.filter(c => !validColIds.has(c)); if (invalidColIds.length === 0) { return ''; } return `Invalid columns in table ${tableId}: ${invalidColIds.join(', ')}`; @@ -378,9 +378,7 @@ export class AccessRules extends Disposable { // Returns a list of valid colIds for the given table, or undefined if the table isn't valid. public getValidColIds(tableId: string): string[]|undefined { - return this._gristDoc.docData.getTable(tableId)?.getColIds() - .filter(id => !isHiddenCol(id)) - .sort(); + return this._aclResources[tableId]?.filter(id => !isHiddenCol(id)).sort(); } private _addTableRules(tableId: string) { diff --git a/app/client/components/DocComm.ts b/app/client/components/DocComm.ts index 9ff2797b..5b9c043e 100644 --- a/app/client/components/DocComm.ts +++ b/app/client/components/DocComm.ts @@ -54,6 +54,7 @@ export class DocComm extends Disposable implements ActiveDocAPI { public reloadDoc = this._wrapMethod("reloadDoc"); public fork = this._wrapMethod("fork"); public checkAclFormula = this._wrapMethod("checkAclFormula"); + public getAclResources = this._wrapMethod("getAclResources"); public changeUrlIdEmitter = this.autoDispose(new Emitter()); diff --git a/app/common/ActiveDocAPI.ts b/app/common/ActiveDocAPI.ts index 1184b30c..cbc3b740 100644 --- a/app/common/ActiveDocAPI.ts +++ b/app/common/ActiveDocAPI.ts @@ -227,4 +227,11 @@ export interface ActiveDocAPI { * Check if an ACL formula is valid. If not, will throw an error with an explanation. */ checkAclFormula(text: string): Promise; + + /** + * Returns the full set of tableIds, with the list of colIds for each table. This is intended + * for editing ACLs. It is only available to users who can edit ACLs, and lists all resources + * regardless of rules that may block access to them. + */ + getAclResources(): Promise<{[tableId: string]: string[]}>; } diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index 3a9e4f90..17299924 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -932,7 +932,7 @@ export class ActiveDoc extends EventEmitter { */ public async checkAclFormula(docSession: DocSession, text: string): Promise { // Checks can leak names of tables and columns. - if (!await this._granularAccess.canReadEverything(docSession)) { return; } + if (await this._granularAccess.hasNuancedAccess(docSession)) { return; } await this.waitForInitialization(); try { const parsedAclFormula = await this._pyCall('parse_acl_formula', text); @@ -945,6 +945,28 @@ export class ActiveDoc extends EventEmitter { } } + /** + * Returns the full set of tableIds, with the list of colIds for each table. This is intended + * for editing ACLs. It is only available to users who can edit ACLs, and lists all resources + * regardless of rules that may block access to them. + */ + public async getAclResources(docSession: DocSession): Promise<{[tableId: string]: string[]}> { + if (await this._granularAccess.hasNuancedAccess(docSession) || !this.docData) { + throw new Error('Cannot list ACL resources'); + } + const result: {[tableId: string]: string[]} = {}; + const tables = this.docData.getTable('_grist_Tables')!; + for (const tableId of tables.getColValues('tableId')!) { + result[tableId as string] = ['id']; + } + const columns = this.docData.getTable('_grist_Tables_column')!; + for (const col of columns.getRecords()) { + const tableId = tables.getValue(col.parentId as number, 'tableId'); + result[tableId as string].push(col.colId as string); + } + return result; + } + public getGristDocAPI(): GristDocAPI { return this.docPluginManager.gristDocAPI; } @@ -1023,7 +1045,7 @@ export class ActiveDoc extends EventEmitter { if (!this.isOwner(docSession)) { throw new Error('cannot delete actions, access denied'); } - this._actionHistory.deleteActions(keepN); + await this._actionHistory.deleteActions(keepN); } /** diff --git a/app/server/lib/DocWorker.ts b/app/server/lib/DocWorker.ts index 0976f090..5172127c 100644 --- a/app/server/lib/DocWorker.ts +++ b/app/server/lib/DocWorker.ts @@ -111,6 +111,7 @@ export class DocWorker { reloadDoc: activeDocMethod.bind(null, 'editors', 'reloadDoc'), fork: activeDocMethod.bind(null, 'viewers', 'fork'), checkAclFormula: activeDocMethod.bind(null, 'viewers', 'checkAclFormula'), + getAclResources: activeDocMethod.bind(null, 'viewers', 'getAclResources'), }); }