(core) check for +S bit early for changes that include formulas

Summary:
Currently, to compute intermediate steps in a bundle, the bundle
is sent to the data engine to process.  Then, if the intermediate
steps break a rule, it is reverted.  One problem introduced by
checking permissions this late is that the data engine can be
exposed for formulas with python code by users who don't have the
right to change formulas.  This diff pre-checks cases that change
formulas.

Test Plan: added a test

Reviewers: dsagal

Reviewed By: dsagal

Differential Revision: https://phab.getgrist.com/D2816
This commit is contained in:
Paul Fitzpatrick 2021-05-14 08:44:11 -04:00
parent dee487684e
commit 15723d1300

View File

@ -69,10 +69,9 @@ const STRUCTURAL_TABLES = new Set(['_grist_Tables', '_grist_Tables_column', '_gr
// Actions that won't be allowed (yet) for a user with nuanced access to a document.
// A few may be innocuous, but generally I've put them in this list if there are problems
// tracking down what table the refer to, or they could allow creation/modification of a
// formula.
// formula, and are not handled elsewhere.
const SPECIAL_ACTIONS = new Set(['InitNewDoc',
'EvalCode',
'SetDisplayFormula',
'UpdateSummaryViewSection',
'DetachSummaryViewSection',
'GenImporterView',
@ -364,10 +363,22 @@ export class GranularAccess implements GranularAccessForBundle {
* until the data engine translates the user actions to doc actions.
*/
public async assertCanMaybeApplyUserActions(docSession: OptDocSession, actions: UserAction[]): Promise<boolean> {
let canCertainlyApply = true;
for (const action of actions) {
if (!await this.assertCanMaybeApplyUserAction(docSession, action)) { return false; }
if (!await this.assertCanMaybeApplyUserAction(docSession, action)) {
canCertainlyApply = false;
break;
}
}
return true;
// If changes could include Python formulas, then user must have
// +S before we even consider passing these to the data engine.
// Since we don't track rule or schema changes at this stage, we
// approximate with the user's access rights at beginning of
// bundle.
if (!canCertainlyApply && scanActionsRecursively(actions, (a) => this.needEarlySchemaPermission(a))) {
await this._assertSchemaAccess(docSession);
}
return canCertainlyApply;
}
/**
@ -391,9 +402,9 @@ export class GranularAccess implements GranularAccessForBundle {
}
return true;
}
if (a[0] === 'ApplyUndoActions') {
if (name === 'ApplyUndoActions') {
return this.assertCanMaybeApplyUserActions(docSession, a[1] as UserAction[]);
} else if (a[0] === 'ApplyDocActions') {
} else if (name === 'ApplyDocActions') {
return this.assertCanMaybeApplyUserActions(docSession, a[1] as UserAction[]);
} else if (isDataAction(a)) {
const tableId = getTableId(a);
@ -409,6 +420,22 @@ export class GranularAccess implements GranularAccessForBundle {
}
}
/**
* For changes that could include Python formulas, check for schema access early.
*/
public needEarlySchemaPermission(a: UserAction|DocAction): boolean {
const name = a[0] as string;
if (name === 'ModifyColumn' || name === 'SetDisplayFormula') {
return true;
} else if (isDataAction(a)) {
const tableId = getTableId(a);
if (tableId === '_grist_Tables_column' || tableId === '_grist_Validations') {
return true;
}
}
return false;
}
/**
* Check whether access is simple, or there are granular nuances that need to be
* worked through. Currently if there are no owner-only tables, then everyone's
@ -587,6 +614,16 @@ export class GranularAccess implements GranularAccessForBundle {
return getDocSessionAccess(docSession);
}
/**
* Asserts that user has schema access.
*/
private async _assertSchemaAccess(docSession: OptDocSession) {
const permInfo = await this._getAccess(docSession);
if (permInfo.getFullAccess().perms.schemaEdit !== 'allow') {
throw new ErrorWithCode('ACL_DENY', `Schema access required`);
}
}
/**
* This filters a message being broadcast to all clients to be appropriate for one
* particular client, if that client may need some material filtered out.