(core) give more detailed reasons for access denied when memos are present

Summary:
With this change, if a comment is added to an ACL formula, then that comment will be offered to the user if access is denied and that rule could potentially have granted access.

The code is factored so that when access is permitted, or when partially visible tables are being filtered, there is little overhead. Comments are gathered only when an explicit denial of access.

Test Plan: added tests, updated tests

Reviewers: dsagal

Reviewed By: dsagal

Differential Revision: https://phab.getgrist.com/D2730
pull/3/head
Paul Fitzpatrick 3 years ago
parent 422560504e
commit 6af811f7ab

@ -156,6 +156,9 @@ export interface CommResponseError {
error: string;
errorCode: string;
shouldFork?: boolean; // if set, the server suggests forking the document.
details?: any; // if set, error has extra details available. TODO - the treatment of
// details could do with some harmonisation between rest API and ws API,
// and between front-end and back-end types.
}
function isCommResponseError(msg: CommResponse | CommResponseError): msg is CommResponseError {
@ -460,6 +463,9 @@ export class Comm extends dispose.Disposable implements GristServerAPI, DocListA
code = ` [${message.errorCode}]`;
err.code = message.errorCode;
}
if (message.details) {
err.details = message.details;
}
err.shouldFork = message.shouldFork;
console.log(`Comm response #${reqId} ${r.methodName} ERROR:${code} ${message.error}`
+ (message.shouldFork ? ` (should fork)` : ''));

