From d33c15787120e0c1ab0248d0eceed5509c0aa71e Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Wed, 1 May 2024 18:17:26 -0400 Subject: [PATCH 1/2] 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 | 221 ++++++++++++++++++++++++------- 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, 431 insertions(+), 303 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 a37fd363..00000000 --- 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 45de41ed..f8ee5420 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 30ea4799..74839841 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 f02d8351..c776336d 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'; @@ -17,7 +17,7 @@ import {toggle} from 'app/client/ui2018/checkbox'; import {mediaSmall, testId, theme, vars} from 'app/client/ui2018/cssVars'; import {icon} from 'app/client/ui2018/icons'; 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'; @@ -42,7 +42,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() { @@ -79,50 +79,94 @@ 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"}, - cssSection( - cssSectionTitle(t('Support Grist')), - this._buildItem(owner, { - id: 'telemetry', - name: t('Telemetry'), - description: t('Help us make Grist better'), - value: maybeSwitchToggle(this._supportGrist.getTelemetryOptInObservable()), - expandedContent: this._supportGrist.buildTelemetrySection(), - }), - this._buildItem(owner, { - id: 'sponsor', - name: t('Sponsor'), - description: t('Support Grist Labs on GitHub'), - value: this._supportGrist.buildSponsorshipSmallButton(), - expandedContent: this._supportGrist.buildSponsorshipSection(), - }), - ), - cssSection( - cssSectionTitle(t('Security Settings')), - this._buildItem(owner, { - id: 'sandboxing', - name: t('Sandboxing'), - description: t('Sandbox settings for data engine'), - value: this._buildSandboxingDisplay(owner), - expandedContent: this._buildSandboxingNotice(), - }), - ), - cssSection( - cssSectionTitle(t('Version')), - this._buildItem(owner, { - id: 'version', - name: t('Current'), - description: t('Current version of Grist'), - value: cssValueLabel(`Version ${version.version}`), - }), - this._buildUpdates(owner), - ), + 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 cssSection( + cssSectionTitle(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 [cssSection( + cssSectionTitle(t('Support Grist')), + this._buildItem(owner, { + id: 'telemetry', + name: t('Telemetry'), + description: t('Help us make Grist better'), + value: maybeSwitchToggle(this._supportGrist.getTelemetryOptInObservable()), + expandedContent: this._supportGrist.buildTelemetrySection(), + }), + this._buildItem(owner, { + id: 'sponsor', + name: t('Sponsor'), + description: t('Support Grist Labs on GitHub'), + value: this._supportGrist.buildSponsorshipSmallButton(), + expandedContent: this._supportGrist.buildSponsorshipSection(), + }), + ), + cssSection( + cssSectionTitle(t('Security Settings')), + this._buildItem(owner, { + id: 'sandboxing', + name: t('Sandboxing'), + description: t('Sandbox settings for data engine'), + value: this._buildSandboxingDisplay(owner), + expandedContent: this._buildSandboxingNotice(), + }), + ), + cssSection( + cssSectionTitle(t('Version')), + this._buildItem(owner, { + id: 'version', + name: t('Current'), + description: t('Current version of Grist'), + value: cssValueLabel(`Version ${version.version}`), + }), + this._buildUpdates(owner), + ), + cssSection( + cssSectionTitle(t('Self Checks')), + this._buildProbeItems(owner, { + showRedundant: false, + showNovel: true, + }), + this._buildItem(owner, { + id: 'probe-other', + name: 'more...', + description: '', + value: '', + expandedContent: this._buildProbeItems(owner, { + showRedundant: true, + showNovel: false, + }), + }), + )]; + } + private _buildSandboxingDisplay(owner: IDisposableOwner) { return dom.domComputed( use => { @@ -146,11 +190,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'}, @@ -420,12 +462,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 this._buildItem(owner, { + 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; @@ -598,3 +722,10 @@ export const cssError = styled('div', ` export const cssHappy = styled('div', ` 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 23e80112..cd86c24c 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 88cc35a5..b71b1389 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 761e76f2..222698c5 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 7cddb99f..8e3ae366 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() { @@ -75,21 +75,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.getHomeUrl(req); + 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', @@ -100,13 +106,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.getHomeUrl(req); 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}`); } @@ -116,11 +126,15 @@ const _statusCheckProbe: Probe = { } return { success: true, + details, }; } catch (e) { return { success: false, - error: String(e), + details: { + ...details, + error: String(e), + }, severity: 'fault', }; } @@ -129,10 +143,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', @@ -140,6 +158,7 @@ const _userProbe: Probe = { } else { return { success: true, + details, }; } }, @@ -147,14 +166,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', }; }, @@ -169,31 +194,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 17ac46a3..9e55e4bf 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; @@ -543,27 +542,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() { @@ -1859,22 +1848,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(); @@ -1902,7 +1890,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 9c53f347..b2d9b657 100644 --- a/app/server/lib/GristServer.ts +++ b/app/server/lib/GristServer.ts @@ -64,7 +64,7 @@ export interface GristServer { getPlugins(): LocalPlugin[]; servesPlugins(): boolean; getBundledWidgets(): ICustomWidget[]; - hasBoot(): boolean; + getBootKey(): string|undefined; getSandboxInfo(): SandboxInfo|undefined; } @@ -155,7 +155,7 @@ export function createDummyGristServer(): GristServer { servesPlugins() { return false; }, getPlugins() { return []; }, getBundledWidgets() { return []; }, - hasBoot() { return false; }, + getBootKey() { return undefined; }, getSandboxInfo() { return undefined; }, }; } diff --git a/app/server/lib/ICreate.ts b/app/server/lib/ICreate.ts index 8d76d030..3ca48d0c 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 803656aa..0a00bfa1 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 f7ecea7d..722714b4 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 8f67607f..00000000 --- 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 5a78b85c..f048192b 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 230adc85..0529e749 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 e45aa7a973221f5dc39393f1467496e6699550a1 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Thu, 9 May 2024 17:41:24 -0400 Subject: [PATCH 2/2] 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 b71b1389..3f50db49 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'}); }