Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Follow-up: Cleanup tests after APP_HOME_INTERNAL_URL #16

Open
wants to merge 343 commits into
base: introduce-app-home-internal-url
Choose a base branch
from

Conversation

fflorent
Copy link
Collaborator

  • Simplifies some pieces of code
  • Re-enable tests that were missing
  • Cleanups

@fflorent fflorent force-pushed the cleanup-tests-after-app-home-internal-url branch from a8390c9 to 19463ce Compare May 14, 2024 15:18
@fflorent fflorent force-pushed the introduce-app-home-internal-url branch from ceefd82 to e1569b8 Compare May 14, 2024 15:29
@fflorent fflorent force-pushed the cleanup-tests-after-app-home-internal-url branch from 63dbd5d to 106b041 Compare May 14, 2024 16:03
@fflorent fflorent force-pushed the introduce-app-home-internal-url branch from e1569b8 to de29a6a Compare May 14, 2024 16:04
@fflorent fflorent force-pushed the cleanup-tests-after-app-home-internal-url branch from 106b041 to 03a8c4d Compare May 14, 2024 17:11
dsagal and others added 25 commits June 14, 2024 09:52
Summary:
- lookupOne/lookupRecords explain `sort_by` param better, and
  link to more detailed article.
- Incorporate a typo fix from Help Center
- Fix the omission of TASTEME never having been documented.

Test Plan: Corresponding update to Help Center can be reviewed at gristlabs/grist-help#351

Reviewers: jarek

Reviewed By: jarek

Subscribers: jarek

Differential Revision: https://phab.getgrist.com/D4269
Summary: The GRIST_DEFAULT_PRODUCT wasn't used for grist-ee, now it is respected.

Test Plan:
I've build grist-ee docker image from github and run it using our instruction (both for recreating the issue and confirming it is fixed)
```
docker run -p 8484:8484 \
  -v $PWD:/persist \
  -e GRIST_SESSION_SECRET=invent-a-secret-here \
  -e GRIST_SINGLE_ORG=cool-beans
  -it gristlabs/grist-ee
```

For grist-core I recreated/confirmed it is fixed it just by `GRIST_SINGLE_ORG=team npm start` in the core folder.
I also created some team sites using stubbed UI and confirmed that they were using the GRIST_DEFAULT_PRODUCT product.

Reviewers: paulfitz

Reviewed By: paulfitz

Subscribers: paulfitz

Differential Revision: https://phab.getgrist.com/D4271
… product if it was set be default

Summary:
After release on 2024-06-12 (1.1.15) the GRIST_DEFAULT_PRODUCT env variable wasn't respected by the
method that started the server in single org mode. In all deployments (apart from saas), the default product
used for new sites is set to `Free`, but the code that starts the server enforced `teamFree` product.

This change adds a fix routine that fixes this issue by rewriting team sites from `teamFree` product to `Free`
product only if:
- The default product is set to `Free`
- The deployment type is something other then 'saas'.

Additionally there is a test that will fail after 2024.10.01, as this fix should be removed before this date.

Test Plan: Added test

Reviewers: paulfitz

Reviewed By: paulfitz

Subscribers: paulfitz

Differential Revision: https://phab.getgrist.com/D4272
Summary:
For non-owners, the timing section of Document Settings is now disabled.
For non-editors, the "Reload" section is disabled.

Test Plan: Added a test case for timing being disabled.

Reviewers: jarek

Reviewed By: jarek

