From 5deac683151e920e87bce97a39381c0ddcf61e6d Mon Sep 17 00:00:00 2001 From: Dmitry S Date: Mon, 28 Dec 2020 00:40:10 -0500 Subject: [PATCH] (core) Update ACL resources/rules when tables/columns get renamed Summary: - Placed rule-updating functions in acl.py. - Reset UI when rules update externally, or alert the user to reset if there are pending local changes. - Removed some unused and distracting bits from client-side DocModel. A few improvements related to poor error handling: - In case of missing DocActions (tickled by broken ACL rule handling), don't add to confusion by attempting to process bad actions - In case of missing attributes in ACL formulas, return undefined rather than fail; the latter creates more problems. - In case in invalid rules, fail rather than skip; this feels more correct now that we have error checking and recovery option, and helps avoid invalid rules. - Prevent saving invalid rules with an empty ACL formula. - Fix bug with rule positions. Test Plan: Added a python and browser test for table/column renames. Reviewers: paulfitz Reviewed By: paulfitz Differential Revision: https://phab.getgrist.com/D2698 --- app/client/aclui/AccessRules.ts | 56 ++++++-- app/client/models/BaseRowModel.js | 2 + app/client/models/DocModel.ts | 8 -- .../models/entities/ACLMembershipRec.ts | 14 -- app/client/models/entities/ACLPrincipalRec.ts | 29 ---- app/client/models/entities/ACLResourceRec.ts | 7 - app/common/ACLRuleCollection.ts | 10 +- app/server/lib/ACLFormula.ts | 8 +- app/server/lib/GranularAccess.ts | 4 +- sandbox/grist/acl.py | 98 ++++++++++++++ sandbox/grist/acl_formula.py | 46 +++++++ sandbox/grist/docmodel.py | 2 + sandbox/grist/test_acl_renames.py | 127 ++++++++++++++++++ sandbox/grist/useractions.py | 6 + 14 files changed, 338 insertions(+), 79 deletions(-) delete mode 100644 app/client/models/entities/ACLMembershipRec.ts delete mode 100644 app/client/models/entities/ACLPrincipalRec.ts delete mode 100644 app/client/models/entities/ACLResourceRec.ts create mode 100644 sandbox/grist/test_acl_renames.py diff --git a/app/client/aclui/AccessRules.ts b/app/client/aclui/AccessRules.ts index fa8fd8a5..092d5777 100644 --- a/app/client/aclui/AccessRules.ts +++ b/app/client/aclui/AccessRules.ts @@ -79,6 +79,9 @@ export class AccessRules extends Disposable { // Whether the save button should be enabled. private _savingEnabled: Computed; + // Error or warning message to show next to Save/Reset buttons if non-empty. + private _errorMessage = Observable.create(this, ''); + constructor(private _gristDoc: GristDoc) { super(); this._ruleStatus = Computed.create(this, (use) => { @@ -117,7 +120,25 @@ export class AccessRules extends Disposable { return result; }); - this.update().catch(reportError); + // The UI in this module isn't really dynamic (that would be tricky while allowing unsaved + // changes). Instead, react deliberately if rules change. Note that table/column renames would + // trigger changes to rules, so we don't need to listen for those separately. + for (const tableId of ['_grist_ACLResources', '_grist_ACLRules']) { + const tableData = this._gristDoc.docData.getTable(tableId)!; + this.autoDispose(tableData.tableActionEmitter.addListener(this._onChange, this)); + } + this.update().catch((e) => this._errorMessage.set(e.message)); + } + + public _onChange() { + if (this._ruleStatus.get() === RuleStatus.Unchanged) { + // If no changes, it's safe to just reload the rules from docData. + this.update().catch((e) => this._errorMessage.set(e.message)); + } else { + this._errorMessage.set( + 'Access rules have changed. Click Reset to revert your changes and refresh the rules.' + ); + } } public get allTableIds() { return this._allTableIds; } @@ -128,6 +149,7 @@ export class AccessRules extends Disposable { * Replace internal state from the rules in DocData. */ public async update() { + this._errorMessage.set(''); const rules = this._ruleCollection; await rules.update(this._gristDoc.docData, {log: console}); this._tableRules.set( @@ -209,7 +231,7 @@ export class AccessRules extends Disposable { if (pos && pos > lastGoodRulePos) { const step = (pos - lastGoodRulePos) / (i - lastGoodIndex); for (let k = lastGoodIndex + 1; k < i; k++) { - newRules[k].rulePos = step * (k - lastGoodIndex); + newRules[k].rulePos = lastGoodRulePos + step * (k - lastGoodIndex); } lastGoodRulePos = pos; lastGoodIndex = i; @@ -251,7 +273,7 @@ export class AccessRules extends Disposable { dom.on('click', () => this.save()), testId('rules-save'), ), - bigBasicButton('Revert', dom.show(this._savingEnabled), + bigBasicButton('Reset', dom.show(use => use(this._ruleStatus) !== RuleStatus.Unchanged), dom.on('click', () => this.update()), testId('rules-revert'), ), @@ -270,6 +292,9 @@ export class AccessRules extends Disposable { ), bigBasicButton('Add User Attributes', dom.on('click', () => this._addUserAttributes())), ), + cssConditionError(dom.text(this._errorMessage), {style: 'margin-left: 16px'}, + testId('access-rules-error') + ), shadowScroll( dom.maybe(use => use(this._userAttrRules).length, () => cssSection( @@ -541,7 +566,9 @@ abstract class ObsRuleSet extends Disposable { return this._body.get().map(part => ({ ...part.getRulePart(), resourceRec: {tableId, colIds: this.getColIds()} - })); + })) + // Skip entirely empty rule parts: they are invalid and dropping them is the best fix. + .filter(part => part.aclFormula || part.permissionsText); } public getColIds(): string { @@ -723,8 +750,9 @@ class ObsUserAttributeRule extends Disposable { const result = use(this._accessRules.userAttrChoices).filter(c => (c.ruleIndex < index)); // If the currently-selected option isn't one of the choices, insert it too. - if (!result.some(choice => (choice.value === this._charId.get()))) { - result.unshift({ruleIndex: -1, value: this._charId.get(), label: `user.${this._charId.get()}`}); + const charId = use(this._charId); + if (charId && !result.some(choice => (choice.value === charId))) { + result.unshift({ruleIndex: -1, value: charId, label: `user.${charId}`}); } return result; }); @@ -817,11 +845,23 @@ class ObsRulePart extends Disposable { // If the formula failed validation, the error message to show. Blank if valid. private _formulaError = Observable.create(this, ''); + // Error message if any validation failed. + private _error: Computed; + // rulePart is omitted for a new ObsRulePart added by the user. constructor(private _ruleSet: ObsRuleSet, private _rulePart?: RulePart) { super(); + this._error = Computed.create(this, (use) => { + return use(this._formulaError) || + ( !this._ruleSet.isLastCondition(use, this) && + use(this._aclFormula) === '' && + permissionSetToText(use(this._permissions)) !== '' ? + 'Condition cannot be blank' : '' + ); + }); + this.ruleStatus = Computed.create(this, (use) => { - if (use(this._formulaError)) { return RuleStatus.Invalid; } + if (use(this._error)) { return RuleStatus.Invalid; } if (use(this._checkPending)) { return RuleStatus.CheckPending; } return getChangedStatus( use(this._aclFormula) !== this._rulePart?.aclFormula || @@ -883,7 +923,7 @@ class ObsRulePart extends Disposable { ) ), ), - dom.maybe(this._formulaError, (msg) => cssConditionError(msg, testId('rule-error'))), + dom.maybe(this._error, (msg) => cssConditionError(msg, testId('rule-error'))), testId('rule-part'), ); } diff --git a/app/client/models/BaseRowModel.js b/app/client/models/BaseRowModel.js index 91f7237e..6191ec4a 100644 --- a/app/client/models/BaseRowModel.js +++ b/app/client/models/BaseRowModel.js @@ -119,10 +119,12 @@ BaseRowModel.prototype._process_RenameColumn = function(action, tableId, oldColI // handle standard renames differently if (this._fields.indexOf(newColId) !== -1) { console.error("RowModel #RenameColumn %s %s %s: already exists", tableId, oldColId, newColId); + return; } var index = this._fields.indexOf(oldColId); if (index === -1) { console.error("RowModel #RenameColumn %s %s %s: not found", tableId, oldColId, newColId); + return; } this._fields[index] = newColId; diff --git a/app/client/models/DocModel.ts b/app/client/models/DocModel.ts index 5457c116..ebaeab51 100644 --- a/app/client/models/DocModel.ts +++ b/app/client/models/DocModel.ts @@ -24,9 +24,6 @@ import * as rowset from 'app/client/models/rowset'; import {RowId} from 'app/client/models/rowset'; import {schema, SchemaTypes} from 'app/common/schema'; -import {ACLMembershipRec, createACLMembershipRec} from 'app/client/models/entities/ACLMembershipRec'; -import {ACLPrincipalRec, createACLPrincipalRec} from 'app/client/models/entities/ACLPrincipalRec'; -import {ACLResourceRec, createACLResourceRec} from 'app/client/models/entities/ACLResourceRec'; import {ColumnRec, createColumnRec} from 'app/client/models/entities/ColumnRec'; import {createDocInfoRec, DocInfoRec} from 'app/client/models/entities/DocInfoRec'; import {createPageRec, PageRec} from 'app/client/models/entities/PageRec'; @@ -41,8 +38,6 @@ import {createViewSectionRec, ViewSectionRec} from 'app/client/models/entities/V // Re-export all the entity types available. The recommended usage is like this: // import {ColumnRec, ViewFieldRec} from 'app/client/models/DocModel'; -export {ACLMembershipRec} from 'app/client/models/entities/ACLMembershipRec'; -export {ACLPrincipalRec} from 'app/client/models/entities/ACLPrincipalRec'; export {ColumnRec} from 'app/client/models/entities/ColumnRec'; export {DocInfoRec} from 'app/client/models/entities/DocInfoRec'; export {PageRec} from 'app/client/models/entities/PageRec'; @@ -115,9 +110,6 @@ export class DocModel { public tabBar: MTM = this._metaTableModel("_grist_TabBar", createTabBarRec); public validations: MTM = this._metaTableModel("_grist_Validations", createValidationRec); public replHist: MTM = this._metaTableModel("_grist_REPL_Hist", createREPLRec); - public aclPrincipals: MTM = this._metaTableModel("_grist_ACLPrincipals", createACLPrincipalRec); - public aclMemberships: MTM = this._metaTableModel("_grist_ACLMemberships", createACLMembershipRec); - public aclResources: MTM = this._metaTableModel("_grist_ACLResources", createACLResourceRec); public pages: MTM = this._metaTableModel("_grist_Pages", createPageRec); public allTables: KoArray; diff --git a/app/client/models/entities/ACLMembershipRec.ts b/app/client/models/entities/ACLMembershipRec.ts deleted file mode 100644 index b71cf447..00000000 --- a/app/client/models/entities/ACLMembershipRec.ts +++ /dev/null @@ -1,14 +0,0 @@ -import {ACLPrincipalRec, DocModel, IRowModel, refRecord} from 'app/client/models/DocModel'; -import * as ko from 'knockout'; - -// Table for containment relationships between Principals, e.g. user contains multiple -// instances, group contains multiple users, and groups may contain other groups. -export interface ACLMembershipRec extends IRowModel<"_grist_ACLMemberships"> { - parentRec: ko.Computed; - childRec: ko.Computed; -} - -export function createACLMembershipRec(this: ACLMembershipRec, docModel: DocModel): void { - this.parentRec = refRecord(docModel.aclPrincipals, this.parent); - this.childRec = refRecord(docModel.aclPrincipals, this.child); -} diff --git a/app/client/models/entities/ACLPrincipalRec.ts b/app/client/models/entities/ACLPrincipalRec.ts deleted file mode 100644 index b4e25329..00000000 --- a/app/client/models/entities/ACLPrincipalRec.ts +++ /dev/null @@ -1,29 +0,0 @@ -import {KoArray} from 'app/client/lib/koArray'; -import {ACLMembershipRec, DocModel, IRowModel, recordSet} from 'app/client/models/DocModel'; -import {KoSaveableObservable} from 'app/client/models/modelUtil'; -import * as ko from 'knockout'; - -// A principals used by ACL rules, including users, groups, and instances. -export interface ACLPrincipalRec extends IRowModel<"_grist_ACLPrincipals"> { - // Declare a more specific type for 'type' than what's set automatically from schema.ts. - type: KoSaveableObservable<'user'|'instance'|'group'>; - - // KoArray of ACLMembership row models which contain this principal as a child. - parentMemberships: ko.Computed>; - - // Gives an array of ACLPrincipal parents to this row model. - parents: ko.Computed; - - // KoArray of ACLMembership row models which contain this principal as a parent. - childMemberships: ko.Computed>; - - // Gives an array of ACLPrincipal children of this row model. - children: ko.Computed; -} - -export function createACLPrincipalRec(this: ACLPrincipalRec, docModel: DocModel): void { - this.parentMemberships = recordSet(this, docModel.aclMemberships, 'child'); - this.childMemberships = recordSet(this, docModel.aclMemberships, 'parent'); - this.parents = ko.pureComputed(() => this.parentMemberships().all().map(m => m.parentRec())); - this.children = ko.pureComputed(() => this.childMemberships().all().map(m => m.childRec())); -} diff --git a/app/client/models/entities/ACLResourceRec.ts b/app/client/models/entities/ACLResourceRec.ts deleted file mode 100644 index 61d41816..00000000 --- a/app/client/models/entities/ACLResourceRec.ts +++ /dev/null @@ -1,7 +0,0 @@ -import {DocModel, IRowModel} from 'app/client/models/DocModel'; - -export type ACLResourceRec = IRowModel<"_grist_ACLResources">; - -export function createACLResourceRec(this: ACLResourceRec, docModel: DocModel): void { - // no extra fields -} diff --git a/app/common/ACLRuleCollection.ts b/app/common/ACLRuleCollection.ts index 9219ff8c..2ad19b19 100644 --- a/app/common/ACLRuleCollection.ts +++ b/app/common/ACLRuleCollection.ts @@ -256,7 +256,7 @@ function readAclRules(docData: DocData, {log, compile}: ReadAclOptions): ReadAcl for (const [resourceId, rules] of rulesByResource.entries()) { const resourceRec = resourcesTable.getRecord(resourceId as number); if (!resourceRec) { - log.error(`ACLRule ${rules[0].id} ignored; refers to an invalid ACLResource ${resourceId}`); + throw new Error(`ACLRule ${rules[0].id} refers to an invalid ACLResource ${resourceId}`); continue; } if (!resourceRec.tableId || !resourceRec.colIds) { @@ -271,7 +271,7 @@ function readAclRules(docData: DocData, {log, compile}: ReadAclOptions): ReadAcl for (const rule of rules) { if (rule.userAttributes) { if (tableId !== '*' || colIds !== '*') { - log.warn(`ACLRule ${rule.id} ignored; user attributes must be on the default resource`); + throw new Error(`ACLRule ${rule.id} invalid; user attributes must be on the default resource`); continue; } const parsed = JSON.parse(String(rule.userAttributes)); @@ -279,15 +279,15 @@ function readAclRules(docData: DocData, {log, compile}: ReadAclOptions): ReadAcl if (!(parsed && typeof parsed === 'object' && [parsed.name, parsed.tableId, parsed.lookupColId, parsed.charId] .every(p => p && typeof p === 'string'))) { - log.warn(`User attribute rule ${rule.id} is invalid`); + throw new Error(`User attribute rule ${rule.id} is invalid`); continue; } parsed.origRecord = rule; userAttributes.push(parsed as UserAttributeRule); } else if (body.length > 0 && !body[body.length - 1].aclFormula) { - log.warn(`ACLRule ${rule.id} ignored because listed after default rule`); + throw new Error(`ACLRule ${rule.id} invalid because listed after default rule`); } else if (rule.aclFormula && !rule.aclFormulaParsed) { - log.warn(`ACLRule ${rule.id} ignored because missing its parsed formula`); + throw new Error(`ACLRule ${rule.id} invalid because missing its parsed formula`); } else { body.push({ origRecord: rule, diff --git a/app/server/lib/ACLFormula.ts b/app/server/lib/ACLFormula.ts index 9ef235f0..34f50824 100644 --- a/app/server/lib/ACLFormula.ts +++ b/app/server/lib/ACLFormula.ts @@ -86,12 +86,8 @@ function getAttr(value: any, attrName: string, valueNode: ParsedAclFormula): any } throw new Error(`No value for '${describeNode(valueNode)}'`); } - const result = (typeof value.get === 'function' ? value.get(attrName) : // InfoView - value[attrName]); - if (result === undefined) { - throw new Error(`No attribute '${describeNode(valueNode)}.${attrName}'`); - } - return result; + return (typeof value.get === 'function' ? value.get(attrName) : // InfoView + value[attrName]); } /** diff --git a/app/server/lib/GranularAccess.ts b/app/server/lib/GranularAccess.ts index b5eb703b..5af51032 100644 --- a/app/server/lib/GranularAccess.ts +++ b/app/server/lib/GranularAccess.ts @@ -1090,14 +1090,14 @@ export class RecordView implements InfoView { if (colId === 'id') { return this.data[2][this.index]; } - return this.data[3][colId][this.index]; + return this.data[3][colId]?.[this.index]; } public toJSON() { if (this.index === undefined) { return {}; } const results: {[key: string]: any} = {}; for (const key of Object.keys(this.data[3])) { - results[key] = this.data[3][key][this.index]; + results[key] = this.data[3][key]?.[this.index]; } return results; } diff --git a/sandbox/grist/acl.py b/sandbox/grist/acl.py index 20b4bc44..ea764b16 100644 --- a/sandbox/grist/acl.py +++ b/sandbox/grist/acl.py @@ -2,7 +2,15 @@ # It now retains only the minimum needed to keep new documents openable by old code, # and to produce the ActionBundles expected by other code. +import json + +from acl_formula import parse_acl_grist_entities import action_obj +import logger +import textbuilder + +log = logger.Logger(__name__, logger.INFO) + class Permissions(object): # Permission types and their combination are represented as bits of a single integer. @@ -36,3 +44,93 @@ def acl_read_split(action_group): bundle.undo.extend((0, da) for da in action_group.undo) bundle.retValues = action_group.retValues return bundle + + +def prepare_acl_table_renames(docmodel, useractions, table_renames_dict): + """ + Given a dict of table renames of the form {table_id: new_table_id}, returns a callback + that will apply updates to the affected ACL rules and resources. + """ + # If there are ACLResources that refer to the renamed table, prepare updates for those. + resource_updates = [] + for resource_rec in docmodel.aclResources.all: + if resource_rec.tableId in table_renames_dict: + resource_updates.append((resource_rec, {'tableId': table_renames_dict[resource_rec.tableId]})) + + # Collect updates for any ACLRules with UserAttributes that refer to the renamed table. + rule_updates = [] + for rule_rec in docmodel.aclRules.all: + if rule_rec.userAttributes: + try: + rule_info = json.loads(rule_rec.userAttributes) + if rule_info.get("tableId") in table_renames_dict: + rule_info["tableId"] = table_renames_dict[rule_info.get("tableId")] + rule_updates.append((rule_rec, {'userAttributes': json.dumps(rule_info)})) + except Exception, e: + log.warn("Error examining aclRule: %s" % (e,)) + + def do_renames(): + useractions.doBulkUpdateFromPairs('_grist_ACLResources', resource_updates) + useractions.doBulkUpdateFromPairs('_grist_ACLRules', rule_updates) + return do_renames + + +def prepare_acl_col_renames(docmodel, useractions, col_renames_dict): + """ + Given a dict of column renames of the form {(table_id, col_id): new_col_id}, returns a callback + that will apply updates to the affected ACL rules and resources. + """ + # Collect updates for ACLResources that refer to the renamed columns. + resource_updates = [] + for resource_rec in docmodel.aclResources.all: + t = resource_rec.tableId + if resource_rec.colIds and resource_rec.colIds != '*': + new_col_ids = ','.join((col_renames_dict.get((t, c)) or c) + for c in resource_rec.colIds.split(',')) + if new_col_ids != resource_rec.colIds: + resource_updates.append((resource_rec, {'colIds': new_col_ids})) + + # Collect updates for any ACLRules with UserAttributes that refer to the renamed column. + rule_updates = [] + user_attr_tables = {} # Maps name of user attribute to its lookup table + for rule_rec in docmodel.aclRules.all: + if rule_rec.userAttributes: + try: + rule_info = json.loads(rule_rec.userAttributes) + user_attr_tables[rule_info.get('name')] = rule_info.get('tableId') + new_col_id = col_renames_dict.get((rule_info.get("tableId"), rule_info.get("lookupColId"))) + if new_col_id: + rule_info["lookupColId"] = new_col_id + rule_updates.append((rule_rec, {'userAttributes': json.dumps(rule_info)})) + except Exception, e: + log.warn("Error examining aclRule: %s" % (e,)) + + # Go through again checking if anything in ACL formulas is affected by the rename. + for rule_rec in docmodel.aclRules.all: + if rule_rec.aclFormula: + # Positions are obtained from unicode version of formulas, so that's what we must patch + formula = rule_rec.aclFormula.decode('utf8') + patches = [] + + for entity in parse_acl_grist_entities(rule_rec.aclFormula): + if entity.type == 'recCol': + table_id = docmodel.aclResources.table.get_record(int(rule_rec.resource)).tableId + elif entity.type == 'userAttrCol': + table_id = user_attr_tables.get(entity.extra) + else: + continue + col_id = entity.name + new_col_id = col_renames_dict.get((table_id, col_id)) + if not new_col_id: + continue + patch = textbuilder.make_patch( + formula, entity.start_pos, entity.start_pos + len(entity.name), new_col_id) + patches.append(patch) + + replacer = textbuilder.Replacer(textbuilder.Text(formula), patches) + rule_updates.append((rule_rec, {'aclFormula': replacer.get_text().encode('utf8')})) + + def do_renames(): + useractions.doBulkUpdateFromPairs('_grist_ACLResources', resource_updates) + useractions.doBulkUpdateFromPairs('_grist_ACLRules', rule_updates) + return do_renames diff --git a/sandbox/grist/acl_formula.py b/sandbox/grist/acl_formula.py index 73245d49..0db91dd4 100644 --- a/sandbox/grist/acl_formula.py +++ b/sandbox/grist/acl_formula.py @@ -1,5 +1,8 @@ import ast import json +from collections import namedtuple + +import asttokens def parse_acl_formula(acl_formula): """ @@ -35,6 +38,27 @@ def parse_acl_formula_json(acl_formula): return json.dumps(parse_acl_formula(acl_formula)) if acl_formula else "" +# Entities encountered in ACL formulas, which may get renamed. +# type: 'recCol'|'userAttr'|'userAttrCol', +# start_pos: number, # start position of the token in the code. +# name: string, # the name that may be updated by a rename. +# extra: string|None, # name of userAttr in case of userAttrCol; otherwise None. +NamedEntity = namedtuple('NamedEntity', ('type', 'start_pos', 'name', 'extra')) + +def parse_acl_grist_entities(acl_formula): + """ + Parse the ACL formula collecting any entities that may be subject to renaming. Returns a + NamedEntity list. + """ + try: + atok = asttokens.ASTTokens(acl_formula, tree=ast.parse(acl_formula, mode='eval')) + converter = _EntityCollector() + converter.visit(atok.tree) + return converter.entities + except SyntaxError as err: + return [] + + named_constants = { 'True': True, 'False': False, @@ -89,3 +113,25 @@ class _TreeConverter(ast.NodeVisitor): def generic_visit(self, node): raise ValueError("Unsupported syntax at %s:%s" % (node.lineno, node.col_offset + 1)) + + +class _EntityCollector(_TreeConverter): + def __init__(self): + self.entities = [] # NamedEntity list + + def visit_Attribute(self, node): + parent = self.visit(node.value) + + # We recognize a couple of specific patterns for entities that may be affected by renames. + if parent == ['Name', 'rec']: + # rec.COL refers to the column from the table that the rule is on. + self.entities.append(NamedEntity('recCol', node.last_token.startpos, node.attr, None)) + if parent == ['Name', 'user']: + # user.ATTR is a user attribute. + self.entities.append(NamedEntity('userAttr', node.last_token.startpos, node.attr, None)) + elif parent[0] == 'Attr' and parent[1] == ['Name', 'user']: + # user.ATTR.COL is a column from the lookup table of the UserAttribute ATTR. + self.entities.append( + NamedEntity('userAttrCol', node.last_token.startpos, node.attr, parent[2])) + + return ["Attr", parent, node.attr] diff --git a/sandbox/grist/docmodel.py b/sandbox/grist/docmodel.py index 67facb19..8def5561 100644 --- a/sandbox/grist/docmodel.py +++ b/sandbox/grist/docmodel.py @@ -147,6 +147,8 @@ class DocModel(object): self.repl_hist = self._prep_table("_grist_REPL_Hist") self.attachments = self._prep_table("_grist_Attachments") self.pages = self._prep_table("_grist_Pages") + self.aclResources = self._prep_table("_grist_ACLResources") + self.aclRules = self._prep_table("_grist_ACLRules") def _prep_table(self, name): """ diff --git a/sandbox/grist/test_acl_renames.py b/sandbox/grist/test_acl_renames.py new file mode 100644 index 00000000..be342e39 --- /dev/null +++ b/sandbox/grist/test_acl_renames.py @@ -0,0 +1,127 @@ +# -*- coding: utf-8 -*- + +import json + +import test_engine +import testsamples +import useractions + +user_attr1 = { + 'name': 'School', + 'charId': 'Email', + 'tableId': 'Schools', + 'lookupColId': 'LiasonEmail', +} + +class TestACLRenames(test_engine.EngineTestCase): + + def setUp(self): + super(TestACLRenames, self).setUp() + + self.load_sample(testsamples.sample_students) + + # Add column to Schools to use with User Attribute. + self.engine.apply_user_actions([useractions.from_repr(ua) for ua in ( + ['AddColumn', 'Schools', 'LiasonEmail', {'type': 'Text'}], + ['AddRecord', '_grist_ACLResources', -1, {'tableId': '*', 'colIds': '*'}], + ['AddRecord', '_grist_ACLRules', None, { + 'resource': -1, + 'userAttributes': json.dumps(user_attr1), + }], + ['AddRecord', '_grist_ACLResources', -2, { + 'tableId': 'Students', 'colIds': 'firstName,lastName' + }], + ['AddRecord', '_grist_ACLResources', -3, { + 'tableId': 'Students', 'colIds': '*' + }], + ['AddRecord', '_grist_ACLRules', None, { + 'resource': -2, + # Include comments and unicode to check that renaming respects all that. + 'aclFormula': '( rec.schoolName != # ünîcødé comment\n user.School.name)', + 'permissionsText': 'none', + }], + ['AddRecord', '_grist_ACLRules', None, { + 'resource': -3, + 'permissionsText': 'all' + }], + )]) + + # Here's what we expect to be in the ACL tables (for reference in tests below). + self.assertTableData('_grist_ACLResources', cols="subset", data=[ + ['id', 'tableId', 'colIds'], + [1, '*', '*'], + [2, 'Students', 'firstName,lastName'], + [3, 'Students', '*'], + ]) + self.assertTableData('_grist_ACLRules', cols="subset", data=[ + ['id', 'resource', 'aclFormula', 'permissionsText', 'userAttributes'], + [1, 1, '', '', json.dumps(user_attr1)], + [2, 2, '( rec.schoolName != # ünîcødé comment\n user.School.name)', 'none', ''], + [3, 3, '', 'all', ''], + ]) + + def test_acl_table_renames(self): + # Rename some tables. + self.apply_user_action(['RenameTable', 'Students', 'Estudiantes']) + self.apply_user_action(['RenameTable', 'Schools', 'Escuelas']) + + user_attr1_renamed = dict(user_attr1, tableId='Escuelas') + + # Check the result of both renames. + self.assertTableData('_grist_ACLResources', cols="subset", data=[ + ['id', 'tableId', 'colIds'], + [1, '*', '*'], + [2, 'Estudiantes', 'firstName,lastName'], + [3, 'Estudiantes', '*'], + ]) + self.assertTableData('_grist_ACLRules', cols="subset", data=[ + ['id', 'resource', 'aclFormula', 'permissionsText', 'userAttributes'], + [1, 1, '', '', json.dumps(user_attr1_renamed)], + [2, 2, '( rec.schoolName != # ünîcødé comment\n user.School.name)', 'none', ''], + [3, 3, '', 'all', ''], + ]) + + def test_acl_column_renames(self): + # Rename some columns. + self.apply_user_action(['RenameColumn', 'Students', 'lastName', 'Family_Name']) + self.apply_user_action(['RenameColumn', 'Schools', 'name', 'schoolName']) + self.apply_user_action(['RenameColumn', 'Students', 'schoolName', 'escuela']) + self.apply_user_action(['RenameColumn', 'Schools', 'LiasonEmail', 'AdminEmail']) + + user_attr1_renamed = dict(user_attr1, lookupColId='AdminEmail') + + # Check the result of both renames. + self.assertTableData('_grist_ACLResources', cols="subset", data=[ + ['id', 'tableId', 'colIds'], + [1, '*', '*'], + [2, 'Students', 'firstName,Family_Name'], + [3, 'Students', '*'], + ]) + self.assertTableData('_grist_ACLRules', cols="subset", data=[ + ['id', 'resource', 'aclFormula', 'permissionsText', 'userAttributes'], + [1, 1, '', '', json.dumps(user_attr1_renamed)], + [2, 2, '( rec.escuela != # ünîcødé comment\n user.School.schoolName)', 'none', ''], + [3, 3, '', 'all', ''], + ]) + + def test_multiple_renames(self): + # Combine several renames into one bundle. + self.engine.apply_user_actions([useractions.from_repr(ua) for ua in ( + ['RenameColumn', 'Students', 'firstName', 'Given_Name'], + ['RenameColumn', 'Students', 'lastName', 'Family_Name'], + ['RenameTable', 'Students', 'Students2'], + ['RenameColumn', 'Students2', 'schoolName', 'escuela'], + ['RenameColumn', 'Schools', 'name', 'schoolName'], + )]) + self.assertTableData('_grist_ACLResources', cols="subset", data=[ + ['id', 'tableId', 'colIds'], + [1, '*', '*'], + [2, 'Students2', 'Given_Name,Family_Name'], + [3, 'Students2', '*'], + ]) + self.assertTableData('_grist_ACLRules', cols="subset", data=[ + ['id', 'resource', 'aclFormula', 'permissionsText', 'userAttributes'], + [1, 1, '', '', json.dumps(user_attr1)], + [2, 2, '( rec.escuela != # ünîcødé comment\n user.School.schoolName)', 'none', ''], + [3, 3, '', 'all', ''], + ]) diff --git a/sandbox/grist/useractions.py b/sandbox/grist/useractions.py index cedf86dc..430bdd21 100644 --- a/sandbox/grist/useractions.py +++ b/sandbox/grist/useractions.py @@ -465,6 +465,8 @@ class UserActions(object): if 'type' in values: self.doModifyColumn(col.tableId, col.colId, {'type': 'Int'}) + make_acl_updates = acl.prepare_acl_table_renames(self._docmodel, self, table_renames) + # Collect all the table renames, and do the actual schema actions to apply them. for tbl, values in update_pairs: if has_diff_value(values, 'tableId', tbl.tableId): @@ -480,6 +482,7 @@ class UserActions(object): for col, values in col_updates.iteritems(): self.doModifyColumn(col.tableId, col.colId, values) self.doBulkUpdateFromPairs('_grist_Tables_column', col_updates.items()) + make_acl_updates() @override_action('BulkUpdateRecord', '_grist_Tables_column') @@ -531,6 +534,8 @@ class UserActions(object): for key, value in values.iteritems()): raise ValueError("Cannot modify summary group-by column '%s'" % col.colId) + make_acl_updates = acl.prepare_acl_col_renames(self._docmodel, self, renames) + for c, values in update_pairs: # Trigger ModifyColumn and RenameColumn as necessary schema_colinfo = select_keys(values, _modify_col_schema_props) @@ -546,6 +551,7 @@ class UserActions(object): widgetOptions='', displayCol=0) self.doBulkUpdateFromPairs(table_id, update_pairs) + make_acl_updates() @override_action('BulkUpdateRecord', '_grist_Views')