From d140b49ba39f8492954180f2d4752d03387fc039 Mon Sep 17 00:00:00 2001 From: Alex Hall Date: Mon, 26 Sep 2022 12:05:59 +0200 Subject: [PATCH] (core) Include helper columns in ACL rules Summary: Extend the way ACL resources are read in the server so that if a rule applies to a specific column then that rule also applies to helper columns belonging to that column, as well as helper columns belonging to fields which display that column. This is particularly intended for display columns of reference columns, but it also applies to conditional formatting rule columns. Test Plan: Added a server test Reviewers: paulfitz, jarek Reviewed By: paulfitz, jarek Differential Revision: https://phab.getgrist.com/D3628 --- app/common/ACLRuleCollection.ts | 67 +++++++++++++++++++++++++++++++- app/common/TableData.ts | 2 +- app/server/lib/GranularAccess.ts | 29 +++++++++++--- 3 files changed, 90 insertions(+), 8 deletions(-) diff --git a/app/common/ACLRuleCollection.ts b/app/common/ACLRuleCollection.ts index bb418146..1fc1d525 100644 --- a/app/common/ACLRuleCollection.ts +++ b/app/common/ACLRuleCollection.ts @@ -4,6 +4,7 @@ import {DocData} from 'app/common/DocData'; import {AclMatchFunc, ParsedAclFormula, RulePart, RuleSet, UserAttributeRule} from 'app/common/GranularAccessClause'; import {getSetMapValue} from 'app/common/gutil'; import {MetaRowRecord} from 'app/common/TableData'; +import {decodeObject} from 'app/plugin/objtypes'; import sortBy = require('lodash/sortBy'); const defaultMatchFunc: AclMatchFunc = () => true; @@ -290,6 +291,11 @@ export class ACLRuleCollection { export interface ReadAclOptions { log: ILogger; // For logging warnings during rule processing. compile?: (parsed: ParsedAclFormula) => AclMatchFunc; + // If true, call addHelperCols to add helper columns of restricted columns to rule sets. + // Used in the server for extra filtering, but not in the client, because: + // 1. They would show in the UI + // 2. They would be saved back after editing, causing them to accumulate + includeHelperCols?: boolean; } export interface ReadAclResults { @@ -297,11 +303,66 @@ export interface ReadAclResults { userAttributes: UserAttributeRule[]; } +/** + * For each column in colIds, return the colIds of any hidden helper columns it has, + * i.e. display columns of references, and conditional formatting rule columns. + */ +function getHelperCols(docData: DocData, tableId: string, colIds: string[], log: ILogger): string[] { + const tablesTable = docData.getMetaTable('_grist_Tables'); + const columnsTable = docData.getMetaTable('_grist_Tables_column'); + const fieldsTable = docData.getMetaTable('_grist_Views_section_field'); + + const tableRef = tablesTable.findRow('tableId', tableId); + if (!tableRef) { + return []; + } + + const result: string[] = []; + for (const colId of colIds) { + const [column] = columnsTable.filterRecords({parentId: tableRef, colId}); + if (!column) { + continue; + } + + function addColsFromRefs(colRefs: unknown) { + if (!Array.isArray(colRefs)) { + return; + } + for (const colRef of colRefs) { + if (typeof colRef !== 'number') { + continue; + } + const extraCol = columnsTable.getRecord(colRef); + if (!extraCol) { + continue; + } + if (extraCol.colId.startsWith("gristHelper_") && extraCol.parentId === tableRef) { + result.push(extraCol.colId); + } else { + log.error(`Invalid helper column ${extraCol.colId} of ${tableId}:${colId}`); + } + } + } + + function addColsFromMetaRecord(rec: MetaRowRecord<'_grist_Tables_column' | '_grist_Views_section_field'>) { + addColsFromRefs([rec.displayCol]); + addColsFromRefs(decodeObject(rec.rules)); + } + + addColsFromMetaRecord(column); + for (const field of fieldsTable.filterRecords({colRef: column.id})) { + addColsFromMetaRecord(field); + } + } + return result; +} + + /** * Parse all ACL rules in the document from DocData into a list of RuleSets and of * UserAttributeRules. This is used by both client-side code and server-side. */ -function readAclRules(docData: DocData, {log, compile}: ReadAclOptions): ReadAclResults { +function readAclRules(docData: DocData, {log, compile, includeHelperCols}: ReadAclOptions): ReadAclResults { const resourcesTable = docData.getMetaTable('_grist_ACLResources'); const rulesTable = docData.getMetaTable('_grist_ACLRules'); @@ -328,6 +389,10 @@ function readAclRules(docData: DocData, {log, compile}: ReadAclOptions): ReadAcl const tableId = resourceRec.tableId; const colIds = resourceRec.colIds === '*' ? '*' : resourceRec.colIds.split(','); + if (includeHelperCols && Array.isArray(colIds)) { + colIds.push(...getHelperCols(docData, tableId, colIds, log)); + } + const body: RulePart[] = []; for (const rule of rules) { if (rule.userAttributes) { diff --git a/app/common/TableData.ts b/app/common/TableData.ts index 57b24ef5..73c4f65e 100644 --- a/app/common/TableData.ts +++ b/app/common/TableData.ts @@ -110,7 +110,7 @@ export class TableData extends ActionDispatcher implements SkippableRows { reassignArray(this._rowIdCol, rowIds); for (const colData of this._colArray) { - const values = colValues[colData.colId]; + const values = colData.colId === 'id' ? rowIds : colValues[colData.colId]; // If colId is missing from tableData, use an array of default values. Note that reusing // default value like this is only OK because all default values we use are primitive. reassignArray(colData.values, values || this._rowIdCol.map(() => colData.defl)); diff --git a/app/server/lib/GranularAccess.ts b/app/server/lib/GranularAccess.ts index 776faaf2..9d358b4f 100644 --- a/app/server/lib/GranularAccess.ts +++ b/app/server/lib/GranularAccess.ts @@ -212,7 +212,7 @@ export class GranularAccess implements GranularAccessForBundle { this._activeBundle.hasDeliberateRuleChange = scanActionsRecursively(userActions, (a) => isAclTable(String(a[1]))); this._activeBundle.hasAnyRuleChange = - scanActionsRecursively(docActions, (a) => isAclTable(String(a[1]))); + scanActionsRecursively(docActions, a => actionHasRuleChange(a)); } /** @@ -1715,7 +1715,7 @@ export class GranularAccess implements GranularAccessForBundle { } step.metaAfter = meta; // replaceRuler logic avoids updating rules between paired changes of resources and rules. - if (isAclTable(tableId)) { + if (actionHasRuleChange(docAction)) { replaceRuler = true; } else if (replaceRuler) { ruler = new Ruler(this); @@ -1977,7 +1977,7 @@ export class Ruler { * Update granular access from DocData. */ public async update(docData: DocData) { - await this.ruleCollection.update(docData, {log, compile: compileAclFormula}); + await this.ruleCollection.update(docData, {log, compile: compileAclFormula, includeHelperCols: true}); // Also clear the per-docSession cache of rule evaluations. this.clearCache(); @@ -2334,11 +2334,11 @@ function getCensorMethod(tableId: string): (rec: RecordEditor) => void { } } -function scanActionsRecursively(actions: (DocAction|UserAction)[], - check: (action: DocAction|UserAction) => boolean): boolean { +function scanActionsRecursively(actions: T[], + check: (action: T) => boolean): boolean { for (const a of actions) { if (a[0] === 'ApplyUndoActions' || a[0] === 'ApplyDocActions') { - return scanActionsRecursively(a[1] as UserAction[], check); + return scanActionsRecursively(a[1] as T[], check); } if (check(a)) { return true; } } @@ -2486,3 +2486,20 @@ export function validTableIdString(tableId: any): string { if (typeof tableId !== 'string') { throw new Error(`Expected tableId to be a string`); } return tableId; } + +function actionHasRuleChange(a: DocAction): boolean { + return isAclTable(getTableId(a)) || ( + // Check if any helper columns have been specified while adding/updating a metadata record, + // as this will affect the result of `getHelperCols` in `ACLRuleCollection.ts` and thus the set of ACL resources. + // Note that removing a helper column doesn't directly trigger this code, but: + // - It will typically be accompanied closely by unsetting the helper column on the metadata record. + // - `getHelperCols` can handle non-existent helper columns and other similarly invalid metadata. + // - Since the column is removed, ACL restrictions on it don't really matter. + isDataAction(a) + && ["_grist_Tables_column", "_grist_Views_section_field"].includes(getTableId(a)) + && Boolean( + a[3]?.hasOwnProperty('rules') || + a[3]?.hasOwnProperty('displayCol') + ) + ); +}