Differential Revision: https://phab.getgrist.com/D4275
…nt in its own module (gristlabs#1049)

The HomeDBManager remains the exposed class to the other parts of the code: any module under gen-server/lib/homedb like UsersManager is intended to be used solely by HomeDBManager, and in order to use their methods, an indirection has to be created to pass through HomeDBManager.
Summary:
fixSiteProducts was always called with a dry option.
This option was just added for debuging test failure, it should
have been removed.

Test Plan:
Manual.
- on grist core, prepare site with `teamFree` product
- then to recreate run the previous version as
`GRIST_SINGLE_ORG=cool-beans GRIST_DEFAULT_PRODUCT=Free npm start`
- then to confirm it is fixed, run the same command as above
Site should be changed from `teamFree` to `Free`.

Reviewers: paulfitz

Reviewed By: paulfitz

Subscribers: paulfitz

Differential Revision: https://phab.getgrist.com/D4276
This is a new entrypoint, mostly intended for Docker, so we have one
simple process controlling the main Grist process. The purpose of this
is to be able to make Grist easily restartable with a new environment.
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`.
When rewriting 1a64910, I
accidentally left a stray reference to docker-runner.mjs in there.
Since this file doesn't exist anymore, this prevents Docker builds
from happening.
Start documenting the databases including:
* document ACL and other tables
* Permissions
* Groups, secrets, and other tables

---------

Co-authored-by: jordigh <[email protected]>
We removed the boot page in 5dc4706,
but we forgot to update the README to reflect this.
Currently translated at 27.5% (368 of 1334 strings)

Translation: Grist/client
Translate-URL: https://hosted.weblate.org/projects/grist/client/sk/
Summary:
- Fixing port allocation in TestServer
- Extending logging in the Billing test
- Fixing negative rowIds support for add/remove actions
- Making FormulaEditor and CardView tests less flacky

Test Plan: Existing

Reviewers: paulfitz

Reviewed By: paulfitz

Subscribers: paulfitz, dsagal

Differential Revision: https://phab.getgrist.com/D4280
Currently translated at 100.0% (1336 of 1336 strings)

Translation: Grist/client
Translate-URL: https://hosted.weblate.org/projects/grist/client/pt_BR/
Currently translated at 100.0% (1336 of 1336 strings)

Translation: Grist/client
Translate-URL: https://hosted.weblate.org/projects/grist/client/es/
Currently translated at 100.0% (1336 of 1336 strings)

Translation: Grist/client
Translate-URL: https://hosted.weblate.org/projects/grist/client/de/
Currently translated at 29.7% (398 of 1336 strings)

Translation: Grist/client
Translate-URL: https://hosted.weblate.org/projects/grist/client/sk/
Co-authored-by: Florent FAYOLLE <[email protected]>
Follow-up of gristlabs#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.
Currently translated at 100.0% (1336 of 1336 strings)

Translation: Grist/client
Translate-URL: https://hosted.weblate.org/projects/grist/client/sl/
Currently translated at 37.1% (496 of 1336 strings)

Translation: Grist/client
Translate-URL: https://hosted.weblate.org/projects/grist/client/sk/
berhalak and others added 28 commits September 24, 2024 10:20
Summary:
- Fixing BulkRemoveRecord bug
- Rewriting copy on the `delete reverse column` dialog
- Rewriting text on the `reassign reference dialog`
- Adding tooltip that explains why 2-way references are not enabled for formula columns

Test Plan: Added tests

Reviewers: georgegevoian

Reviewed By: georgegevoian

Subscribers: paulfitz

Differential Revision: https://phab.getgrist.com/D4355
The chat endpoint we used is broadly supported across models on
OpenAI now. One more random self-hostable tool has been tested and
shown compatible with Grist's use.
Grist has needed a job queue for some time. This adds one, using
BullMQ. BullMQ however requires Redis, meaning we couldn't use
jobs for the large subset of Grist that needs to be runnable without
Redis (e.g. for use on desktop, or on simple self-hosted sites).
So simple immediate, delayed, and repeated jobs are supported also
in a crude single-process form when Redis is not available.

This code isn't ready for actual use since an important issue
remains to be worked out, specifically how to handle draining
the queue during deployments to avoid mixing versions (or - if
allowing mixed versions - thinking through any extra support needed
for the developer to avoid introducing hard-to-test code paths).
Needed to fix the build
* Add "Copy with headers" to grid cell popup.

This is what you want when you're going to paste into e.g. an email.

Tested just by manually trying copy and paste into an editor and an
email, and then again using the new variant to confirm the headers show
up.

gristlabs#1208
Summary:
The scrollbar wasn't properly positioned inside the
bounds of the main panel.

Test Plan: Manual.

Reviewers: jarek

Reviewed By: jarek

Subscribers: paulfitz, jarek

Differential Revision: https://phab.getgrist.com/D4357
Summary: The previous behavior didn't match HyperLink cells.

Test Plan: Browser test.

Reviewers: jarek

Reviewed By: jarek

Subscribers: jarek

Differential Revision: https://phab.getgrist.com/D4358
Summary:
Adds the remaining batch of audit log events, and a CLI
utility to generate documentation for installation and
site audit events.

Test Plan: Manual.

Reviewers: jarek

Reviewed By: jarek

Differential Revision: https://phab.getgrist.com/D4356
This checks whether code can successfully run in the
sandbox only when the admin panel needs to report that,
rather than at start up. This is motivated by two things:

  - The desktop app became a lot slower to open with this
    check, since it uses pyodide by default, and there's
    been no work on optimizing the pyodide sandbox load
    times (as opposed to gvisor, where a lot of work was
    done, and it is also fundamentally faster).
  - The messages logged by a test sandbox starting and
    stopping have been confusing people.

There is a case for doing the check on startup, especially
on servers, so that we can fail early. Still, that isn't
what we were doing, and we'd also like to move away from the
server refusing to start because of a problem and
towards an always-reachable admin page that reports
the nature of problems in a clearer way.
Context:
Removing an obsolete method that was fixing an issue with default site products.
Details can be found here 76d9448

Proposed solution:
Removing this method and its test.

Test plan:
not needed

gristlabs#1236
Currently translated at 96.4% (1410 of 1462 strings)

Translation: Grist/client
Translate-URL: https://hosted.weblate.org/projects/grist/client/eu/
Currently translated at 99.1% (1449 of 1462 strings)

Translation: Grist/client
Translate-URL: https://hosted.weblate.org/projects/grist/client/sl/
Currently translated at 100.0% (1462 of 1462 strings)

Translation: Grist/client
Translate-URL: https://hosted.weblate.org/projects/grist/client/sl/
## Context

Error is caused due to these steps:
- File is uploaded to Home server and attempts to import
- Import ends up in `claimDocument` in `HostedStorageManager`
- Tries to read doc metadata from DocWorkerMap, gets 'unknown' as md5 hash
- Thinks local doc is out of date and erases it.
- Downloads a non-existent file from S3, so import fails as it has no data.

## Proposed solution

This fixes it by checking for DummyDocWorker's special 'unknown' MD5, forcing an S3 check.

## Related issues

https://community.getgrist.com/t/no-metadata-for-imported-grist-document/6029/32
@fflorent fflorent force-pushed the cleanup-tests-after-app-home-internal-url branch from 579ed39 to 354e177 Compare October 8, 2024 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.