diff --git a/app/server/lib/BootProbes.ts b/app/server/lib/BootProbes.ts index adef48110b..36c3786c56 100644 --- a/app/server/lib/BootProbes.ts +++ b/app/server/lib/BootProbes.ts @@ -6,7 +6,7 @@ import { GristServer } from 'app/server/lib/GristServer'; import * as express from 'express'; import WS from 'ws'; import fetch from 'node-fetch'; -import { DEFAULT_SESSION_SECRET } from 'app/server/lib/coreCreator'; +import { DEFAULT_SESSION_SECRET } from 'app/server/lib/ICreate'; /** * Self-diagnostics useful when installing Grist. diff --git a/app/server/lib/ICreate.ts b/app/server/lib/ICreate.ts index 4b4d66eeea..6e6ed5a79b 100644 --- a/app/server/lib/ICreate.ts +++ b/app/server/lib/ICreate.ts @@ -13,6 +13,29 @@ import {createSandbox, SpawnFn} from 'app/server/lib/NSandbox'; import {SqliteVariant} from 'app/server/lib/SqliteCommon'; import {ITelemetry} from 'app/server/lib/Telemetry'; +// In the past, the session secret was used as an additional +// protection passed on to expressjs-session for security when +// generating session IDs, in order to make them less guessable. +// Quoting the upstream documentation, +// +// Using a secret that cannot be guessed will reduce the ability +// to hijack a session to only guessing the session ID (as +// determined by the genid option). +// +// https://expressjs.com/en/resources/middleware/session.html +// +// However, since this change, +// +// https://github.com/gristlabs/grist-core/commit/24ce54b586e20a260376a9e3d5b6774e3fa2b8b8#diff-d34f5357f09d96e1c2ba63495da16aad7bc4c01e7925ab1e96946eacd1edb094R121-R124 +// +// session IDs are now completely randomly generated in a cryptographically +// secure way, so there is no danger of session IDs being guessable. +// This makes the value of the session secret less important. The only +// concern is that changing the secret will invalidate existing +// sessions and force users to log in again. +export const DEFAULT_SESSION_SECRET = + 'Phoo2ag1jaiz6Moo2Iese2xoaphahbai3oNg7diemohlah0ohtae9iengafieS2Hae7quungoCi9iaPh'; + export interface ICreate { Billing(dbManager: HomeDBManager, gristConfig: GristServer): IBilling; @@ -72,6 +95,15 @@ export interface ICreateTelemetryOptions { create(dbManager: HomeDBManager, gristConfig: GristServer): ITelemetry|undefined; } +/** + * This function returns a `create` object that defines various core + * aspects of a Grist installation, such as what kind of billing or + * sandbox to use, if any. + * + * The intended use of this function is to initialise Grist with + * different settings and providers, to facilitate different editions + * such as standard, enterprise or cloud-hosted. + */ export function makeSimpleCreator(opts: { deploymentType: GristDeploymentType, sessionSecret?: string, @@ -116,11 +148,7 @@ export function makeSimpleCreator(opts: { return createSandbox(opts.sandboxFlavor || 'unsandboxed', options); }, sessionSecret() { - const secret = process.env.GRIST_SESSION_SECRET || sessionSecret; - if (!secret) { - throw new Error('need GRIST_SESSION_SECRET'); - } - return secret; + return process.env.GRIST_SESSION_SECRET || sessionSecret || DEFAULT_SESSION_SECRET; }, async configure() { for (const s of storage || []) { diff --git a/app/server/lib/coreCreator.ts b/app/server/lib/coreCreator.ts index 477c970b4d..2eda4e9f9b 100644 --- a/app/server/lib/coreCreator.ts +++ b/app/server/lib/coreCreator.ts @@ -3,14 +3,8 @@ import { checkMinIOBucket, checkMinIOExternalStorage, import { makeSimpleCreator } from 'app/server/lib/ICreate'; import { Telemetry } from 'app/server/lib/Telemetry'; -export const DEFAULT_SESSION_SECRET = - 'Phoo2ag1jaiz6Moo2Iese2xoaphahbai3oNg7diemohlah0ohtae9iengafieS2Hae7quungoCi9iaPh'; - export const makeCoreCreator = () => makeSimpleCreator({ deploymentType: 'core', - // This can and should be overridden by GRIST_SESSION_SECRET - // (or generated randomly per install, like grist-omnibus does). - sessionSecret: DEFAULT_SESSION_SECRET, storage: [ { name: 'minio',