(core) discount indirect changes for access control purposes

Summary:
This diff discounts indirect changes for access control purposes.  A UserAction that updates a cell A, which in turn causes changes in other dependent cells, will be considered a change to cell A for access control purposes.

The `engine.apply_user_actions` method now returns a `direct` array, with a boolean for each `stored` action, set to `true` if the action is attributed to the user or `false` if it is attributed to the engine.  `GranularAccess` ignores actions attributed to the engine when checking for edit rights.

Subtleties:
 * Removal of references to a removed row are considered direct changes.
 * Doesn't play well with undos as yet.  An action that indirectly modifies a cell the user doesn't have rights to may succeed, but it will not be reversible.

Test Plan: added tests, updated tests

Reviewers: dsagal

Reviewed By: dsagal

Differential Revision: https://phab.getgrist.com/D2806
This commit is contained in:
Paul Fitzpatrick
2021-05-12 11:04:37 -04:00
parent 8d62a857e1
commit d0d3d3d0c9
14 changed files with 175 additions and 48 deletions

View File

@@ -1015,6 +1015,7 @@ export class ActiveDoc extends EventEmitter {
// Note: onDemand stored/undo actions are arbitrarily processed/added after normal actions
// and do not support access control.
sandboxActionBundle.stored.push(...stored.map(a => [allIndex, a] as [number, DocAction]));
sandboxActionBundle.direct.push(...stored.map(a => [allIndex, true] as [number, boolean]));
sandboxActionBundle.undo.push(...undo.map(a => [allIndex, a] as [number, DocAction]));
sandboxActionBundle.retValues.push(retValues);
}
@@ -1078,8 +1079,8 @@ export class ActiveDoc extends EventEmitter {
* granular access rules.
*/
public getGranularAccessForBundle(docSession: OptDocSession, docActions: DocAction[], undo: DocAction[],
userActions: UserAction[]): GranularAccessForBundle {
this._granularAccess.getGranularAccessForBundle(docSession, docActions, undo, userActions);
userActions: UserAction[], isDirect: boolean[]): GranularAccessForBundle {
this._granularAccess.getGranularAccessForBundle(docSession, docActions, undo, userActions, isDirect);
return this._granularAccess;
}
@@ -1411,6 +1412,7 @@ function createEmptySandboxActionBundle(): SandboxActionBundle {
return {
envelopes: [],
stored: [],
direct: [],
calc: [],
undo: [],
retValues: []

View File

@@ -531,6 +531,7 @@ export class FlexServer implements GristServer {
}
public async close() {
if (this.usage) { await this.usage.close(); }
if (this._hosts) { this._hosts.close(); }
if (this.dbManager) {
this.dbManager.removeAllListeners();
@@ -538,7 +539,6 @@ export class FlexServer implements GristServer {
}
if (this.server) { this.server.close(); }
if (this.httpsServer) { this.httpsServer.close(); }
if (this.usage) { this.usage.close(); }
if (this.housekeeper) { await this.housekeeper.stop(); }
await this._shutdown();
// Do this after _shutdown, since DocWorkerMap is used during shutdown.

View File

@@ -142,6 +142,7 @@ export class GranularAccess implements GranularAccessForBundle {
docSession: OptDocSession,
userActions: UserAction[],
docActions: DocAction[],
isDirect: boolean[],
undo: DocAction[],
// Flag tracking whether a set of actions have been applied to the database or not.
applied: boolean,
@@ -163,10 +164,10 @@ export class GranularAccess implements GranularAccessForBundle {
}
public getGranularAccessForBundle(docSession: OptDocSession, docActions: DocAction[], undo: DocAction[],
userActions: UserAction[]): void {
userActions: UserAction[], isDirect: boolean[]): void {
if (this._activeBundle) { throw new Error('Cannot start a bundle while one is already in progress'); }
this._activeBundle = {
docSession, docActions, undo, userActions,
docSession, docActions, undo, userActions, isDirect,
applied: false, hasDeliberateRuleChange: false, hasAnyRuleChange: false
};
this._activeBundle.hasDeliberateRuleChange =
@@ -211,13 +212,17 @@ export class GranularAccess implements GranularAccessForBundle {
*/
public async canApplyBundle() {
if (!this._activeBundle) { throw new Error('no active bundle'); }
const {docActions, docSession} = this._activeBundle;
const {docActions, docSession, isDirect} = this._activeBundle;
if (this._activeBundle.hasDeliberateRuleChange && !await this.isOwner(docSession)) {
throw new ErrorWithCode('ACL_DENY', 'Only owners can modify access rules');
}
if (this._ruler.haveRules()) {
await Promise.all(
docActions.map((action, actionIdx) => this._checkIncomingDocAction({docSession, action, actionIdx})));
docActions.map((action, actionIdx) => {
if (isDirect[actionIdx]) {
return this._checkIncomingDocAction({docSession, action, actionIdx});
}
}));
}
if (this._recoveryMode) {

View File

@@ -373,9 +373,10 @@ export class Sharing {
const undo = getEnvContent(sandboxActionBundle.undo);
const docActions = getEnvContent(sandboxActionBundle.stored).concat(
getEnvContent(sandboxActionBundle.calc));
const isDirect = getEnvContent(sandboxActionBundle.direct);
const accessControl = this._activeDoc.getGranularAccessForBundle(
docSession || makeExceptionalDocSession('share'), docActions, undo, userActions
docSession || makeExceptionalDocSession('share'), docActions, undo, userActions, isDirect
);
try {
// TODO: see if any of the code paths that have no docSession are relevant outside