(core) Trim unapplicable permissions bits for column rules, both at parse time, and in UI

Summary:
- UI now trims column rules before saving.
- When rules are loaded, bits that aren't applicable to a resource get ignored.
  This should fix the incorrect behavior in existing docs without a migration.

Test Plan:
- Added test of UI, that it now sends trimmed rules
- Added a unitteset of new trimPermissions() function
- Add test of fixed interpretation of existing rules: now only permission bits
  applicable to a resource get respected. I.e. create/delete/schemaEdit are
  ignored in column rules, and schemaEdit is also ignored in table rules.
- Note that DuplicateTest was affected: updated on the assumption that
  schemaEdit still can't actually apply at a table level.

Reviewers: georgegevoian, paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D4205
This commit is contained in:
Dmitry S 2024-03-03 22:41:48 -05:00
parent 0532ed6547
commit ca8ac806db
6 changed files with 63 additions and 16 deletions

View File

@ -6,7 +6,7 @@ import {aclFormulaEditor} from 'app/client/aclui/ACLFormulaEditor';
import {aclMemoEditor} from 'app/client/aclui/ACLMemoEditor';
import {aclSelect} from 'app/client/aclui/ACLSelect';
import {ACLUsersPopup} from 'app/client/aclui/ACLUsers';
import {PermissionKey, permissionsWidget} from 'app/client/aclui/PermissionsWidget';
import {permissionsWidget} from 'app/client/aclui/PermissionsWidget';
import {GristDoc} from 'app/client/components/GristDoc';
import {logTelemetryEvent} from 'app/client/lib/telemetry';
import {reportError, UserError} from 'app/client/models/errors';
@ -20,13 +20,17 @@ import {textInput} from 'app/client/ui2018/editableLabel';
import {cssIconButton, icon} from 'app/client/ui2018/icons';
import {menu, menuItemAsync} from 'app/client/ui2018/menus';
import {
AVAILABLE_BITS_COLUMNS,
AVAILABLE_BITS_TABLES,
emptyPermissionSet,
MixedPermissionValue,
parsePermissions,
PartialPermissionSet,
PermissionKey,
permissionSetToText,
summarizePermissions,
summarizePermissionSet
summarizePermissionSet,
trimPermissions
} from 'app/common/ACLPermissions';
import {ACLRuleCollection, isSchemaEditResource, SPECIAL_RULES_TABLE_ID} from 'app/common/ACLRuleCollection';
import {AclRuleProblem, AclTableDescription, getTableTitle} from 'app/common/ActiveDocAPI';
@ -990,12 +994,19 @@ abstract class ObsRuleSet extends Disposable {
// Should not happen.
continue;
}
// Include only the permissions for the bits that this RuleSet supports. E.g. this matters
// for seed rules, which may include create/delete bits which shouldn't apply to columns.
const origPermissions = parsePermissions(permissionsText);
const trimmedPermissions = trimPermissions(origPermissions, this.getAvailableBits());
const trimmedPermissionsText = permissionSetToText(trimmedPermissions);
this.addRulePart(
this.getFirst() || null,
{
aclFormula,
permissionsText,
permissions: parsePermissions(permissionsText),
permissionsText: trimmedPermissionsText,
permissions: trimmedPermissions,
memo,
},
true,
@ -1048,7 +1059,7 @@ abstract class ObsRuleSet extends Disposable {
* Which permission bits to allow the user to set.
*/
public getAvailableBits(): PermissionKey[] {
return ['read', 'update', 'create', 'delete'];
return AVAILABLE_BITS_TABLES;
}
/**
@ -1117,8 +1128,7 @@ class ColumnObsRuleSet extends ObsRuleSet {
}
public getAvailableBits(): PermissionKey[] {
// Create/Delete bits can't be set on a column-specific rule.
return ['read', 'update'];
return AVAILABLE_BITS_COLUMNS;
}
public hasColumns() {

View File

@ -6,15 +6,12 @@ import {colors, testId, theme} from 'app/client/ui2018/cssVars';
import {cssIconButton, icon} from 'app/client/ui2018/icons';
import {menu, menuIcon, menuItem} from 'app/client/ui2018/menus';
import {PartialPermissionSet, PartialPermissionValue} from 'app/common/ACLPermissions';
import {ALL_PERMISSION_PROPS, emptyPermissionSet} from 'app/common/ACLPermissions';
import {ALL_PERMISSION_PROPS, emptyPermissionSet, PermissionKey} from 'app/common/ACLPermissions';
import {capitalize} from 'app/common/gutil';
import {dom, DomElementArg, Observable, styled} from 'grainjs';
import isEqual = require('lodash/isEqual');
import {makeT} from 'app/client/lib/localization';
// One of the strings 'read', 'update', etc.
export type PermissionKey = keyof PartialPermissionSet;
// Canonical order of permission bits when rendered in a permissionsWidget.
const PERMISSION_BIT_ORDER = 'RUCDS';

View File

@ -41,7 +41,10 @@ export type PartialPermissionSet = PermissionSet<PartialPermissionValue>;
export type MixedPermissionSet = PermissionSet<MixedPermissionValue>;
export type TablePermissionSet = PermissionSet<TablePermissionValue>;
const PERMISSION_BITS: {[letter: string]: keyof PermissionSet} = {
// One of the strings 'read', 'update', etc.
export type PermissionKey = keyof PermissionSet;
const PERMISSION_BITS: {[letter: string]: PermissionKey} = {
R: 'read',
C: 'create',
U: 'update',
@ -60,6 +63,9 @@ const ALIASES: {[key: string]: string} = {
};
const REVERSE_ALIASES = fromPairs(Object.entries(ALIASES).map(([alias, value]) => [value, alias]));
export const AVAILABLE_BITS_TABLES: PermissionKey[] = ['read', 'update', 'create', 'delete'];
export const AVAILABLE_BITS_COLUMNS: PermissionKey[] = ['read', 'update'];
// Comes in useful for initializing unset PermissionSets.
export function emptyPermissionSet(): PartialPermissionSet {
return {read: "", create: "", update: "", delete: "", schemaEdit: ""};
@ -141,6 +147,20 @@ export function mergePartialPermissions(a: PartialPermissionSet, b: PartialPermi
return mergePermissions([a, b], ([_a, _b]) => combinePartialPermission(_a, _b));
}
/**
* Returns permissions trimmed to include only the available bits, and empty for any other bits.
*/
export function trimPermissions(
permissions: PartialPermissionSet, availableBits: PermissionKey[]
): PartialPermissionSet {
const trimmed = emptyPermissionSet();
for (const bit of availableBits) {
trimmed[bit] = permissions[bit];
}
return trimmed;
}
/**
* Merge a list of PermissionSets by combining individual bits.
*/

View File

@ -1,4 +1,5 @@
import {parsePermissions, permissionSetToText, splitSchemaEditPermissionSet} from 'app/common/ACLPermissions';
import {AVAILABLE_BITS_COLUMNS, AVAILABLE_BITS_TABLES, trimPermissions} from 'app/common/ACLPermissions';
import {ACLShareRules, TableWithOverlay} from 'app/common/ACLShareRules';
import {AclRuleProblem} from 'app/common/ActiveDocAPI';
import {DocData} from 'app/common/DocData';
@ -537,13 +538,18 @@ function readAclRules(docData: DocData, {log, compile, enrichRulesForImplementat
if (hasShares && rule.id >= 0) {
aclFormulaParsed = shareRules.transformNonShareRules({rule, aclFormulaParsed});
}
let permissions = parsePermissions(String(rule.permissionsText));
if (tableId !== '*' && tableId !== SPECIAL_RULES_TABLE_ID) {
const availableBits = (colIds === '*') ? AVAILABLE_BITS_TABLES : AVAILABLE_BITS_COLUMNS;
permissions = trimPermissions(permissions, availableBits);
}
body.push({
origRecord: rule,
aclFormula: String(rule.aclFormula),
matchFunc: rule.aclFormula ? compile?.(aclFormulaParsed) : defaultMatchFunc,
memo: rule.memo,
permissions: parsePermissions(String(rule.permissionsText)),
permissionsText: String(rule.permissionsText),
permissions,
permissionsText: permissionSetToText(permissions)
});
}
}

View File

@ -2,3 +2,6 @@ const chai = require('chai');
const chaiAsPromised = require('chai-as-promised');
chai.use(chaiAsPromised);
// By default this is false, which affects asserts like isRejected and isFulfilled.
chai.config.includeStack = true;

View File

@ -1,7 +1,7 @@
import {emptyPermissionSet, PartialPermissionSet,
import {emptyPermissionSet, PartialPermissionSet, PermissionKey,
summarizePermissions, summarizePermissionSet} from 'app/common/ACLPermissions';
import {makePartialPermissions, parsePermissions, permissionSetToText} from 'app/common/ACLPermissions';
import {mergePartialPermissions, mergePermissions} from 'app/common/ACLPermissions';
import {mergePartialPermissions, mergePermissions, trimPermissions} from 'app/common/ACLPermissions';
import {assert} from 'chai';
describe("ACLPermissions", function() {
@ -112,6 +112,17 @@ describe("ACLPermissions", function() {
);
});
it('should support trimPermissions', function() {
const trim = (permissionsText: string, availableBits: PermissionKey[]) =>
permissionSetToText(trimPermissions(parsePermissions(permissionsText), availableBits));
assert.deepEqual(trim("+CRUD", ["read", "update"]), "+RU");
assert.deepEqual(trim("all", ["read", "update"]), "+RU");
assert.deepEqual(trim("-C+R-U+D-S", ["update", "read"]), "+R-U");
assert.deepEqual(trim("none", ["read", "update", "create", "delete", "schemaEdit"]), "none");
assert.deepEqual(trim("none", ["read", "update", "create", "delete"]), "-CRUD");
assert.deepEqual(trim("none", ["read"]), "-R");
});
it ('should allow summarization of permission sets', function() {
assert.deepEqual(summarizePermissionSet(parsePermissions("+U-D")), 'mixed');
assert.deepEqual(summarizePermissionSet(parsePermissions("+U+D")), 'allow');