diff --git a/app/common/ActiveDocAPI.ts b/app/common/ActiveDocAPI.ts index 2fb484cb..f842c238 100644 --- a/app/common/ActiveDocAPI.ts +++ b/app/common/ActiveDocAPI.ts @@ -10,6 +10,7 @@ export interface ApplyUAOptions { desc?: string; // Overrides the description of the action. otherId?: number; // For undo/redo; the actionNum of the original action to which it applies. linkId?: number; // For bundled actions, actionNum of the previous action in the bundle. + bestEffort?: boolean; // If set, action may be applied in part if it cannot be applied completely. } export interface ApplyUAResult { diff --git a/app/common/gristTypes.ts b/app/common/gristTypes.ts index d93a47ad..851a3281 100644 --- a/app/common/gristTypes.ts +++ b/app/common/gristTypes.ts @@ -20,6 +20,7 @@ export const enum GristObjCode { DateTime = 'D', Date = 'd', Skip = 'S', + Censored = 'C', Reference = 'R', Exception = 'E', Pending = 'P', @@ -131,6 +132,10 @@ export function isSkip(value: CellValue): value is [GristObjCode.Skip] { return getObjCode(value) === GristObjCode.Skip; } +export function isCensored(value: CellValue): value is [GristObjCode.Censored] { + return getObjCode(value) === GristObjCode.Censored; +} + /** * Returns whether a value (as received in a DocAction) represents a list or is null, * which is a valid value for list types in grist. diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index 22145a86..35e394a3 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -783,7 +783,8 @@ export class ActiveDoc extends EventEmitter { // Granular access control implemented ultimately in _applyUserActions. // It could be that error cases and timing etc leak some info prior to this // point. - return this.applyUserActions(docSession, actions, options); + // Undos are best effort now by default. + return this.applyUserActions(docSession, actions, {bestEffort: undo, ...(options||{})}); } /** @@ -1170,6 +1171,9 @@ export class ActiveDoc extends EventEmitter { this.logDebug(docSession, "_applyUserActions(%s, %s)", client, shortDesc(actions)); this._inactivityTimer.ping(); // The doc is in active use; ping it to stay open longer. + if (options?.bestEffort) { + actions = await this._granularAccess.prefilterUserActions(docSession, actions); + } await this._granularAccess.assertCanMaybeApplyUserActions(docSession, actions); const user = docSession.mode === 'system' ? 'grist' : diff --git a/app/server/lib/GranularAccess.ts b/app/server/lib/GranularAccess.ts index 0b3a7135..83a12588 100644 --- a/app/server/lib/GranularAccess.ts +++ b/app/server/lib/GranularAccess.ts @@ -13,6 +13,7 @@ import { UserOverride } from 'app/common/DocListAPI'; import { ErrorWithCode } from 'app/common/ErrorWithCode'; import { AclMatchInput, InfoEditor, InfoView } from 'app/common/GranularAccessClause'; import { UserInfo } from 'app/common/GranularAccessClause'; +import { isCensored } from 'app/common/gristTypes'; import { getSetMapValue, isObject, pruneArray } from 'app/common/gutil'; import { canView, Role } from 'app/common/roles'; import { FullUser } from 'app/common/UserAPI'; @@ -44,14 +45,19 @@ function isDataAction(a: UserAction): a is DataAction { return ACTION_WITH_TABLE_ID.has(String(a[0])); } -function isAddRecordAction(a: DataAction): boolean { +function isAddRecordAction(a: DataAction): a is AddRecord | BulkAddRecord { return ['AddRecord', 'BulkAddRecord'].includes(a[0]); } -function isRemoveRecordAction(a: DataAction): boolean { +function isRemoveRecordAction(a: DataAction): a is RemoveRecord | BulkRemoveRecord { return ['RemoveRecord', 'BulkRemoveRecord'].includes(a[0]); } +function isBulkAction(a: DataAction): a is BulkAddRecord | BulkUpdateRecord | + BulkRemoveRecord | ReplaceTableData | TableDataAction { + return Array.isArray(a[2]); +} + // Check if a tableId is that of an ACL table. Currently just _grist_ACLRules and // _grist_ACLResources are accepted. function isAclTable(tableId: string): boolean { @@ -381,6 +387,89 @@ export class GranularAccess implements GranularAccessForBundle { return canCertainlyApply; } + /** + * Called when it is permissible to partially fulfill the requested actions. + * Will remove forbidden actions in a very limited set of recognized circumstances. + * In fact, currently in only one circumstance: + * + * - If there is a single requested action, and it is an ApplyUndoActions. + * The goal being to let a user undo their action to the extent that it + * is possible to do so. + * + * In this case, the list of actions nested in ApplyUndoActions will be extracted, + * treated as DocActions, and filtered to remove any component parts (at action, + * column, row, or individual cell level) that would be forbidden. + * + * Beyond pure data changes, there are no heroics - any schema change will + * result in prefiltering being skipped. + * + * Any filtering done here is NOT a security measure, and the output should + * not be granted any level of automatic trust. + */ + public async prefilterUserActions(docSession: OptDocSession, actions: UserAction[]): Promise { + // Currently we only attempt prefiltering for an ApplyUndoActions. + if (actions.length !== 1) { return actions; } + const userAction = actions[0]; + if (userAction[0] !== 'ApplyUndoActions') { return actions; } + + // Ok, this is an undo. Unpack the requested undo actions. For a bona + // fide ApplyUndoActions, these would be doc actions generated by the + // data engine and stored in action history. But there is no actual + // restriction in how ApplyUndoActions could be generated. Security + // is enforced separately, so we don't need to be paranoid here. + const docActions = userAction[1] as DocAction[]; + + // Bail out if there is any hint of a schema change. + // TODO: may want to also bail if an action we'd need to filter would + // affect a row id used later in the bundle. Perhaps prefiltering + // should be restricted to bundles of updates only for that reason. + for (const action of docActions) { + if (!isDataAction(action) || getTableId(action).startsWith('_grist')) { + return actions; + } + } + + // Run through a simulation of access control on these actions, + // retaining only permitted material. + const proposedActions: UserAction[] = []; + try { + // Establish our doc actions as the current context for access control. + // We don't have undo information for them, but don't need to because + // they have not been applied to the db. Treat all actions as "direct" + // since we could not trust claims of indirectness currently in + // any case (though we could rearrange to limit how undo actions are + // requested). + this.getGranularAccessForBundle(docSession, docActions, [], docActions, + docActions.map(() => true)); + for (const [actionIdx, action] of docActions.entries()) { + // A single action might contain forbidden material at cell, row, column, + // or table level. Retaining permitted material may require refactoring the + // single action into a series of actions. + try { + await this._checkIncomingDocAction({docSession, action, actionIdx}); + // Nothing forbidden! Keep this action unchanged. + proposedActions.push(action); + } catch (e) { + if (String(e.code) !== 'ACL_DENY') { throw e; } + const acts = await this._prefilterDocAction({docSession, action, actionIdx}); + proposedActions.push(...acts); + // Presumably we've changed the action. Zap our cache of intermediate + // states, since it is stale now. TODO: reorganize cache to so can avoid wasting + // time repeating work unnecessarily. The cache was designed with all-or-nothing + // operations in mind, and is poorly suited to prefiltering. + // Note: the meaning of newRec is slippery in prefiltering, since it depends on + // state at the end of the bundle, but that state is unstable now. + // TODO look into prefiltering in cases using newRec in a many-action bundle. + this._steps = null; + this._metaSteps = null; + } + } + } finally { + await this.finishedBundle(); + } + return [['ApplyUndoActions', proposedActions]]; + } + /** * Check if user may be able to apply a given action. Throws if * user cannot apply the action. Returns true if a user can apply an @@ -566,7 +655,7 @@ export class GranularAccess implements GranularAccessForBundle { const cursor: ActionCursor = {docSession, action: data, actionIdx: null}; const tableId = getTableId(data); if (this.getReadPermission(permInfo.getTableAccess(tableId)) === 'mixed') { - await this._filterRowsAndCells(cursor, data, data, readAccessCheck); + await this._filterRowsAndCells(cursor, data, data, readAccessCheck, true); } // Filter columns, omitting any to which the user has no access, regardless of rows. @@ -800,7 +889,7 @@ export class GranularAccess implements GranularAccessForBundle { // Return the results, also applying any cell-level access control. for (const a of revisedDocActions) { - await this._filterRowsAndCells({...cursor, action: a}, rowsAfter, rowsAfter, readAccessCheck); + await this._filterRowsAndCells({...cursor, action: a}, rowsAfter, rowsAfter, readAccessCheck, false); } return revisedDocActions; } @@ -814,7 +903,7 @@ export class GranularAccess implements GranularAccessForBundle { // This check applies to data changes only. if (!isDataAction(action)) { return; } const {rowsBefore, rowsAfter} = await this._getRowsForRecAndNewRec(cursor); - await this._filterRowsAndCells(cursor, rowsBefore, rowsAfter, accessCheck); + await this._filterRowsAndCells(cursor, rowsBefore, rowsAfter, accessCheck, false); } private async _getRowsBeforeAndAfter(cursor: ActionCursor) { @@ -856,13 +945,28 @@ export class GranularAccess implements GranularAccessForBundle { /** * Modify action in place, scrubbing any rows and cells to which access is not granted. + * Returns filteredAction, which is the provided action or null - it is null if the + * action was entirely eliminated (and was not a bulk action). Also returns + * censoredRows, a set of indexes of rows that have a censored value in them. + * + * If allowRowRemoval is false, then rows will not be removed, and if the user + * does not have access to a row and the action itself is not a remove action, then + * an error will be thrown. This flag setting is used when filtering outgoing + * actions, where actions need rewriting elsewhere to reflect access changes to + * rows for each individual client. */ private async _filterRowsAndCells(cursor: ActionCursor, rowsBefore: TableDataAction, rowsAfter: TableDataAction, - accessCheck: IAccessCheck) { + accessCheck: IAccessCheck, + allowRowRemoval: boolean): Promise<{ + filteredAction: DocAction | null, + censoredRows: Set + }> { + const censoredRows = new Set(); const ruler = await this._getRuler(cursor); const {docSession, action} = cursor; + let filteredAction: DocAction | null = action; if (action && isSchemaAction(action)) { - return []; + return {filteredAction, censoredRows}; } // For user convenience, for creations and deletions we equate rec and newRec. @@ -921,14 +1025,19 @@ export class GranularAccess implements GranularAccessForBundle { const colAccess = rowPermInfo.getColumnAccess(tableId, colId); if (accessCheck.get(colAccess) === 'deny') { censorAt(colId, idx); + censoredRows.add(idx); } } } } if (toRemove.length > 0) { - if (rowsBefore === action) { - this._removeRowsAt(toRemove, rowsBefore[2], rowsBefore[3]); + if (allowRowRemoval) { + if (Array.isArray(action[2])) { + this._removeRowsAt(toRemove, action[2], action[3]); + } else { + filteredAction = null; + } } else { // Artificially introduced removals are ok, otherwise this is suspect. if (action[0] !== 'RemoveRecord' && action[0] !== 'BulkRemoveRecord') { @@ -936,6 +1045,7 @@ export class GranularAccess implements GranularAccessForBundle { } } } + return {filteredAction, censoredRows}; } // Compute which of the row ids supplied are for rows forbidden for this session. @@ -975,7 +1085,7 @@ export class GranularAccess implements GranularAccessForBundle { * * toRemove must be sorted, lowest to highest. */ - private _removeRowsAt(toRemove: number[], rowIds: number[], colValues: BulkColValues|undefined) { + private _removeRowsAt(toRemove: number[], rowIds: number[], colValues: BulkColValues|ColValues|undefined) { if (toRemove.length > 0) { pruneArray(rowIds, toRemove); if (colValues) { @@ -1304,6 +1414,63 @@ export class GranularAccess implements GranularAccessForBundle { return steps; } + /** + * Return any permitted parts of an action. A completely forbidden + * action results in an empty list. Forbidden columns and rows will + * be stripped from a returned action. Rows with forbidden cells are + * extracted and returned in distinct actions (since they will have + * a distinct set of columns). + * + * This method should only be called with data actions, and will throw + * for anything else. + */ + private async _prefilterDocAction(cursor: ActionCursor): Promise { + const {action, docSession} = cursor; + const tableId = getTableId(action); + const permInfo = await this._getStepAccess(cursor); + const tableAccess = permInfo.getTableAccess(tableId); + const accessCheck = await this._getAccessForActionType(docSession, action, 'check'); + const access = accessCheck.get(tableAccess); + if (access === 'deny') { + // Filter out this action entirely. + return []; + } else if (access === 'allow') { + // Retain this action entirely. + return [action]; + } else if (access === 'mixedColumns') { + // Retain some or all columns entirely. + const act = this._pruneColumns(action, permInfo, tableId, accessCheck); + return act ? [act] : []; + } + // The remainder is the mixed condition. + + const {rowsBefore, rowsAfter} = await this._getRowsForRecAndNewRec(cursor); + const {censoredRows, filteredAction} = await this._filterRowsAndCells({...cursor, action: cloneDeep(action)}, + rowsBefore, rowsAfter, accessCheck, + true); + if (filteredAction === null) { + return []; + } + if (!isDataAction(filteredAction)) { + throw new Error('_prefilterDocAction called with unexpected action'); + } + if (isRemoveRecordAction(filteredAction)) { + // removals do not mention columns or cells, so no further complications. + return [filteredAction]; + } + + // Strip any forbidden columns. + this._filterColumns( + filteredAction[3], + (colId) => accessCheck.get(permInfo.getColumnAccess(tableId, colId)) !== 'deny'); + if (censoredRows.size === 0) { + // no cell censorship, so no further complications. + return [filteredAction]; + } + + return filterColValues(filteredAction, (idx) => censoredRows.has(idx), isCensored); + } + /** * Tailor the information about a change reported to a given client. The action passed in * is never modified. The actions output may differ in the following ways: @@ -1805,3 +1972,92 @@ function scanActionsRecursively(actions: (DocAction|UserAction)[], } return false; } + +/** + * Takes an action, and removes certain cells from it. The action + * passed in is modified in place, and also returned as part of a list + * of derived actions. + * + * For a non-bulk action, any cell values that return true for + * shouldFilterCell are removed. For a bulk action, there's no way to + * express that in general in a single action. For a bulk action, for + * any row (identified by row index, not rowId) that returns true for + * shouldFilterRow, we remove cell values based on shouldFilterCell + * and add the row to an action with just the remaining cell values. + * + * This is by no means a general-purpose function. It is used only in + * the implementation of partial undos. If is factored out for + * testing purposes. + * + * This method could be made unnecessary if a way were created to have + * unambiguous "holes" in column value arrays, where values for some + * rows are omitted. + */ +export function filterColValues(action: DataAction, + shouldFilterRow: (idx: number) => boolean, + shouldFilterCell: (value: CellValue) => boolean): DataAction[] { + if (isRemoveRecordAction(action)) { + // removals do not have cells, so nothing to do. + return [action]; + } + + const colIds = Object.keys(action[3]).sort(); + const colValues = action[3]; + + if (!isBulkAction(action)) { + for (const colId of colIds) { + if (shouldFilterCell((colValues as ColValues)[colId])) { + delete colValues[colId]; + } + } + return [action]; + } + + const rowIds = action[2]; + + // For bulk operations, censored cells require us to reorganize into a set of actions + // with different columns. + const parts: Map = new Map(); + let at = 0; + for (let idx = 0; idx < rowIds.length; idx++) { + if (!shouldFilterRow(idx)) { + if (idx !== at) { + // Shuffle columnar data up as we remove rows. + rowIds[at] = rowIds[idx]; + for (const colId of colIds) { + (colValues as BulkColValues)[colId][at] = (colValues as BulkColValues)[colId][idx]; + } + } + at++; + continue; + } + // Some censored data in this row, so move the row to an action specialized + // for the set of columns this row has. + const keys: string[] = []; + const values: BulkColValues = {}; + for (const colId of colIds) { + const value = (colValues as BulkColValues)[colId][idx]; + if (!shouldFilterCell(value)) { + values[colId] = [value]; + keys.push(colId); + } + } + const mergedKey = keys.join(' '); + const peers = parts.get(mergedKey); + if (!peers) { + parts.set(mergedKey, [action[0], action[1], [rowIds[idx]], values]); + } else { + peers[2].push(rowIds[idx]); + for (const key of keys) { + peers[3][key].push(values[key][0]); + } + } + } + // Truncate columnar data. + rowIds.length = at; + for (const colId of colIds) { + (colValues as BulkColValues)[colId].length = at; + } + // Return all actions, in a consistent order for test purposes. + return [action, ...[...parts.keys()].sort().map(key => parts.get(key)!)]; +}