mirror of
				https://github.com/gristlabs/grist-core.git
				synced 2025-06-13 20:53:59 +00:00 
			
		
		
		
	(core) Decode cell values to prevent working around rule using 'in' on lists
Summary: Fixes bug described in https://grist.slack.com/archives/C069RUP71/p1699643458649019 Decodes cell values obtained from `InfoView.get` when evaluating user-defined ACL formulas, i.e. the result of `rec.foo` in such a formula. In particular this is so that `rec.some_list` loses the leading `L` type code and behaves sensibly in an expression like `thing in rec.some_list`. `InfoView.get` is called in many places, but for every usage I found other than here, leaving the cell values encoded was best. Test Plan: Added two unit server tests. The first is for the main bug involving lists. The second checks the only other plausible way I could think of that this change affects behaviour, and it seems to be for the better since both tests failed before. Most operations involving non-primitive cell values don't do anything sensible with or without decoding, so behaviour shouldn't change meaningfully in those cases. Reviewers: georgegevoian, paulfitz Reviewed By: georgegevoian, paulfitz Subscribers: paulfitz Differential Revision: https://phab.getgrist.com/D4123
This commit is contained in:
		
							parent
							
								
									bcb9740d89
								
							
						
					
					
						commit
						887717bb15
					
				| @ -11,6 +11,7 @@ | ||||
| import {CellValue} from 'app/common/DocActions'; | ||||
| import {ErrorWithCode} from 'app/common/ErrorWithCode'; | ||||
| import {AclMatchFunc, AclMatchInput, ParsedAclFormula} from 'app/common/GranularAccessClause'; | ||||
| import {decodeObject} from "app/plugin/objtypes"; | ||||
| import constant = require('lodash/constant'); | ||||
| 
 | ||||
| const GRIST_CONSTANTS: Record<string, string> = { | ||||
| @ -94,7 +95,7 @@ function getAttr(value: any, attrName: string, valueNode: ParsedAclFormula): any | ||||
|     } | ||||
|     throw new Error(`No value for '${describeNode(valueNode)}'`); | ||||
|   } | ||||
|   return (typeof value.get === 'function' ? value.get(attrName) : // InfoView
 | ||||
|   return (typeof value.get === 'function' ? decodeObject(value.get(attrName)) : // InfoView
 | ||||
|           value[attrName]); | ||||
| } | ||||
| 
 | ||||
|  | ||||
| @ -1,5 +1,6 @@ | ||||
| import {CellValue} from 'app/common/DocActions'; | ||||
| import {AclMatchFunc, InfoView} from 'app/common/GranularAccessClause'; | ||||
| import {GristObjCode} from 'app/plugin/GristData'; | ||||
| import {compileAclFormula} from 'app/server/lib/ACLFormula'; | ||||
| import {makeExceptionalDocSession} from 'app/server/lib/DocSession'; | ||||
| import {User} from 'app/server/lib/GranularAccess'; | ||||
| @ -193,4 +194,48 @@ describe('ACLFormula', function() { | ||||
|     assert.equal(compiled({user, rec: V({emails: null})}), true); | ||||
|     assert.equal(compiled({user, rec: V({emails: 'X@'})}), false); | ||||
|   }); | ||||
| 
 | ||||
|   it('should decode cell values so that "in" is safe to use with lists', async function () { | ||||
|     const user = new User({Email: 'L'}); | ||||
| 
 | ||||
|     // A previous bug meant that the above user would always pass this formula,
 | ||||
|     // because an encoded list always starts with the 'L' type code,
 | ||||
|     // and encoded cell values were used in evaluating formulas.
 | ||||
|     let compiled = await setAndCompile('user.Email in rec.emails'); | ||||
|     assert.equal(compiled({user, rec: V({emails: [GristObjCode.List]})}), false); | ||||
|     assert.equal(compiled({user, rec: V({emails: [GristObjCode.List, "X"]})}), false); | ||||
|     assert.equal(compiled({user, rec: V({emails: [GristObjCode.List, "L"]})}), true); | ||||
| 
 | ||||
|     // This should never happen (nothing should be encoded as an empty list),
 | ||||
|     // this just shows what would happen.
 | ||||
|     assert.throws(() => compiled({user, rec: V({emails: [] as any})}), | ||||
|       /\.includes is not a function/); | ||||
| 
 | ||||
|     // List literals aren't decoded and work as expected.
 | ||||
|     compiled = await setAndCompile('user.Email in []'); | ||||
|     assert.equal(compiled({user, rec: V({})}), false); | ||||
| 
 | ||||
|     compiled = await setAndCompile('user.Email in ["X"]'); | ||||
|     assert.equal(compiled({user, rec: V({})}), false); | ||||
| 
 | ||||
|     compiled = await setAndCompile('user.Email in ["L"]'); | ||||
|     assert.equal(compiled({user, rec: V({})}), true); | ||||
|   }); | ||||
| 
 | ||||
|   it('should allow comparing dates', async function () { | ||||
|     const user = new User({}); | ||||
| 
 | ||||
|     const compiled = await setAndCompile('rec.date1 < rec.date2'); | ||||
|     for (let i = 0; i < 150; i++) { | ||||
|       const date1 = i * 10000000000; | ||||
|       for (let j = 0; j < 150; j++) { | ||||
|         const date2 = j * 10000000000; | ||||
|         const rec = V({ | ||||
|           date1: [GristObjCode.Date, date1], | ||||
|           date2: [GristObjCode.Date, date2], | ||||
|         }); | ||||
|         assert.equal(compiled({user, rec}), date1 < date2); | ||||
|       } | ||||
|     } | ||||
|   }); | ||||
| }); | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user