Summary:
A user without SchemaEdit permission was able to reorder pages, since
this changes _grist_Pages, and that table was left under control of
regular access rules. This diff tightens things up, to require
SchemaEdit for all metadata edits. The one remaining exception is
_grist_Attachments, which needs some reworking to play well with
granular access.
Test Plan: extended test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D3025
Summary:
The import dialog now has an option to 'Update existing records',
which when checked will allow for selection of 1 or more fields
to match source and destination tables on.
If all fields match, then the matched record in the
destination table will be merged with the incoming record
from the source table. This means the incoming values will
replace the destination table values, unless the incoming
values are blank.
Additional merge strategies are implemented in the data
engine, but the import dialog only uses one of the
strategies currently. The others can be exposed in the UI
in the future, and tweak the behavior of how source
and destination values should be merged in different contexts,
such as when blank values exist.
Test Plan: Python and browser tests.
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: alexmojaki
Differential Revision: https://phab.getgrist.com/D3020
Summary: This adds a dropdown to the document settings model in staging/dev to set the python engine to Python2 or Python3. The setting is saved in `_grist_DocInfo.documentSettings.engine`.
Test Plan: tested manually for now - separate diff needed to add runsc to jenkins setup and make this testable
Reviewers: dsagal, alexmojaki
Reviewed By: alexmojaki
Differential Revision: https://phab.getgrist.com/D3014
Summary: This tests site deletion with and without a plan.
Test Plan: adding tests
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D3017
Summary:
This adds a `site:delete` target to `cli.sh` for deleting sites. Sites should be specified by numeric org id, and for confirmation their name also needs to be given.
All the docs in the site are deleted permanently, and the workspaces, and the site, and the stripe customer (if any).
Test Plan: manual
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D3015
Summary:
Moves CSV and XLSX export urls under /download/, and
removes the document title query parameter which is now
retrieved from the backend.
Test Plan: No new tests. Existing tests that verify endpoints still function.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3010
Summary:
The endpoints for exporting CSV and Excel are now under
/api/docs/:docId/ and are forwarded to a doc worker for export.
The Share Menu has been updated to use the new endpoints.
Test Plan: No new tests. Existing tests that verify endpoints work correctly.
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3007
Summary:
- Grist document has a associated "locale" setting that affects how currency is formatted.
- Currency selector for number format.
Test Plan: not done
Reviewers: dsagal
Reviewed By: dsagal
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D2977
Summary:
This adds `runsc` and `python3` to the grist-server images. For deployments with GRIST_EXPERIMENTAL_PLUGINS=1 (dev + staging but not prod) a hack is added to use `python3` under `runsc` for documents with a special title (`activate-python3-magic` or similar).
This will simplify experiments on behavior of this configuration under realistic conditions.
Hopefully, before landing this, I'll be able to switch to storing a python flag in a document options cell being added by @georgegevoian in a parallel diff, since using the doc title is super hacky :-).
Test Plan: tested manually on worker built locally
Reviewers: dsagal, alexmojaki
Reviewed By: dsagal, alexmojaki
Subscribers: georgegevoian
Differential Revision: https://phab.getgrist.com/D2998
Summary:
Currently actions blocked early because they could modify the
schema (e.g. changing formulas) do not report memo information
(comments in relevant rules). This diff fixes that by using
more of the same code path in the two situations. It also
adds information about what type of action was blocked to
error messages.
Test Plan: extended a test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2995
Summary:
Currently, if access rules are set to allow edits unconditionally,
and an owner does "View As" a user who is a viewer only, they will
be allowed to make edits. This catches that condition and adds a
test.
Test Plan: added test
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D2991
Summary:
Decoding large actions is a plausible culprit for hogging CPU time for
certain documents. To begin with, log the time taken for this operation,
so that we can tell if it's a problem in practice.
Test Plan: Should not affect any current behaviors
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2989
Summary: This is a documentation update, and version bump on grist-core.
Test Plan: No code changes.
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2982
Summary:
* Remove adjustSession hack, interfering with loading docs under saml.
* Allow the anonymous user to receive an empty list of workspaces for
the merged org.
* Behave better on first page load when org is in path - this used to
fail because of lack of cookie. This is very visible in grist-core,
as a failure to load localhost:8484 on first visit.
* Mark cookie explicitly as SameSite=Lax to remove a warning in firefox.
* Make errorPages available in grist-core.
This changes the default behavior of grist-core to now start off in
anonymous mode, with an explicit sign-in step available. If SAML is not configured,
the sign-in operation will unconditionally sign the user in as a default
user, without any password check or other security. The user email is
taken from GRIST_DEFAULT_EMAIL if set. This is a significant change, but
makes anonymous mode available in grist-core (which is convenient
for testing) and makes behavior with and without SAML much more consistent.
Test Plan: updated test; manual (time to start adding grist-core tests though!)
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2980
Summary:
SAML support had broken due to SameSite changes in browsers. This
makes it work again, and tests it against Auth0 (now owned by Okta).
Logging in and out works. The logged out state is confusing, and may
not be complete. The "Add Account" menu item doesn't work.
But with this, an important part of self-hosting becomes easier.
SAML support works also in grist-core, for site pages, but there
is a glitch on document pages that I'll look into separately.
Test Plan: tested manually
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2976
Summary:
Applies simple data transformations to the existing /data API.
Mimics the Airtable API. Designed in https://grist.quip.com/RZh9AEbPaj8x/Doc-API#FZfACAAZ9a0
Haven't done deletion because it seems like less of a priority and also not fully designed.
Test Plan: Added basic server tests similar to the /data tests. Haven't tested edge cases like bad input.
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D2974
Summary:
Prefix keys of `LinkingState.filterColValues` with `_contains:` when the source column is a ChoiceList or ReferenceList.
This is parsed out to make a boolean `isContainsFilter` which is kept in each value of `QueryRefs.filterTuples` (previously `filterPairs`).
Then when converting back in `convertQueryFromRefs` we construct `Query.contains: {[colId: string]: boolean}`.
Finally `getFilterFunc` uses `Query.contains` to decide what kind of filtering to do.
This is not pretty, but the existing code is already very complex and it was hard to find something that wouldn't require touching loads of code just to make things compile.
Test Plan: Added a new nbrowser test and fixture, tests that selecting a source table by summary tables grouped by a choicelist column, non-list column, and both all filter the correct data.
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2940
Summary:
Move all the plugins python code into the main folder with the core code.
Register file importing functions in the same main.py entrypoint as the data engine.
Remove options relating to different entrypoints and code directories. The only remaining plugin-specific option in NSandbox is the import directory/mount, i.e. where files to be parsed are placed.
Test Plan: this
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D2965
Summary:
Exceptional sessions had lost full read access to documents; this
restores it. Exceptional sessions are used for system actions or
while creating documents.
Test Plan: added test
Reviewers: alexmojaki
Reviewed By: alexmojaki
Differential Revision: https://phab.getgrist.com/D2966
Summary: Importing from google drive from home screen (also for anonymous users)
Test Plan: Browser tests
Reviewers: dsagal, paulfitz
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2943
Summary:
When importing from url, user types a url for google spreadsheet,
Grist will switch to Google Drive plugin to allow user to choose file manualy.
Test Plan: Browser tests
Reviewers: paulfitz, dsagal
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2945
Summary: Deletes code which was previously only used by SharedSharing.ts, which was deleted in D2894
Test Plan: no
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2960
Summary:
This adds python2 to the gvisor sandbox image. It can be used instead
of the default python3 by setting PYTHON_VERSION to 2 (or calling run.py with python2).
This is useful for making side-by-side comparisons with code running python3.
Test Plan: manual
Reviewers: alexmojaki
Reviewed By: alexmojaki
Differential Revision: https://phab.getgrist.com/D2957
Summary: Improving error messages that get returned from "Import from URL" plugin.
Test Plan: browser tests
Reviewers: alexmojaki
Reviewed By: alexmojaki
Subscribers: dsagal, alexmojaki
Differential Revision: https://phab.getgrist.com/D2946
Summary:
Processing these calls in the client, rather than passing them on
to the backend, means that access rules are more straightforward to
apply.
An unrelated fix is included to filter _grist_ tables when fetched
individually - metadata could leak through this path.
Test Plan: added tests
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2954
Summary:
* Moves essential plugins to grist-core, so that basic imports (e.g. csv) work.
* Adds support for a `GRIST_SANDBOX_FLAVOR` flag that can systematically override how the data engine is run.
- `GRIST_SANDBOX_FLAVOR=pynbox` is "classic" nacl-based sandbox.
- `GRIST_SANDBOX_FLAVOR=docker` runs engines in individual docker containers. It requires an image specified in `sandbox/docker` (alternative images can be named with `GRIST_SANDBOX` flag - need to contain python and engine requirements). It is a simple reference implementation for sandboxing.
- `GRIST_SANDBOX_FLAVOR=unsandboxed` runs whatever local version of python is specified by a `GRIST_SANDBOX` flag directly, with no sandboxing. Engine requirements must be installed, so an absolute path to a python executable in a virtualenv is easiest to manage.
- `GRIST_SANDBOX_FLAVOR=gvisor` runs the data engine via gvisor's runsc. Experimental, with implementation not included in grist-core. Since gvisor runs on Linux only, this flavor supports wrapping the sandboxes in a single shared docker container.
* Tweaks some recent express query parameter code to work in grist-core, which has a slightly different version of express (smoke test doesn't catch this since in Jenkins core is built within a workspace that has node_modules, and wires get crossed - in a dev environment the problem on master can be seen by doing `buildtools/build_core.sh /tmp/any_path_outside_grist`).
The new sandbox options do not have tests yet, nor does this they change the behavior of grist servers today. They are there to clean up and consolidate a collection of patches I've been using that were getting cumbersome, and make it easier to run experiments.
I haven't looked closely at imports beyond core.
Test Plan: tested manually against regular grist and grist-core, including imports
Reviewers: alexmojaki, dsagal
Reviewed By: alexmojaki
Differential Revision: https://phab.getgrist.com/D2942
Summary:
This makes it possible to set the type of a column to ReferenceList, but the UI is terrible
ReferenceList.ts is a mishmash of ChoiceList and Reference that sort of works but something about the CSS is clearly broken
ReferenceListEditor is just a text editor, you have to type in a JSON array of row IDs. Ignore the value that's present when you start editing. I can maybe try mashing together ReferenceEditor and ChoiceListEditor but it doesn't seem wise.
I think @georgegevoian should take over here. Reviewing the diff as it is to check for obvious issues is probably good but I don't think it's worth trying to land/merge anything.
Test Plan: none
Reviewers: dsagal
Reviewed By: dsagal
Subscribers: georgegevoian
Differential Revision: https://phab.getgrist.com/D2914
Summary:
Fixing two bugs
- Google Auth Endpoint wasn't resolving protocol in a correct way
- Google Auth Popup was navigationg to endpoint url based on home url, which
was diffent from current page origin
Test Plan: n/a
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D2937
Summary:
This tweaks pre-fork mode to make the user's experience a bit more seamless.
Pre-fork mode is where the user has opened a document with intent to
fork it, but actual forking (with allocation of a new document id)
is postponed until they make their first change.
The tweak makes the user an owner for granular access purposes, if
forking is permitted. So data visible only to owners because of
access rules will be visible to them. As always, any edits would
go to a separate new copy.
A remaining tricky corner is what to do about "View As" functionality
on forks. Fork sharing cannot be controlled, so the "Users -> View As"
functionality isn't available. Perhaps the "Users" button on a fork
could encourage doing a save-copy and inviting users, or offer some
dummy users? In any case, this diff doesn't change anything with
that corner.
Test Plan: added test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2931
Summary:
Google Auth popup wasn't able to resolve origin from gristConfig.
Moving this reponsability to server side, where it gets calculated from initial request.
Test Plan: n/a
Reviewers: dsagal, paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2935
Summary:
Implementing export to excel and send to Google Drive feature.
As part of this feature few things were implemented:
- Server side google authentication exposed on url: (docs, docs-s, or localhost:8080)/auth/google
- Exporting grist documents as an excel file (xlsx)
- Storing exported grist document (in excel format) in Google Drive as a spreadsheet document.
Server side google authentication requires one new environmental variables
- GOOGLE_CLIENT_SECRET (required) used by authentication handler
Test Plan: Browser tests for exporting to excel.
Reviewers: paulfitz, dsagal
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2924
Summary:
Bundles some new document options into a JSON column.
The icon option is treated somewhat gingerly. It is intended, at
least initially, to store an image thumbnail for a document as a
url to hand-prepared assets (for examples and templates), so it is
locked down to a particular url prefix to avoid opening the door to
mischief.
Test Plan: added test
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D2916
Summary:
The 'user' variable has a similar API to the one from access rules: it
contains properties about a user, such as their full name and email
address, as well as optional, user-defined attributes that are populated
via user attribute tables.
Test Plan: Python unit tests.
Reviewers: alexmojaki, paulfitz, dsagal
Reviewed By: alexmojaki, dsagal
Subscribers: paulfitz, dsagal, alexmojaki
Differential Revision: https://phab.getgrist.com/D2898
Summary:
When redirecting to login, it's important to have a valid session set. This was
done by middleware that only applies to home pages. We need to set session to
live when redirecting in case of doc pages too.
Test Plan: Added a test case for fixed behavior by applying an existing case to doc pages too
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2915
Summary:
This applies some mitigations suggested by SQLite authors when
opening untrusted SQLite databases, as we do when Grist docs
are uploaded by the user. See:
https://www.sqlite.org/security.html#untrusted_sqlite_database_files
Steps implemented in this diff are:
* Setting `trusted_schema` to off
* Running a SQLite-level integrity check on uploads
Other steps will require updates to our node-sqlite3 fork, since they
are not available via the node-sqlite3 api (one more reason to migrate
to better-sqlite3).
I haven't yet managed to create a file that triggers an integrity
check failure without also being detected as corruption by sqlite
at a more basic level, so that is a TODO for testing.
Test Plan:
existing tests pass; need to come up with exploits to
actually test the defences and have not yet
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2909
Summary:
Finishing implementation for google drive plugin.
- Refactoring plugin code to make it more robust and to follow grist ux
- Changing the way server hosts untrusted user content, from different domain to different port
Test Plan: Browser tests
Reviewers: dsagal, paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2881
Summary:
API signature for autocomplete updated to add column ID, which is
necessary for exposing correct types for 'value'.
Test Plan: Unit tests.
Reviewers: alexmojaki
Reviewed By: alexmojaki
Subscribers: jarek, alexmojaki
Differential Revision: https://phab.getgrist.com/D2896
Summary:
In the past, Cognito sign-ins were intended to give authorization to some AWS
services (like SQS); various tokens were stored in the session for this
purpose. This is no longer used. Profiles from Cognito now serve a limited
purpose: first-time initialization of name and picture, and keeping track of
which login method was used. For these remaining needs, ScopedSession is
sufficient.
Test Plan:
Existing test pass. Tested manually that logins work with Google and
Email + Password. Tested manually that on a clean database, name and picture
are picked up from a Google Login.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2907
Test Plan: Checked manually for a long-opening document that the time reported is correct.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2906
Summary:
This switches to using stdin/stdout for RPC calls to the sandbox, rather than specially allocated side channels. Plain text error information remains on stderr.
The motivation for the change is to simplify use of sandboxes, some of which support extra file descriptors and some of which don't.
The new style of communication is made the default, but I'm not committed to this, just that it be easy to switch to if needed. It is possible I'll need to switch the communication method again in the near future.
One reason not to make this default would be windows support, which is likely broken since stdin/stdout are by default in text mode.
Test Plan: existing tests pass
Reviewers: dsagal, alexmojaki
Reviewed By: dsagal, alexmojaki
Differential Revision: https://phab.getgrist.com/D2897
Summary: Removed test/aws/, most of app/server/lib/, 3 dirs in app/lambda/, corresponding tests, and more!
Test Plan: a lot of this is quite the opposite...
Reviewers: dsagal, paulfitz
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2894
Summary:
Run JS with a value for SANDBOX_BUFFERS_DIR, then run test_replay in python with the same value to replay just the python code.
See test_replay.py for more info.
Test Plan:
Record some data, e.g. `SANDBOX_BUFFERS_DIR=manual npm start` or `SANDBOX_BUFFERS_DIR=server ./test/testrun.sh server`.
Then run `SANDBOX_BUFFERS_DIR=server python -m unittest test_replay` from within `core/sandbox/grist` to replay the input from the JS.
Sample of the output will look like this:
```
Checking /tmp/sandbox_buffers/server/2021-06-16T15:13:59.958Z
True
Checking /tmp/sandbox_buffers/server/2021-06-16T15:16:37.170Z
True
Checking /tmp/sandbox_buffers/server/2021-06-16T15:14:22.378Z
True
```
Reviewers: paulfitz, dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2866
Summary:
Current appsumo sign-up flow doesn't reach the billing pages.
This diff nudges user on through that extra step.
It also tweaks plan summaries to say what special appsumo
features are in effect (member count prepaid for).
Test Plan: manual
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2882
Summary:
This adds a new landing page for cognito sign-up, intended for
use by new appsumo users.
Their email address is pre-filled and locked down, and sign-up
is by entering a password.
The page is very crude compared to hosted cognito - especially
in error reporting! - but having the address filled in more
than makes up for that.
The flow does not quite connect with the new billing signup.
I think we can do that through the regular "welcome" process,
which will list the user's team site. When the user visits
that site, we could detect that we are on a site with no
domain set yet and for which the user is a billing manager,
and trigger a visit to the appropriate billing page.
Test Plan: manual - hard to test through cognito email step
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2880
Test Plan: Enhanced the test case for memos to check these cases too (fails without this fix).
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2876
Summary:
This adds appsumo /token and /notification endpoints, with some
tests. The stub implementation is sufficient for AppSumo
activation to succeed (when exposed via port forwarding for testing).
It needs fleshing out:
* Implement upgrade/downgrade/refund and stripe subscription.
* Implement custom landing page and flow.
Test Plan: added tests
Reviewers: dsagal, georgegevoian
Reviewed By: dsagal
Subscribers: alexmojaki
Differential Revision: https://phab.getgrist.com/D2864
Summary:
Replaces https://phab.getgrist.com/D2854
Refactoring of NSandbox:
- Simplify arguments to NSandbox.spawn. Only half the arguments were used depending on the flavour, adding a layer of confusion.
- Ensure the same environment variables are passed to both flavours of sandbox
- Simplify passing down environment variables.
Implement deterministic mode with libfaketime and a seeded random instance.
- Include static prebuilt libfaketime.so.1, may need another solution in future for other platforms.
Recording pycalls:
- Add script recordDocumentPyCalls.js to open a single document outside of tests.
- Refactor out recordPyCalls.ts to support various uses.
- Add afterEach hook to save all pycalls from server tests under $PYCALLS_DIR
- Make docTools usable without mocha.
- Add useLocalDoc and loadLocalDoc for loading non-fixture documents
Test Plan:
Made a document with formulas NOW() and UUID()
Compare two document openings in normal mode:
diff <(test/recordDocumentPyCalls.js samples/d4W6NrzCMNVSVD6nWgNrGC.grist /dev/stdout) \
<(test/recordDocumentPyCalls.js samples/d4W6NrzCMNVSVD6nWgNrGC.grist /dev/stdout)
Output:
< 1623407499.58132,
---
> 1623407499.60376,
1195c1195
< "B": "bd2487f6-63c9-4f02-bbbc-5c0d674a2dc6"
---
> "B": "22e1a4fd-297f-4b86-91a2-bc42cc6da4b2"
`export DETERMINISTIC_MODE=1` and repeat. diff is empty!
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2857
Test Plan: Wrote unit and browser tests that verify new behavior.
Reviewers: paulfitz, dsagal
Reviewed By: dsagal
Subscribers: alexmojaki
Differential Revision: https://phab.getgrist.com/D2855
Summary:
This is a somewhat experimental change, that will implement permitted parts of an undo if not all parts are permitted. This is in preparation for trigger columns, where it may become common for a change in a record resulting in a change to an automatic change to another that the user cannot edit directly. How to undo such an action is somewhat unclear. One option is to undo the permitted parts, and then the triggers can rerun.
The general case is a bit of a can of worms, and feels adjacent to merging/rebasing etc.
Oh: it would probably be important in general to communicate to the user that an undo was partial, but this diff doesn't do that. It would need some new plumbing.
Test Plan: added test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2839
Test Plan: Block read access to column A based on the condition rec.B == 1. Then setting B = 1 in a row makes the cell under A grey.
Reviewers: dsagal
Reviewed By: dsagal
Subscribers: paulfitz, dsagal
Differential Revision: https://phab.getgrist.com/D2828
Summary:
The `_repairWorkspaceGuests` method is slow for workspaces with large numbers of documents. It makes a query that produces a lot of rows. The query itself is tolerable, but TypeORM processing uses enough CPU to be a likely culprit in some production instability. This diff splits the query into two pieces that are logically independent, but which when combined were resulting in the number of rows being the product of the two pieces. Once split, there is also a where clause that can be applied to one of the pieces.
The purpose of the method is to add every user that a document within a workspace is shared with to a "guest" group of the workspace itself. The design of "guest" groups is not ideal, but this diff leaves the design unchanged and is intended only to speed up operation.
Made some small tweaks to the timing of a flakey test, and temporarily recreated the `samples` directory removed in a previous diff (this is currently breaking tests badly on a fresh worker without a `samples` directory lying around)
Test Plan: added test; existing tests pass
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D2844
Summary:
This cleans up a few things about SELF_HYPERLINK urls:
* Use `urlId` rather than `docId`.
* Correctly merge personal org subdomain.
* In dev environment, use clearer port number.
Test Plan: updated test
Reviewers: alexmojaki, dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2841
Summary:
Names of private tables and columns were leaking via Code View.
This plugs that leak.
Test Plan: adds test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2840
Summary:
We used tslint earlier, and on switching to eslint, some rules were not
transfered. This moves more rules over, for consistent conventions or helpful
warnings.
- Name private members with a leading underscore.
- Prefer interface over a type alias.
- Use consistent spacing around ':' in type annotations.
- Use consistent spacing around braces of code blocks.
- Use semicolons consistently at the ends of statements.
- Use braces around even one-liner blocks, like conditionals and loops.
- Warn about shadowed variables.
Test Plan: Fixed all new warnings. Should be no behavior changes in code.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2831
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
Summary: For row creations and deletions, treat `rec` and `newRec` variables as identical. This simplifies writing a single rule that controls multiple permissions.
Test Plan: added test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2812
Summary:
This diff discounts indirect changes for access control purposes. A UserAction that updates a cell A, which in turn causes changes in other dependent cells, will be considered a change to cell A for access control purposes.
The `engine.apply_user_actions` method now returns a `direct` array, with a boolean for each `stored` action, set to `true` if the action is attributed to the user or `false` if it is attributed to the engine. `GranularAccess` ignores actions attributed to the engine when checking for edit rights.
Subtleties:
* Removal of references to a removed row are considered direct changes.
* Doesn't play well with undos as yet. An action that indirectly modifies a cell the user doesn't have rights to may succeed, but it will not be reversible.
Test Plan: added tests, updated tests
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2806
Summary:
- Adds a new ChoiceList type, and widgets to view and edit it.
- Store in SQLite as a JSON string
- Support conversions between ChoiceList and other types
Test Plan: Added browser tests, and a test for how these values are stored
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2803
Summary:
When a document has an exception to allow copies,
unset that option on any copies of the document.
Test Plan: added test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2794
Summary:
Exceptional document operations (particularly `system` and `nascent`
operations) should never be denied by a granular access rule.
Test Plan: added test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2792
Summary: Access to structural tables currently depends on SchemaEdit permission. We now make an exception for owner access to _grist_ACLResources and _grist_ACLRules, giving them unconditional access. It was too easy for owners to lock themselves out of editing access rules.
Test Plan: added test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2790
Summary:
- Update rules to be more like we've had with tslint
- Switch tsserver plugin to eslint (tsserver makes for a much faster way to lint in editors)
- Apply suggested auto-fixes
- Fix all lint errors and warnings in core/, app/, test/
Test Plan: Some behavior may change subtly (e.g. added missing awaits), relying on existing tests to catch problems.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2785
Summary:
attribute ActiveDoc log messages to users regardless of whether
they were triggered via a client or directly via api
Test Plan: log messages checked manually
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2786
Summary: add user and docId to heartbeat logging
Test Plan: checked manually
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2784
Summary:
This removes some unintentional repetition of work when there are
no row-level rules (there was a missing `return`).
Test Plan: existing tests pass
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2782
Summary: This addresses a weakness in the following case: rules controlling view access for a column, with a dependency on the values of other columns. We had disabled support for such rules, since the existing implementation worked only on table loads and not on broadcast changes. This diff adds in logic to enrich broadcasts as needed, and allows such rules.
Test Plan: added test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2774
Summary:
When adding robustness to schema changes to granular access control,
a calculation of intermediate row states that was previously done
semi-intelligently on need started happening less intelligently.
This diff separates out the row state calculations from metadata
state calculations so that one can happen without the other.
Test Plan: extended a test. Also did some manual checks.
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2773
Summary:
For a long array with removals proportional to that length,
lodash/pullAt becomes slow due to doing one splice per removal.
This diff swaps in an alternate implementation that doesn't become
quadratic. On a 250k-row doc with a row-level access rule, this improves
initial page load for a viewer with access to half the rows from minutes
to seconds.
Test Plan: added test; did manual benchmarking
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2777
Summary:
A client hit a situation where a granular access control "bundle"
was not closed, leaving the document locked until reset. I don't
yet have a replication. This diff is a possible mitigation,
trusting various methods less.
Test Plan: existing tests pass
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2775
Summary:
This fixes DocStorage.fetchQuery when the number of parameters
exceeds the maximum that can be passed directly to sqlite.
In this case, parameters are now stored and used from a temporary
table.
Problem first noticed via a use of DocStorage.fetchQuery by
granular access controls. Access control should be optimized
to make fewer such queries, but that is a separate issue.
Test Plan: added tests
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2772
Summary:
* adds a smoke test to grist-core
* fixes a problem with highlight.js failing to load correctly
* skips survey for default user
* freshens docker build
Utility files in test/nbrowser are moved to core/test/nbrowser, so that gristUtils are available there. This increased the apparent size of the diff as "./" import paths needed replacing with "test/nbrowser/" paths. The utility files are untouched, except for the code to start a server - it now has a small grist-core specific conditional in it.
Test Plan: adds test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2768
Summary:
This allows `*SPECIAL:AccessRules` to give read access to the access rules to more users, and `*SPECIAL:FullCopies` to grant download/copy rights to more users.
This diff also changes forks to be owned by the user who forked them (previously they were an editor), since that feels more natural.
Test Plan: Added and updated tests.
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2760
Summary:
- Use special ACLResources of the form "*SPECIAL:<RuleType>" to represent
special document-wide rules.
- Include default rules that give Read access to these resources to Owners only.
- Add UI with a checkbox to give access to everyone instead.
- Allow expanding the UI for advanced configuration.
- These rules don't actually have any behavior yet.
Test Plan: WIP
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2764
Summary:
Previously in {{D1053}} we switched to using BLOB as the "type" for all columns, to prevent SQLite from casting data unexpectedly. This diff now returns to more meaningful types. We apply marshalling to values when being placed in a column where a cast might occur, to inhibit such casting.
The benefit is that Grist documents become easier to interact with via regular database clients/libraries, which often rely on the column type more than a purely SQLite tool would.
On column type conversion, we run all blobs in the column through a decode/encode cycle so if they no longer need to be marshalled they revert to native type. This could be optimized further, it is somewhat brute force.
Test Plan: Updated tests and reference document
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2755
Summary: this adds constants for user access roles, to facilitate autocomplete.
Test Plan: updated tests
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2761
Summary:
* Adds a `SELF_HYPERLINK()` python function, with optional keyword arguments to set a label, the page, and link parameters.
* Adds a `UUID()` python function, since using python's uuid.uuidv4 hits a problem accessing /dev/urandom in the sandbox. UUID makes no particular quality claims since it doesn't use an audited implementation. A difficult to guess code is convenient for some use cases that `SELF_HYPERLINK()` enables.
The canonical URL for a document is mutable, but older versions generally forward. So for implementation simplicity the document url is passed it on sandbox creation and remains fixed throughout the lifetime of the sandbox. This could and should be improved in future.
The URL is passed into the sandbox as a `DOC_URL` environment variable.
The code for creating the URL is factored out of `Notifier.ts`. Since the url is a function of the organization as well as the document, some rejiggering is needed to make that information available to DocManager.
On document imports, the new document is registered in the database slightly earlier now, in order to keep the procedure for constructing the URL in different starting conditions more homogeneous.
Test Plan: updated test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2759
Summary:
This particular combination of features is not built out - data will be
censored but changes to data will not. So the user will now get an error
if they try to do it. Existing rules of this kind will continue to
operate as before, and can be set via the api.
Test Plan: added test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2751
Summary: This fixes a bug where one client's access control limits could remove data from others via a cache.
Test Plan: added test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2748
Summary:
Now as the user an owner might choose to view their document as
is likely to not have access to rules, it is better to start
viewing on the default document page rather than /p/acl.
The "Access Rules" link is grayed out when in "view as" mode for
now (improvements are planned).
Test Plan: updated test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2743
Summary:
Previously, if some columns are allowed and the rest are denied,
the client could see unnecessary blank columns. This diff cleans
up column metadata censorship. It also adds a small tweak to
retain the `manualSort` column when filtering columns for a mixed
access table.
Test Plan: added tests
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2742
Summary:
- Support schema changes in the presence of non-trivial ACL rules.
- Fix update of `aclFormulaParsed` when updating formulas automatically after schema change.
- Filter private metadata in broadcasts, not just fetches. Censorship method is unchanged, just refactored.
- Allow only owners to change ACL rules.
- Force reloads if rules are changed.
- Track rule changes within bundle, for clarity during schema changes - tableId and colId changes create a muddle otherwise.
- Show or forbid pages dynamically depending on user's access to its sections. Logic unchanged, just no longer requires reload.
- Fix calculation of pre-existing rows touched by a bundle, in the presence of schema changes.
- Gray out acl page for non-owners.
Test Plan: added tests
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2734
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:
Column conversions involve changes to metadata tables such as
_grist_Tables_column. When fetched (from GranularAccess), ExpandedQuery used to
fail with 'table not found' because there is no metadata for metadata tables.
This diff limits the need for metadata in ExpandedQuery to when it's actually
needed (to implmement some formulas for on-demand tables), which no longer
interferes with GranularAccess.
Test Plan: Added a test case that reproduces the issue before the fix.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2729
Test Plan: Added a test case to tickle the bug this was causing.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2718
Summary:
The way linkId was set on actions to tie them together for undo bundling was
incorrect. This diff fixes it by moves the setting of linkIds to Sharing.ts,
which already serializes the processing of actions.
Test Plan: Added a test case for submitting actions together while bundling (which fails without this change).
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2716
Summary:
- This replaces the message "Unexpected Error / Access Denied / Report a problem" with a
one-line "Blocked by access rules".
Test Plan: Only tested manually
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2712
Summary:
This redefines `newRec` to be the state at the end of a bundle,
for the purposes of modifying a document. Updates and adds tests
for creation/updates of rows that are now more intuitive hopefully.
Test Plan: added tests
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2707
Summary:
This adds back-end support for query parameters `aclAsUser_` and
`aclAsUserId_` which, when either is present, direct Grist to
process granular access control rules from the point of view
of that user (specified by email or id respectively).
Some front end support is added, in the form of a tag that
shows up when in this mode, and a way to cancel the mode.
No friendly way to initiate the mode is offered yet.
Test Plan: added test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2704
Summary:
The goal is that those who can edit ACL rules can create or change rules for
any resource, even if the rules block their own ability to see the resource.
Test Plan: Added a browser test, and a server test for who can call the new method.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2703
Summary:
This allows a fork to be made by a user if:
* That user is an owner of the document being forked, or
* That user has full read access to the document being forked.
The bulk of the diff is reorganization of how forking is done. ActiveDoc.fork is now responsible for creating a fork, not just a docId/urlId for the fork. Since fork creation should not be limited to the doc worker hosting the trunk, a helper endpoint is added for placing the fork.
The change required sanitizing worker allocation a bit, and allowed session knowledge to be removed from HostedStorageManager.
Test Plan: Added test; existing tests pass.
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2700
Summary:
This adds endpoints that allow the support user to remove unlisted
snapshots for a document, and to remove all action history for
a document.
This does increase what the support user can do, but not in a way
that would be particularly valuable to attack. It would have some
destructive value, for removing history (removing unlisted
snapshots doesn't impact the user, by contrast).
This would simplify some maintenance operations.
Test Plan: added test for snapshots; tested states manually
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2699
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:
- Add headers to tables.
- Change styles to reduce boxes-within-boxes.
- Add validation of table and column IDs, both in UI and on server when saving rules.
- Add autocomplete for tables/columns used for UserAttribute rules.
- Add a fancy widget to set permission bits.
Test Plan: Updated browser test for new UI, added a test case for user attributes.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2695
Summary:
Currently, if a document is created by importing a file, inventory
creation is a little haphazard - it works, but triggers a
"surprise" message. This diff makes initialization of inventory
explicit, so that surprise messages shouldn't happen during
document creation.
Test Plan: manual
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2696
Summary:
This adds a snapshots/remove and states/remove endpoint, primarily
for maintenance work rather than for the end user. If some secret
gets into document history, it is useful to be able to purge it
in an orderly way.
Test Plan: added tests
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2694
Summary:
After switch to using an inventory file, old document versions were
not in fact being pruned. This corrects that and adds a test
that fails with the previous implementation.
The pruner was operating correctly, but was being applied to an
inventory list rather than s3 directly - and the inventory list
did not pass through version removals to s3.
This fix will leave a stock of undeleted versions that can
be eliminated by an external script (there are alternatives
but that seems simplest overall).
Test Plan: updated test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2693
