(core) support subscriptions to a doc with row-dependent column read permissions

Summary: This addresses a weakness in the following case: rules controlling view access for a column, with a dependency on the values of other columns. We had disabled support for such rules, since the existing implementation worked only on table loads and not on broadcast changes. This diff adds in logic to enrich broadcasts as needed, and allows such rules.

Test Plan: added test

Reviewers: dsagal

Reviewed By: dsagal

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

app/client/aclui/AccessRules.ts

@@ -20,7 +20,6 @@ import {emptyPermissionSet, MixedPermissionValue, PartialPermissionSet} from 'ap
 import {parsePermissions, permissionSetToText} from 'app/common/ACLPermissions';
 import {summarizePermissions, summarizePermissionSet} from 'app/common/ACLPermissions';
 import {ACLRuleCollection, SPECIAL_RULES_TABLE_ID} from 'app/common/ACLRuleCollection';
-import {ApiError} from 'app/common/ApiError';
 import {BulkColValues, RowRecord, UserAction} from 'app/common/DocActions';
 import {FormulaProperties, RulePart, RuleSet, UserAttributeRule} from 'app/common/GranularAccessClause';
 import {getFormulaProperties} from 'app/common/GranularAccessClause';
@@ -1146,19 +1145,7 @@ class ObsRulePart extends Disposable {
    * Verify that the rule is in a good state, optionally given a proposed permission change.
    */
   public sanityCheck(pset?: PartialPermissionSet) {
-    if (this._ruleSet.hasColumns() &&
+    // Nothing to do!  We now support all expressible rule permutations.
-        (pset || this._permissions.get()).read &&
-        this._formulaProperties.get().hasRecOrNewRec) {
-      if (pset && pset.read && this._permissions.get().read) {
-        // We don't want users to set either allow or deny read permissions in
-        // row-dependent rules, but if such a permission is set, allow it to be
-        // toggled to enable the deny->allow->unset cycling in the UI.
-        return;
-      }
-      throw new ApiError('Control of the read permission for column rules is unavailable ' +
-                         'when the formula uses the rec variable. ' +
-                         'Sorry! We will get to it, promise.', 400);
-    }
   }
 
   public buildRulePartDom(wide: boolean = false) {

app/server/lib/GranularAccess.ts

@@ -651,7 +651,8 @@ export class GranularAccess implements GranularAccessForBundle {
     // Figure out which rows were forbidden to this session before this action vs
     // after this action.  We need to know both so that we can infer the state of the
     // client and send the correct change.
-    const ids = new Set(getRowIdsFromDocAction(action));
+    const orderedIds = getRowIdsFromDocAction(action);
+    const ids = new Set(orderedIds);
     const forbiddenBefores = new Set(await this._getForbiddenRows(cursor, rowsBefore, ids));
     const forbiddenAfters = new Set(await this._getForbiddenRows(cursor, rowsAfter, ids));
 
@@ -714,6 +715,37 @@ export class GranularAccess implements GranularAccessForBundle {
       this._makeRemovals(rowsAfter, forceRemoves),
     ].filter(isObject);
 
+    // Check whether there are column rules for this table, and if so whether they are row
+    // dependent.  If so, we may need to update visibility of cells not mentioned in the
+    // original DocAction.
+    // No censorship is done here, all we do at this point is pull in any extra cells that need
+    // to be updated for the current client.  Censorship for these cells, and any cells already
+    // present in the DocAction, is done by _filterRowsAndCells.
+    const ruler = await this._getRuler(cursor);
+    const tableId = getTableId(action);
+    const ruleSets = ruler.ruleCollection.getAllColumnRuleSets(tableId);
+    const colIds = new Set(([] as string[]).concat(...ruleSets.map(ruleSet => ruleSet.colIds === '*' ? [] : ruleSet.colIds)));
+    const access = await ruler.getAccess(cursor.docSession);
+    // Check columns in a consistent order, for determinism (easier testing).
+    // TODO: could pool some work between columns by doing them together rather than one by one.
+    for (const colId of [...colIds].sort()) {
+      // If the column is already in the DocAction, we can skip checking if we need to add it.
+      if (!action[3] || (colId in action[3])) { continue; }
+      // If the column is not row dependent, we have nothing to do.
+      if (access.getColumnAccess(tableId, colId).perms.read !== 'mixed') { continue; }
+      // Check column accessibility before and after.
+      const forbiddenBefores = new Set(await this._getForbiddenRows(cursor, rowsBefore, ids, colId));
+      const forbiddenAfters = new Set(await this._getForbiddenRows(cursor, rowsAfter, ids, colId));
+      // For any column that is in a visible row and for which accessibility has changed,
+      // pull it into the doc actions.  We don't censor cells yet, that happens later
+      // (if that's what needs doing).
+      const changedIds = orderedIds.filter(id => !forceRemoves.has(id) && !removals.has(id) &&
+                                        (forbiddenBefores.has(id) !== forbiddenAfters.has(id)));
+      if (changedIds.length > 0) {
+        revisedDocActions.push(this._makeColumnUpdate(rowsAfter, colId, new Set(changedIds)));
+      }
+    }
+
     // Return the results, also applying any cell-level access control.
     for (const action of revisedDocActions) {
       await this._filterRowsAndCells({...cursor, action}, rowsAfter, rowsAfter, readAccessCheck);
@@ -842,7 +874,9 @@ export class GranularAccess implements GranularAccessForBundle {
   }
 
   // Compute which of the row ids supplied are for rows forbidden for this session.
-  private async _getForbiddenRows(cursor: ActionCursor, data: TableDataAction, ids: Set<number>): Promise<number[]> {
+  // If colId is supplied, check instead whether that specific column is forbidden.
+  private async _getForbiddenRows(cursor: ActionCursor, data: TableDataAction, ids: Set<number>,
+                                  colId?: string): Promise<number[]> {
     const ruler = await this._getRuler(cursor);
     const rec = new RecordView(data, undefined);
     const input: AclMatchInput = {user: await this._getUser(cursor.docSession), rec};
@@ -856,8 +890,15 @@ export class GranularAccess implements GranularAccessForBundle {
       const rowPermInfo = new PermissionInfo(ruler.ruleCollection, input);
       // getTableAccess() evaluates all column rules for THIS record. So it's really rowAccess.
       const rowAccess = rowPermInfo.getTableAccess(tableId);
-      if (this.getReadPermission(rowAccess) === 'deny') {
+      if (!colId) {
-        toRemove.push(rowIds[idx]);
+        if (this.getReadPermission(rowAccess) === 'deny') {
+          toRemove.push(rowIds[idx]);
+        }
+      } else {
+        const colAccess = rowPermInfo.getColumnAccess(tableId, colId);
+        if (this.getReadPermission(colAccess) === 'deny') {
+          toRemove.push(rowIds[idx]);
+        }
       }
     }
     return toRemove;
@@ -1051,6 +1092,16 @@ export class GranularAccess implements GranularAccessForBundle {
     return ['BulkRemoveRecord', getTableId(data), [...rowIds]];
   }
 
+  /**
+   * Make a BulkUpdateRecord for a particular column across a set of rows.
+   */
+  private _makeColumnUpdate(data: TableDataAction, colId: string, rowIds: Set<number>): BulkUpdateRecord {
+    const dataRowIds = data[2];
+    const selectedRowIds = dataRowIds.filter(r => rowIds.has(r));
+    const colData = data[3][colId].filter((value, idx) => rowIds.has(dataRowIds[idx]));
+    return ['BulkUpdateRecord', getTableId(data), selectedRowIds, {[colId]: colData}];
+  }
+
   private async _getSteps(): Promise<Array<ActionStep>> {
     if (!this._steps) {
       this._steps = this._getUncachedSteps().catch(e => {