(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
This commit is contained in:
Dmitry S 2021-01-14 11:10:42 -05:00
parent ffe4a34335
commit d8e742aa0d
6 changed files with 50 additions and 19 deletions

View File

@ -3,7 +3,7 @@
* add a new column, and allows removing already-added columns. * add a new column, and allows removing already-added columns.
*/ */
import {aclSelect, cssSelect} from 'app/client/aclui/ACLSelect'; 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 {icon} from 'app/client/ui2018/icons';
import {Computed, dom, Observable, styled} from 'grainjs'; import {Computed, dom, Observable, styled} from 'grainjs';
@ -53,8 +53,10 @@ export function aclColumnList(colIds: Observable<string[]>, validColIds: string[
cssColItem( cssColItem(
cssColId(colId), cssColId(colId),
cssColItemIcon(icon('CrossSmall'), cssColItemIcon(icon('CrossSmall'),
dom.on('click', () => removeColId(colId)) dom.on('click', () => removeColId(colId)),
) testId('acl-col-remove'),
),
testId('acl-column'),
) )
), ),
cssNewColItem( cssNewColItem(

View File

@ -6,7 +6,6 @@ import {aclFormulaEditor} from 'app/client/aclui/ACLFormulaEditor';
import {aclSelect} from 'app/client/aclui/ACLSelect'; import {aclSelect} from 'app/client/aclui/ACLSelect';
import {PermissionKey, permissionsWidget} from 'app/client/aclui/PermissionsWidget'; import {PermissionKey, permissionsWidget} from 'app/client/aclui/PermissionsWidget';
import {GristDoc} from 'app/client/components/GristDoc'; import {GristDoc} from 'app/client/components/GristDoc';
import {createObsArray} from 'app/client/lib/koArrayWrap';
import {reportError, UserError} from 'app/client/models/errors'; import {reportError, UserError} from 'app/client/models/errors';
import {TableData} from 'app/client/models/TableData'; import {TableData} from 'app/client/models/TableData';
import {shadowScroll} from 'app/client/ui/shadowScroll'; import {shadowScroll} from 'app/client/ui/shadowScroll';
@ -66,9 +65,6 @@ export class AccessRules extends Disposable {
// The default rule set for the document (for "*:*"). // The default rule set for the document (for "*:*").
private _docDefaultRuleSet = Observable.create<DefaultObsRuleSet|null>(this, null); private _docDefaultRuleSet = Observable.create<DefaultObsRuleSet|null>(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. // Array of all UserAttribute rules.
private _userAttrRules = this.autoDispose(obsArray<ObsUserAttributeRule>()); private _userAttrRules = this.autoDispose(obsArray<ObsUserAttributeRule>());
@ -82,6 +78,9 @@ export class AccessRules extends Disposable {
// Error or warning message to show next to Save/Reset buttons if non-empty. // Error or warning message to show next to Save/Reset buttons if non-empty.
private _errorMessage = Observable.create(this, ''); 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) { constructor(private _gristDoc: GristDoc) {
super(); super();
this._ruleStatus = Computed.create(this, (use) => { 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 userAttrRules() { return this._userAttrRules; }
public get userAttrChoices() { return this._userAttrChoices; } public get userAttrChoices() { return this._userAttrChoices; }
@ -152,6 +151,7 @@ export class AccessRules extends Disposable {
this._errorMessage.set(''); this._errorMessage.set('');
const rules = this._ruleCollection; const rules = this._ruleCollection;
await rules.update(this._gristDoc.docData, {log: console}); await rules.update(this._gristDoc.docData, {log: console});
this._aclResources = await this._gristDoc.docComm.getAclResources();
this._tableRules.set( this._tableRules.set(
rules.getAllTableIds().map(tableId => TableRules.create(this._tableRules, rules.getAllTableIds().map(tableId => TableRules.create(this._tableRules,
tableId, this, rules.getAllColumnRuleSets(tableId), rules.getTableDefaultRuleSet(tableId))) 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'}, bigBasicButton('Add Table Rules', cssDropdownIcon('Dropdown'), {style: 'margin-left: auto'},
menu(() => [ menu(() =>
dom.forEach(this._allTableIds, (tableId) => this.allTableIds.map((tableId) =>
// Add the table on a timeout, to avoid disabling the clicked menu item // Add the table on a timeout, to avoid disabling the clicked menu item
// synchronously, which prevents the menu from closing on click. // synchronously, which prevents the menu from closing on click.
menuItemAsync(() => this._addTableRules(tableId), 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)), dom.cls('disabled', (use) => use(this._tableRules).some(t => t.tableId === tableId)),
) )
), ),
]), ),
), ),
bigBasicButton('Add User Attributes', dom.on('click', () => this._addUserAttributes())), 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. // 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 { public checkTableColumns(tableId: string, colIds?: string[], exemptColIds?: string[]): string {
if (!tableId) { return ''; } if (!tableId) { return ''; }
const table = this._gristDoc.docData.getTable(tableId); const tableColIds = this._aclResources[tableId];
if (!table) { return `Invalid table: ${tableId}`; } if (!tableColIds) { return `Invalid table: ${tableId}`; }
if (colIds) { if (colIds) {
const validColIds = new Set([...table.getColIds(), ...exemptColIds || []]); const validColIds = new Set([...tableColIds, ...exemptColIds || []]);
const invalidColIds = colIds.filter(c => !validColIds.has(c)); const invalidColIds = colIds.filter(c => !validColIds.has(c));
if (invalidColIds.length === 0) { return ''; } if (invalidColIds.length === 0) { return ''; }
return `Invalid columns in table ${tableId}: ${invalidColIds.join(', ')}`; 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. // Returns a list of valid colIds for the given table, or undefined if the table isn't valid.
public getValidColIds(tableId: string): string[]|undefined { public getValidColIds(tableId: string): string[]|undefined {
return this._gristDoc.docData.getTable(tableId)?.getColIds() return this._aclResources[tableId]?.filter(id => !isHiddenCol(id)).sort();
.filter(id => !isHiddenCol(id))
.sort();
} }
private _addTableRules(tableId: string) { private _addTableRules(tableId: string) {

View File

@ -54,6 +54,7 @@ export class DocComm extends Disposable implements ActiveDocAPI {
public reloadDoc = this._wrapMethod("reloadDoc"); public reloadDoc = this._wrapMethod("reloadDoc");
public fork = this._wrapMethod("fork"); public fork = this._wrapMethod("fork");
public checkAclFormula = this._wrapMethod("checkAclFormula"); public checkAclFormula = this._wrapMethod("checkAclFormula");
public getAclResources = this._wrapMethod("getAclResources");
public changeUrlIdEmitter = this.autoDispose(new Emitter()); public changeUrlIdEmitter = this.autoDispose(new Emitter());

View File

@ -227,4 +227,11 @@ export interface ActiveDocAPI {
* Check if an ACL formula is valid. If not, will throw an error with an explanation. * Check if an ACL formula is valid. If not, will throw an error with an explanation.
*/ */
checkAclFormula(text: string): Promise<void>; checkAclFormula(text: string): Promise<void>;
/**
* 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[]}>;
} }

View File

@ -932,7 +932,7 @@ export class ActiveDoc extends EventEmitter {
*/ */
public async checkAclFormula(docSession: DocSession, text: string): Promise<void> { public async checkAclFormula(docSession: DocSession, text: string): Promise<void> {
// Checks can leak names of tables and columns. // 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(); await this.waitForInitialization();
try { try {
const parsedAclFormula = await this._pyCall('parse_acl_formula', text); 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 { public getGristDocAPI(): GristDocAPI {
return this.docPluginManager.gristDocAPI; return this.docPluginManager.gristDocAPI;
} }
@ -1023,7 +1045,7 @@ export class ActiveDoc extends EventEmitter {
if (!this.isOwner(docSession)) { if (!this.isOwner(docSession)) {
throw new Error('cannot delete actions, access denied'); throw new Error('cannot delete actions, access denied');
} }
this._actionHistory.deleteActions(keepN); await this._actionHistory.deleteActions(keepN);
} }
/** /**

View File

@ -111,6 +111,7 @@ export class DocWorker {
reloadDoc: activeDocMethod.bind(null, 'editors', 'reloadDoc'), reloadDoc: activeDocMethod.bind(null, 'editors', 'reloadDoc'),
fork: activeDocMethod.bind(null, 'viewers', 'fork'), fork: activeDocMethod.bind(null, 'viewers', 'fork'),
checkAclFormula: activeDocMethod.bind(null, 'viewers', 'checkAclFormula'), checkAclFormula: activeDocMethod.bind(null, 'viewers', 'checkAclFormula'),
getAclResources: activeDocMethod.bind(null, 'viewers', 'getAclResources'),
}); });
} }