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
Summary:
An incorrect DocAction (as possible from an Undo of a non-last action)
could cause RemoveRecord on an already missing record. This used to
create an invalid undo, and wreak havoc when a series of DocActions
later fails and needs to be reverted.
To fix, consider RemoveRecord of a missing record to be a no-op.
Test Plan: Includes a new test case that triggers the problem.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2717
Summary:
Currently, an undo of a non-last action can leave the doc in an inconsistent
state. For example, it may remove a table, but fail to remove all columns of
it from metadata. We normally check that schema corresponds to metadata, but
stray columns were not visible to this check, and instead caused later table
additions to fail.
This diff fixes the check to fail the action that causes stray columns, and
to restore the doc to a consistent state.
Note that this only handles schema-metadata inconsistencies, but an undo of a
non-last action can easily create other surprises.
Test Plan: Added a test case that triggered inconsistency before, and now triggers a failed undo.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2715
Summary:
- Placed rule-updating functions in acl.py.
- Reset UI when rules update externally, or alert the user to reset if there
are pending local changes.
- Removed some unused and distracting bits from client-side DocModel.
A few improvements related to poor error handling:
- In case of missing DocActions (tickled by broken ACL rule handling), don't
add to confusion by attempting to process bad actions
- In case of missing attributes in ACL formulas, return undefined rather than
fail; the latter creates more problems.
- In case in invalid rules, fail rather than skip; this feels more correct now
that we have error checking and recovery option, and helps avoid invalid rules.
- Prevent saving invalid rules with an empty ACL formula.
- Fix bug with rule positions.
Test Plan: Added a python and browser test for table/column renames.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2698
Summary:
- When adding records, negative rowIds may now be specified. They'll be replaced by proper IDs.
- If these negative IDs are used in Reference columns in subsequent actions in
the same bundle of UserActions, they get replaced with the proper rowIds.
- Use this to sync ACLResources and ACLRules from UI in a single batch of UserActions.
- Resolve the TODOs in GranularAccess test, to no longer need to guess resource rowIds.
Test Plan: Added a python unittest for mapping IDs; updated browser tests.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2691
Summary:
- Fix error-handling in bundleActions(), and wait for the full bundle to complete.
(The omissions here were making it impossibly to react to errors from inside bundleActions())
- Catch problematic rules early enough to undo them, by trying out ruleCollection.update()
on updated rules before the updates are applied.
- Added checkAclFormula() call to DocComm that checks parsing and compiling
formula, and reports errors.
- In UI, prevent saving if any aclFormulas are invalid, or while waiting for the to get checked.
- Also fixed some lint errors
Test Plan: Added a test case of error reporting in ACL formulas.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2689
Summary:
- For comparing for equality of encoding, do better at approximating what's equal in JSON.
- Fix loading of "RaisedException" values so that they can match an equivalent
exception raised when the formula is re-evaluated.
Test Plan: Added another column to the test that verifies the Calculate action.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2682
Summary:
- Replace unicode strings with byte strings when decoding values in sandbox.
- Columns that rely on float values should derive from NumericColumn, so
that set() ensures that a float is stored even if loading an int.
- Parse unmarshallable values (['U']) into an object that can be encoded
back to the same value (rather than info a RaisedException).
- Compare NaN's as equal for deciding whether a change is a no-op.
Unrelated:
- Removed a tiny bit of unhelpful logging
Test Plan:
Added a test case that reproduces several causes of Calculate
discrepancies by loading various values into various types of formula columns.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2676
Summary:
This can happen after "Detach" is used on a summary table.
Includes a unittest reproducing the problem case.
Test Plan: Included unittest fails without the one-line tweak.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2672
Summary:
- Added fields to _grist_ACLRules for the new Granular ACL representation
- Include a corresponding migration.
- Added ACLPermissions module with merging PermissionSets and converting to/from string.
- Implemented parsing of ACL formulas and compiling them into JS functions.
- Add automatic parsing of ACL formulas when ACLRules are added or updated.
- Convert GranularAccess to load and interpret new-style rules.
- Convert ACL UI to load and save new-style rules.
For now, no attempt to do anything better on the server or UI side, only to
reproduce previous behavior.
Test Plan: Added unittests for new files; fixed those for existing files.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2664
Summary:
The new plans for granular access control are different and handled by
node.js. Some of the same tables will be reused, of which we never made
real use before except for expecting certain specific initial records.
This diff removes the old logic, replacing it with a stub that satisfies
the interface expected by other code.
It also removes several unused UserActions: AddUser/RemoveUser/
AddInstance/RemoveInstance.
Test Plan: Existing tests should pass.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2662
Summary:
Loading all user data to run a migration is risky (creates more than usual
memory pressure), and almost never needed (only one migration requires it).
This diff attempts to run migrations using only metadata (_grist_* tables),
but retries if the sandbox tells it that all data is needed.
The intent is for new migrations to avoid needing all data.
Test Plan: Added a somewhat contrived unittest.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2659
Summary:
- Introduce a new SQLiteDB migration, which adds DB columns for formula columns
- Newly added columns have the special ['P'] (pending) value in them
(in order to show the usual "Loading..." on the first load that triggers the migration)
- Calculated values are added to .stored/.undo fields of user actions.
- Various changes made in the sandbox to include .stored/.undo in the right order.
- OnDemand tables ignore stored formula columns, replacing them with special SQL as before
- In particular, converting to OnDemand table leaves stale values in those
columns, we should maybe clean those out.
Some tweaks on the side:
- Allow overriding chai assertion truncateThreshold with CHAI_TRUNCATE_THRESHOLD
- Rebuild python automatically in watch mode
Test Plan: Fixed various tests, updated some fixtures. Many python tests that check actions needed adjustments because actions moved from .stored to .undo. Some checks added to catch situations previously only caught in browser tests.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2645
Summary:
This is a hacky solution that unfortunately relies on internal workings of ACE
autocomplete popups. I don't see a less hacky one if we stick with ACE
autocomplete.
Test Plan: Added a test case for links to test/nbrowser/Formulas.ts
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2610
Summary:
This makes it possible to serve a table or tables only to owners.
* The _grist_ACLResources table is abused (temporarily) such that rows of the form `{colId: '~o', tableId}` are interpreted as meaning that `tableId` is private to owners.
* Many websocket and api endpoints are updated to preserve the privacy of these tables.
* In a document where some tables are private, a lot of capabilities are turned off for non-owners to avoid leaking info indirectly.
* The client is tweaked minimally, to show '-' where a page with some private material would otherwise go.
No attempt is made to protect data from private tables pulled into non-private tables via formulas.
There are some known leaks remaining:
* Changes to the schema of private tables are still broadcast to all clients (fixable).
* Non-owner may be able to access snapshots or make forks or use other corners of API (fixable).
* Changing name of table makes it public, since tableId in ACLResource is not updated (fixable).
Security will require some work, the attack surface is large.
Test Plan: added tests
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2604
Summary:
- Disable a pointless timing check in Python that occasionally fails.
- Make the test of InvalidValues more robust by waiting for calculated values to load.
- Make Snapshots test more robust by waiting for an action that precedes a URL change.
Test Plan: These tests should fail less often when the tested logic is correct.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2609
Summary:
- Make suggestions less case-sensitive (not entirely case-insensitive, but
allow top-level suggestions to match in all-lowercase)
- Add function signatures to suggestions for Grist functions.
- Excel-like functions that are present but not implemented are no longer
offered as suggestions.
Test Plan:
Added a test case on python side, and a browser test case for how suggestions
are rendered and inserted.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2608
Summary:
- Change RECORD's dates_as_str default to False.
- Reimplement objtype encode_object/decode_object with less machinery.
- Implement encoding of dicts (with string keys).
- Make lists and dicts encode values recursively.
- Implement encoding/decoding in the client
- Decode automatically in plugins' fetchSelectedTable/Record, with an option to skip.
Test Plan: Tested manually, not sure what tests may be affected yet.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2593
Summary:
- Instead of sending an "UnmarshallableError" as an exception, introduce an
"Unmarshallable" type of value, represented as ['U', repr(value)]
- Unmarshallable values are rendered using a bluish text color, no longer a
pink background.
- Factor out ErrorDom to be simpler and cleaner.
- Add GristObjCode enum, and simplify related helpers.
- Use safe_repr() for when repr() itself fails
- Handle conversion errors using safe_repr() when str() fails
Test Plan: Added a test case based on a fixture covering a bunch of cases.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2584
Summary:
Examining a Record is currently difficult, because its columns are hard to
list (and to use), and CircularRef errors hard to avoid. The RECORD function
takes care of this mess to return a simple dictionary of values.
- Supports dates_as_iso=False flag to turn off the translation of date/datetime
objects to strings.
- Supports expand_refs=True flag to apply RECORD() to encountered values of
type Record, for a single level of nesting.
Test Plan: Added a unittest for RECORD()
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2582
Test Plan: Added a test case for this, which fails without the change.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2569
Summary:
this moves sandbox/grist to core, and adds a requirements.txt
file for reconstructing the content of sandbox/thirdparty.
Test Plan:
existing tests pass.
Tested core functionality manually. Tested docker build manually.
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2563
