diff --git a/app/client/aclui/AccessRules.ts b/app/client/aclui/AccessRules.ts index 6d9c6d96..d9c113f7 100644 --- a/app/client/aclui/AccessRules.ts +++ b/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() && - (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); - } + // Nothing to do! We now support all expressible rule permutations. } public buildRulePartDom(wide: boolean = false) { diff --git a/app/server/lib/GranularAccess.ts b/app/server/lib/GranularAccess.ts index 41d68ea8..d2462d24 100644 --- a/app/server/lib/GranularAccess.ts +++ b/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): Promise { + // If colId is supplied, check instead whether that specific column is forbidden. + private async _getForbiddenRows(cursor: ActionCursor, data: TableDataAction, ids: Set, + colId?: string): Promise { 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') { - toRemove.push(rowIds[idx]); + if (!colId) { + 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): 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> { if (!this._steps) { this._steps = this._getUncachedSteps().catch(e => {