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'), }); }