(core) Implement checkbox for SchemaEdit permission in Access Rules UI.

Summary:
- Introduces a fictitious "*SPECIAL:SchemaEdit" resource in UI only.
- Hides "S" bit for the default rule section.
- Shows a checkbox UI similar to other checkboxes, with an additional
  dismissable warning.

Test Plan: Added a browser test

Reviewers: paulfitz, georgegevoian

Reviewed By: paulfitz, georgegevoian

Differential Revision: https://phab.getgrist.com/D3765

app/client/aclui/AccessRules.ts

@@ -26,7 +26,7 @@ import {
   summarizePermissions,
   summarizePermissionSet
 } from 'app/common/ACLPermissions';
-import {ACLRuleCollection, SPECIAL_RULES_TABLE_ID} from 'app/common/ACLRuleCollection';
+import {ACLRuleCollection, isSchemaEditResource, SPECIAL_RULES_TABLE_ID} from 'app/common/ACLRuleCollection';
 import {AclRuleProblem, AclTableDescription, getTableTitle} from 'app/common/ActiveDocAPI';
 import {BulkColValues, getColValues, RowRecord, UserAction} from 'app/common/DocActions';
 import {
@@ -45,6 +45,7 @@ import {
   Computed,
   Disposable,
   dom,
+  DomContents,
   DomElementArg,
   IDisposableOwner,
   MutableObsArray,
@@ -201,7 +202,7 @@ export class AccessRules extends Disposable {
     const rules = this._ruleCollection;
 
     const [ , , aclResources] = await Promise.all([
-      rules.update(this._gristDoc.docData, {log: console}),
+      rules.update(this._gristDoc.docData, {log: console, pullOutSchemaEdit: true}),
       this._updateDocAccessData(),
       this._gristDoc.docComm.getAclResources(),
     ]);
@@ -217,7 +218,7 @@ export class AccessRules extends Disposable {
     );
 
     const withDefaultRules = ['SeedRule'];
-    const separateRules = ['FullCopies', 'AccessRules'];
+    const separateRules = ['SchemaEdit', 'FullCopies', 'AccessRules'];
 
     SpecialRules.create(
       this._specialRulesWithDefault, SPECIAL_RULES_TABLE_ID, this,
@@ -252,12 +253,20 @@ export class AccessRules extends Disposable {
       [{tableId: '*', colIds: '*'}],
       this._specialRulesWithDefault.get()?.getResources() || [],
       this._specialRulesSeparate.get()?.getResources() || [],
-      ...this._tableRules.get().map(tr => tr.getResources()))
-      .map(r => ({id: -1, ...r}));
+      ...this._tableRules.get().map(tr => tr.getResources())
+    )
+    // Skip the fake "*SPECIAL:SchemaEdit" resource (frontend-specific); these rules are saved to the default resource.
+    .filter(resource => !isSchemaEditResource(resource))
+    .map(r => ({id: -1, ...r}));
 
     // Prepare userActions and a mapping of serializedResource to rowIds.
     const resourceSync = syncRecords(resourcesTable, newResources, serializeResource);
 
+    const defaultResourceRowId = resourceSync.rowIdMap.get(serializeResource({id: -1, tableId: '*', colIds: '*'}));
+    if (!defaultResourceRowId) {
+      throw new Error('Default resource missing in resource map');
+    }
+
     // For syncing rules, we'll go by rowId that we store with each RulePart and with the RuleSet.
     const newRules: RowRecord[] = [];
     for (const rule of this.getRules()) {
@@ -267,10 +276,16 @@ export class AccessRules extends Disposable {
       }
 
       // Look up the rowId for the resource.
-      const resourceKey = serializeResource(rule.resourceRec as RowRecord);
-      const resourceRowId = resourceSync.rowIdMap.get(resourceKey);
-      if (!resourceRowId) {
-        throw new Error(`Resource missing in resource map: ${resourceKey}`);
+      let resourceRowId: number|undefined;
+      // Assign the rules for the fake "*SPECIAL:SchemaEdit" resource to the default resource where they belong.
+      if (isSchemaEditResource(rule.resourceRec!)) {
+        resourceRowId = defaultResourceRowId;
+      } else {
+        const resourceKey = serializeResource(rule.resourceRec as RowRecord);
+        resourceRowId = resourceSync.rowIdMap.get(resourceKey);
+        if (!resourceRowId) {
+          throw new Error(`Resource missing in resource map: ${resourceKey}`);
+        }
       }
       newRules.push({
         id: rule.id || -1,
@@ -283,10 +298,6 @@ export class AccessRules extends Disposable {
     }
 
     // UserAttribute rules are listed in the same rulesTable.
-    const defaultResourceRowId = resourceSync.rowIdMap.get(serializeResource({id: -1, tableId: '*', colIds: '*'}));
-    if (!defaultResourceRowId) {
-      throw new Error('Default resource missing in resource map');
-    }
     for (const userAttr of this._userAttrRules.get()) {
       const rule = userAttr.getRule();
       newRules.push({
@@ -829,7 +840,12 @@ class SpecialRules extends TableRules {
     owner: IDisposableOwner, accessRules: AccessRules, tableRules: TableRules,
     ruleSet: RuleSet|undefined, initialColIds: string[],
   ): ColumnObsRuleSet {
-    return SpecialObsRuleSet.create(owner, accessRules, tableRules, ruleSet, initialColIds);
+    if (isEqual(ruleSet?.colIds, ['SchemaEdit'])) {
+      // The special rule for "schemaEdit" permissions.
+      return SpecialSchemaObsRuleSet.create(owner, accessRules, tableRules, ruleSet, initialColIds);
+    } else {
+      return SpecialObsRuleSet.create(owner, accessRules, tableRules, ruleSet, initialColIds);
+    }
   }
 }
 
@@ -1014,12 +1030,7 @@ abstract class ObsRuleSet extends Disposable {
    * Which permission bits to allow the user to set.
    */
   public getAvailableBits(): PermissionKey[] {
-    if (this._tableRules) {
-      return ['read', 'update', 'create', 'delete'];
-    } else {
-      // For the doc-wide rule set, expose the schemaEdit bit too.
-      return ['read', 'update', 'create', 'delete', 'schemaEdit'];
-    }
+    return ['read', 'update', 'create', 'delete'];
   }
 
   /**
@@ -1112,18 +1123,32 @@ class DefaultObsRuleSet extends ObsRuleSet {
   }
 }
 
+interface SpecialRuleBody {
+  permissions: string;
+  formula: string;
+}
+
 /**
  * Properties we need to know about how a special rule should function and
  * be rendered.
  */
-interface SpecialRuleProperties {
+interface SpecialRuleProperties extends SpecialRuleBody {
   description: string;
   name: string;
   availableBits: PermissionKey[];
-  permissions: string;
-  formula: string;
 }
 
+const schemaEditRules: {[key: string]: SpecialRuleBody} = {
+  allowEditors: {
+    permissions: '+S',
+    formula: 'user.Access == EDITOR',
+  },
+  denyEditors: {
+    permissions: '-S',
+    formula: 'user.Access != OWNER',
+  },
+};
+
 const specialRuleProperties: Record<string, SpecialRuleProperties> = {
   AccessRules: {
     name: t('Permission to view Access Rules'),
@@ -1147,6 +1172,13 @@ Useful for examples and templates, but not for sensitive data.`),
     permissions: '+CRUD',
     formula: 'user.Access in [OWNER]',
   },
+  SchemaEdit: {
+    name: t("Permission to edit document structure"),
+    description: t("Allow editors to edit structure (e.g. modify and delete tables, columns, " +
+        "layouts), and to write formulas, which give access to all data regardless of read restrictions."),
+    availableBits: ['schemaEdit'],
+    ...schemaEditRules.denyEditors,
+  },
 };
 
 function getSpecialRuleProperties(name: string): SpecialRuleProperties {
@@ -1165,32 +1197,29 @@ class SpecialObsRuleSet extends ColumnObsRuleSet {
   }
 
   public buildRuleSetDom() {
-    const isNonStandard: Observable<boolean> = Computed.create(null, this._body, (use, body) =>
-      !body.every(rule => rule.isBuiltInOrEmpty(use) || rule.matches(use, this.props.formula, this.props.permissions)));
-
-    const allowEveryone: Observable<boolean> = Computed.create(null, this._body,
-      (use, body) => !use(isNonStandard) && !body.every(rule => rule.isBuiltInOrEmpty(use)))
-      .onWrite(val => this._allowEveryone(val));
-
+    const isNonStandard = this._createIsNonStandardObs();
+    const isChecked = this._createIsCheckedObs(isNonStandard);
     if (isNonStandard.get()) {
       this._isExpanded.set(true);
     }
 
     return dom('div',
-      dom.autoDispose(allowEveryone),
+      dom.autoDispose(isChecked),
+      dom.autoDispose(isNonStandard),
       cssRuleDescription(
-        {style: 'white-space: pre-line;'},  // preserve line breaks in long descriptions
         cssIconButton(icon('Expand'),
           dom.style('transform', (use) => use(this._isExpanded) ? 'rotate(90deg)' : ''),
           dom.on('click', () => this._isExpanded.set(!this._isExpanded.get())),
           testId('rule-special-expand'),
+          {style: 'margin: -4px'},  // subtract padding to align better.
         ),
-        squareCheckbox(allowEveryone,
+        cssCheckbox(isChecked,
           dom.prop('disabled', isNonStandard),
           testId('rule-special-checkbox'),
         ),
         this.props.description,
       ),
+      this._buildDomWarning(),
       dom.maybe(this._isExpanded, () =>
         cssTableRounded(
           {style: 'margin-left: 56px'},
@@ -1238,14 +1267,29 @@ class SpecialObsRuleSet extends ColumnObsRuleSet {
     }
   }
 
+  protected _buildDomWarning(): DomContents {
+    return null;
+  }
+
+  // Observable for whether this ruleSet is "standard", i.e. checked or unchecked state, without
+  // any strange rules that need to be shown expanded with the checkbox greyed out.
+  protected _createIsNonStandardObs(): Observable<boolean> {
+    return Computed.create(null, this._body, (use, body) =>
+      !body.every(rule => rule.isBuiltInOrEmpty(use) || rule.matches(use, this.props.formula, this.props.permissions)));
+  }
+
+  // Observable for whether the checkbox should be shown as checked. Writing to it will update
+  // rules so as to toggle the checkbox.
+  protected _createIsCheckedObs(isNonStandard: Observable<boolean>): Observable<boolean> {
+    return Computed.create(null, this._body,
+      (use, body) => !use(isNonStandard) && !body.every(rule => rule.isBuiltInOrEmpty(use)))
+      .onWrite(val => this._allowEveryone(val));
+  }
+
   private _allowEveryone(value: boolean) {
     const builtInRules = this._body.get().filter(r => r.isBuiltIn());
     if (value) {
-      const rulePart: RulePart = {
-        aclFormula: this.props.formula,
-        permissionsText: this.props.permissions,
-        permissions: parsePermissions(this.props.permissions),
-      };
+      const rulePart = makeRulePart(this.props);
       this._body.set([ObsRulePart.create(this._body, this, rulePart, true), ...builtInRules]);
     } else {
       this._body.set(builtInRules);
@@ -1256,6 +1300,63 @@ class SpecialObsRuleSet extends ColumnObsRuleSet {
   }
 }
 
+function makeRulePart({permissions, formula}: SpecialRuleBody): RulePart {
+  const rulePart: RulePart = {
+    aclFormula: formula,
+    permissionsText: permissions,
+    permissions: parsePermissions(permissions),
+  };
+  return rulePart;
+}
+
+/**
+ * SchemaEdit permissions are moved out to a special fake resource "*SPECIAL:SchemaEdit" in the
+ * frontend, to be presented under their own checkbox option. Its behaviors are a bit different
+ * from other checkbox options; the differences are in the overridden methods here.
+ */
+class SpecialSchemaObsRuleSet extends SpecialObsRuleSet {
+  protected _buildDomWarning(): DomContents {
+    return dom.maybe(
+      (use) => use(this._body).every(rule => rule.isBuiltInOrEmpty(use)),
+      () => cssConditionError({style: 'margin-left: 56px; margin-bottom: 8px;'},
+        "This default should be changed if editors' access is to be limited. ",
+        dom('a', {style: 'color: inherit; text-decoration: underline'},
+          'Dismiss', dom.on('click', () => this._allowEditors('confirm'))),
+        testId('rule-schema-edit-warning'),
+      )
+    );
+  }
+
+  // SchemaEdit rules support an extra "standard" state, where a no-op rule exists (explicit rule
+  // allowing EDITORs SchemaEdit permission), in which case we don't show a warning.
+  protected _createIsNonStandardObs(): Observable<boolean> {
+    return Computed.create(null, this._body, (use, body) =>
+      !body.every(rule => rule.isBuiltInOrEmpty(use) || rule.matches(use, this.props.formula, this.props.permissions)
+        || rule.matches(use, schemaEditRules.allowEditors.formula, schemaEditRules.allowEditors.permissions)));
+  }
+
+  protected _createIsCheckedObs(isNonStandard: Observable<boolean>): Observable<boolean> {
+    return Computed.create(null, this._body,
+      (use, body) => body.every(rule => rule.isBuiltInOrEmpty(use)
+        || rule.matches(use, schemaEditRules.allowEditors.formula, schemaEditRules.allowEditors.permissions)))
+      .onWrite(val => this._allowEditors(val));
+  }
+
+  // The third "confirm" option is used by the "Dismiss" link in the warning.
+  private _allowEditors(value: boolean|'confirm') {
+    const builtInRules = this._body.get().filter(r => r.isBuiltIn());
+    if (value === 'confirm') {
+      const rulePart = makeRulePart(schemaEditRules.allowEditors);
+      this._body.set([ObsRulePart.create(this._body, this, rulePart, true), ...builtInRules]);
+    } else if (!value) {
+      const rulePart = makeRulePart(schemaEditRules.denyEditors);
+      this._body.set([ObsRulePart.create(this._body, this, rulePart, true), ...builtInRules]);
+    } else {
+      this._body.set(builtInRules);
+    }
+  }
+}
+
 class ObsUserAttributeRule extends Disposable {
   public ruleStatus: Computed<RuleStatus>;
 
@@ -1943,9 +2044,14 @@ const cssRuleBody = styled('div', `
 const cssRuleDescription = styled('div', `
   color: ${theme.text};
   display: flex;
-  align-items: center;
+  align-items: top;
   margin: 16px 0 8px 0;
-  gap: 8px;
+  gap: 12px;
+  white-space: pre-line;  /* preserve line breaks in long descriptions */
+`);
+
+const cssCheckbox = styled(squareCheckbox, `
+  flex: none;
 `);
 
 const cssCellContent = styled('div', `

app/common/ACLPermissions.ts

@@ -190,3 +190,25 @@ export function summarizePermissions(perms: MixedPermissionValue[]): MixedPermis
   const perm = perms[0];
   return perms.some(p => p !== perm) ? 'mixed' : perm;
 }
+
+
+function isEmpty(permissions: PartialPermissionSet): boolean {
+  return Object.values(permissions).every(v => v === "");
+}
+
+
+/**
+ * Divide up a PartialPermissionSet into two: one containing only the 'schemaEdit' permission bit,
+ * and the other containing everything else. Empty parts will be returned as undefined, except
+ * when both are empty, in which case nonSchemaEdit will be returned as an empty permission set.
+ */
+export function splitSchemaEditPermissionSet(permissions: PartialPermissionSet):
+    {schemaEdit?: PartialPermissionSet, nonSchemaEdit?: PartialPermissionSet} {
+
+  const schemaEdit = {...emptyPermissionSet(), schemaEdit: permissions.schemaEdit};
+  const nonSchemaEdit: PartialPermissionSet = {...permissions, schemaEdit: ""};
+  return {
+    schemaEdit: !isEmpty(schemaEdit) ? schemaEdit : undefined,
+    nonSchemaEdit: !isEmpty(nonSchemaEdit) || isEmpty(schemaEdit) ? nonSchemaEdit : undefined,
+  };
+}

app/common/ACLRuleCollection.ts

@@ -1,9 +1,9 @@
-import {parsePermissions} from 'app/common/ACLPermissions';
+import {parsePermissions, permissionSetToText, splitSchemaEditPermissionSet} 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';
-import {getSetMapValue} from 'app/common/gutil';
+import {getSetMapValue, isNonNullish} from 'app/common/gutil';
 import {MetaRowRecord} from 'app/common/TableData';
 import {decodeObject} from 'app/plugin/objtypes';
 import sortBy = require('lodash/sortBy');
@@ -34,7 +34,28 @@ const DEFAULT_RULE_SET: RuleSet = {
   }],
 };
 
+// Check if the given resource is the special "SchemaEdit" resource, which only exists as a
+// frontend representation.
+export function isSchemaEditResource(resource: {tableId: string, colIds: string}): boolean {
+  return resource.tableId === SPECIAL_RULES_TABLE_ID && resource.colIds === 'SchemaEdit';
+}
+
 const SPECIAL_RULE_SETS: Record<string, RuleSet> = {
+  SchemaEdit: {
+    tableId: SPECIAL_RULES_TABLE_ID,
+    colIds: ['SchemaEdit'],
+    body: [{
+      aclFormula: "user.Access in [EDITOR, OWNER]",
+      matchFunc:  (input) => ['editors', 'owners'].includes(String(input.user.Access)),
+      permissions: parsePermissions('+S'),
+      permissionsText: '+S',
+    }, {
+      aclFormula: "",
+      matchFunc: defaultMatchFunc,
+      permissions: parsePermissions('-S'),
+      permissionsText: '-S',
+    }],
+  },
   AccessRules: {
     tableId: SPECIAL_RULES_TABLE_ID,
     colIds: ['AccessRules'],
@@ -191,6 +212,21 @@ export class ACLRuleCollection {
         } else {
           specialRuleSets.set(specialType, {...ruleSet, body: [...ruleSet.body, ...specialDefault.body]});
         }
+      } else if (options.pullOutSchemaEdit && ruleSet.tableId === '*' && ruleSet.colIds === '*') {
+        // If pullOutSchemaEdit is requested, we move out rules with SchemaEdit permissions from
+        // the default resource into the ficticious "*SPECIAL:SchemaEdit" resource. This is used
+        // in the frontend only, to present those rules in a separate section.
+        const schemaParts = ruleSet.body.map(part => splitSchemaEditRulePart(part).schemaEdit).filter(isNonNullish);
+
+        if (schemaParts.length > 0) {
+          const specialType = 'SchemaEdit';
+          const specialDefault = specialRuleSets.get(specialType)!;
+          specialRuleSets.set(specialType, {
+            tableId: SPECIAL_RULES_TABLE_ID,
+            colIds: ['SchemaEdit'],
+            body: [...schemaParts, ...specialDefault.body]
+          });
+        }
       }
     }
 
@@ -203,9 +239,15 @@ export class ACLRuleCollection {
     for (const ruleSet of ruleSets) {
       if (ruleSet.tableId === '*') {
         if (ruleSet.colIds === '*') {
+          // If pullOutSchemaEdit is requested, skip the SchemaEdit rules for the default resource;
+          // those got pulled out earlier into the fictitious "*SPECIAL:SchemaEdit" resource.
+          const body = options.pullOutSchemaEdit ?
+            ruleSet.body.map(part => splitSchemaEditRulePart(part).nonSchemaEdit).filter(isNonNullish) :
+            ruleSet.body;
+
           defaultRuleSet = {
             ...ruleSet,
-            body: [...ruleSet.body, ...DEFAULT_RULE_SET.body],
+            body: [...body, ...DEFAULT_RULE_SET.body],
           };
         } else {
           // tableId of '*' cannot list particular columns.
@@ -341,6 +383,11 @@ export interface ReadAclOptions {
   // 1. They would show in the UI
   // 2. They would be saved back after editing, causing them to accumulate
   includeHelperCols?: boolean;
+
+  // If true, rules with 'schemaEdit' permission are moved out of the '*:*' resource into a
+  // fictitious '*SPECIAL:SchemaEdit' resource. This is used only on the client, to present
+  // schemaEdit as a separate checkbox. Such rules are saved back to the '*:*' resource.
+  pullOutSchemaEdit?: boolean;
 }
 
 export interface ReadAclResults {
@@ -474,3 +521,37 @@ function readAclRules(docData: DocData, {log, compile, includeHelperCols}: ReadA
   }
   return {ruleSets, userAttributes};
 }
+
+
+/**
+ * In the UI, we present SchemaEdit rules in a separate section, even though in reality they live
+ * as schemaEdit permission bits among the rules for the default resource. This function splits a
+ * RulePart into two: one containing the schemaEdit permission bit, and the other containing the
+ * other bits. If either part is empty, it will be returned as undefined, but if both are empty,
+ * nonSchemaEdit will be included as a rule with empty permission bits.
+ *
+ * It's possible for both parts to be non-empty (for rules created before the updated UI), in
+ * which case the schemaEdit one will have a fake origRecord, to cause it to be saved as a new
+ * record when saving.
+ */
+function splitSchemaEditRulePart(rulePart: RulePart): {schemaEdit?: RulePart, nonSchemaEdit?: RulePart} {
+  const p = splitSchemaEditPermissionSet(rulePart.permissions);
+  let schemaEdit: RulePart|undefined;
+  let nonSchemaEdit: RulePart|undefined;
+  if (p.schemaEdit) {
+    schemaEdit = {...rulePart,
+      permissions: p.schemaEdit,
+      permissionsText: permissionSetToText(p.schemaEdit),
+    };
+  }
+  if (p.nonSchemaEdit) {
+    nonSchemaEdit = {...rulePart,
+      permissions: p.nonSchemaEdit,
+      permissionsText: permissionSetToText(p.nonSchemaEdit),
+    };
+  }
+  if (schemaEdit && nonSchemaEdit) {
+    schemaEdit.origRecord = {id: -1} as MetaRowRecord<'_grist_ACLRules'>;
+  }
+  return {schemaEdit, nonSchemaEdit};
+}

test/nbrowser/gristUtils.ts

@@ -121,10 +121,12 @@ export function startsWith(value: string): RegExp {
 }
 
 /**
- * Helper to scroll an element into view.
+ * Helper to scroll an element into view. Returns the passed-in element.
  */
-export function scrollIntoView(elem: WebElement): Promise<void> {
-  return driver.executeScript((el: any) => el.scrollIntoView({behavior: 'auto'}), elem);
+export function scrollIntoView(elem: WebElement): WebElementPromise {
+  return new WebElementPromise(driver,
+    driver.executeScript((el: any) => el.scrollIntoView({behavior: 'auto'}), elem)
+    .then(() => elem));
 }
 
 /**