Archives
/
gristlabs_grist-core
mirror of https://github.com/gristlabs/grist-core
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

(core) add buttons to delete bad rules

Browse Source 
Summary:
When access rules refer to tables and/or columns that no longer exist, offer convenient buttons to remove these rules.

It could alternatively be useful to generate errors when deleting tables or columns that are mentioned in access rules, and refuse to do so unless the access rules are updated first.

Test Plan: added and updated tests

Reviewers: georgegevoian

Reviewed By: georgegevoian

Subscribers: jarek

Differential Revision: https://phab.getgrist.com/D3718
pull/371/head
Paul Fitzpatrick 1 year ago
parent 8c610dcb33
commit ebaf04dace
5 changed files with 201 additions and 11 deletions
Show Stats Download Patch File Download Diff File

129
app/client/aclui/AccessRules.ts
Unescape View File

@ -26,7 +26,7 @@ import {
summarizePermissionSet
} from 'app/common/ACLPermissions';
import {ACLRuleCollection, SPECIAL_RULES_TABLE_ID} from 'app/common/ACLRuleCollection';
import {AclTableDescription, getTableTitle} from 'app/common/ActiveDocAPI';
import {AclRuleProblem, AclTableDescription, getTableTitle} from 'app/common/ActiveDocAPI';
import {BulkColValues, getColValues, RowRecord, UserAction} from 'app/common/DocActions';
import {
FormulaProperties,
@ -112,6 +112,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, '');
// Details of rule problems, for offering solutions to the user.
private _ruleProblems = this.autoDispose(obsArray<AclRuleProblem>());
// Map of tableId to basic metadata for all tables in the document.
private _aclResources = new Map<string, AclTableDescription>();
@ -196,7 +199,8 @@ export class AccessRules extends Disposable {
this._updateDocAccessData(),
this._gristDoc.docComm.getAclResources(),
]);
this._aclResources = new Map(Object.entries(aclResources));
this._aclResources = new Map(Object.entries(aclResources.tables));
this._ruleProblems.set(aclResources.problems);
if (this.isDisposed()) { return; }
this._tableRules.set(
@ -354,6 +358,14 @@ export class AccessRules extends Disposable {
dom.text(this._errorMessage),
testId('access-rules-error')
),
dom.maybe(use => {
const ruleProblems = use(this._ruleProblems);
return ruleProblems.length > 0 ? ruleProblems : null;
}, ruleProblems =>
cssSection(
cssRuleProblems(
this.buildRuleProblemsDom(ruleProblems)))),
shadowScroll(
dom.maybe(use => use(this._userAttrRules).length, () =>
cssSection(
@ -398,6 +410,26 @@ export class AccessRules extends Disposable {
);
}
public buildRuleProblemsDom(ruleProblems: AclRuleProblem[]) {
const buttons: Array<HTMLAnchorElement | HTMLButtonElement> = [];
for (const problem of ruleProblems) {
// Is the problem a missing table?
if (problem.tables) {
this._addButtonsForMissingTables(buttons, problem.tables.tableIds);
}
// Is the problem a missing column?
if (problem.columns) {
this._addButtonsForMissingColumns(buttons, problem.columns.tableId, problem.columns.colIds);
}
// Is the problem a misconfigured user attribute?
if (problem.userAttributes) {
const names = problem.userAttributes.names;
this._addButtonsForMisconfiguredUserAttributes(buttons, names);
}
}
return buttons.map(button => dom('span', button));
}
/**
* Get a list of all rule records, for saving.
*/
@ -476,6 +508,63 @@ export class AccessRules extends Disposable {
this._aclUsersPopup.init(pageModel, permissionData);
}
private _addButtonsForMissingTables(buttons: Array<HTMLAnchorElement | HTMLButtonElement>, tableIds: string[]) {
for (const tableId of tableIds) {
// We don't know what the table's name was, just its tableId.
// Hopefully, the user will understand.
const title = t('RemoveRulesMentioningTable', { tableId });
const button = bigBasicButton(title, cssRemoveIcon('Remove'), dom.on('click', async () => {
await Promise.all(this._tableRules.get()
.filter(rules => rules.tableId === tableId)
.map(rules => rules.remove()));
button.style.display = 'none';
}));
buttons.push(button);
}
}
private _addButtonsForMissingColumns(buttons: Array<HTMLAnchorElement | HTMLButtonElement>,
tableId: string, colIds: string[]) {
const removeColRules = (rules: TableRules, colId: string) => {
for (const rule of rules.columnRuleSets.get()) {
const ruleColIds = new Set(rule.getColIdList());
if (!ruleColIds.has(colId)) { continue; }
if (ruleColIds.size === 1) {
rule.remove();
} else {
rule.removeColId(colId);
}
}
};
for (const colId of colIds) {
// TODO: we could translate tableId to table name in this case.
const title = t('RemoveRulesMentioningColumn', { tableId, colId });
const button = bigBasicButton(title, cssRemoveIcon('Remove'), dom.on('click', async () => {
await Promise.all(this._tableRules.get()
.filter(rules => rules.tableId === tableId)
.map(rules => removeColRules(rules, colId)));
button.style.display = 'none';
}));
buttons.push(button);
}
}
private _addButtonsForMisconfiguredUserAttributes(
buttons: Array<HTMLAnchorElement | HTMLButtonElement>,
names: string[]
) {
for (const name of names) {
const title = t('RemoveUserAttribute', {name});
const button = bigBasicButton(title, cssRemoveIcon('Remove'), dom.on('click', async () => {
await Promise.all(this._userAttrRules.get()
.filter(rule => rule.name.get() === name)
.map(rule => rule.remove()));
button.style.display = 'none';
}));
buttons.push(button);
}
}
}
// Represents all rules for a table.
@ -521,6 +610,14 @@ class TableRules extends Disposable {
});
}
public remove() {
this._accessRules.removeTableRules(this);
}
public get columnRuleSets() {
return this._columnRuleSets;
}
public buildDom() {
return cssSection(
cssSectionHeading(
@ -530,7 +627,7 @@ class TableRules extends Disposable {
menuItemAsync(() => this._addColumnRuleSet(), t('AddColumnRule')),
menuItemAsync(() => this._addDefaultRuleSet(), t('AddDefaultRule'),
dom.cls('disabled', use => Boolean(use(this._defaultRuleSet)))),
menuItemAsync(() => this._accessRules.removeTableRules(this), t('DeleteTableRules')),
menuItemAsync(() => this.remove(), t('DeleteTableRules')),
]),
testId('rule-table-menu-btn'),
),
@ -707,6 +804,10 @@ abstract class ObsRuleSet extends Disposable {
});
}
public remove() {
this._tableRules?.removeRuleSet(this);
}
public getRules(tableId: string): RuleRec[] {
// Return every part in the body, tacking on resourceRec to each rule.
return this._body.get().map(part => ({
@ -864,6 +965,10 @@ class ColumnObsRuleSet extends ObsRuleSet {
return this._colIds.get();
}
public removeColId(colId: string) {
this._colIds.set(this._colIds.get().filter(c => (c !== colId)));
}
public getColIds(): string {
return this._colIds.get().join(",");
}
@ -1031,6 +1136,10 @@ class ObsUserAttributeRule extends Disposable {
});
}
public remove() {
this._accessRules.removeUserAttributes(this);
}
public get name() { return this._name; }
public get tableId() { return this._tableId; }
@ -1440,6 +1549,10 @@ const cssDropdownIcon = styled(icon, `
margin: -2px -2px 0 4px;
`);
const cssRemoveIcon = styled(icon, `
margin: -2px -2px 0 4px;
`);
const cssSection = styled('div', `
margin: 16px 16px 24px 16px;
`);
@ -1581,3 +1694,13 @@ const cssDefaultLabel = styled('div', `
color: ${theme.accessRulesTableBodyFg};
font-weight: bold;
`);
const cssRuleProblems = styled('div', `
flex: auto;
height: 100%;
width: 100%;
display: flex;
flex-direction: row;
flex-wrap: wrap;
gap: 8px;
`);

44
app/common/ACLRuleCollection.ts
Unescape View File

@ -1,4 +1,5 @@
import {parsePermissions} from 'app/common/ACLPermissions';
import {AclRuleProblem} from 'app/common/ActiveDocAPI';
import {ILogger} from 'app/common/BaseAPI';
import {DocData} from 'app/common/DocData';
import {AclMatchFunc, ParsedAclFormula, RulePart, RuleSet, UserAttributeRule} from 'app/common/GranularAccessClause';
@ -232,6 +233,20 @@ export class ACLRuleCollection {
* Check that all references to table and column IDs in ACL rules are valid.
*/
public checkDocEntities(docData: DocData) {
const problems = this.findRuleProblems(docData);
if (problems.length === 0) { return; }
throw new Error(problems[0].comment);
}
/**
* Enumerate rule problems caused by table and column IDs that are not valid.
* Problems include:
* - Rules for a table that does not exist
* - Rules for columns that include a column that does not exist
* - User attributes links to a column that does not exist
*/
public findRuleProblems(docData: DocData): AclRuleProblem[] {
const problems: AclRuleProblem[] = [];
const tablesTable = docData.getMetaTable('_grist_Tables');
const columnsTable = docData.getMetaTable('_grist_Tables_column');
@ -239,7 +254,12 @@ export class ACLRuleCollection {
const validTableIds = new Set(tablesTable.getColValues('tableId'));
const invalidTables = this.getAllTableIds().filter(t => !validTableIds.has(t));
if (invalidTables.length > 0) {
throw new Error(`Invalid tables in rules: ${invalidTables.join(', ')}`);
problems.push({
tables: {
tableIds: invalidTables,
},
comment: `Invalid tables in rules: ${invalidTables.join(', ')}`,
});
}
// Collect valid columns, grouped by tableRef (rowId of table record).
@ -249,15 +269,22 @@ export class ACLRuleCollection {
getSetMapValue(validColumns, colTableRefs[i], () => new Set()).add(colId);
}
// For each table, check that any explicitly mentioned columns are valid.
// For each valid table, check that any explicitly mentioned columns are valid.
for (const tableId of this.getAllTableIds()) {
if (!validTableIds.has(tableId)) { continue; }
const tableRef = tablesTable.findRow('tableId', tableId);
const validTableCols = validColumns.get(tableRef);
for (const ruleSet of this.getAllColumnRuleSets(tableId)) {
if (Array.isArray(ruleSet.colIds)) {
const invalidColIds = ruleSet.colIds.filter(c => !validTableCols?.has(c));
if (invalidColIds.length > 0) {
throw new Error(`Invalid columns in rules for table ${tableId}: ${invalidColIds.join(', ')}`);
problems.push({
columns: {
tableId,
colIds: invalidColIds,
},
comment: `Invalid columns in rules for table ${tableId}: ${invalidColIds.join(', ')}`,
});
}
}
}
@ -265,16 +292,25 @@ export class ACLRuleCollection {
// Check for valid tableId/lookupColId combinations in UserAttribute rules.
const invalidUAColumns: string[] = [];
const names: string[] = [];
for (const rule of this.getUserAttributeRules().values()) {
const tableRef = tablesTable.findRow('tableId', rule.tableId);
const colRef = columnsTable.findMatchingRowId({parentId: tableRef, colId: rule.lookupColId});
if (!colRef) {
invalidUAColumns.push(`${rule.tableId}.${rule.lookupColId}`);
names.push(rule.name);
}
}
if (invalidUAColumns.length > 0) {
throw new Error(`Invalid columns in User Attribute rules: ${invalidUAColumns.join(', ')}`);
problems.push({
userAttributes: {
invalidUAColumns,
names,
},
comment: `Invalid columns in User Attribute rules: ${invalidUAColumns.join(', ')}`,
});
}
return problems;
}
private _safeReadAclRules(docData: DocData, options: ReadAclOptions): ReadAclResults {

22
app/common/ActiveDocAPI.ts
Unescape View File

@ -176,6 +176,26 @@ export interface AclTableDescription {
groupByColLabels: string[] | null; // Labels of groupby columns for summary tables, or null.
}
export interface AclResources {
tables: {[tableId: string]: AclTableDescription};
problems: AclRuleProblem[];
}
export interface AclRuleProblem {
tables?: {
tableIds: string[],
};
columns?: {
tableId: string,
colIds: string[],
};
userAttributes?: {
invalidUAColumns: string[],
names: string[],
}
comment: string;
}
export function getTableTitle(table: AclTableDescription): string {
let {title} = table;
if (table.groupByColLabels) {
@ -349,7 +369,7 @@ export interface ActiveDocAPI {
* 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]: AclTableDescription}>;
getAclResources(): Promise<AclResources>;
/**
* Wait for document to finish initializing.

12
app/server/lib/ActiveDoc.ts
Unescape View File

@ -4,6 +4,7 @@
* change events.
*/
import {ACLRuleCollection} from 'app/common/ACLRuleCollection';
import {
getEnvContent,
LocalActionBundle,
@ -14,6 +15,7 @@ import {
import {ActionGroup, MinimalActionGroup} from 'app/common/ActionGroup';
import {ActionSummary} from "app/common/ActionSummary";
import {
AclResources,
AclTableDescription,
ApplyUAExtendedOptions,
ApplyUAOptions,
@ -1427,8 +1429,11 @@ export class ActiveDoc extends EventEmitter {
* Returns the full set of tableIds, with basic metadata 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.
*
* Also returns information about resources mentioned in rules that no longer
* exist.
*/
public async getAclResources(docSession: DocSession): Promise<{[tableId: string]: AclTableDescription}> {
public async getAclResources(docSession: DocSession): Promise<AclResources> {
if (!this.docData || !await this._granularAccess.hasAccessRulesPermission(docSession)) {
throw new Error('Cannot list ACL resources');
}
@ -1453,7 +1458,10 @@ export class ActiveDoc extends EventEmitter {
result[tableId].groupByColLabels!.push(sourceCol.label);
}
}
return result;
const ruleCollection = new ACLRuleCollection();
await ruleCollection.update(this.docData, {log});
const problems = ruleCollection.findRuleProblems(this.docData);
return {tables: result, problems};
}
/**

5
static/locales/en.client.json
Unescape View File

@ -584,7 +584,10 @@
"AttributeNamePlaceholder": "Attribute name",
"Everyone": "Everyone",
"EveryoneElse": "Everyone Else",
"EnterCondition": "Enter Condition"
"EnterCondition": "Enter Condition",
"RemoveRulesMentioningTable": "Remove {{- tableId }} rules",
"RemoveRulesMentioningColumn": "Remove column {{- colId }} from {{- tableId }} rules",
"RemoveUserAttribute": "Remove {{- name }} user attribute"
},
"PermissionsWidget": {
"AllowAll": "Allow All",

Write Preview
Loading…
Cancel
Save