Skip to content

Commit

Permalink
add more tests
Browse files Browse the repository at this point in the history
  • Loading branch information
paulfitz committed May 9, 2024
1 parent 986fc64 commit d40457b
Show file tree
Hide file tree
Showing 9 changed files with 107 additions and 43 deletions.
7 changes: 5 additions & 2 deletions app/client/boot.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { AppModel } from 'app/client/models/AppModel';
import { AppModel, getHomeUrl } from 'app/client/models/AppModel';
import { AdminChecks } from 'app/client/models/AdminChecks';
import { AdminPanel } from 'app/client/ui/AdminPanel';
import { createAppPage } from 'app/client/ui/createAppPage';
import { pagePanels } from 'app/client/ui/PagePanels';
import { getGristConfig } from 'app/common/urlUtils';
import { Disposable, dom, Observable, styled, UseCBOwner } from 'grainjs';
import { InstallAPI, InstallAPIImpl } from 'app/common/InstallAPI';

const cssBody = styled('div', `
padding: 20px;
Expand Down Expand Up @@ -38,12 +39,14 @@ export class Boot extends Disposable {

private _checks: AdminChecks;

private readonly _installAPI: InstallAPI = new InstallAPIImpl(getHomeUrl());

constructor(private _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);
this._checks = new AdminChecks(this, this._installAPI);
}

/**
Expand Down
35 changes: 14 additions & 21 deletions app/client/models/AdminChecks.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -19,7 +19,7 @@ export class AdminChecks {
// Keep track of probe results we have received, by probe ID.
private _results: Map<string, Observable<BootProbeResult>>;

constructor(private _parent: Disposable) {
constructor(private _parent: Disposable, private _installAPI: InstallAPI) {
this.probes = Observable.create(_parent, []);
this._results = new Map();
this._requests = new Map();
Expand All @@ -32,20 +32,14 @@ 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();
const _probes = await this._installAPI.getChecks().catch(() => undefined);
if (!this._parent.isDisposed()) {
this.probes.set(_probes.probes);
// Currently, no probes are allowed if not admin.
// May want to relax this to allow some probes that help
// diagnose some initial auth problems.
this.probes.set(_probes ? _probes.probes : []);
}
return _probes.probes;
return _probes;
}
return [];
}
Expand All @@ -63,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();
Expand Down Expand Up @@ -97,16 +91,15 @@ export interface AdminCheckRequest {
* Manage a single check.
*/
export class AdminCheckRunner {
constructor(public id: string, public results: Map<string, Observable<BootProbeResult>>,
constructor(private _installAPI: InstallAPI,
public id: string,
public results: Map<string, Observable<BootProbeResult>>,
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(async result => {
if (parent.isDisposed()) { return; }
const ob = results.get(id);
if (ob) {
ob.set(_probes);
ob.set(result);
}
}).catch(e => console.error(e));
}
Expand Down
27 changes: 25 additions & 2 deletions app/client/ui/AdminPanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,15 @@ export class AdminPanel extends Disposable {
constructor(private _appModel: AppModel, private _fullScreen: boolean = false) {
super();
document.title = getAdminPanelName() + getPageTitleSuffix(getGristConfig());
this._checks = new AdminChecks(this);
this._checks = new AdminChecks(this, this._installAPI);
}

public buildDom() {
this._checks.fetchAvailableChecks().catch(err => {
reportError(err);
});
if (this._fullScreen) {
return dom.create(this._buildMainContent.bind(this));
return dom.create(this._buildMainContentForAdmin.bind(this));
}
const panelOpen = Observable.create(this, false);
return pagePanels({
Expand Down Expand Up @@ -82,6 +82,29 @@ export class AdminPanel extends Disposable {
}

private _buildMainContent(owner: MultiHolder) {
return dom.maybe(this._checks.probes, probes => {
return probes.length > 0
? this._buildMainContentForAdmin(owner)
: this._buildMainContentForOthers(owner);
});
}

private _buildMainContentForOthers(owner: MultiHolder) {
return cssPageContainer(
dom.cls('clipboard'),
{tabIndex: "-1"},
cssSection(
cssSectionTitle(t('Admin Page Unavailable')),
`
You are not logged in as an administrator.
If logging in is broken, you can set GRIST_BOOT_KEY=secret in
the environment and visit /admin?key=secret.`
),
testId('admin-panel'),
);
}

private _buildMainContentForAdmin(owner: MultiHolder) {
return cssPageContainer(
dom.cls('clipboard'),
{tabIndex: "-1"},
Expand Down
15 changes: 11 additions & 4 deletions app/common/BaseAPI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,17 @@ export class BaseAPI {
'X-Requested-With': 'XMLHttpRequest',
...options.headers
};
if (typeof window !== 'undefined' && (window as any)?.isGristBootPage) {
const parts = (new URL(window.location.href).pathname).split('/');
if (parts[0] === '' && parts[1] === 'boot' && parts[2] !== undefined) {
this._headers['X-Boot-Key'] = parts[2];
if (typeof window !== 'undefined') {
const url = new URL(window.location.href);
if ((window as any)?.isGristBootPage) {
const parts = (url.pathname).split('/');
if (parts[0] === '' && parts[1] === 'boot' && parts[2] !== undefined) {
this._headers['X-Boot-Key'] = parts[2];
}
}
const bootKey = url.searchParams.get('boot');
if (bootKey) {
this._headers['X-Boot-Key'] = bootKey;
}
}
this._extraParameters = options.extraParameters;
Expand Down
11 changes: 11 additions & 0 deletions app/common/InstallAPI.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -56,6 +57,8 @@ export interface InstallAPI {
* Returns information about latest version of Grist
*/
checkUpdates(): Promise<LatestVersion>;
getChecks(): Promise<{probes: BootProbeInfo[]}>;
runCheck(id: string): Promise<BootProbeResult>;
}

export class InstallAPIImpl extends BaseAPI implements InstallAPI {
Expand All @@ -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[]}> {

Check failure on line 84 in app/common/InstallAPI.ts

View workflow job for this annotation

GitHub Actions / build_and_test (3.9, 18.x, :lint:python:client:common:smoke:stubs:)

Missing accessibility modifier on method definition getChecks

Check failure on line 84 in app/common/InstallAPI.ts

View workflow job for this annotation

GitHub Actions / build_and_test (:lint:python:client:common:smoke:, 18.x, 3.10)

Missing accessibility modifier on method definition getChecks

Check failure on line 84 in app/common/InstallAPI.ts

View workflow job for this annotation

GitHub Actions / build_and_test (:lint:python:client:common:smoke:, 18.x, 3.11)

Missing accessibility modifier on method definition getChecks
return this.requestJson(`${this._url}/api/probes`, {method: 'GET'});
}

runCheck(id: string): Promise<BootProbeResult> {

Check failure on line 88 in app/common/InstallAPI.ts

View workflow job for this annotation

GitHub Actions / build_and_test (3.9, 18.x, :lint:python:client:common:smoke:stubs:)

Missing accessibility modifier on method definition runCheck

Check failure on line 88 in app/common/InstallAPI.ts

View workflow job for this annotation

GitHub Actions / build_and_test (:lint:python:client:common:smoke:, 18.x, 3.10)

Missing accessibility modifier on method definition runCheck

Check failure on line 88 in app/common/InstallAPI.ts

View workflow job for this annotation

GitHub Actions / build_and_test (:lint:python:client:common:smoke:, 18.x, 3.11)

Missing accessibility modifier on method definition runCheck
return this.requestJson(`${this._url}/api/probes/${id}`, {method: 'GET'});
}

private get _url(): string {
return addCurrentOrgToPath(this._homeUrl);
}
Expand Down
2 changes: 2 additions & 0 deletions app/server/lib/Authorizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,9 @@ export async function addRequestUser(
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
Expand Down
6 changes: 3 additions & 3 deletions app/server/lib/BootProbes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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);
Expand All @@ -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() {
Expand Down
16 changes: 12 additions & 4 deletions app/server/lib/FlexServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1859,19 +1859,27 @@ export class FlexServer implements GristServer {

const requireInstallAdmin = this.getInstallAdmin().getMiddlewareRequireAdmin();

const adminPageMiddleware = [
this._redirectToHostMiddleware,
const adminPageFullMiddleware = [
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,
];

const adminPageMiddleware = [
// this._redirectToHostMiddleware,
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', adminPageFullMiddleware);
probes.addEndpoints();

// Restrict this endpoint to install admins too, for the same reason as the /admin page.
Expand Down
31 changes: 24 additions & 7 deletions test/nbrowser/AdminPanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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('/');

Expand All @@ -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(), /not logged in/);
});

it('should be shown to managers', async function() {
Expand Down Expand Up @@ -330,14 +331,30 @@ describe('AdminPanel', function() {
});

it('should survive APP_HOME_URL misconfiguration', async function() {
// TODO: this works in theory, but admin page is in practice hard
// to access unless other pages work (e.g. to log in). So falling
// back on boot page for now.
process.env.APP_HOME_URL = 'http://misconfigured.invalid';
process.env.GRIST_BOOT_KEY = 'zig';
await server.restart(true);
await driver.get(`${server.getHost()}/boot/zig`);
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(), /not logged in/);

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(), /not logged in/);
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(), /not logged in/);
});
});

Expand Down

0 comments on commit d40457b

Please sign in to comment.