From 7a57a8c6ee04beb3c1954b8fc57af17be590d8ac Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Wed, 1 May 2024 18:17:26 -0400 Subject: [PATCH 01/17] reconcile boot and admin pages further This adds some remaining parts of the boot page to the admin panel, and then removes the boot page. --- app/client/boot.ts | 160 ------------------------------- app/client/models/AdminChecks.ts | 51 ++++++---- app/client/models/AppModel.ts | 46 ++++++++- app/client/ui/AdminPanel.ts | 156 +++++++++++++++++++++++++++--- app/common/BaseAPI.ts | 12 +++ app/common/InstallAPI.ts | 11 +++ app/server/lib/Authorizer.ts | 18 ++++ app/server/lib/BootProbes.ts | 61 +++++++++--- app/server/lib/FlexServer.ts | 48 ++++------ app/server/lib/GristServer.ts | 4 +- app/server/lib/ICreate.ts | 2 +- app/server/lib/InstallAdmin.ts | 9 +- buildtools/webpack.config.js | 1 - static/boot.html | 15 --- test/nbrowser/AdminPanel.ts | 49 +++++++++- test/nbrowser/Boot.ts | 26 +++-- 16 files changed, 400 insertions(+), 269 deletions(-) delete mode 100644 app/client/boot.ts delete mode 100644 static/boot.html diff --git a/app/client/boot.ts b/app/client/boot.ts deleted file mode 100644 index a37fd363f7..0000000000 --- a/app/client/boot.ts +++ /dev/null @@ -1,160 +0,0 @@ -import { AppModel } from 'app/client/models/AppModel'; -import { AdminChecks, ProbeDetails } from 'app/client/models/AdminChecks'; -import { createAppPage } from 'app/client/ui/createAppPage'; -import { pagePanels } from 'app/client/ui/PagePanels'; -import { BootProbeInfo, BootProbeResult } from 'app/common/BootProbe'; -import { getGristConfig } from 'app/common/urlUtils'; -import { Disposable, dom, Observable, styled, UseCBOwner } from 'grainjs'; - -const cssBody = styled('div', ` - padding: 20px; - overflow: auto; -`); - -const cssHeader = styled('div', ` - padding: 20px; -`); - -const cssResult = styled('div', ` - max-width: 500px; -`); - -/** - * - * A "boot" page for inspecting the state of the Grist installation. - * - * TODO: deferring using any localization machinery so as not - * to have to worry about its failure modes yet, but it should be - * fine as long as assets served locally are used. - * - */ -export class Boot extends Disposable { - - private _checks: AdminChecks; - - constructor(_appModel: AppModel) { - super(); - // Setting title in constructor seems to be how we are doing this, - // based on other similar pages. - document.title = 'Booting Grist'; - this._checks = new AdminChecks(this); - } - - /** - * Set up the page. Uses the generic Grist layout with an empty - * side panel, just for convenience. Could be made a lot prettier. - */ - public buildDom() { - this._checks.fetchAvailableChecks().catch(e => reportError(e)); - - const config = getGristConfig(); - const errMessage = config.errMessage; - const rootNode = dom('div', - dom.domComputed( - use => { - return pagePanels({ - leftPanel: { - panelWidth: Observable.create(this, 240), - panelOpen: Observable.create(this, false), - hideOpener: true, - header: null, - content: null, - }, - headerMain: cssHeader(dom('h1', 'Grist Boot')), - contentMain: this.buildBody(use, {errMessage}), - }); - } - ), - ); - return rootNode; - } - - /** - * The body of the page is very simple right now, basically a - * placeholder. Make a section for each probe, and kick them off in - * parallel, showing results as they come in. - */ - public buildBody(use: UseCBOwner, options: {errMessage?: string}) { - if (options.errMessage) { - return cssBody(cssResult(this.buildError())); - } - return cssBody([ - ...use(this._checks.probes).map(probe => { - const req = this._checks.requestCheck(probe); - return cssResult( - this.buildResult(req.probe, use(req.result), req.details)); - }), - ]); - } - - /** - * This is used when there is an attempt to access the boot page - * but something isn't right - either the page isn't enabled, or - * the key in the URL is wrong. Give the user some information about - * how to set things up. - */ - public buildError() { - return dom( - 'div', - dom('p', - 'A diagnostics page can be made available at:', - dom('blockquote', '/boot/GRIST_BOOT_KEY'), - 'GRIST_BOOT_KEY is an environment variable ', - ' set before Grist starts. It should only', - ' contain characters that are valid in a URL.', - ' It should be a secret, since no authentication is needed', - ' to visit the diagnostics page.'), - dom('p', - 'You are seeing this page because either the key is not set,', - ' or it is not in the URL.'), - ); - } - - /** - * An ugly rendering of information returned by the probe. - */ - public buildResult(info: BootProbeInfo, result: BootProbeResult, - details: ProbeDetails|undefined) { - const out: (HTMLElement|string|null)[] = []; - out.push(dom('h2', info.name)); - if (details) { - out.push(dom('p', '> ', details.info)); - } - if (result.verdict) { - out.push(dom('pre', result.verdict)); - } - if (result.success !== undefined) { - out.push(result.success ? '✅' : '❌'); - } - if (result.done === true) { - out.push(dom('p', 'no fault detected')); - } - if (result.details) { - for (const [key, val] of Object.entries(result.details)) { - out.push(dom( - 'div', - cssLabel(key), - dom('input', dom.prop('value', JSON.stringify(val))))); - } - } - return out; - } -} - -/** - * Create a stripped down page to show boot information. - * Make sure the API isn't used since it may well be unreachable - * due to a misconfiguration, especially in multi-server setups. - */ -createAppPage(appModel => { - return dom.create(Boot, appModel); -}, { - useApi: false, -}); - -export const cssLabel = styled('div', ` - display: inline-block; - min-width: 100px; - text-align: right; - padding-right: 5px; -`); diff --git a/app/client/models/AdminChecks.ts b/app/client/models/AdminChecks.ts index 45de41ed36..f8ee542021 100644 --- a/app/client/models/AdminChecks.ts +++ b/app/client/models/AdminChecks.ts @@ -1,5 +1,5 @@ import { BootProbeIds, BootProbeInfo, BootProbeResult } from 'app/common/BootProbe'; -import { removeTrailingSlash } from 'app/common/gutil'; +import { InstallAPI } from 'app/common/InstallAPI'; import { getGristConfig } from 'app/common/urlUtils'; import { Disposable, Observable, UseCBOwner } from 'grainjs'; @@ -19,7 +19,7 @@ export class AdminChecks { // Keep track of probe results we have received, by probe ID. private _results: Map>; - constructor(private _parent: Disposable) { + constructor(private _parent: Disposable, private _installAPI: InstallAPI) { this.probes = Observable.create(_parent, []); this._results = new Map(); this._requests = new Map(); @@ -32,18 +32,16 @@ export class AdminChecks { const config = getGristConfig(); const errMessage = config.errMessage; if (!errMessage) { - // Probe tool URLs are relative to the current URL. Don't trust configuration, - // because it may be buggy if the user is here looking at the boot page - // to figure out some problem. - // - // We have been careful to make URLs available with appropriate - // middleware relative to both of the admin panel and the boot page. - const url = new URL(removeTrailingSlash(document.location.href)); - url.pathname += '/probe'; - const resp = await fetch(url.href); - const _probes = await resp.json(); - this.probes.set(_probes.probes); + const _probes = await this._installAPI.getChecks().catch(() => undefined); + if (!this._parent.isDisposed()) { + // Currently, probes are forbidden if not admin. + // TODO: May want to relax this to allow some probes that help + // diagnose some initial auth problems. + this.probes.set(_probes ? _probes.probes : []); + } + return _probes; } + return []; } /** @@ -59,7 +57,7 @@ export class AdminChecks { } let request = this._requests.get(id); if (!request) { - request = new AdminCheckRunner(id, this._results, this._parent); + request = new AdminCheckRunner(this._installAPI, id, this._results, this._parent); this._requests.set(id, request); } request.start(); @@ -93,15 +91,15 @@ export interface AdminCheckRequest { * Manage a single check. */ export class AdminCheckRunner { - constructor(public id: string, public results: Map>, + constructor(private _installAPI: InstallAPI, + public id: string, + public results: Map>, public parent: Disposable) { - const url = new URL(removeTrailingSlash(document.location.href)); - url.pathname = url.pathname + '/probe/' + id; - fetch(url.href).then(async resp => { - const _probes: BootProbeResult = await resp.json(); + this._installAPI.runCheck(id).then(result => { + if (parent.isDisposed()) { return; } const ob = results.get(id); if (ob) { - ob.set(_probes); + ob.set(result); } }).catch(e => console.error(e)); } @@ -120,7 +118,7 @@ export class AdminCheckRunner { * but it can be useful to show extra details and tips in the * client. */ -const probeDetails: Record = { +export const probeDetails: Record = { 'boot-page': { info: ` This boot page should not be too easy to access. Either turn @@ -144,6 +142,17 @@ is set. `, }, + 'sandboxing': { + info: ` +Grist allows for very powerful formulas, using Python. +We recommend setting the environment variable +GRIST_SANDBOX_FLAVOR to gvisor if your hardware +supports it (most will), to run formulas in each document +within a sandbox isolated from other documents and isolated +from the network. +` + }, + 'system-user': { info: ` It is good practice not to run Grist as the root user. diff --git a/app/client/models/AppModel.ts b/app/client/models/AppModel.ts index 30ea479957..74839841a8 100644 --- a/app/client/models/AppModel.ts +++ b/app/client/models/AppModel.ts @@ -505,10 +505,52 @@ export function getOrgNameOrGuest(org: Organization|null, user: FullUser|null) { return getOrgName(org); } -export function getHomeUrl(): string { +/** + * If we don't know what the home URL is, the top level of the site + * we are on may work. This should always work for single-server installs + * that don't encode organization information in domains. Even for other + * cases, this should be a good enough home URL for many purposes, it + * just may still have some organization information encoded in it from + * the domain that could influence results that might be supposed to be + * organization-neutral. + */ +export function getFallbackHomeUrl(): string { const {host, protocol} = window.location; + return `${protocol}//${host}`; +} + +/** + * Get the official home URL sent to us from the back end. + */ +export function getConfiguredHomeUrl(): string { const gristConfig: any = (window as any).gristConfig; - return (gristConfig && gristConfig.homeUrl) || `${protocol}//${host}`; + return (gristConfig && gristConfig.homeUrl) || getFallbackHomeUrl(); +} + +/** + * Get the home URL, using fallback if on admin page rather + * than trusting back end configuration. + */ +export function getPreferredHomeUrl(): string|undefined { + const gristUrl = urlState().state.get(); + if (gristUrl.adminPanel) { + // On the admin panel, we should not trust configuration much, + // since we want the user to be able to access it to diagnose + // problems with configuration. So we access the API via the + // site we happen to be on rather than anything configured on + // the back end. Couldn't we just always do this? Maybe! + // It could require adjustments for calls that are meant + // to be site-neutral if the domain has an org encoded in it. + // But that's a small price to pay. Grist Labs uses a setup + // where api calls go to a dedicated domain distinct from all + // other sites, but there's no particular advantage to it. + return getFallbackHomeUrl(); + } + return getConfiguredHomeUrl(); +} + +export function getHomeUrl(): string { + return getPreferredHomeUrl() || getConfiguredHomeUrl(); } export function newUserAPIImpl(): UserAPIImpl { diff --git a/app/client/ui/AdminPanel.ts b/app/client/ui/AdminPanel.ts index 3a229714e5..ba61fd9f3e 100644 --- a/app/client/ui/AdminPanel.ts +++ b/app/client/ui/AdminPanel.ts @@ -2,8 +2,8 @@ import {buildHomeBanners} from 'app/client/components/Banners'; import {makeT} from 'app/client/lib/localization'; import {localStorageJsonObs} from 'app/client/lib/localStorageObs'; import {getTimeFromNow} from 'app/client/lib/timeUtils'; +import {AdminChecks, probeDetails, ProbeDetails} from 'app/client/models/AdminChecks'; import {AppModel, getHomeUrl, reportError} from 'app/client/models/AppModel'; -import {AdminChecks} from 'app/client/models/AdminChecks'; import {urlState} from 'app/client/models/gristUrlState'; import {AppHeader} from 'app/client/ui/AppHeader'; import {leftPanelBasic} from 'app/client/ui/LeftPanelCommon'; @@ -15,7 +15,7 @@ import {basicButton} from 'app/client/ui2018/buttons'; import {toggle} from 'app/client/ui2018/checkbox'; import {mediaSmall, testId, theme, vars} from 'app/client/ui2018/cssVars'; import {cssLink, makeLinks} from 'app/client/ui2018/links'; -import {SandboxingBootProbeDetails} from 'app/common/BootProbe'; +import {BootProbeInfo, BootProbeResult, SandboxingBootProbeDetails} from 'app/common/BootProbe'; import {commonUrls, getPageTitleSuffix} from 'app/common/gristUrls'; import {InstallAPI, InstallAPIImpl, LatestVersion} from 'app/common/InstallAPI'; import {naturalCompare} from 'app/common/SortFunc'; @@ -41,7 +41,7 @@ export class AdminPanel extends Disposable { constructor(private _appModel: AppModel) { super(); document.title = getAdminPanelName() + getPageTitleSuffix(getGristConfig()); - this._checks = new AdminChecks(this); + this._checks = new AdminChecks(this, this._installAPI); } public buildDom() { @@ -78,9 +78,38 @@ export class AdminPanel extends Disposable { } private _buildMainContent(owner: MultiHolder) { + // If probes are available, show the panel as normal. + // Otherwise say it is unavailable, and describe a fallback + // mechanism for access. return cssPageContainer( dom.cls('clipboard'), {tabIndex: "-1"}, + dom.maybe(this._checks.probes, probes => { + return probes.length > 0 + ? this._buildMainContentForAdmin(owner) + : this._buildMainContentForOthers(owner); + }), + testId('admin-panel'), + ); + } + + /** + * Show something helpful to those without access to the panel, + * which could include a legit adminstrator if auth is misconfigured. + */ + private _buildMainContentForOthers(owner: MultiHolder) { + return dom.create(AdminSection, t('Administrator Panel Unavailable'), [ + dom('p', `You do not have access to the administrator panel. +Please log in as an administrator.`), + dom('p', `Or, as a fallback, you can set:`), + dom('pre', 'GRIST_BOOT_KEY=secret'), + dom('p', ` in the environment and visit: `), + dom('pre', `/admin?key=secret`) + ]); + } + + private _buildMainContentForAdmin(owner: MultiHolder) { + return [ dom.create(AdminSection, t('Support Grist'), [ dom.create(AdminSectionItem, { id: 'telemetry', @@ -113,7 +142,6 @@ export class AdminPanel extends Disposable { expandedContent: this._buildAuthenticationNotice(owner), }) ]), - dom.create(AdminSection, t('Version'), [ dom.create(AdminSectionItem, { id: 'version', @@ -123,8 +151,23 @@ export class AdminPanel extends Disposable { }), this._buildUpdates(owner), ]), - testId('admin-panel'), - ); + dom.create(AdminSection, t('Self Checks'), [ + this._buildProbeItems(owner, { + showRedundant: false, + showNovel: true, + }), + dom.create(AdminSectionItem, { + id: 'probe-other', + name: 'more...', + description: '', + value: '', + expandedContent: this._buildProbeItems(owner, { + showRedundant: true, + showNovel: false, + }), + }), + ]), + ]; } private _buildSandboxingDisplay(owner: IDisposableOwner) { @@ -150,11 +193,9 @@ export class AdminPanel extends Disposable { private _buildSandboxingNotice() { return [ - t('Grist allows for very powerful formulas, using Python. \ -We recommend setting the environment variable GRIST_SANDBOX_FLAVOR to gvisor \ -if your hardware supports it (most will), \ -to run formulas in each document within a sandbox \ -isolated from other documents and isolated from the network.'), + // Use AdminChecks text for sandboxing, in order not to + // duplicate. + probeDetails['sandboxing'].info, dom( 'div', {style: 'margin-top: 8px'}, @@ -412,8 +453,94 @@ isolated from other documents and isolated from the network.'), ) }); } + + /** + * Show the results of various checks. Of the checks, some are considered + * "redundant" (already covered elsewhere in the Admin Panel) and the + * remainder are "novel". + */ + private _buildProbeItems(owner: MultiHolder, options: { + showRedundant: boolean, + showNovel: boolean, + }) { + return dom.domComputed( + use => [ + ...use(this._checks.probes).map(probe => { + const isRedundant = probe.id === 'sandboxing'; + const show = isRedundant ? options.showRedundant : options.showNovel; + if (!show) { return null; } + const req = this._checks.requestCheck(probe); + return this._buildProbeItem(owner, req.probe, use(req.result), req.details); + }), + ] + ); + } + + /** + * Show the result of an individual check. + */ + private _buildProbeItem(owner: MultiHolder, + info: BootProbeInfo, + result: BootProbeResult, + details: ProbeDetails|undefined) { + + const status = (result.success !== undefined) ? + (result.success ? '✅' : '❗') : '―'; + + return dom.create(AdminSectionItem, { + id: `probe-${info.id}`, + name: info.id, + description: info.name, + value: cssStatus(status), + expandedContent: [ + cssCheckHeader( + 'Results', + { style: 'margin-top: 0px; padding-top: 0px;' }, + ), + result.verdict ? dom('pre', result.verdict) : null, + (result.success === undefined) ? null : + dom('p', + result.success ? 'Check succeeded.' : 'Check failed.'), + (result.done !== true) ? null : + dom('p', 'No fault detected.'), + (details?.info === undefined) ? null : [ + cssCheckHeader('Notes'), + details.info, + ], + (result.details === undefined) ? null : [ + cssCheckHeader('Details'), + ...Object.entries(result.details).map(([key, val]) => { + return dom( + 'div', + cssLabel(key), + dom('input', dom.prop( + 'value', + typeof val === 'string' ? val : JSON.stringify(val)))); + }), + ], + ], + }); + } } +//function maybeSwitchToggle(value: Observable): DomContents { +// return toggle(value, dom.hide((use) => use(value) === null)); +//} + +// Ugh I'm not a front end person. h5 small-caps, sure why not. +// Hopefully someone with taste will edit someday! +const cssCheckHeader = styled('h5', ` + margin-bottom: 5px; + font-variant: small-caps; +`); + +const cssStatus = styled('div', ` + display: inline-block; + text-align: center; + width: 40px; + padding: 5px; +`); + const cssPageContainer = styled('div', ` overflow: auto; padding: 40px; @@ -491,3 +618,10 @@ export const cssDangerText = styled('div', ` const cssHappyText = styled('span', ` color: ${theme.controlFg}; `); + +export const cssLabel = styled('div', ` + display: inline-block; + min-width: 100px; + text-align: right; + padding-right: 5px; +`); diff --git a/app/common/BaseAPI.ts b/app/common/BaseAPI.ts index 23e8011273..cd86c24c03 100644 --- a/app/common/BaseAPI.ts +++ b/app/common/BaseAPI.ts @@ -61,6 +61,18 @@ export class BaseAPI { 'X-Requested-With': 'XMLHttpRequest', ...options.headers }; + // If we are in the client, and have a boot key query parameter, + // pass it on as a header to make it available for authentication. + // This is a fallback mechanism if auth is broken to access the + // admin panel. + // TODO: should this be more selective? + if (typeof window !== 'undefined') { + const url = new URL(window.location.href); + const bootKey = url.searchParams.get('boot'); + if (bootKey) { + this._headers['X-Boot-Key'] = bootKey; + } + } this._extraParameters = options.extraParameters; } diff --git a/app/common/InstallAPI.ts b/app/common/InstallAPI.ts index 88cc35a554..b71b1389a8 100644 --- a/app/common/InstallAPI.ts +++ b/app/common/InstallAPI.ts @@ -1,4 +1,5 @@ import {BaseAPI, IOptions} from 'app/common/BaseAPI'; +import {BootProbeInfo, BootProbeResult} from 'app/common/BootProbe'; import {InstallPrefs} from 'app/common/Install'; import {TelemetryLevel} from 'app/common/Telemetry'; import {addCurrentOrgToPath} from 'app/common/urlUtils'; @@ -56,6 +57,8 @@ export interface InstallAPI { * Returns information about latest version of Grist */ checkUpdates(): Promise; + getChecks(): Promise<{probes: BootProbeInfo[]}>; + runCheck(id: string): Promise; } export class InstallAPIImpl extends BaseAPI implements InstallAPI { @@ -78,6 +81,14 @@ export class InstallAPIImpl extends BaseAPI implements InstallAPI { return this.requestJson(`${this._url}/api/install/updates`, {method: 'GET'}); } + getChecks(): Promise<{probes: BootProbeInfo[]}> { + return this.requestJson(`${this._url}/api/probes`, {method: 'GET'}); + } + + runCheck(id: string): Promise { + return this.requestJson(`${this._url}/api/probes/${id}`, {method: 'GET'}); + } + private get _url(): string { return addCurrentOrgToPath(this._homeUrl); } diff --git a/app/server/lib/Authorizer.ts b/app/server/lib/Authorizer.ts index b8c859815b..6f9a674125 100644 --- a/app/server/lib/Authorizer.ts +++ b/app/server/lib/Authorizer.ts @@ -193,6 +193,24 @@ export async function addRequestUser( } } + // Check if we have a boot key. This is a fallback mechanism for an + // administrator to authenticate themselves by demonstrating access + // to the environment. + if (!authDone && mreq.headers && mreq.headers['x-boot-key']) { + const reqBootKey = String(mreq.headers['x-boot-key']); + const bootKey = options.gristServer.getBootKey(); + if (!bootKey || bootKey !== reqBootKey) { + return res.status(401).send('Bad request: invalid Boot key'); + } + const userId = dbManager.getSupportUserId(); + const user = await dbManager.getUser(userId); + mreq.user = user; + mreq.userId = userId; + mreq.users = [dbManager.makeFullUser(user!)]; + mreq.userIsAuthorized = true; + authDone = true; + } + // Special permission header for internal housekeeping tasks if (!authDone && mreq.headers && mreq.headers.permit) { const permitKey = String(mreq.headers.permit); diff --git a/app/server/lib/BootProbes.ts b/app/server/lib/BootProbes.ts index 954294910f..11e2d9616d 100644 --- a/app/server/lib/BootProbes.ts +++ b/app/server/lib/BootProbes.ts @@ -25,7 +25,7 @@ export class BootProbes { public addEndpoints() { // Return a list of available probes. - this._app.use(`${this._base}/probe$`, + this._app.use(`${this._base}/probes$`, ...this._middleware, expressWrap(async (_, res) => { res.json({ @@ -36,7 +36,7 @@ export class BootProbes { })); // Return result of running an individual probe. - this._app.use(`${this._base}/probe/:probeId`, + this._app.use(`${this._base}/probes/:probeId`, ...this._middleware, expressWrap(async (req, res) => { const probe = this._probeById.get(req.params.probeId); @@ -48,7 +48,7 @@ export class BootProbes { })); // Fall-back for errors. - this._app.use(`${this._base}/probe`, jsonErrorHandler); + this._app.use(`${this._base}/probes`, jsonErrorHandler); } private _addProbes() { @@ -76,21 +76,27 @@ export interface Probe { const _homeUrlReachableProbe: Probe = { id: 'reachable', - name: 'Grist is reachable', + name: 'Is home page available at expected URL', apply: async (server, req) => { const url = server.getHomeInternalUrl(); + const details: Record = { + url, + }; try { const resp = await fetch(url); + details.status = resp.status; if (resp.status !== 200) { throw new ApiError(await resp.text(), resp.status); } return { success: true, + details, }; } catch (e) { return { success: false, details: { + ...details, error: String(e), }, severity: 'fault', @@ -101,13 +107,17 @@ const _homeUrlReachableProbe: Probe = { const _statusCheckProbe: Probe = { id: 'health-check', - name: 'Built-in Health check', + name: 'Is an internal health check passing', apply: async (server, req) => { const baseUrl = server.getHomeInternalUrl(); const url = new URL(baseUrl); url.pathname = removeTrailingSlash(url.pathname) + '/status'; + const details: Record = { + url: url.href, + }; try { const resp = await fetch(url); + details.status = resp.status; if (resp.status !== 200) { throw new Error(`Failed with status ${resp.status}`); } @@ -117,11 +127,15 @@ const _statusCheckProbe: Probe = { } return { success: true, + details, }; } catch (e) { return { success: false, - error: String(e), + details: { + ...details, + error: String(e), + }, severity: 'fault', }; } @@ -130,10 +144,14 @@ const _statusCheckProbe: Probe = { const _userProbe: Probe = { id: 'system-user', - name: 'System user is sane', + name: 'Is the system user following best practice', apply: async () => { + const details = { + uid: process.getuid ? process.getuid() : 'unavailable', + }; if (process.getuid && process.getuid() === 0) { return { + details, success: false, verdict: 'User appears to be root (UID 0)', severity: 'warning', @@ -141,6 +159,7 @@ const _userProbe: Probe = { } else { return { success: true, + details, }; } }, @@ -148,14 +167,20 @@ const _userProbe: Probe = { const _bootProbe: Probe = { id: 'boot-page', - name: 'Boot page exposure', + name: 'Is the boot page adequately protected', apply: async (server) => { - if (!server.hasBoot) { - return { success: true }; + const bootKey = server.getBootKey; + const hasBoot = Boolean(bootKey); + const details: Record = { + bootKeySet: hasBoot, + }; + if (!hasBoot) { + return { success: true, details }; } - const maybeSecureEnough = String(process.env.GRIST_BOOT_KEY).length > 10; + details.bootKeyLength = bootKey.length; return { - success: maybeSecureEnough, + success: bootKey.length > 10, + details, severity: 'hmm', }; }, @@ -170,31 +195,37 @@ const _bootProbe: Probe = { */ const _hostHeaderProbe: Probe = { id: 'host-header', - name: 'Host header is sane', + name: 'Does the host header look correct', apply: async (server, req) => { const host = req.header('host'); const url = new URL(server.getHomeUrl(req)); + const details = { + homeUrlHost: url.hostname, + headerHost: host, + }; if (url.hostname === 'localhost') { return { done: true, + details, }; } if (String(url.hostname).toLowerCase() !== String(host).toLowerCase()) { return { success: false, + details, severity: 'hmm', }; } return { done: true, + details, }; }, }; - const _sandboxingProbe: Probe = { id: 'sandboxing', - name: 'Sandboxing is working', + name: 'Is document sandboxing effective', apply: async (server, req) => { const details = server.getSandboxInfo(); return { diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 0e8b28a6a2..b2666ccf35 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -181,7 +181,6 @@ export class FlexServer implements GristServer { private _getLoginSystem?: () => Promise; // Set once ready() is called private _isReady: boolean = false; - private _probes: BootProbes; private _updateManager: UpdateManager; private _sandboxInfo: SandboxInfo; @@ -558,27 +557,17 @@ export class FlexServer implements GristServer { */ public addBootPage() { if (this._check('boot')) { return; } - const bootKey = appSettings.section('boot').flag('key').readString({ - envVar: 'GRIST_BOOT_KEY' - }); - const base = `/boot/${bootKey}`; - this._probes = new BootProbes(this.app, this, base); - // Respond to /boot, /boot/, /boot/KEY, /boot/KEY/ to give - // a helpful message even if user gets KEY wrong or omits it. this.app.get('/boot(/(:bootKey/?)?)?$', async (req, res) => { - const goodKey = bootKey && req.params.bootKey === bootKey; - return this._sendAppPage(req, res, { - path: 'boot.html', status: 200, config: goodKey ? { - } : { - errMessage: 'not-the-key', - }, tag: 'boot', - }); + // Doing a good redirect is actually pretty subtle and we might + // get it wrong, so just say /boot got moved. + res.send('The /boot/key page is now /admin?boot=key'); }); - this._probes.addEndpoints(); } - public hasBoot(): boolean { - return Boolean(this._probes); + public getBootKey(): string|undefined { + return appSettings.section('boot').flag('key').readString({ + envVar: 'GRIST_BOOT_KEY' + }); } public denyRequestsIfNotReady() { @@ -1879,22 +1868,21 @@ export class FlexServer implements GristServer { const requireInstallAdmin = this.getInstallAdmin().getMiddlewareRequireAdmin(); - const adminPageMiddleware = [ - this._redirectToHostMiddleware, + // Admin endpoint needs to have very little middleware since each + // piece of middleware creates a new way to fail and leave the admin + // panel inaccessible. Generally the admin panel should report problems + // rather than failing entirely. + this.app.get('/admin', this._userIdMiddleware, expressWrap(async (req, resp) => { + return this.sendAppPage(req, resp, {path: 'app.html', status: 200, config: {}}); + })); + const adminMiddleware = [ this._userIdMiddleware, - this._redirectToLoginWithoutExceptionsMiddleware, - // In principle, it may be safe to show the Admin Panel to non-admins but let's protect it - // since it's intended for admins, and it's easier not to have to worry how it should behave - // for others. requireInstallAdmin, ]; - this.app.get('/admin', ...adminPageMiddleware, expressWrap(async (req, resp) => { - return this.sendAppPage(req, resp, {path: 'app.html', status: 200, config: {}}); - })); - const probes = new BootProbes(this.app, this, '/admin', adminPageMiddleware); + const probes = new BootProbes(this.app, this, '/api', adminMiddleware); probes.addEndpoints(); - // Restrict this endpoint to install admins too, for the same reason as the /admin page. + // Restrict this endpoint to install admins this.app.get('/api/install/prefs', requireInstallAdmin, expressWrap(async (_req, resp) => { const activation = await this._activations.current(); @@ -1922,7 +1910,7 @@ export class FlexServer implements GristServer { // GET api/checkUpdates // Retrieves the latest version of the client from Grist SAAS endpoint. - this.app.get('/api/install/updates', adminPageMiddleware, expressWrap(async (req, res) => { + this.app.get('/api/install/updates', adminMiddleware, expressWrap(async (req, res) => { // Prepare data for the telemetry that endpoint might expect. const installationId = (await this.getActivations().current()).id; const deploymentType = this.getDeploymentType(); diff --git a/app/server/lib/GristServer.ts b/app/server/lib/GristServer.ts index 57e6638912..1e92639544 100644 --- a/app/server/lib/GristServer.ts +++ b/app/server/lib/GristServer.ts @@ -65,7 +65,7 @@ export interface GristServer { getPlugins(): LocalPlugin[]; servesPlugins(): boolean; getBundledWidgets(): ICustomWidget[]; - hasBoot(): boolean; + getBootKey(): string|undefined; getSandboxInfo(): SandboxInfo|undefined; getInfo(key: string): any; } @@ -158,7 +158,7 @@ export function createDummyGristServer(): GristServer { servesPlugins() { return false; }, getPlugins() { return []; }, getBundledWidgets() { return []; }, - hasBoot() { return false; }, + getBootKey() { return undefined; }, getSandboxInfo() { return undefined; }, getInfo(key: string) { return undefined; } }; diff --git a/app/server/lib/ICreate.ts b/app/server/lib/ICreate.ts index 8d76d030ef..3ca48d0cd9 100644 --- a/app/server/lib/ICreate.ts +++ b/app/server/lib/ICreate.ts @@ -158,6 +158,6 @@ export function makeSimpleCreator(opts: { }, getSqliteVariant: opts.getSqliteVariant, getSandboxVariants: opts.getSandboxVariants, - createInstallAdmin: opts.createInstallAdmin || (async () => new SimpleInstallAdmin()), + createInstallAdmin: opts.createInstallAdmin || (async (dbManager) => new SimpleInstallAdmin(dbManager)), }; } diff --git a/app/server/lib/InstallAdmin.ts b/app/server/lib/InstallAdmin.ts index 803656aada..0a00bfa18e 100644 --- a/app/server/lib/InstallAdmin.ts +++ b/app/server/lib/InstallAdmin.ts @@ -1,4 +1,5 @@ import {ApiError} from 'app/common/ApiError'; +import {HomeDBManager} from 'app/gen-server/lib/HomeDBManager'; import {appSettings} from 'app/server/lib/AppSettings'; import {getUser, RequestWithLogin} from 'app/server/lib/Authorizer'; import {User} from 'app/gen-server/entity/User'; @@ -40,13 +41,19 @@ export abstract class InstallAdmin { } // Considers the user whose email matches GRIST_DEFAULT_EMAIL env var, if given, to be the -// installation admin. If not given, then there is no admin. +// installation admin. The support user is also accepted. +// Otherwise, there is no admin. export class SimpleInstallAdmin extends InstallAdmin { private _installAdminEmail = appSettings.section('access').flag('installAdminEmail').readString({ envVar: 'GRIST_DEFAULT_EMAIL', }); + public constructor(private _dbManager: HomeDBManager) { + super(); + } + public override async isAdminUser(user: User): Promise { + if (user.id === this._dbManager.getSupportUserId()) { return true; } return this._installAdminEmail ? (user.loginEmail === this._installAdminEmail) : false; } } diff --git a/buildtools/webpack.config.js b/buildtools/webpack.config.js index f7ecea7df0..722714b4a6 100644 --- a/buildtools/webpack.config.js +++ b/buildtools/webpack.config.js @@ -14,7 +14,6 @@ module.exports = { main: "app/client/app", errorPages: "app/client/errorMain", apiconsole: "app/client/apiconsole", - boot: "app/client/boot", billing: "app/client/billingMain", form: "app/client/formMain", // Include client test harness if it is present (it won't be in diff --git a/static/boot.html b/static/boot.html deleted file mode 100644 index 8f67607f00..0000000000 --- a/static/boot.html +++ /dev/null @@ -1,15 +0,0 @@ - - - - - - - - - - Loading...<!-- INSERT TITLE SUFFIX --> - - - - - diff --git a/test/nbrowser/AdminPanel.ts b/test/nbrowser/AdminPanel.ts index 5a78b85cd8..f048192b8f 100644 --- a/test/nbrowser/AdminPanel.ts +++ b/test/nbrowser/AdminPanel.ts @@ -31,7 +31,7 @@ describe('AdminPanel', function() { await server.restart(true); }); - it('should not be shown to non-managers', async function() { + it('should show an explanation to non-managers', async function() { session = await gu.session().user('user2').personalSite.login(); await session.loadDocMenu('/'); @@ -42,8 +42,9 @@ describe('AdminPanel', function() { // Try loading the URL directly. await driver.get(`${server.getHost()}/admin`); - assert.match(await driver.findWait('.test-error-header', 2000).getText(), /Access denied/); - assert.equal(await driver.find('.test-admin-panel').isPresent(), false); + await waitForAdminPanel(); + assert.equal(await driver.find('.test-admin-panel').isDisplayed(), true); + assert.match(await driver.find('.test-admin-panel').getText(), /Administrator Panel Unavailable/); }); it('should be shown to managers', async function() { @@ -192,6 +193,21 @@ describe('AdminPanel', function() { // useful there yet. }); + it('should show various self checks', async function() { + await driver.get(`${server.getHost()}/admin`); + await waitForAdminPanel(); + assert.equal(await driver.find('.test-admin-panel-item-name-probe-reachable').isDisplayed(), true); + await gu.waitToPass( + async () => assert.match(await driver.find('.test-admin-panel-item-value-probe-reachable').getText(), /✅/), + 3000, + ); + assert.equal(await driver.find('.test-admin-panel-item-name-probe-system-user').isDisplayed(), true); + await gu.waitToPass( + async () => assert.match(await driver.find('.test-admin-panel-item-value-probe-system-user').getText(), /✅/), + 3000, + ); + }); + const upperCheckNow = () => driver.find('.test-admin-panel-updates-upper-check-now'); const lowerCheckNow = () => driver.find('.test-admin-panel-updates-lower-check-now'); const autoCheckToggle = () => driver.find('.test-admin-panel-updates-auto-check'); @@ -313,6 +329,33 @@ describe('AdminPanel', function() { }); assert.isNotEmpty(fakeServer.payload.installationId); }); + + it('should survive APP_HOME_URL misconfiguration', async function() { + process.env.APP_HOME_URL = 'http://misconfigured.invalid'; + process.env.GRIST_BOOT_KEY = 'zig'; + await server.restart(true); + await driver.get(`${server.getHost()}/admin`); + await waitForAdminPanel(); + }); + + it('should honor GRIST_BOOT_KEY fallback', async function() { + await gu.removeLogin(); + await driver.get(`${server.getHost()}/admin`); + await waitForAdminPanel(); + assert.equal(await driver.find('.test-admin-panel').isDisplayed(), true); + assert.match(await driver.find('.test-admin-panel').getText(), /Administrator Panel Unavailable/); + + process.env.GRIST_BOOT_KEY = 'zig'; + await server.restart(true); + await driver.get(`${server.getHost()}/admin?boot=zig`); + await waitForAdminPanel(); + assert.equal(await driver.find('.test-admin-panel').isDisplayed(), true); + assert.notMatch(await driver.find('.test-admin-panel').getText(), /Administrator Panel Unavailable/); + await driver.get(`${server.getHost()}/admin?boot=zig-wrong`); + await waitForAdminPanel(); + assert.equal(await driver.find('.test-admin-panel').isDisplayed(), true); + assert.match(await driver.find('.test-admin-panel').getText(), /Administrator Panel Unavailable/); + }); }); async function assertTelemetryLevel(level: TelemetryLevel) { diff --git a/test/nbrowser/Boot.ts b/test/nbrowser/Boot.ts index 230adc8542..0529e749a9 100644 --- a/test/nbrowser/Boot.ts +++ b/test/nbrowser/Boot.ts @@ -3,6 +3,10 @@ import * as gu from 'test/nbrowser/gristUtils'; import {server, setupTestSuite} from 'test/nbrowser/testUtils'; import * as testUtils from 'test/server/testUtils'; +/** + * The boot page functionality has been merged with the Admin Panel. + * Check that it behaves as a boot page did now. + */ describe('Boot', function() { this.timeout(30000); setupTestSuite(); @@ -13,12 +17,20 @@ describe('Boot', function() { async function hasPrompt() { assert.include( - await driver.findContentWait('p', /diagnostics page/, 2000).getText(), - 'A diagnostics page can be made available'); + await driver.findContentWait('pre', /GRIST_BOOT_KEY/, 2000).getText(), + 'GRIST_BOOT_KEY=secret'); } - it('gives prompt about how to enable boot page', async function() { + it('tells user about /admin', async function() { await driver.get(`${server.getHost()}/boot`); + assert.match(await driver.getPageSource(), /\/admin/); + // Switch to a regular place to that gu.checkForErrors won't panic - + // it needs a Grist page. + await driver.get(`${server.getHost()}`); + }); + + it('gives prompt about how to enable boot page', async function() { + await driver.get(`${server.getHost()}/admin`); await hasPrompt(); }); @@ -35,18 +47,18 @@ describe('Boot', function() { }); it('gives prompt when key is missing', async function() { - await driver.get(`${server.getHost()}/boot`); + await driver.get(`${server.getHost()}/admin`); await hasPrompt(); }); it('gives prompt when key is wrong', async function() { - await driver.get(`${server.getHost()}/boot/bilbo`); + await driver.get(`${server.getHost()}/admin?boot=bilbo`); await hasPrompt(); }); it('gives page when key is right', async function() { - await driver.get(`${server.getHost()}/boot/lala`); - await driver.findContentWait('h2', /Grist is reachable/, 2000); + await driver.get(`${server.getHost()}/admin?boot=lala`); + await driver.findContentWait('div', /Is home page available/, 2000); }); }); }); From 4ee3ff9eb7fce8809049271ca9ee2468a379ab54 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Thu, 9 May 2024 17:41:24 -0400 Subject: [PATCH 02/17] add missing modifiers --- app/common/InstallAPI.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/common/InstallAPI.ts b/app/common/InstallAPI.ts index b71b1389a8..3f50db49db 100644 --- a/app/common/InstallAPI.ts +++ b/app/common/InstallAPI.ts @@ -81,11 +81,11 @@ export class InstallAPIImpl extends BaseAPI implements InstallAPI { return this.requestJson(`${this._url}/api/install/updates`, {method: 'GET'}); } - getChecks(): Promise<{probes: BootProbeInfo[]}> { + public getChecks(): Promise<{probes: BootProbeInfo[]}> { return this.requestJson(`${this._url}/api/probes`, {method: 'GET'}); } - runCheck(id: string): Promise { + public runCheck(id: string): Promise { return this.requestJson(`${this._url}/api/probes/${id}`, {method: 'GET'}); } From ae781ff0d4ee0ed81e468bb5a7609dbac143afe3 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Mon, 13 May 2024 16:25:24 -0400 Subject: [PATCH 03/17] mark some, but not all, strings for translation --- app/client/ui/AdminPanel.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/client/ui/AdminPanel.ts b/app/client/ui/AdminPanel.ts index ba61fd9f3e..ac231960d8 100644 --- a/app/client/ui/AdminPanel.ts +++ b/app/client/ui/AdminPanel.ts @@ -494,21 +494,21 @@ Please log in as an administrator.`), value: cssStatus(status), expandedContent: [ cssCheckHeader( - 'Results', + t('Results'), { style: 'margin-top: 0px; padding-top: 0px;' }, ), result.verdict ? dom('pre', result.verdict) : null, (result.success === undefined) ? null : dom('p', - result.success ? 'Check succeeded.' : 'Check failed.'), + result.success ? t('Check succeeded.') : t('Check failed.')), (result.done !== true) ? null : - dom('p', 'No fault detected.'), + dom('p', t('No fault detected.')), (details?.info === undefined) ? null : [ - cssCheckHeader('Notes'), + cssCheckHeader(t('Notes')), details.info, ], (result.details === undefined) ? null : [ - cssCheckHeader('Details'), + cssCheckHeader(t('Details')), ...Object.entries(result.details).map(([key, val]) => { return dom( 'div', From 632a2b00c5d7b0adbf99d1a2562aeacdfda6d3ac Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Mon, 13 May 2024 17:18:29 -0400 Subject: [PATCH 04/17] wait for text to appear --- test/nbrowser/Boot.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/nbrowser/Boot.ts b/test/nbrowser/Boot.ts index 0529e749a9..25e5b788c0 100644 --- a/test/nbrowser/Boot.ts +++ b/test/nbrowser/Boot.ts @@ -16,9 +16,12 @@ describe('Boot', function() { afterEach(() => gu.checkForErrors()); async function hasPrompt() { - assert.include( - await driver.findContentWait('pre', /GRIST_BOOT_KEY/, 2000).getText(), - 'GRIST_BOOT_KEY=secret'); + // There is some glitchiness to when the text appears. + gu.waitToPass(async () => { + assert.include( + await driver.findContentWait('pre', /GRIST_BOOT_KEY/, 2000).getText(), + 'GRIST_BOOT_KEY=secret'); + }, 3000); } it('tells user about /admin', async function() { From c1070fa059e06739b59678d0f2d3dcbcafa715f8 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Mon, 13 May 2024 17:36:22 -0400 Subject: [PATCH 05/17] missing await in test --- test/nbrowser/Boot.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/nbrowser/Boot.ts b/test/nbrowser/Boot.ts index 25e5b788c0..e4630220dd 100644 --- a/test/nbrowser/Boot.ts +++ b/test/nbrowser/Boot.ts @@ -18,7 +18,7 @@ describe('Boot', function() { async function hasPrompt() { // There is some glitchiness to when the text appears. gu.waitToPass(async () => { - assert.include( + await assert.include( await driver.findContentWait('pre', /GRIST_BOOT_KEY/, 2000).getText(), 'GRIST_BOOT_KEY=secret'); }, 3000); From 8645128b89a1a92e0d54ebf210ecad2b2127609a Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Mon, 13 May 2024 17:55:56 -0400 Subject: [PATCH 06/17] missing await in test, corrected --- test/nbrowser/Boot.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/nbrowser/Boot.ts b/test/nbrowser/Boot.ts index e4630220dd..ec86585bc6 100644 --- a/test/nbrowser/Boot.ts +++ b/test/nbrowser/Boot.ts @@ -17,8 +17,8 @@ describe('Boot', function() { async function hasPrompt() { // There is some glitchiness to when the text appears. - gu.waitToPass(async () => { - await assert.include( + await gu.waitToPass(async () => { + assert.include( await driver.findContentWait('pre', /GRIST_BOOT_KEY/, 2000).getText(), 'GRIST_BOOT_KEY=secret'); }, 3000); From 19f40085fa8a3fb54ff006db0a645f0e222cc1f1 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Mon, 13 May 2024 18:20:58 -0400 Subject: [PATCH 07/17] there is a situation in tests where window is defined but window.location is undefined --- app/common/BaseAPI.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/common/BaseAPI.ts b/app/common/BaseAPI.ts index cd86c24c03..0cdc9e38e7 100644 --- a/app/common/BaseAPI.ts +++ b/app/common/BaseAPI.ts @@ -66,7 +66,7 @@ export class BaseAPI { // This is a fallback mechanism if auth is broken to access the // admin panel. // TODO: should this be more selective? - if (typeof window !== 'undefined') { + if (typeof window !== 'undefined' && window.location) { const url = new URL(window.location.href); const bootKey = url.searchParams.get('boot'); if (bootKey) { From 9947eabdbce0871db09fc0c69e5942c56d049c98 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Wed, 15 May 2024 16:17:57 -0400 Subject: [PATCH 08/17] be fussier about boot key; log more problems --- app/client/models/AdminChecks.ts | 3 ++- app/client/ui/AdminPanel.ts | 18 +++++++++++++++--- app/server/lib/BootProbes.ts | 15 ++++++++++++--- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/app/client/models/AdminChecks.ts b/app/client/models/AdminChecks.ts index f8ee542021..51ff8b2bd7 100644 --- a/app/client/models/AdminChecks.ts +++ b/app/client/models/AdminChecks.ts @@ -1,3 +1,4 @@ +import { reportError } from 'app/client/models/errors'; import { BootProbeIds, BootProbeInfo, BootProbeResult } from 'app/common/BootProbe'; import { InstallAPI } from 'app/common/InstallAPI'; import { getGristConfig } from 'app/common/urlUtils'; @@ -32,7 +33,7 @@ export class AdminChecks { const config = getGristConfig(); const errMessage = config.errMessage; if (!errMessage) { - const _probes = await this._installAPI.getChecks().catch(() => undefined); + const _probes = await this._installAPI.getChecks().catch(reportError); if (!this._parent.isDisposed()) { // Currently, probes are forbidden if not admin. // TODO: May want to relax this to allow some probes that help diff --git a/app/client/ui/AdminPanel.ts b/app/client/ui/AdminPanel.ts index ac231960d8..0da4c3aa7a 100644 --- a/app/client/ui/AdminPanel.ts +++ b/app/client/ui/AdminPanel.ts @@ -484,9 +484,7 @@ Please log in as an administrator.`), result: BootProbeResult, details: ProbeDetails|undefined) { - const status = (result.success !== undefined) ? - (result.success ? '✅' : '❗') : '―'; - + const status = this._encodeSuccess(result); return dom.create(AdminSectionItem, { id: `probe-${info.id}`, name: info.id, @@ -521,6 +519,20 @@ Please log in as an administrator.`), ], }); } + + /** + * Give an icon summarizing success or failure. Factor in the + * severity of the result for failures. This is crude, the + * visualization of the results can be elaborated in future. + */ + private _encodeSuccess(result: BootProbeResult) { + if (result.success === undefined) { return '―'; } + if (result.success) { return '✅'; } + if (result.severity === 'warning') { return '❗'; } + if (result.severity === 'hmm') { return '?'; } + // remaining case is a fault. + return '❌'; + } } //function maybeSwitchToggle(value: Observable): DomContents { diff --git a/app/server/lib/BootProbes.ts b/app/server/lib/BootProbes.ts index 11e2d9616d..b1186c51a4 100644 --- a/app/server/lib/BootProbes.ts +++ b/app/server/lib/BootProbes.ts @@ -169,7 +169,7 @@ const _bootProbe: Probe = { id: 'boot-page', name: 'Is the boot page adequately protected', apply: async (server) => { - const bootKey = server.getBootKey; + const bootKey = server.getBootKey() || ''; const hasBoot = Boolean(bootKey); const details: Record = { bootKeySet: hasBoot, @@ -178,10 +178,19 @@ const _bootProbe: Probe = { return { success: true, details }; } details.bootKeyLength = bootKey.length; + if (bootKey.length < 10) { + return { + success: false, + verdict: 'Boot key length is shorter than 10.', + details, + severity: 'fault', + }; + } return { - success: bootKey.length > 10, + success: false, + verdict: 'Boot key ideally should be removed after installation.', details, - severity: 'hmm', + severity: 'warning', }; }, }; From 67e8ac477946e5941606372b7c163a27c36c46f1 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Thu, 16 May 2024 13:59:34 -0400 Subject: [PATCH 09/17] remove stray commented out function, did it come from a rebase? Don't recognize it! Co-authored-by: George Gevoian <85144792+georgegevoian@users.noreply.github.com> --- app/client/ui/AdminPanel.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/client/ui/AdminPanel.ts b/app/client/ui/AdminPanel.ts index 0da4c3aa7a..2f7241177c 100644 --- a/app/client/ui/AdminPanel.ts +++ b/app/client/ui/AdminPanel.ts @@ -535,9 +535,6 @@ Please log in as an administrator.`), } } -//function maybeSwitchToggle(value: Observable): DomContents { -// return toggle(value, dom.hide((use) => use(value) === null)); -//} // Ugh I'm not a front end person. h5 small-caps, sure why not. // Hopefully someone with taste will edit someday! From 9a7fe249e03947185718b8f1c53e1df200f9201d Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Thu, 16 May 2024 14:00:24 -0400 Subject: [PATCH 10/17] make non-admin section better formatted Co-authored-by: George Gevoian <85144792+georgegevoian@users.noreply.github.com> --- app/client/ui/AdminPanel.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/app/client/ui/AdminPanel.ts b/app/client/ui/AdminPanel.ts index 2f7241177c..eaf12e89fa 100644 --- a/app/client/ui/AdminPanel.ts +++ b/app/client/ui/AdminPanel.ts @@ -99,12 +99,9 @@ export class AdminPanel extends Disposable { */ private _buildMainContentForOthers(owner: MultiHolder) { return dom.create(AdminSection, t('Administrator Panel Unavailable'), [ - dom('p', `You do not have access to the administrator panel. -Please log in as an administrator.`), - dom('p', `Or, as a fallback, you can set:`), - dom('pre', 'GRIST_BOOT_KEY=secret'), - dom('p', ` in the environment and visit: `), - dom('pre', `/admin?key=secret`) + dom('p', t(`You do not have access to the administrator panel. +Please log in as an administrator.`)), + dom('p', t(`Or, as a fallback, you can set: {{bootKey}} in the environment and visit: {{url}}`), {bootKey: dom('pre', 'GRIST_BOOT_KEY=secret'), url: dom('pre', `/admin?key=secret`)}), ]); } From bc4a8a90951f9f448032073c584971f3d89bd862 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Thu, 16 May 2024 14:16:20 -0400 Subject: [PATCH 11/17] fix message for non-admins --- app/client/ui/AdminPanel.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/client/ui/AdminPanel.ts b/app/client/ui/AdminPanel.ts index eaf12e89fa..f13c5f37f8 100644 --- a/app/client/ui/AdminPanel.ts +++ b/app/client/ui/AdminPanel.ts @@ -101,7 +101,13 @@ export class AdminPanel extends Disposable { return dom.create(AdminSection, t('Administrator Panel Unavailable'), [ dom('p', t(`You do not have access to the administrator panel. Please log in as an administrator.`)), - dom('p', t(`Or, as a fallback, you can set: {{bootKey}} in the environment and visit: {{url}}`), {bootKey: dom('pre', 'GRIST_BOOT_KEY=secret'), url: dom('pre', `/admin?key=secret`)}), + dom( + 'p', + t(`Or, as a fallback, you can set: {{bootKey}} in the environment and visit: {{url}}`, { + bootKey: dom('pre', 'GRIST_BOOT_KEY=secret'), + url: dom('pre', `/admin?key=secret`) + }), + ), ]); } From 54257c28657133720f9d9bb1533a207d2cd7d3e9 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Thu, 16 May 2024 15:34:32 -0400 Subject: [PATCH 12/17] rebase and tweak test to handle initial UI instability --- test/nbrowser/AdminPanel.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/nbrowser/AdminPanel.ts b/test/nbrowser/AdminPanel.ts index f048192b8f..72ca0909f8 100644 --- a/test/nbrowser/AdminPanel.ts +++ b/test/nbrowser/AdminPanel.ts @@ -196,9 +196,11 @@ describe('AdminPanel', function() { it('should show various self checks', async function() { await driver.get(`${server.getHost()}/admin`); await waitForAdminPanel(); - assert.equal(await driver.find('.test-admin-panel-item-name-probe-reachable').isDisplayed(), true); await gu.waitToPass( - async () => assert.match(await driver.find('.test-admin-panel-item-value-probe-reachable').getText(), /✅/), + async () => { + assert.equal(await driver.find('.test-admin-panel-item-name-probe-reachable').isDisplayed(), true); + assert.match(await driver.find('.test-admin-panel-item-value-probe-reachable').getText(), /✅/); + }, 3000, ); assert.equal(await driver.find('.test-admin-panel-item-name-probe-system-user').isDisplayed(), true); From 088578f5414f629c1113dd106e78b2cb40037c91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Guti=C3=A9rrez=20Hermoso?= Date: Thu, 9 May 2024 16:43:01 -0400 Subject: [PATCH 13/17] debugging: add a separate start:debug-brk target --inspect-brk is useful if you want to debug something during startup --- package.json | 3 ++- sandbox/watch.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 6968344356..bbe1c5e588 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,8 @@ "repository": "git://github.com/gristlabs/grist-core.git", "scripts": { "start": "sandbox/watch.sh", - "start:debug": "NODE_INSPECT=1 sandbox/watch.sh", + "start:debug": "NODE_INSPECT=--inspect sandbox/watch.sh", + "start:debug-brk": "NODE_INSPECT=--inspect-brk sandbox/watch.sh", "install:python": "buildtools/prepare_python.sh", "install:python2": "buildtools/prepare_python2.sh", "install:python3": "buildtools/prepare_python3.sh", diff --git a/sandbox/watch.sh b/sandbox/watch.sh index 6cb7a750ac..531c97f62c 100755 --- a/sandbox/watch.sh +++ b/sandbox/watch.sh @@ -19,6 +19,6 @@ tsc --build -w --preserveWatchOutput $PROJECT & css_files="app/client/**/*.css" chokidar "${css_files}" -c "bash -O globstar -c 'cat ${css_files} > static/bundle.css'" & webpack --config $WEBPACK_CONFIG --mode development --watch & -NODE_PATH=_build:_build/stubs:_build/ext nodemon ${NODE_INSPECT:+--inspect} --delay 1 -w _build/app/server -w _build/app/common _build/stubs/app/server/server.js & +NODE_PATH=_build:_build/stubs:_build/ext nodemon ${NODE_INSPECT} --delay 1 -w _build/app/server -w _build/app/common _build/stubs/app/server/server.js & wait From 07b80b1110b2bb7dd04ff924855fa2293116fd6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Guti=C3=A9rrez=20Hermoso?= Date: Thu, 9 May 2024 16:46:25 -0400 Subject: [PATCH 14/17] BootProbes: add a websocket probe This self-test isn't perfect because we're running it from the backend instead of the frontend. Conceivably the backend might have trouble resolving its own url. Eventually we should move this test or something like it to something that executes in the frontend. --- app/client/models/AdminChecks.ts | 9 +++++++++ app/common/BootProbe.ts | 3 ++- app/server/lib/BootProbes.ts | 33 ++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/app/client/models/AdminChecks.ts b/app/client/models/AdminChecks.ts index 51ff8b2bd7..bd5a8cee49 100644 --- a/app/client/models/AdminChecks.ts +++ b/app/client/models/AdminChecks.ts @@ -163,6 +163,15 @@ It is good practice not to run Grist as the root user. 'reachable': { info: ` The main page of Grist should be available. +` + }, + + 'websockets': { + // TODO: add a link to https://support.getgrist.com/self-managed/#how-do-i-run-grist-on-a-server + info: ` +Websocket connections need HTTP 1.1 and the ability to pass a few +extra headers in order to work. Sometimes a reverse proxy can +interfere with these requirements. ` }, }; diff --git a/app/common/BootProbe.ts b/app/common/BootProbe.ts index dd86704937..0da3c859fc 100644 --- a/app/common/BootProbe.ts +++ b/app/common/BootProbe.ts @@ -7,7 +7,8 @@ export type BootProbeIds = 'host-header' | 'sandboxing' | 'system-user' | - 'authentication' + 'authentication' | + 'websockets' ; export interface BootProbeResult { diff --git a/app/server/lib/BootProbes.ts b/app/server/lib/BootProbes.ts index b1186c51a4..8145f2a2ce 100644 --- a/app/server/lib/BootProbes.ts +++ b/app/server/lib/BootProbes.ts @@ -4,6 +4,7 @@ import { removeTrailingSlash } from 'app/common/gutil'; import { expressWrap, jsonErrorHandler } from 'app/server/lib/expressWrap'; import { GristServer } from 'app/server/lib/GristServer'; import * as express from 'express'; +import WS from 'ws'; import fetch from 'node-fetch'; /** @@ -59,6 +60,7 @@ export class BootProbes { this._probes.push(_hostHeaderProbe); this._probes.push(_sandboxingProbe); this._probes.push(_authenticationProbe); + this._probes.push(_webSocketsProbe); this._probeById = new Map(this._probes.map(p => [p.id, p])); } } @@ -105,6 +107,37 @@ const _homeUrlReachableProbe: Probe = { } }; +const _webSocketsProbe: Probe = { + id: 'websockets', + name: 'Can we open a websocket with the server', + apply: async (server, req) => { + return new Promise((resolve) => { + const url = new URL(server.getHomeUrl(req)); + url.protocol = (url.protocol === 'https:') ? 'wss:' : 'ws:'; + const ws = new WS.WebSocket(url.href); + const details: Record = { + url, + }; + ws.on('open', () => { + ws.send('Just nod if you can hear me.'); + resolve({ + success: true, + details, + }); + ws.close(); + }); + ws.on('error', (ev) => { + details.error = ev.message; + resolve({ + success: false, + details, + }); + ws.close(); + }); + }); + } +}; + const _statusCheckProbe: Probe = { id: 'health-check', name: 'Is an internal health check passing', From 7383b3f8f6c0debf4b63f49f0b4f6d94555c0e0e Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Thu, 23 May 2024 14:59:58 -0400 Subject: [PATCH 15/17] use boot-key query parameter, tighten url match, put randomness in suggestions --- app/client/ui/AdminPanel.ts | 5 +++-- app/common/BaseAPI.ts | 6 +++--- app/server/lib/FlexServer.ts | 2 +- test/nbrowser/AdminPanel.ts | 4 ++-- test/nbrowser/Boot.ts | 6 +++--- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/app/client/ui/AdminPanel.ts b/app/client/ui/AdminPanel.ts index f13c5f37f8..4870344476 100644 --- a/app/client/ui/AdminPanel.ts +++ b/app/client/ui/AdminPanel.ts @@ -98,14 +98,15 @@ export class AdminPanel extends Disposable { * which could include a legit adminstrator if auth is misconfigured. */ private _buildMainContentForOthers(owner: MultiHolder) { + const exampleKey = 'example-' + window.crypto.randomUUID(); return dom.create(AdminSection, t('Administrator Panel Unavailable'), [ dom('p', t(`You do not have access to the administrator panel. Please log in as an administrator.`)), dom( 'p', t(`Or, as a fallback, you can set: {{bootKey}} in the environment and visit: {{url}}`, { - bootKey: dom('pre', 'GRIST_BOOT_KEY=secret'), - url: dom('pre', `/admin?key=secret`) + bootKey: dom('pre', `GRIST_BOOT_KEY=${exampleKey}`), + url: dom('pre', `/admin?boot-key=${exampleKey}`) }), ), ]); diff --git a/app/common/BaseAPI.ts b/app/common/BaseAPI.ts index 0cdc9e38e7..09e1ae496f 100644 --- a/app/common/BaseAPI.ts +++ b/app/common/BaseAPI.ts @@ -66,9 +66,9 @@ export class BaseAPI { // This is a fallback mechanism if auth is broken to access the // admin panel. // TODO: should this be more selective? - if (typeof window !== 'undefined' && window.location) { - const url = new URL(window.location.href); - const bootKey = url.searchParams.get('boot'); + if (typeof window !== 'undefined' && window.location && + window.location.pathname.endsWith('/admin')) { + const bootKey = new URLSearchParams(window.location.search).get('boot-key') if (bootKey) { this._headers['X-Boot-Key'] = bootKey; } diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index b2666ccf35..5f787886dd 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -560,7 +560,7 @@ export class FlexServer implements GristServer { this.app.get('/boot(/(:bootKey/?)?)?$', async (req, res) => { // Doing a good redirect is actually pretty subtle and we might // get it wrong, so just say /boot got moved. - res.send('The /boot/key page is now /admin?boot=key'); + res.send('The /boot/KEY page is now /admin?boot-key=KEY'); }); } diff --git a/test/nbrowser/AdminPanel.ts b/test/nbrowser/AdminPanel.ts index 72ca0909f8..d93de13ce6 100644 --- a/test/nbrowser/AdminPanel.ts +++ b/test/nbrowser/AdminPanel.ts @@ -349,11 +349,11 @@ describe('AdminPanel', function() { process.env.GRIST_BOOT_KEY = 'zig'; await server.restart(true); - await driver.get(`${server.getHost()}/admin?boot=zig`); + await driver.get(`${server.getHost()}/admin?boot-key=zig`); await waitForAdminPanel(); assert.equal(await driver.find('.test-admin-panel').isDisplayed(), true); assert.notMatch(await driver.find('.test-admin-panel').getText(), /Administrator Panel Unavailable/); - await driver.get(`${server.getHost()}/admin?boot=zig-wrong`); + await driver.get(`${server.getHost()}/admin?boot-key=zig-wrong`); await waitForAdminPanel(); assert.equal(await driver.find('.test-admin-panel').isDisplayed(), true); assert.match(await driver.find('.test-admin-panel').getText(), /Administrator Panel Unavailable/); diff --git a/test/nbrowser/Boot.ts b/test/nbrowser/Boot.ts index ec86585bc6..1a8f043cc8 100644 --- a/test/nbrowser/Boot.ts +++ b/test/nbrowser/Boot.ts @@ -20,7 +20,7 @@ describe('Boot', function() { await gu.waitToPass(async () => { assert.include( await driver.findContentWait('pre', /GRIST_BOOT_KEY/, 2000).getText(), - 'GRIST_BOOT_KEY=secret'); + 'GRIST_BOOT_KEY=example-'); }, 3000); } @@ -55,12 +55,12 @@ describe('Boot', function() { }); it('gives prompt when key is wrong', async function() { - await driver.get(`${server.getHost()}/admin?boot=bilbo`); + await driver.get(`${server.getHost()}/admin?boot-key=bilbo`); await hasPrompt(); }); it('gives page when key is right', async function() { - await driver.get(`${server.getHost()}/admin?boot=lala`); + await driver.get(`${server.getHost()}/admin?boot-key=lala`); await driver.findContentWait('div', /Is home page available/, 2000); }); }); From 307334e8dd239fbf373bfb564f4b9d945904f558 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Thu, 23 May 2024 15:05:21 -0400 Subject: [PATCH 16/17] add missing semicolon --- app/common/BaseAPI.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/common/BaseAPI.ts b/app/common/BaseAPI.ts index 09e1ae496f..c9062ab0e2 100644 --- a/app/common/BaseAPI.ts +++ b/app/common/BaseAPI.ts @@ -68,7 +68,7 @@ export class BaseAPI { // TODO: should this be more selective? if (typeof window !== 'undefined' && window.location && window.location.pathname.endsWith('/admin')) { - const bootKey = new URLSearchParams(window.location.search).get('boot-key') + const bootKey = new URLSearchParams(window.location.search).get('boot-key'); if (bootKey) { this._headers['X-Boot-Key'] = bootKey; } From fc4e43785e4b3f7e692fc29dd34e97f312441014 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Thu, 23 May 2024 16:24:38 -0400 Subject: [PATCH 17/17] collapse success/severity/done into status --- app/client/models/AdminChecks.ts | 4 ++-- app/client/ui/AdminPanel.ts | 32 ++++++++++++++++++--------- app/common/BootProbe.ts | 9 +++++--- app/server/lib/BootProbes.ts | 38 ++++++++++++++------------------ 4 files changed, 45 insertions(+), 38 deletions(-) diff --git a/app/client/models/AdminChecks.ts b/app/client/models/AdminChecks.ts index bd5a8cee49..bbea371bb5 100644 --- a/app/client/models/AdminChecks.ts +++ b/app/client/models/AdminChecks.ts @@ -53,7 +53,7 @@ export class AdminChecks { const {id} = probe; let result = this._results.get(id); if (!result) { - result = Observable.create(this._parent, {}); + result = Observable.create(this._parent, {status: 'none'}); this._results.set(id, result); } let request = this._requests.get(id); @@ -108,7 +108,7 @@ export class AdminCheckRunner { public start() { let result = this.results.get(this.id); if (!result) { - result = Observable.create(this.parent, {}); + result = Observable.create(this.parent, {status: 'none'}); this.results.set(this.id, result); } } diff --git a/app/client/ui/AdminPanel.ts b/app/client/ui/AdminPanel.ts index 4870344476..dfecb1ff8a 100644 --- a/app/client/ui/AdminPanel.ts +++ b/app/client/ui/AdminPanel.ts @@ -179,7 +179,7 @@ Please log in as an administrator.`)), use => { const req = this._checks.requestCheckById(use, 'sandboxing'); const result = req ? use(req.result) : undefined; - const success = result?.success; + const success = result?.status === 'success'; const details = result?.details as SandboxingBootProbeDetails|undefined; if (!details) { return cssValueLabel(t('unknown')); @@ -217,7 +217,8 @@ Please log in as an administrator.`)), return cssValueLabel(cssErrorText('unavailable')); } - const { success, details } = result; + const { status, details } = result; + const success = status === 'success'; const loginSystemId = details?.loginSystemId; if (!success || !loginSystemId) { @@ -500,10 +501,10 @@ Please log in as an administrator.`)), { style: 'margin-top: 0px; padding-top: 0px;' }, ), result.verdict ? dom('pre', result.verdict) : null, - (result.success === undefined) ? null : + (result.status === 'none') ? null : dom('p', - result.success ? t('Check succeeded.') : t('Check failed.')), - (result.done !== true) ? null : + (result.status === 'success') ? t('Check succeeded.') : t('Check failed.')), + (result.status !== 'none') ? null : dom('p', t('No fault detected.')), (details?.info === undefined) ? null : [ cssCheckHeader(t('Notes')), @@ -530,12 +531,21 @@ Please log in as an administrator.`)), * visualization of the results can be elaborated in future. */ private _encodeSuccess(result: BootProbeResult) { - if (result.success === undefined) { return '―'; } - if (result.success) { return '✅'; } - if (result.severity === 'warning') { return '❗'; } - if (result.severity === 'hmm') { return '?'; } - // remaining case is a fault. - return '❌'; + switch (result.status) { + case 'success': + return '✅'; + case 'fault': + return '❌'; + case 'warning': + return '❗'; + case 'hmm': + return '?'; + case 'none': + return '―'; + default: + // should not arrive here + return '??'; + } } } diff --git a/app/common/BootProbe.ts b/app/common/BootProbe.ts index 0da3c859fc..49fb99115a 100644 --- a/app/common/BootProbe.ts +++ b/app/common/BootProbe.ts @@ -13,9 +13,12 @@ export type BootProbeIds = export interface BootProbeResult { verdict?: string; - success?: boolean; - done?: boolean; - severity?: 'fault' | 'warning' | 'hmm'; + // Result of check. + // "success" is a positive outcome. + // "none" means no fault detected (but that the test is not exhaustive + // enough to claim "success"). + // "fault" is a bad error, "warning" a ... warning, "hmm" almost a debug message. + status: 'success' | 'fault' | 'warning' | 'hmm' | 'none'; details?: Record; } diff --git a/app/server/lib/BootProbes.ts b/app/server/lib/BootProbes.ts index 8145f2a2ce..df9427f231 100644 --- a/app/server/lib/BootProbes.ts +++ b/app/server/lib/BootProbes.ts @@ -91,17 +91,16 @@ const _homeUrlReachableProbe: Probe = { throw new ApiError(await resp.text(), resp.status); } return { - success: true, + status: 'success', details, }; } catch (e) { return { - success: false, details: { ...details, error: String(e), }, - severity: 'fault', + status: 'fault', }; } } @@ -121,7 +120,7 @@ const _webSocketsProbe: Probe = { ws.on('open', () => { ws.send('Just nod if you can hear me.'); resolve({ - success: true, + status: 'success', details, }); ws.close(); @@ -129,7 +128,7 @@ const _webSocketsProbe: Probe = { ws.on('error', (ev) => { details.error = ev.message; resolve({ - success: false, + status: 'fault', details, }); ws.close(); @@ -159,17 +158,16 @@ const _statusCheckProbe: Probe = { throw new Error(`Failed, page has unexpected content`); } return { - success: true, + status: 'success', details, }; } catch (e) { return { - success: false, details: { ...details, error: String(e), }, - severity: 'fault', + status: 'fault', }; } }, @@ -185,13 +183,12 @@ const _userProbe: Probe = { if (process.getuid && process.getuid() === 0) { return { details, - success: false, verdict: 'User appears to be root (UID 0)', - severity: 'warning', + status: 'warning', }; } else { return { - success: true, + status: 'success', details, }; } @@ -208,22 +205,20 @@ const _bootProbe: Probe = { bootKeySet: hasBoot, }; if (!hasBoot) { - return { success: true, details }; + return { status: 'success', details }; } details.bootKeyLength = bootKey.length; if (bootKey.length < 10) { return { - success: false, verdict: 'Boot key length is shorter than 10.', details, - severity: 'fault', + status: 'fault', }; } return { - success: false, verdict: 'Boot key ideally should be removed after installation.', details, - severity: 'warning', + status: 'warning', }; }, }; @@ -247,19 +242,18 @@ const _hostHeaderProbe: Probe = { }; if (url.hostname === 'localhost') { return { - done: true, + status: 'none', details, }; } if (String(url.hostname).toLowerCase() !== String(host).toLowerCase()) { return { - success: false, details, - severity: 'hmm', + status: 'hmm', }; } return { - done: true, + status: 'none', details, }; }, @@ -271,7 +265,7 @@ const _sandboxingProbe: Probe = { apply: async (server, req) => { const details = server.getSandboxInfo(); return { - success: details?.configured && details?.functional, + status: (details?.configured && details?.functional) ? 'success' : 'fault', details, }; }, @@ -283,7 +277,7 @@ const _authenticationProbe: Probe = { apply: async(server, req) => { const loginSystemId = server.getInfo('loginMiddlewareComment'); return { - success: loginSystemId != undefined, + status: (loginSystemId != undefined) ? 'success' : 'fault', details: { loginSystemId, }