Archives
/
gristlabs_grist-core
mirror of https://github.com/gristlabs/grist-core
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Dump the rule for ACL formula warnings (#639)

Browse Source 
When an ACL formula fails to be run, a warning is printed. However, it is painful to know which formula is concerned by the warning.

ACL: if RHS is null, return false for "in" and "not in"
pull/643/head
Florent 8 months ago committed by GitHub
parent fe12562ad7
commit 9dfebefc9b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 77 additions and 21 deletions
Show Stats Download Patch File Download Diff File

2
.editorconfig
Unescape View File

@ -6,7 +6,7 @@ root = true
# Unix-style newlines with a newline ending every file
[*.{ts,js}]
end_of_line = lf
insert_final_newline = false
insert_final_newline = true
charset = utf-8
# indent with 2 spaces
indent_style = space

1
app/common/GranularAccessClause.ts
Unescape View File

@ -58,6 +58,7 @@ export interface AclMatchInput {
user: UserInfo;
rec?: InfoView;
newRec?: InfoView;
docId?: string;
}
/**

4
app/server/lib/ACLFormula.ts
Unescape View File

@ -55,8 +55,8 @@ function _compileNode(parsedAclFormula: ParsedAclFormula): AclEvalFunc {
case 'GtE': return _compileAndCombine(args, ([a, b]) => a >= b);
case 'Is': return _compileAndCombine(args, ([a, b]) => a === b);
case 'IsNot': return _compileAndCombine(args, ([a, b]) => a !== b);
case 'In': return _compileAndCombine(args, ([a, b]) => b.includes(a));
case 'NotIn': return _compileAndCombine(args, ([a, b]) => !b.includes(a));
case 'In': return _compileAndCombine(args, ([a, b]) => Boolean(b?.includes(a)));
case 'NotIn': return _compileAndCombine(args, ([a, b]) => !b?.includes(a));
case 'List': return _compileAndCombine(args, (values) => values);
case 'Const': return constant(parsedAclFormula[1] as CellValue);
case 'Name': {

1
app/server/lib/GranularAccess.ts
Unescape View File

@ -342,6 +342,7 @@ export class GranularAccess implements GranularAccessForBundle {
public async inputs(docSession: OptDocSession): Promise<AclMatchInput> {
return {
user: await this._getUser(docSession),
docId: this._docId
};
}

3
app/server/lib/PermissionInfo.ts
Unescape View File

@ -219,7 +219,8 @@ function evaluateRule(ruleSet: RuleSet, input: AclMatchInput): PartialPermission
// 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);
const prefixedTableName = input.docId ? `${input.docId}.${ruleSet.tableId}` : ruleSet.tableId;
log.warn("ACLRule for %s (`%s`) failed: %s", prefixedTableName, rule.aclFormula, e.stack);
}
}
}

87
test/server/lib/ACLFormula.ts
Unescape View File

@ -13,7 +13,7 @@ describe('ACLFormula', function() {
// Turn off logging for this test, and restore afterwards.
testUtils.setTmpLogLevel('error');
const docTools = createDocTools();
const docTools = createDocTools({ persistAcrossCases: true });
const fakeSession = makeExceptionalDocSession('system');
function getInfoView(row: Record<string, CellValue>): InfoView {
@ -25,47 +25,72 @@ describe('ACLFormula', function() {
const V = getInfoView; // A shortcut.
it('should correctly interpret parsed ACL formulas', async function() {
type SetAndCompile = (aclFormula: string) => Promise<AclMatchFunc>;
let setAndCompile: SetAndCompile;
before(async function () {
const docName = 'docdata1';
const activeDoc1 = await docTools.createDoc(docName);
let compiled: AclMatchFunc;
const resourceRef = (await activeDoc1.applyUserActions(fakeSession,
[['AddRecord', '_grist_ACLResources', null, {tableId: '*', colIds: '*'}]])).retValues[0];
const ruleRef = (await activeDoc1.applyUserActions(fakeSession,
[['AddRecord', '_grist_ACLRules', null, {resource: resourceRef}]])).retValues[0];
async function setAndCompile(aclFormula: string): Promise<AclMatchFunc> {
setAndCompile = async function setAndCompile(aclFormula) {
await activeDoc1.applyUserActions(fakeSession, [['UpdateRecord', '_grist_ACLRules', ruleRef, {aclFormula}]]);
const {tableData} = await activeDoc1.fetchQuery(
fakeSession, {tableId: '_grist_ACLRules', filters: {id: [ruleRef]}});
assert(tableData[3].aclFormulaParsed, "Expected aclFormulaParsed to be populated");
const parsedFormula = String(tableData[3].aclFormulaParsed[0]);
return compileAclFormula(JSON.parse(parsedFormula));
}
};
});
compiled = await setAndCompile("user.Email == 'X@'");
it('should handle a comparison', async function() {
const compiled = await setAndCompile("user.Email == 'X@'");
assert.equal(compiled({user: new User({Email: 'X@'})}), true);
assert.equal(compiled({user: new User({Email: 'Y@'})}), false);
assert.equal(compiled({user: new User({Email: 'X'}), rec: V({Email: 'Y@'})}), false);
assert.equal(compiled({user: new User({Name: 'X@'})}), false);
});
compiled = await setAndCompile("user.Role in ('editors', 'owners')");
it('should handle the "in" operator', async function () {
const compiled = await setAndCompile("user.Role in ('editors', 'owners')");
assert.equal(compiled({user: new User({Role: 'editors'})}), true);
assert.equal(compiled({user: new User({Role: 'owners'})}), true);
assert.equal(compiled({user: new User({Role: 'viewers'})}), false);
assert.equal(compiled({user: new User({Role: null})}), false);
assert.equal(compiled({user: new User({})}), false);
});
// Opposite of the previous formula.
compiled = await setAndCompile("user.Role not in ('editors', 'owners')");
it('should handle the "not in" operator', async function () {
const compiled = await setAndCompile("user.Role not in ('editors', 'owners')");
assert.equal(compiled({user: new User({Role: 'editors'})}), false);
assert.equal(compiled({user: new User({Role: 'owners'})}), false);
assert.equal(compiled({user: new User({Role: 'viewers'})}), true);
assert.equal(compiled({user: new User({Role: null})}), true);
assert.equal(compiled({user: new User({})}), true);
});
[{
op: 'in'
}, {
op: 'not in'
}].forEach(ctx => {
it(`should handle the "${ctx.op}" operator with a string RHS to check if substring exist`, async function() {
const compiled = await setAndCompile(`user.Name ${ctx.op} 'FooBar'`);
assert.equal(compiled({user: new User({Name: 'FooBar'})}), ctx.op === 'in');
assert.equal(compiled({user: new User({Name: 'Foo'})}), ctx.op === 'in');
assert.equal(compiled({user: new User({Name: 'Bar'})}), ctx.op === 'in');
assert.equal(compiled({user: new User({Name: 'bar'})}), ctx.op === 'not in');
assert.equal(compiled({user: new User({Name: 'qux'})}), ctx.op === 'not in');
assert.equal(compiled({user: new User({Name: null})}), ctx.op === 'not in');
});
});
compiled = await setAndCompile("rec.office == 'Seattle' and user.email in ['sally@', 'xie@']");
it('should handle the "and" operator', async function () {
const compiled = await setAndCompile("rec.office == 'Seattle' and user.email in ['sally@', 'xie@']");
assert.throws(() => compiled({user: new User({email: 'xie@'})}), /Missing row data 'rec'/);
assert.equal(compiled({user: new User({email: 'xie@'}), rec: V({})}), false);
assert.equal(compiled({user: new User({email: 'xie@'}), rec: V({office: null})}), false);
@ -75,9 +100,19 @@ describe('ACLFormula', function() {
assert.equal(compiled({user: new User({email: 'sally@'}), rec: V({office: 'Chicago'})}), false);
assert.equal(compiled({user: new User({email: null}), rec: V({office: null})}), false);
assert.equal(compiled({user: new User({}), rec: V({})}), false);
});
it('should handle the "or" operator', async function () {
const compiled = await setAndCompile('user.Email=="X@" or user.Email is None');
assert.equal(compiled({user: new User({Email: 'X@'})}), true);
assert.equal(compiled({user: new User({})}), true);
assert.equal(compiled({user: new User({Email: 'Y@'})}), false);
});
it('should handle a complex combination of operators', async function () {
// This is not particularly meaningful, but involves more combinations.
compiled = await setAndCompile(
const compiled = await setAndCompile(
"user.IsAdmin or rec.assigned is None or (not newRec.HasDuplicates and rec.StatusIndex <= newRec.StatusIndex)");
assert.equal(compiled({user: new User({IsAdmin: true})}), true);
assert.equal(compiled({user: new User({IsAdmin: 17})}), true);
@ -102,9 +137,10 @@ describe('ACLFormula', function() {
newRec: V({HasDuplicates: false, StatusIndex: 1})}), false);
assert.equal(compiled({user: new User({IsAdmin: false}), rec: V({assigned: 1, StatusIndex: 1}),
newRec: V({HasDuplicates: true, StatusIndex: 17})}), false);
});
// Test arithmetic.
compiled = await setAndCompile(
it('should handle arithmetic tests', async function () {
const compiled = await setAndCompile(
"rec.A <= rec.B + 1 and rec.A >= rec.B - 1 and rec.A < rec.C * 2.5 and rec.A > rec.C / 2.5 and rec.A % 2 != 0");
assert.equal(compiled({user: new User({}), rec: V({A: 3, B: 3, C: 3})}), true);
assert.equal(compiled({user: new User({}), rec: V({A: 3, B: 4, C: 3})}), true);
@ -117,9 +153,10 @@ describe('ACLFormula', function() {
assert.equal(compiled({user: new User({}), rec: V({A: 1.3, B: 1, C: 3})}), true);
assert.equal(compiled({user: new User({}), rec: V({A: 7.4, B: 7, C: 3})}), true);
assert.equal(compiled({user: new User({}), rec: V({A: 7.5, B: 7, C: 3})}), false);
});
// Test is/is-not.
compiled = await setAndCompile(
it('should handle "is" and "is not" operators', async function () {
const compiled = await setAndCompile(
"rec.A is True or rec.B is not False");
assert.equal(compiled({user: new User({}), rec: V({A: true})}), true);
assert.equal(compiled({user: new User({}), rec: V({A: 2})}), true);
@ -130,9 +167,10 @@ describe('ACLFormula', function() {
assert.equal(compiled({user: new User({}), rec: V({A: null, B: 2})}), true);
assert.equal(compiled({user: new User({}), rec: V({A: null, B: false})}), false);
assert.equal(compiled({user: new User({}), rec: V({A: null, B: 0})}), true);
});
// Test nested attribute lookups.
compiled = await setAndCompile('user.office.city == "New York"');
it('should handle nested attribute lookups', async function () {
const compiled = await setAndCompile('user.office.city == "New York"');
assert.equal(compiled({user: new User({office: V({city: "New York"})})}), true);
assert.equal(compiled({user: new User({office: V({city: "Boston"})})}), false);
assert.equal(compiled({user: new User({office: V({city: null})})}), false);
@ -140,4 +178,19 @@ describe('ACLFormula', function() {
assert.equal(compiled({user: new User({office: 5})}), false);
assert.throws(() => compiled({user: new User({office: null})}), /No value for 'user.office'/);
});
it('should handle "in" and "not in" when RHS is nullish', async function() {
let compiled = await setAndCompile('user.Email in rec.emails');
const user = new User({Email: 'X@'});
assert.equal(compiled({user, rec: V({emails: null})}), false);
assert.equal(compiled({user, rec: V({unrelated: 'X@'})}), false);
assert.equal(compiled({user, rec: V({emails: 'X@'})}), true);
compiled = await setAndCompile('user.Email not in rec.emails');
assert.equal(compiled({user, rec: V({emails: null})}), true);
assert.equal(compiled({user, rec: V({unrelated: 'X@'})}), true);
assert.equal(compiled({user, rec: V({emails: 'X@'})}), false);
compiled = await setAndCompile('(user.Email in rec.emails) == (user.Name in rec.emails)');
assert.equal(compiled({user, rec: V({emails: null})}), true);
assert.equal(compiled({user, rec: V({emails: 'X@'})}), false);
});
});

Write Preview
Loading…
Cancel
Save