@ -7,6 +7,7 @@ import {timeFormat} from 'app/common/timeFormat';
import {bundleChanges, Disposable, Holder, IDisposable, IDisposableOwner } from 'grainjs';
import {Computed, dom, DomElementArg, MutableObsArray, obsArray, Observable} from 'grainjs';
import clamp = require('lodash/clamp');
import defaults = require('lodash/defaults');
// When rendering app errors, we'll only show the last few.
const maxAppErrors = 5;
@ -45,6 +46,8 @@ export interface INotifyOptions {
expireSec?: number;
badgeCounter?: boolean;
memos?: string[]; // A list of relevant notes.
// cssToastAction class from NotifyUI will be applied automatically to action elements.
actions?: NotifyAction[];
@ -87,12 +90,13 @@ export class Notification extends Expirable implements INotification {
expireSec: 0,
canUserClose: false,
actions: [],
memos: [],
key: null,
};
constructor(_opts: INotifyOptions) {
super();
Object.assign(this.options, _opts);
this.options = defaults({}, _opts, this.options)
if (this.options.expireSec > 0) {
const expireTimer = setTimeout(() => this.expire(), 1000 * this.options.expireSec);

@ -84,7 +84,7 @@ export function reportError(err: Error|string): void {
} else if (err.name === 'NeedUpgradeError') {
_notifier.createUserError(err.message, {actions: ['upgrade'], key: 'NEED_UPGRADE'});
} else if (code === 'AUTH_NO_EDIT' || code === 'ACL_DENY') {
_notifier.createUserError(message, {key: code});
_notifier.createUserError(err.message, {key: code, memos: details?.memos});
} else {
// If we don't recognize it, consider it an application error (bug) that the user should be
// able to report.

@ -57,6 +57,9 @@ function buildNotificationDom(item: Notification, options: IBeaconOpenOptions) {
item.options.actions.length ? cssToastActions(
item.options.actions.map((action) => buildAction(action, item, options))
) : null,
item.options.memos.length ? cssToastMemos(
item.options.memos.map(memo => cssToastMemo(memo))
) : null,
),
dom.maybe(item.options.canUserClose, () =>
cssToastClose(testId('toast-close'),
@ -300,6 +303,19 @@ const cssToastAction = styled('div', `
}
`);
const cssToastMemos = styled('div', `
margin-top: 16px;
display: flex;
flex-direction: column;
`);
const cssToastMemo = styled('div', `
margin: 3px;
color: ${colors.dark};
background: ${colors.light};
padding: 3px;
`);
const cssProgressBarWrapper = styled('div', `
margin-top: 18px;
margin-bottom: 11px;

@ -272,7 +272,6 @@ function readAclRules(docData: DocData, {log, compile}: ReadAclOptions): ReadAcl
if (rule.userAttributes) {
if (tableId !== '*' || colIds !== '*') {
throw new Error(`ACLRule ${rule.id} invalid; user attributes must be on the default resource`);
continue;
}
const parsed = JSON.parse(String(rule.userAttributes));
// TODO: could perhaps use ts-interface-checker here.
@ -280,7 +279,6 @@ function readAclRules(docData: DocData, {log, compile}: ReadAclOptions): ReadAcl
[parsed.name, parsed.tableId, parsed.lookupColId, parsed.charId]
.every(p => p && typeof p === 'string'))) {
throw new Error(`User attribute rule ${rule.id} is invalid`);
continue;
}
parsed.origRecord = rule;
userAttributes.push(parsed as UserAttributeRule);
@ -289,10 +287,12 @@ function readAclRules(docData: DocData, {log, compile}: ReadAclOptions): ReadAcl
} else if (rule.aclFormula && !rule.aclFormulaParsed) {
throw new Error(`ACLRule ${rule.id} invalid because missing its parsed formula`);
} else {
const aclFormulaParsed = rule.aclFormula && JSON.parse(String(rule.aclFormulaParsed));
body.push({
origRecord: rule,
aclFormula: String(rule.aclFormula),
matchFunc: rule.aclFormula ? compile?.(JSON.parse(String(rule.aclFormulaParsed))) : defaultMatchFunc,
matchFunc: rule.aclFormula ? compile?.(aclFormulaParsed) : defaultMatchFunc,
memo: aclFormulaParsed && aclFormulaParsed[0] === 'Comment' && aclFormulaParsed[2],
permissions: parsePermissions(String(rule.permissionsText)),
permissionsText: String(rule.permissionsText),
});

@ -28,6 +28,8 @@ export interface ApiErrorDetails {
// If set, contains suggestions for fixing a problem.
tips?: ApiTip[];
memos?: string[];
}
/**

@ -136,6 +136,9 @@ function throwApiError(url: string, resp: Response | AxiosResponse, body: any) {
if (body.error) {
details.userError = body.error;
}
if (body.memos) {
details.memos = body.memos;
}
throw new ApiError(`Request to ${url} failed with status ${resp.status}: ` +
`${resp.statusText} (${body.error || 'unknown cause'})`, resp.status, details);
}

@ -3,6 +3,7 @@ import {OpenDocMode} from 'app/common/DocListAPI';
interface ErrorDetails {
status?: number;
accessMode?: OpenDocMode;
memos?: string[];
}
/**
@ -13,11 +14,9 @@ interface ErrorDetails {
*
*/
export class ErrorWithCode extends Error {
public accessMode?: OpenDocMode;
public status?: number;
constructor(public code: string, message: string, details: ErrorDetails = {}) {
constructor(public code: string, message: string, public details: ErrorDetails = {}) {
super(message);
this.status = details.status;
this.accessMode = details.accessMode;
}
public get accessMode() { return this.details?.accessMode; }
public get status() { return this.details?.status; }
}

@ -17,6 +17,9 @@ export interface RulePart {
// Compiled version of aclFormula.
matchFunc?: AclMatchFunc;
// Optional memo, currently extracted from comment in formula.
memo?: string;
}
// Light wrapper around characteristics or records.

@ -64,6 +64,7 @@ function _compileNode(parsedAclFormula: ParsedAclFormula): AclEvalFunc {
const attrName = rawArgs[1] as string;
return _compileAndCombine([args[0]], ([value]) => getAttr(value, attrName, args[0]));
}
case 'Comment': return _compileNode(args[0]);
}
throw new Error(`Unknown node type '${parsedAclFormula[0]}'`);
}

@ -29,7 +29,6 @@ import {toTableDataAction} from 'app/common/DocActions';
import {DocData} from 'app/common/DocData';
import {DocSnapshots} from 'app/common/DocSnapshot';
import {EncActionBundleFromHub} from 'app/common/EncActionBundle';
import {ErrorWithCode} from 'app/common/ErrorWithCode';
import {byteString, countIf} from 'app/common/gutil';
import {InactivityTimer} from 'app/common/InactivityTimer';
import * as marshal from 'app/common/marshal';
@ -604,9 +603,8 @@ export class ActiveDoc extends EventEmitter {
// If user does not have rights to access what this query is asking for, fail.
const tableAccess = await this._granularAccess.getTableAccess(docSession, query.tableId);
if (tableAccess.read === 'deny') {
throw new Error('not authorized to read table');
}
this._granularAccess.assertCanRead(tableAccess);
// Some tests read _grist_ tables via the api. The _fetchQueryFromDB method
// currently cannot read those tables, so we load them from the data engine
@ -614,7 +612,7 @@ export class ActiveDoc extends EventEmitter {
// Also, if row-level access is being controlled, we wait for formula columns
// to be populated.
const wantFull = waitForFormulas || query.tableId.startsWith('_grist_') ||
tableAccess.read === 'mixed';
this._granularAccess.getReadPermission(tableAccess) === 'mixed';
const onDemand = this._onDemandActions.isOnDemand(query.tableId);
this.logInfo(docSession, "fetchQuery %s %s", JSON.stringify(query),
onDemand ? "(onDemand)" : "(regular)");
@ -636,7 +634,7 @@ export class ActiveDoc extends EventEmitter {
}
// If row-level access is being controlled, filter the data appropriately.
// Likewise if column-level access is being controlled.
if (tableAccess.read !== 'allow') {
if (this._granularAccess.getReadPermission(tableAccess) !== 'allow') {
await this._granularAccess.filterData(docSession, data!);
}
this.logInfo(docSession, "fetchQuery -> %d rows, cols: %s",
@ -1181,14 +1179,12 @@ export class ActiveDoc extends EventEmitter {
protected async _applyUserActions(docSession: OptDocSession, actions: UserAction[],
options: ApplyUAOptions = {}): Promise<ApplyUAResult> {
if (!await this._granularAccess.canMaybeApplyUserActions(docSession, actions)) {
throw new ErrorWithCode('ACL_DENY', 'Action blocked by access rules');
}
const client = docSession.client;
this.logDebug(docSession, "_applyUserActions(%s, %s)", client, shortDesc(actions));
this._inactivityTimer.ping(); // The doc is in active use; ping it to stay open longer.
await this._granularAccess.assertCanMaybeApplyUserActions(docSession, actions);
const user = docSession.mode === 'system' ? 'grist' :
(client && client.session ? (await client.session.getEmail()) : "");

@ -276,6 +276,9 @@ export class Client {
if (err.code) {
response.errorCode = err.code;
}
if (err.details) {
response.details = err.details;
}
if (typeof code === 'string' && code === 'AUTH_NO_EDIT' && err.accessMode === 'fork') {
response.shouldFork = true;
}

@ -1,6 +1,4 @@
import { MixedPermissionSet, PartialPermissionSet, PermissionSet, TablePermissionSet } from 'app/common/ACLPermissions';
import { makePartialPermissions, mergePartialPermissions, mergePermissions } from 'app/common/ACLPermissions';
import { emptyPermissionSet, toMixed } from 'app/common/ACLPermissions';
import { ALL_PERMISSION_PROPS } from 'app/common/ACLPermissions';
import { ACLRuleCollection } from 'app/common/ACLRuleCollection';
import { ActionGroup } from 'app/common/ActionGroup';
import { createEmptyActionSummary } from 'app/common/ActionSummary';
@ -15,7 +13,7 @@ import { DocData } from 'app/common/DocData';
import { UserOverride } from 'app/common/DocListAPI';
import { ErrorWithCode } from 'app/common/ErrorWithCode';
import { AclMatchInput, InfoView } from 'app/common/GranularAccessClause';
import { RuleSet, UserInfo } from 'app/common/GranularAccessClause';
import { UserInfo } from 'app/common/GranularAccessClause';
import { getSetMapValue, isObject } from 'app/common/gutil';
import { canView, Role } from 'app/common/roles';
import { FullUser } from 'app/common/UserAPI';
@ -23,9 +21,11 @@ import { HomeDBManager } from 'app/gen-server/lib/HomeDBManager';
import { compileAclFormula } from 'app/server/lib/ACLFormula';
import { getDocSessionAccess, getDocSessionUser, OptDocSession } from 'app/server/lib/DocSession';
import * as log from 'app/server/lib/log';
import { PermissionInfo, PermissionSetWithContext, TablePermissionSetWithContext } from 'app/server/lib/PermissionInfo';
import { integerParam } from 'app/server/lib/requestUtils';
import { getRelatedRows, getRowIdsFromDocAction } from 'app/server/lib/RowAccess';
import cloneDeep = require('lodash/cloneDeep');
import fromPairs = require('lodash/fromPairs');
import get = require('lodash/get');
import pullAt = require('lodash/pullAt');
@ -85,7 +85,7 @@ const OK_ACTIONS = new Set(['Calculate', 'AddEmptyTable']);
* steps and clients. We expect modifications to be serialized, and the following
* pattern of calls for modifications:
*
* - canMaybeApplyUserActions(), called with UserActions for an initial access check.
* - assertCanMaybeApplyUserActions(), called with UserActions for an initial access check.
* Since not all checks can be done without analyzing UserActions into DocActions,
* it is ok for this call to pass even if a more definitive test later will fail.
* - canApplyDocActions(), called when DocActions have been produced from UserActions,
@ -146,7 +146,7 @@ export class GranularAccess {
*/
public async hasTableAccess(docSession: OptDocSession, tableId: string) {
const pset = await this.getTableAccess(docSession, tableId);
return pset.read !== 'deny';
return this.getReadPermission(pset) !== 'deny';
}
/**
@ -280,9 +280,11 @@ export class GranularAccess {
* TODO: not smart about intermediate states, if there is a table or column rename it will
* have trouble, and might forbid something that should be allowed.
*/
public async canMaybeApplyUserActions(docSession: OptDocSession, actions: UserAction[]): Promise<boolean> {
public async assertCanMaybeApplyUserActions(docSession: OptDocSession, actions: UserAction[]): Promise<boolean> {
for (const action of actions) {
if (!await this.canMaybeApplyUserAction(docSession, action)) { return false; }
if (!await this.assertCanMaybeApplyUserAction(docSession, action)) {
throw new ErrorWithCode('ACL_DENY', 'Action blocked by access rules');
}
}
return true;
}
@ -290,7 +292,7 @@ export class GranularAccess {
/**
* Check if user can apply a given action to the document.
*/
public async canMaybeApplyUserAction(docSession: OptDocSession, a: UserAction|DocAction): Promise<boolean> {
public async assertCanMaybeApplyUserAction(docSession: OptDocSession, a: UserAction|DocAction): Promise<boolean> {
const name = a[0] as string;
if (OK_ACTIONS.has(name)) { return true; }
if (SPECIAL_ACTIONS.has(name)) {
@ -300,9 +302,9 @@ export class GranularAccess {
return this.hasFullAccess(docSession);
}
if (a[0] === 'ApplyUndoActions') {
return this.canMaybeApplyUserActions(docSession, a[1] as UserAction[]);
return this.assertCanMaybeApplyUserActions(docSession, a[1] as UserAction[]);
} else if (a[0] === 'ApplyDocActions') {
return this.canMaybeApplyUserActions(docSession, a[1] as UserAction[]);
return this.assertCanMaybeApplyUserActions(docSession, a[1] as UserAction[]);
} else if (isTableAction(a)) {
const tableId = getTableId(a);
// If there are any access control nuances, deny _grist_* tables.
@ -311,8 +313,8 @@ export class GranularAccess {
return !this.hasNuancedAccess(docSession);
}
const tableAccess = await this.getTableAccess(docSession, tableId);
const accessFn = getAccessForActionType(a);
const access = accessFn(tableAccess);
const accessCheck = getAccessForActionType(a, 'fatal');
const access = accessCheck.get(tableAccess);
// if access is mixed, leave this to be checked in detail later.
return access === 'allow' || access === 'mixed' || access === 'mixedColumns';
}
@ -337,7 +339,7 @@ export class GranularAccess {
const access = getDocSessionAccess(docSession);
if (!canView(access)) { return false; }
const permInfo = await this._getAccess(docSession);
return permInfo.getFullAccess().read === 'allow';
return this.getReadPermission(permInfo.getFullAccess()) === 'allow';
}
/**
@ -399,7 +401,7 @@ export class GranularAccess {
const tableId = rec.tableId as string;
const tableRef = rec.id;
const tableAccess = permInfo.getTableAccess(tableId);
if (tableAccess.read === 'deny') {
if (this.getReadPermission(tableAccess) === 'deny') {
censoredTables.add(tableRef);
}
// TODO If some columns are allowed and the rest (*) are denied, we need to be able to
@ -407,7 +409,7 @@ export class GranularAccess {
for (const ruleSet of this._ruleCollection.getAllColumnRuleSets(tableId)) {
if (Array.isArray(ruleSet.colIds)) {
for (const colId of ruleSet.colIds) {
if (permInfo.getColumnAccess(tableId, colId).read === 'deny') {
if (this.getReadPermission(permInfo.getColumnAccess(tableId, colId)) === 'deny') {
censoredColumnCodes.add(columnCode(tableRef, colId));
}
}
@ -475,7 +477,7 @@ export class GranularAccess {
* Distill the clauses for the given session and table, to figure out the
* access level and any row-level access functions needed.
*/
public async getTableAccess(docSession: OptDocSession, tableId: string): Promise<TablePermissionSet> {
public async getTableAccess(docSession: OptDocSession, tableId: string): Promise<TablePermissionSetWithContext> {
return (await this._getAccess(docSession)).getTableAccess(tableId);
}
@ -486,12 +488,14 @@ export class GranularAccess {
public async filterData(docSession: OptDocSession, data: TableDataAction) {
const permInfo = await this._getAccess(docSession);
const tableId = getTableId(data);
if (permInfo.getTableAccess(tableId).read === 'mixed') {
await this._filterRowsAndCells(docSession, data, data, data, canRead);
if (this.getReadPermission(permInfo.getTableAccess(tableId)) === 'mixed') {
await this._filterRowsAndCells(docSession, data, data, data, readAccessCheck);
}
// Filter columns, omitting any to which the user has no access, regardless of rows.
this._filterColumns(data[3], (colId) => permInfo.getColumnAccess(tableId, colId).read !== 'deny');
this._filterColumns(
data[3],
(colId) => this.getReadPermission(permInfo.getColumnAccess(tableId, colId)) !== 'deny');
}
public async getUserOverride(docSession: OptDocSession): Promise<UserOverride|undefined> {
@ -499,25 +503,33 @@ export class GranularAccess {
return this._getUserAttributes(docSession).override;
}
public getReadPermission(ps: PermissionSetWithContext) {
return ps.perms.read;
}
public assertCanRead(ps: PermissionSetWithContext) {
readAccessCheck.throwIfDenied(ps);
}
/**
* Strip out any denied columns from an action. Returns null if nothing is left.
* accessFn may throw if denials are fatal.
* accessCheck may throw if denials are fatal.
*/
private _pruneColumns(a: DocAction, permInfo: PermissionInfo, tableId: string,
accessFn: AccessFn): DocAction|null {
accessCheck: AccessCheck): DocAction|null {
if (a[0] === 'RemoveRecord' || a[0] === 'BulkRemoveRecord') {
return a;
} else if (a[0] === 'AddRecord' || a[0] === 'BulkAddRecord' || a[0] === 'UpdateRecord' ||
a[0] === 'BulkUpdateRecord' || a[0] === 'ReplaceTableData' || a[0] === 'TableData') {
const na = cloneDeep(a);
this._filterColumns(na[3], (colId) => accessFn(permInfo.getColumnAccess(tableId, colId)) !== 'deny');
this._filterColumns(na[3], (colId) => accessCheck.get(permInfo.getColumnAccess(tableId, colId)) !== 'deny');
if (Object.keys(na[3]).length === 0) { return null; }
return na;
} else if (a[0] === 'AddColumn' || a[0] === 'RemoveColumn' || a[0] === 'RenameColumn' ||
a[0] === 'ModifyColumn') {
const na = cloneDeep(a);
const colId: string = na[2];
if (accessFn(permInfo.getColumnAccess(tableId, colId)) === 'deny') { return null; }
if (accessCheck.get(permInfo.getColumnAccess(tableId, colId)) === 'deny') { return null; }
throw new ErrorWithCode('NEED_RELOAD', 'document needs reload');
} else {
// Remaining cases of AddTable, RemoveTable, RenameTable should have
@ -614,22 +626,22 @@ export class GranularAccess {
// Return the results, also applying any cell-level access control.
for (const docAction of revisedDocActions) {
await this._filterRowsAndCells(docSession, rowsAfter, rowsAfter, docAction, canRead);
await this._filterRowsAndCells(docSession, rowsAfter, rowsAfter, docAction, readAccessCheck);
}
return revisedDocActions;
}
/**
* Like _pruneRows, but fails immediately if access to any row is forbidden.
* The accessFn supplied should throw an error on denial.
* The accessCheck supplied should throw an error on denial.
*/
private async _checkRows(docSession: OptDocSession, a: DocAction, idx: number,
accessFn: AccessFn): Promise<void> {
accessCheck: AccessCheck): Promise<void> {
// For the moment, only deal with Record-related actions.
// TODO: process table/column schema changes more carefully.
if (isSchemaAction(a)) { return; }
const {rowsBefore, rowsAfter} = await this._getRowsBeforeAndAfter(idx);
await this._filterRowsAndCells(docSession, rowsBefore, rowsAfter, a, accessFn);
await this._filterRowsAndCells(docSession, rowsBefore, rowsAfter, a, accessCheck);
}
private async _getRowsBeforeAndAfter(idx: number) {
@ -656,7 +668,7 @@ export class GranularAccess {
* Modify action in place, scrubbing any rows and cells to which access is not granted.
*/
private async _filterRowsAndCells(docSession: OptDocSession, rowsBefore: TableDataAction, rowsAfter: TableDataAction,
docAction: DocAction, accessFn: AccessFn) {
docAction: DocAction, accessCheck: AccessCheck) {
if (docAction && isSchemaAction(docAction)) {
// TODO should filter out metadata about an unavailable column, probably.
return [];
@ -696,14 +708,14 @@ export class GranularAccess {
const rowPermInfo = new PermissionInfo(this._ruleCollection, input);
// getTableAccess() evaluates all column rules for THIS record. So it's really rowAccess.
const rowAccess = rowPermInfo.getTableAccess(tableId);
const access = accessFn(rowAccess);
const access = accessCheck.get(rowAccess);
if (access === 'deny') {
toRemove.push(idx);
} else if (access !== 'allow' && colValues) {
// Go over column rules.
for (const colId of Object.keys(colValues)) {
const colAccess = rowPermInfo.getColumnAccess(tableId, colId);
if (accessFn(colAccess) === 'deny') {
if (accessCheck.get(colAccess) === 'deny') {
censorAt(colId, idx);
}
}
@ -737,7 +749,7 @@ export class GranularAccess {
const rowPermInfo = new PermissionInfo(this._ruleCollection, input);
// getTableAccess() evaluates all column rules for THIS record. So it's really rowAccess.
const rowAccess = rowPermInfo.getTableAccess(tableId);
if (canRead(rowAccess) === 'deny') {
if (this.getReadPermission(rowAccess) === 'deny') {
toRemove.push(rowIds[idx]);
}
}
@ -1003,134 +1015,36 @@ export class GranularAccess {
const tableId = getTableId(a);
const permInfo = await this._getAccess(docSession);
const tableAccess = permInfo.getTableAccess(tableId);
const access = tableAccess.read;
const access = this.getReadPermission(tableAccess);
if (access === 'deny') { return []; }
if (access === 'allow') { return [a]; }
if (access === 'mixedColumns') {
return [this._pruneColumns(a, permInfo, tableId, canRead)].filter(isObject);
return [this._pruneColumns(a, permInfo, tableId, readAccessCheck)].filter(isObject);
}
// The remainder is the mixed condition.
const revisedDocActions = await this._pruneRows(docSession, a, idx);
const result = revisedDocActions.map(na => this._pruneColumns(na, permInfo, tableId,
canRead)).filter(isObject);
readAccessCheck)).filter(isObject);
return result;
}
private async _checkIncomingDocAction(docSession: OptDocSession, a: DocAction, idx: number): Promise<void> {
const accessFn = denyIsFatal(getAccessForActionType(a));
const accessCheck = getAccessForActionType(a, 'fatal');
const tableId = getTableId(a);
const permInfo = await this._getAccess(docSession);
const tableAccess = permInfo.getTableAccess(tableId);
const access = accessFn(tableAccess);
const access = accessCheck.get(tableAccess);
if (access === 'allow') { return; }
if (access === 'mixedColumns') {
// Somewhat abusing prune method by calling it with an access function that
// throws on denial.
this._pruneColumns(a, permInfo, tableId, accessFn);
this._pruneColumns(a, permInfo, tableId, accessCheck);
}
// The remainder is the mixed condition.
await this._checkRows(docSession, a, idx, accessFn);
await this._checkRows(docSession, a, idx, accessCheck);
// Somewhat abusing prune method by calling it with an access function that
// throws on denial.
this._pruneColumns(a, permInfo, tableId, accessFn);
}
}
/**
* Evaluate a RuleSet on a given input (user and optionally record). If a record is needed but not
* included, the result may include permission values like 'allowSome', 'denySome'.
*/
function evaluateRule(ruleSet: RuleSet, input: AclMatchInput): PartialPermissionSet {
let pset: PartialPermissionSet = emptyPermissionSet();
for (const rule of ruleSet.body) {
try {
if (rule.matchFunc!(input)) {
pset = mergePartialPermissions(pset, rule.permissions);
}
} catch (e) {
if (e.code === 'NEED_ROW_DATA') {
pset = mergePartialPermissions(pset, makePartialPermissions(rule.permissions));
} else {
// For other errors, assume the rule is invalid, and treat as a non-match.
// TODO An appropriate user should be alerted that a clause is not being honored.
log.warn("ACLRule for %s failed: %s", ruleSet.tableId, e.message);
}
}
}
return pset;
}
/**
* Helper for evaluating rules given a particular user and optionally a record. It evaluates rules
* for a column, table, or document, with caching to avoid evaluating the same rule multiple times.
*/
class PermissionInfo {
private _ruleResults = new Map<RuleSet, MixedPermissionSet>();
// Construct a PermissionInfo for a particular input, which is a combination of user and
// optionally a record.
constructor(private _acls: ACLRuleCollection, private _input: AclMatchInput) {}
// Get permissions for "tableId:colId", defaulting to "tableId:*" and "*:*" as needed.
// If 'mixed' is returned, different rows may have different permissions. It should never return
// 'mixed' if the input includes `rec`.
public getColumnAccess(tableId: string, colId: string): MixedPermissionSet {
const ruleSet: RuleSet|undefined = this._acls.getColumnRuleSet(tableId, colId);
return ruleSet ? this._processColumnRule(ruleSet) : this._getTableDefaultAccess(tableId);
}
// Combine permissions from all rules for the given table.
// If 'mixedColumns' is returned, different columns have different permissions, but they do NOT
// depend on rows. If 'mixed' is returned, some permissions depend on rows.
public getTableAccess(tableId: string): TablePermissionSet {
const columnAccess = this._acls.getAllColumnRuleSets(tableId).map(rs => this._processColumnRule(rs));
columnAccess.push(this._getTableDefaultAccess(tableId));
return mergePermissions(columnAccess, (bits) => (
bits.every(b => b === 'allow') ? 'allow' :
bits.every(b => b === 'deny') ? 'deny' :
bits.every(b => b === 'allow' || b === 'deny') ? 'mixedColumns' :
'mixed'
));
}
// Combine permissions from all rules throughout.
// If 'mixed' is returned, then different tables, rows, or columns have different permissions.
public getFullAccess(): MixedPermissionSet {
const tableAccess = this._acls.getAllTableIds().map(tableId => this.getTableAccess(tableId));
tableAccess.push(this._getDocDefaultAccess());
return mergePermissions(tableAccess, (bits) => (
bits.every(b => b === 'allow') ? 'allow' :
bits.every(b => b === 'deny') ? 'deny' :
'mixed'
));
}
// Get permissions for "tableId:*", defaulting to "*:*" as needed.
// If 'mixed' is returned, different rows may have different permissions.
private _getTableDefaultAccess(tableId: string): MixedPermissionSet {
const ruleSet: RuleSet|undefined = this._acls.getTableDefaultRuleSet(tableId);
return ruleSet ? this._processRule(ruleSet, () => this._getDocDefaultAccess()) :
this._getDocDefaultAccess();
}
// Get permissions for "*:*".
private _getDocDefaultAccess(): MixedPermissionSet {
return this._processRule(this._acls.getDocDefaultRuleSet());
}
// Evaluate and cache the given column rule, falling back to the corresponding table default.
private _processColumnRule(ruleSet: RuleSet): MixedPermissionSet {
return this._processRule(ruleSet, () => this._getTableDefaultAccess(ruleSet.tableId));
}
// Evaluate the given rule, with the default fallback, and cache the result.
private _processRule(ruleSet: RuleSet, defaultAccess?: () => MixedPermissionSet): MixedPermissionSet {
return getSetMapValue(this._ruleResults, ruleSet, () => {
const pset = evaluateRule(ruleSet, this._input);
return toMixed(defaultAccess ? mergePartialPermissions(pset, defaultAccess()) : pset);
});
this._pruneColumns(a, permInfo, tableId, accessCheck);
}
}
@ -1173,34 +1087,49 @@ class UserAttributes {
public override?: UserOverride;
}
// A function for extracting one of the create/read/update/delete/schemaEdit permissions
// from a permission set.
type AccessFn = (ps: PermissionSet<string>) => string;
// Get an AccessFn appropriate for the specific action.
class AccessCheck {
constructor(public access: 'update'|'delete'|'create'|'schemaEdit'|'read',
public severity: 'check'|'fatal') {
}
public get(ps: PermissionSetWithContext): string {
const result = ps.perms[this.access];
if (result !== 'deny' || this.severity !== 'fatal') { return result; }
this.throwIfDenied(ps);
return result;
}
public throwIfDenied(ps: PermissionSetWithContext): void {
const result = ps.perms[this.access];
if (result !== 'deny') { return; }
const memos = ps.getMemos()[this.access];
throw new ErrorWithCode('ACL_DENY', `Blocked by ${ps.ruleType} access rules`, {
memos,
status: 403
});
}
}
export const accessChecks = {
check: fromPairs(ALL_PERMISSION_PROPS.map(prop => [prop, new AccessCheck(prop, 'check')])),
fatal: fromPairs(ALL_PERMISSION_PROPS.map(prop => [prop, new AccessCheck(prop, 'fatal')])),
};
// The AccessCheck for the "read" permission is used enough to merit a shortcut.
const readAccessCheck = accessChecks.check.read;
// Get an AccessCheck appropriate for the specific action.
// TODO: deal with ReplaceTableData, which both deletes and creates rows.
function getAccessForActionType(a: DocAction): AccessFn {
function getAccessForActionType(a: DocAction, severity: 'check'|'fatal'): AccessCheck {
if (a[0] === 'UpdateRecord' || a[0] === 'BulkUpdateRecord') {
return (ps) => ps.update;
return accessChecks[severity].update;
} else if (a[0] === 'RemoveRecord' || a[0] === 'BulkRemoveRecord') {
return (ps) => ps.delete;
return accessChecks[severity].delete;
} else if (a[0] === 'AddRecord' || a[0] === 'BulkAddRecord') {
return (ps) => ps.create;
return accessChecks[severity].create;
} else {
return (ps) => ps.schemaEdit;
return accessChecks[severity].schemaEdit;
}
}
// Tweak an AccessFn so that it throws an exception if access is denied.
function denyIsFatal(fn: AccessFn): AccessFn {
return (ps) => {
const result = fn(ps);
if (result === 'deny') { throw new ErrorWithCode('ACL_DENY', 'Blocked by access rules'); }
return result;
};
}
// A simple access function that returns the "read" permission.
function canRead(ps: PermissionSet<string>) {
return ps.read;
}

@ -0,0 +1,239 @@
import { ALL_PERMISSION_PROPS, emptyPermissionSet,
makePartialPermissions, mergePartialPermissions, mergePermissions,
MixedPermissionSet, PartialPermissionSet, PermissionSet, TablePermissionSet,
toMixed } from 'app/common/ACLPermissions';
import { ACLRuleCollection } from 'app/common/ACLRuleCollection';
import { AclMatchInput, RuleSet } from 'app/common/GranularAccessClause';
import { getSetMapValue } from 'app/common/gutil';
import * as log from 'app/server/lib/log';
import { mapValues } from 'lodash';
/**
* A PermissionSet with context about how it was created. Allows us to produce more
* informative error messages.
*/
export interface PermissionSetWithContextOf<T = PermissionSet> {
perms: T;
ruleType: 'full'|'table'|'column';
getMemos: () => MemoSet;
}
export type MixedPermissionSetWithContext = PermissionSetWithContextOf<MixedPermissionSet>;
export type TablePermissionSetWithContext = PermissionSetWithContextOf<TablePermissionSet>;
export type PermissionSetWithContext = PermissionSetWithContextOf<PermissionSet<string>>;
// Accumulator for memos of relevant rules.
export type MemoSet = PermissionSet<string[]>;
// Merge MemoSets straightforwardly, by concatenation.
export function mergeMemoSets(psets: MemoSet[]): MemoSet {
const result: Partial<MemoSet> = {};
for (const prop of ALL_PERMISSION_PROPS) {
result[prop] = ([] as string[]).concat(...psets.map(p => p[prop]));
}
return result as MemoSet;
}
export function emptyMemoSet(): MemoSet {
return {
read: [],
create: [],
update: [],
delete: [],
schemaEdit: [],
};
}
/**
* Abstract base class for processing rules given a particular input.
* Main use of this class will be to calculate permissions, but will also
* be used to calculate metadata about permissions.
*/
abstract class RuleInfo<MixedT extends TableT, TableT> {
// Construct a RuleInfo for a particular input, which is a combination of user and
// optionally a record.
constructor(protected _acls: ACLRuleCollection, protected _input: AclMatchInput) {}
public getColumnAspect(tableId: string, colId: string): MixedT {
const ruleSet: RuleSet|undefined = this._acls.getColumnRuleSet(tableId, colId);
return ruleSet ? this._processColumnRule(ruleSet) : this._getTableDefaultAspect(tableId);
}
public getTableAspect(tableId: string): TableT {
const columnAccess = this._acls.getAllColumnRuleSets(tableId).map(rs => this._processColumnRule(rs));
columnAccess.push(this._getTableDefaultAspect(tableId));
return this._mergeTableAccess(columnAccess);
}
public getFullAspect(): MixedT {
const tableAccess = this._acls.getAllTableIds().map(tableId => this.getTableAspect(tableId));
tableAccess.push(this._getDocDefaultAspect());
return this._mergeFullAccess(tableAccess);
}
protected abstract _processRule(ruleSet: RuleSet, defaultAccess?: () => MixedT): MixedT;
protected abstract _mergeTableAccess(access: MixedT[]): TableT;
protected abstract _mergeFullAccess(access: TableT[]): MixedT;
private _getTableDefaultAspect(tableId: string): MixedT {
const ruleSet: RuleSet|undefined = this._acls.getTableDefaultRuleSet(tableId);
return ruleSet ? this._processRule(ruleSet, () => this._getDocDefaultAspect()) :
this._getDocDefaultAspect();
}
private _getDocDefaultAspect(): MixedT {
return this._processRule(this._acls.getDocDefaultRuleSet());
}
private _processColumnRule(ruleSet: RuleSet): MixedT {
return this._processRule(ruleSet, () => this._getTableDefaultAspect(ruleSet.tableId));
}
}
/**
* Pool memos from rules, on the assumption that access has been denied and we are looking
* for possible explanations to offer the user.
*/
export class MemoInfo extends RuleInfo<MemoSet, MemoSet> {
protected _processRule(ruleSet: RuleSet, defaultAccess?: () => MemoSet): MemoSet {
const pset = extractMemos(ruleSet, this._input);
return defaultAccess ? mergeMemoSets([pset, defaultAccess()]) : pset;
}
protected _mergeTableAccess(access: MemoSet[]): MemoSet {
return mergeMemoSets(access);
}
protected _mergeFullAccess(access: MemoSet[]): MemoSet {
return mergeMemoSets(access);
}
}
/**
* Helper for evaluating rules given a particular user and optionally a record. It evaluates rules
* for a column, table, or document, with caching to avoid evaluating the same rule multiple times.
*/
export class PermissionInfo extends RuleInfo<MixedPermissionSet, TablePermissionSet> {
private _ruleResults = new Map<RuleSet, MixedPermissionSet>();
// Get permissions for "tableId:colId", defaulting to "tableId:*" and "*:*" as needed.
// If 'mixed' is returned, different rows may have different permissions. It should never return
// 'mixed' if the input includes `rec`.
// Wrap permissions with information about how they were computed. This allows
// us to issue more informative error messages.
public getColumnAccess(tableId: string, colId: string): MixedPermissionSetWithContext {
return {
perms: this.getColumnAspect(tableId, colId),
ruleType: 'column',
getMemos: () => new MemoInfo(this._acls, this._input).getColumnAspect(tableId, colId)
};
}
// Combine permissions from all rules for the given table.
// If 'mixedColumns' is returned, different columns have different permissions, but they do NOT
// depend on rows. If 'mixed' is returned, some permissions depend on rows.
// Wrap permission sets for better error messages.
public getTableAccess(tableId: string): TablePermissionSetWithContext {
return {
perms: this.getTableAspect(tableId),
ruleType: 'table',
getMemos: () => new MemoInfo(this._acls, this._input).getTableAspect(tableId)
};
}
// Combine permissions from all rules throughout.
// If 'mixed' is returned, then different tables, rows, or columns have different permissions.
// Wrap permission sets for better error messages.
public getFullAccess(): MixedPermissionSetWithContext {
return {
perms: this.getFullAspect(),
ruleType: 'full',
getMemos: () => new MemoInfo(this._acls, this._input).getFullAspect()
};
}
protected _processRule(ruleSet: RuleSet, defaultAccess?: () => MixedPermissionSet): MixedPermissionSet {
return getSetMapValue(this._ruleResults, ruleSet, () => {
const pset = evaluateRule(ruleSet, this._input);
return toMixed(defaultAccess ? mergePartialPermissions(pset, defaultAccess()) : pset);
});
}
protected _mergeTableAccess(access: MixedPermissionSet[]): TablePermissionSet {
return mergePermissions(access, (bits) => (
bits.every(b => b === 'allow') ? 'allow' :
bits.every(b => b === 'deny') ? 'deny' :
bits.every(b => b === 'allow' || b === 'deny') ? 'mixedColumns' :
'mixed'
));
}
protected _mergeFullAccess(access: TablePermissionSet[]): MixedPermissionSet {
return mergePermissions(access, (bits) => (
bits.every(b => b === 'allow') ? 'allow' :
bits.every(b => b === 'deny') ? 'deny' :
'mixed'
));
}
}
/**
* Evaluate a RuleSet on a given input (user and optionally record). If a record is needed but not
* included, the result may include permission values like 'allowSome', 'denySome'.
*/
function evaluateRule(ruleSet: RuleSet, input: AclMatchInput): PartialPermissionSet {
let pset: PartialPermissionSet = emptyPermissionSet();
for (const rule of ruleSet.body) {
try {
if (rule.matchFunc!(input)) {
pset = mergePartialPermissions(pset, rule.permissions);
}
} catch (e) {
if (e.code === 'NEED_ROW_DATA') {
pset = mergePartialPermissions(pset, makePartialPermissions(rule.permissions));
} else {
// Unexpected error. Interpret rule pessimistically.
// Anything it would explicitly allow, no longer allow through this rule.
// Anything it would explicitly deny, go ahead and deny.
pset = mergePartialPermissions(pset, mapValues(rule.permissions, val => (val === 'allow' ? "" : val)));
log.warn("ACLRule for %s failed: %s", ruleSet.tableId, e.message);
}
}
}
return pset;
}
/**
* If a rule has a memo, and passes, add that memo for all permissions it denies.
* If a rule has a memo, and fails, add that memo for all permissions it allows.
*/
function extractMemos(ruleSet: RuleSet, input: AclMatchInput): MemoSet {
const pset = emptyMemoSet();
for (const rule of ruleSet.body) {
try {
const passing = rule.matchFunc!(input);
for (const prop of ALL_PERMISSION_PROPS) {
const p = rule.permissions[prop];
const memos: string[] = pset[prop];
if (rule.memo) {
if (passing && p === 'deny') {
memos.push(rule.memo);
} else if (!passing && p === 'allow') {
memos.push(rule.memo);
}
}
}
} catch (e) {
if (e.code !== 'NEED_ROW_DATA') {
// If a rule is failing unexpectedly, give some information via memos.
// TODO: Could give a more structured result.
for (const prop of ALL_PERMISSION_PROPS) {
pset[prop].push(`Rule [${rule.aclFormula}] for ${ruleSet.tableId} has an error: ${e.message}`);
}
}
}
}
return pset;
}

@ -24,8 +24,15 @@ export const jsonErrorHandler: express.ErrorRequestHandler = (err, req, res, nex
log.warn("Error during api call to %s: (%s) user %d params %s body %s", req.path, err.message,
mreq.userId,
JSON.stringify(req.params), JSON.stringify(req.body));
res.status(err.status || 500).json({error: err.message || 'internal error',
details: err.details});
let details = err.details && {...err.details};
const status = details?.status || err.status || 500;
if (details) {
// Remove some details exposed for websocket API only.
delete details.accessMode;
delete details.status; // TODO: reconcile err.status and details.status, no need for both.
if (Object.keys(details).length === 0) { details = undefined; }
}
res.status(status).json({error: err.message || 'internal error', details});
};
/**

@ -1,5 +1,7 @@
import ast
import io
import json
import tokenize
from collections import namedtuple
import asttokens
@ -22,10 +24,16 @@ def parse_acl_formula(acl_formula):
Const value (number, string, bool)
Name name (string)
Attr node, attr_name
Comment node, comment
"""
try:
tree = ast.parse(acl_formula, mode='eval')
return _TreeConverter().visit(tree)
result = _TreeConverter().visit(tree)
for part in tokenize.generate_tokens(io.StringIO(acl_formula.decode('utf-8')).readline):
if part[0] == tokenize.COMMENT and part[1].startswith('#'):
result = ['Comment', result, part[1][1:].strip()]
break
return result
except SyntaxError as err:
# In case of an error, include line and offset.
raise SyntaxError("%s on line %s col %s" % (err.args[0], err.lineno, err.offset))

@ -79,6 +79,24 @@ class TestACLFormula(unittest.TestCase):
['Attr', ['Attr', ['Name', 'user'], 'Status'], 'IsActive']
])
self.assertEqual(parse_acl_formula(
"True # Comment! "),
['Comment', ['Const', True], 'Comment!'])
self.assertEqual(parse_acl_formula(
"\"#x\" == \" # Not a comment \"#Comment!"),
['Comment',
['Eq', ['Const', '#x'], ['Const', ' # Not a comment ']],
'Comment!'
])
self.assertEqual(parse_acl_formula(
"# Allow owners\nuser.Access == 'owners' # ignored\n# comment ignored"),
['Comment',
['Eq', ['Attr', ['Name', 'user'], 'Access'], ['Const', 'owners']],
'Allow owners'
])
def test_unsupported(self):
# Test a few constructs we expect to fail
# Not an expression

Loading…
Cancel
Save