When a document has too many requests, one may want to force a document to be reopened. However, the /force-reload endpoint may raise a 429 (TOO_MANY_REQUESTS) error, because it uses the throttled middleware.
Context
HomeDBManager lacks of direct tests, which makes hard to make rework or refactorations.
Proposed solution
Specifically here, I introduce tests which call exposed UsersManager methods directly and check their result.
Also:
I removed updateUserName which seems to me useless (updateUser does the same work)
Taking a look at the getUserByLogin methods, it appears that Typescirpt infers it returns a Promise<User|null> while in no case it may resolve a nullish value, therefore I have forced to return a Promise<User> and have changed the call sites to reflect the change.
Related issues
I make this change for then working on #870
Summary:
Before this change we would always say there are 14 days remaining,
regardless of how many actually are remaining. Let's pass around a
different `dataLimitsInfo` object that also reports the number of days
remaining.
Test Plan: Ensure the test suite passes.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D4332
Summary:
- When sandbox is down, report failing UpdateCurrentTime calls as warnings instead of errors.
- When applying system actions (such as updating current time), don't treat
them as user activity for the purpose of keeping the doc open.
Test Plan: Added a test case for the fixed behavior.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D4324
Add body in log requests.
GRIST_LOG_SKIP_HTTP is a badly named environment variable and its
expected values are confusing (to log the requests, you actually have to
set its value to "", and setting to "false" actually is equivalent to
setting to "true").
We deprecate this env variable in favor of GRIST_LOG_HTTP which is more
convenient and understandable:
- by default, its undefined, so nothing is logged;
- to enable the logs, you just have to set GRIST_LOG_HTTP=true
Also this commit removes the default value for GRIST_LOG_SKIP_HTTP,
because we don't have to set it to "true" to actually disable the
requests logging thanks to GRIST_LOG_HTTP. FlexServer now handles
the historical behavior for this deprecated variable.
---------
Co-authored-by: Jonathan Perret <j-github@jonathanperret.net>
Summary:
In the pure OSS `grist-oss` image, the `ActivationPage` module from
stubs is used, as the `ext` code is completely missing. We can easily
just always return `false` here.
In the case when the `ext` directory exists, this may mean we're in
the standard `grist` image or the `grist-ee` image. The latter is
distinguished by having `GRIST_FORCE_ENABLE_ENTERPRISE` so we check if
that's on, and hide the toggle accordingly if so.
Test Plan:
Use these changes to build the three Docker images
(`grist-oss`, `grist`, and `grist-ee`) and verify that only `grist`
shows the toggle.
Reviewers: jarek
Reviewed By: jarek
Subscribers: jarek
Differential Revision: https://phab.getgrist.com/D4321
Summary:
This includes two fixes: one to ensure that any exception from websocket
upgrade handlers are handled (by destroying the socket). A test case is
added for this.
The other is to ensure verifyClient returns false instead of failing; this
should lead to a better error to the client (Forbidden, rather than just socket
close). This is only tested manually with a curl request.
Test Plan: Added a test case for the more sensitive half of the fix.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Subscribers: georgegevoian
Differential Revision: https://phab.getgrist.com/D4323
Summary:
The name of this env var has bothered me for a little while.
Let's rename it more meaningfully.
Test Plan: No need to test, cosmetic change only.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D4320
Summary:
Since formula errors are typically obtained from the Python data engine, they
were not returning any info for errors in on-demand tables (not loaded into the
data engine). This change implements a detailed message to explain such errors,
mainly to point out that on-demand table is the reason.
Test Plan: Added a check to the OnDemand test that formula error details are shown.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D4317
* Introduces new configuration variables for OIDC:
- GRIST_OIDC_IDP_ENABLED_PROTECTIONS
- GRIST_OIDC_IDP_ACR_VALUES
- GRIST_OIDC_IDP_EXTRA_CLIENT_METADATA
* Implements all supported protections in oidc/Protections.ts
* Includes a better error page for failed OIDC logins
* Includes some other improvements, e.g. to logging, to OIDC
* Adds a large unit test for OIDCConfig
* Adds support for SERVER_NODE_OPTIONS for running tests
* Adds to documentation/develop.md info about GREP_TESTS, VERBOSE, and SERVER_NODE_OPTIONS.
The problem here is that making it this optional meant that it wasn't
supplied by [the enterprise creation
function](fb22d94878/ext/app/server/lib/create.ts (L10)).
This resulted in an odd situation where the secret was required for
the enterprise edition, even though it offers no additional security.
Without this key, the enterprise code crashes.
The requirement to supply a secret key would make a Grist instance
crash if you start in normal mode but switch to enterprise, as the
enterprise creator does not supply a default secret key.
Access control for ConvertFromColumn in the presence of access rules had previously been left as a TODO. This change allows the action when the user has schema rights. Because schema rights let you create formulas, they let you read anything, so there is currently no value in nuance here.
Summary:
A new onboarding page is now shown to all new users visiting the doc
menu for the first time. Tutorial cards on the doc menu have been
replaced with a new version that tracks completion progress, alongside
a new card that opens the orientation video.
Test Plan: Browser tests.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D4296
I need to be able to read the config at module load time, which makes
async difficult if not impossible.
This will make read config operations synchronous, which is fine. The
file is tiny and seldom read.
Summary: Path for the HomeDbManager has beed updated after merging with core.
Test Plan: Existing
Reviewers: georgegevoian
Reviewed By: georgegevoian
Subscribers: georgegevoian
Differential Revision: https://phab.getgrist.com/D4288
This adds a config file that's loaded very early on during startup.
It enables us to save/load settings from within Grist's admin panel, that affect the startup of the FlexServer.
The config file loading:
- Is type-safe,
- Validates the config file on startup
- Provides a path to upgrade to future versions.
It should be extensible from other versions of Grist (such as desktop), by overriding `getGlobalConfig` in stubs.
----
Some minor refactors needed to occur to make this possible. This includes:
- Extracting config loading into its own module (out of FlexServer).
- Cleaning up the `loadConfig` function in FlexServer into `loadLoginSystem` (which is what its main purpose was before).
Summary:
Version API endpoint wasn't logging telemetry from POST requests. The issue was in registration
order, this endpoint was registered before `expressJson` and it couldn't read json body in the handler.
Test Plan: Added new test
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D4277
Follow-up of #994. This PR revises the session ID generation logic to improve security in the absence of a secure session secret. It also adds a section in the admin panel "security" section to nag system admins when GRIST_SESSION_SECRET is not set.
Following is an excerpt from internal conversation.
TL;DR: Grist's current implementation generates semi-secure session IDs and uses a publicly known default signing key to sign them when the environment variable GRIST_SESSION_SECRET is not set. This PR generates cryptographically secure session IDs to dismiss security concerns around an insecure signing key, and encourages system admins to configure their own signing key anyway.
> The session secret is required by expressjs/session to sign its session IDs. It's designed as an extra protection against session hijacking by randomly guessing session IDs and hitting a valid one. While it is easy to encourage users to set a distinct session secret, this is unnecessary if session IDs are generated in a cryptographically secure way. As of now Grist uses version 4 UUIDs as session IDs (see app/server/lib/gristSessions.ts - it uses shortUUID.generate which invokes uuid.v4 under the hood). These contain 122 bits of entropy, technically insufficient to be considered cryptographically secure. In practice, this is never considered a real vulnerability. To compare, RSA2048 is still very commonly used in web servers, yet it only has 112 bits of security (>=128 bits = "secure", rule of thumb in cryptography). But for peace of mind I propose using crypto.getRandomValues to generate real 128-bit random values. This should render session ID signing unnecessary and hence dismiss security concerns around an insecure signing key.
This adds an endpoint for the admin user to be able to signal to a
controlling process to restart the server. This is intended for
`docker-runner.mjs`.
Summary:
CheckUpdateAPI is now storing client's installation id in a new field called 'deploymentId'.
Previously it was using installationId which is reserved (and overriden) by the home server.
Test Plan: Existing and manual
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D4268
This is a small thing, but when visiting the admin, the websocket test
doesn't send valid JSON, which the receiving endpoint expects. This
results in a harmless exception being thrown.
While this test should eventually be modified to be run from the
frontend, for now let's just make a small fix and send valid JSON in
order to avoid that JSON parsing exception.
Summary:
Dropdown conditions can now reference a `user` variable, similar to the
one available in Access Rules.
Test Plan: Browser test.
Reviewers: jarek, paulfitz
Reviewed By: jarek, paulfitz
Differential Revision: https://phab.getgrist.com/D4255
Summary:
When the ACL page was visited by an anonymous user (for a temporary doc), it
tried to load the "View as" list, which failed. Apart from example users, it was
also trying to load all users the document is shared with by checking the home db.
However, since the document is not persisted, it failed.
Test Plan: Added new test
Reviewers: Spoffy
Reviewed By: Spoffy
Subscribers: Spoffy
Differential Revision: https://phab.getgrist.com/D4257
This enables code in ext/ to be able to access it (e.g for proxying / interception).
Additionally adds getCreate() to enable future refactoring of `const create` away from being a global singleton constant.
Summary:
Adding new buttons to control the `timing` API and a way to view the results
using virtual table features.
Test Plan: Added new
Reviewers: georgegevoian
Reviewed By: georgegevoian
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D4252
