(core) move more tests to grist-core

Summary:
 * Tie build and run-time docker base images to a consistent version (buster)
 * Extend the test login system activated by GRIST_TEST_LOGIN to ease porting tests that currently rely on cognito (many)
 * Make org resets work in absence of billing endpoints
 * When in-memory session caches are used, add missing invalidation steps
 * Pass org information through sign-ups/sign-ins more carefully
 * For CORS, explicitly trust GRIST_HOST origin when set
 * Move some fixtures and tests to core, focussing on tests that cover existing failures or are in the set of tests run on deployments
 * Retain regular `test` target to run the test suite directly, without docker
 * Add a `test:smoke` target to run a single simple test without `GRIST_TEST_LOGIN` activated
 * Add a `test:docker` target to run the tests against a grist-core docker image - since tests rely on certain fixture teams/docs, added `TEST_SUPPORT_API_KEY` and `TEST_ADD_SAMPLES` flags to ease porting

The tests ported were `nbrowser` tests: `ActionLog.ts` (the first test I tend to port to anything, out of habit), `Fork.ts` (exercises a lot of doc creation paths), `HomeIntro.ts` (a lot of DocMenu exercise), and `DuplicateDocument.ts` (covers a feature known to be failing prior to this diff, the CORS tweak resolves it).

Test Plan: Manually tested via `buildtools/build_core.sh`. In follow up, I want to add running the `test:docker` target in grist-core's workflows. In jenkins, only the smoke test is run. There'd be an argument for running all tests, but they include particularly slow tests, and are duplicates of tests already run (in different configuration admittedly), so I'd like to try first just using them in grist-core to gate updates to any packaged version of Grist (the docker image currently).

Reviewers: alexmojaki

Reviewed By: alexmojaki

Subscribers: alexmojaki

Differential Revision: https://phab.getgrist.com/D3176
This commit is contained in:
Paul Fitzpatrick
2021-12-10 17:42:54 -05:00
parent 307966e84f
commit d99db8d016
29 changed files with 1612 additions and 77 deletions

View File

@@ -33,7 +33,7 @@ import {DocWorkerInfo, IDocWorkerMap} from 'app/server/lib/DocWorkerMap';
import {expressWrap, jsonErrorHandler} from 'app/server/lib/expressWrap';
import {Hosts, RequestWithOrg} from 'app/server/lib/extractOrg';
import {addGoogleAuthEndpoint} from "app/server/lib/GoogleAuth";
import {GristLoginMiddleware, GristServer} from 'app/server/lib/GristServer';
import {GristLoginMiddleware, GristServer, RequestWithGrist} from 'app/server/lib/GristServer';
import {initGristSessions, SessionStore} from 'app/server/lib/gristSessions';
import {HostedStorageManager} from 'app/server/lib/HostedStorageManager';
import {IBilling} from 'app/server/lib/IBilling';
@@ -45,7 +45,8 @@ import {IPermitStore} from 'app/server/lib/Permit';
import {getAppPathTo, getAppRoot, getUnpackedAppRoot} from 'app/server/lib/places';
import {addPluginEndpoints, limitToPlugins} from 'app/server/lib/PluginEndpoint';
import {PluginManager} from 'app/server/lib/PluginManager';
import {adaptServerUrl, addOrgToPath, addPermit, getScope, optStringParam, RequestWithGristInfo, stringParam,
import {adaptServerUrl, addOrgToPath, addOrgToPathIfNeeded, addPermit, getScope,
optStringParam, RequestWithGristInfo, stringParam,
TEST_HTTPS_OFFSET, trustOrigin} from 'app/server/lib/requestUtils';
import {ISendAppPageOptions, makeGristConfig, makeMessagePage, makeSendAppPage} from 'app/server/lib/sendAppPage';
import {getDatabaseUrl} from 'app/server/lib/serverUtils';
@@ -53,6 +54,7 @@ import {Sessions} from 'app/server/lib/Sessions';
import * as shutdown from 'app/server/lib/shutdown';
import {TagChecker} from 'app/server/lib/TagChecker';
import {startTestingHooks} from 'app/server/lib/TestingHooks';
import {getTestLoginSystem} from 'app/server/lib/TestLogin';
import {addUploadRoute} from 'app/server/lib/uploads';
import {buildWidgetRepository, IWidgetRepository} from 'app/server/lib/WidgetRepository';
import axios from 'axios';
@@ -86,10 +88,6 @@ export interface FlexServerOptions {
pluginUrl?: string;
}
export interface RequestWithGrist extends express.Request {
gristServer?: GristServer;
}
export class FlexServer implements GristServer {
public readonly create = create;
public tagChecker: TagChecker;
@@ -761,7 +759,7 @@ export class FlexServer implements GristServer {
// TODO: We could include a third mock provider of login/logout URLs for better tests. Or we
// could create a mock SAML identity provider for testing this using the SAML flow.
const loginSystem = await getLoginSystem();
const loginSystem = await (process.env.GRIST_TEST_LOGIN ? getTestLoginSystem() : getLoginSystem());
this._loginMiddleware = await loginSystem.getMiddleware(this);
this._getLoginRedirectUrl = tbind(this._loginMiddleware.getLoginRedirectUrl, this._loginMiddleware);
this._getSignUpRedirectUrl = tbind(this._loginMiddleware.getSignUpRedirectUrl, this._loginMiddleware);
@@ -800,7 +798,7 @@ export class FlexServer implements GristServer {
}
public async addLoginRoutes() {
if (this._check('login', 'org', 'sessions', 'homedb')) { return; }
if (this._check('login', 'org', 'sessions', 'homedb', 'hosts')) { return; }
// TODO: We do NOT want Comm here at all, it's only being used for handling sessions, which
// should be factored out of it.
this.addComm();
@@ -813,7 +811,7 @@ export class FlexServer implements GristServer {
// we'll need it when we come back from Cognito.
forceSessionChange(mreq.session);
// Redirect to "/" on our requested hostname (in test env, this will redirect further)
const next = req.protocol + '://' + req.get('host') + '/';
const next = getOrgUrl(req);
if (signUp === null) {
// Like redirectToLogin in Authorizer, redirect to sign up if it doesn't look like the
// user has ever logged in on this browser.
@@ -839,20 +837,38 @@ export class FlexServer implements GristServer {
this.app.get('/test/login', expressWrap(async (req, res) => {
log.warn("Serving unauthenticated /test/login endpoint, made available because GRIST_TEST_LOGIN is set.");
const scopedSession = this._sessions.getOrCreateSessionFromRequest(req);
const profile: UserProfile = {
email: optStringParam(req.query.email) || 'chimpy@getgrist.com',
name: optStringParam(req.query.name) || 'Chimpy McBanana',
};
await scopedSession.updateUserProfile(req, profile);
// Query parameter is called "username" for compatibility with Cognito.
const email = optStringParam(req.query.username);
if (email) {
const redirect = optStringParam(req.query.next);
const profile: UserProfile = {
email,
name: optStringParam(req.query.name) || email,
};
const url = new URL(redirect || getOrgUrl(req));
// Make sure we update session for org we'll be redirecting to.
const {org} = await this._hosts.getOrgInfoFromParts(url.hostname, url.pathname);
const scopedSession = this._sessions.getOrCreateSessionFromRequest(req, { org });
await scopedSession.updateUserProfile(req, profile);
this._sessions.clearCacheIfNeeded({email, org});
if (redirect) { return res.redirect(redirect); }
}
res.send(`<!doctype html>
<html><body>
<p>Logged in as ${JSON.stringify(profile)}.<p>
<form>
<input type=text name=email placeholder=email>
<input type=text name=name placeholder=name>
<input type=submit value=login>
</form>
<div class="modal-content-desktop">
<h1>A Very Creduluous Login Page</h1>
<p>
A minimal login screen to facilitate testing.
I'll believe anything you tell me.
</p>
<form>
<div>Email <input type=text name=username placeholder=email /></div>
<div>Name <input type=text name=name placeholder=name /></div>
<div>Dummy password <input type=text name=password placeholder=unused ></div>
<input type=hidden name=next value="${req.query.next || ''}">
<div><input type=submit name=signInSubmitButton value=login></div>
</form>
</div>
</body></html>
`);
}));
@@ -862,7 +878,7 @@ export class FlexServer implements GristServer {
const scopedSession = this._sessions.getOrCreateSessionFromRequest(req);
// If 'next' param is missing, redirect to "/" on our requested hostname.
const next = optStringParam(req.query.next) || (req.protocol + '://' + req.get('host') + '/');
const next = optStringParam(req.query.next) || getOrgUrl(req);
const redirectUrl = await this._getLogoutRedirectUrl(req, new URL(next));
// Clear session so that user needs to log in again at the next request.
@@ -871,6 +887,8 @@ export class FlexServer implements GristServer {
const expressSession = (req as any).session;
if (expressSession) { expressSession.users = []; expressSession.orgToUser = {}; }
await scopedSession.clearScopedSession(req);
// TODO: limit cache clearing to specific user.
this._sessions.clearCacheIfNeeded();
resp.redirect(redirectUrl);
}));
@@ -1639,6 +1657,11 @@ function trustOriginHandler(req: express.Request, res: express.Response, next: e
}
}
// Get url to the org associated with the request.
function getOrgUrl(req: express.Request) {
return req.protocol + '://' + req.get('host') + addOrgToPathIfNeeded(req, '/');
}
// Set Cache-Control header to "no-cache"
function noCaching(req: express.Request, res: express.Response, next: express.NextFunction) {
res.header("Cache-Control", "no-cache");

View File

@@ -47,3 +47,7 @@ export interface GristLoginMiddleware {
// Returns arbitrary string for log.
addEndpoints(app: express.Express): Promise<string>;
}
export interface RequestWithGrist extends express.Request {
gristServer?: GristServer;
}

View File

@@ -193,7 +193,7 @@ export class SamlConfig {
const samlNameId = samlUser.name_id;
log.info(`SamlConfig: got SAML response for ${profile.email} (${profile.name}) redirecting to ${redirectUrl}`);
const scopedSession = sessions.getOrCreateSessionFromRequest(req, state.sessionId);
const scopedSession = sessions.getOrCreateSessionFromRequest(req, {sessionId: state.sessionId});
await scopedSession.operateOnScopedSession(req, async (user) => Object.assign(user, {
profile,
samlSessionIndex,

View File

@@ -32,9 +32,12 @@ export class Sessions {
* Get the session id and organization from the request (or just pass it in if known), and
* return the identified session.
*/
public getOrCreateSessionFromRequest(req: Request, sessionId?: string): ScopedSession {
const sid = sessionId || this.getSessionIdFromRequest(req);
const org = (req as any).org;
public getOrCreateSessionFromRequest(req: Request, options?: {
sessionId?: string,
org?: string
}): ScopedSession {
const sid = options?.sessionId ?? this.getSessionIdFromRequest(req);
const org = options?.org ?? (req as any).org;
if (!sid) { throw new Error("session not found"); }
return this.getOrCreateSession(sid, org, ''); // TODO: allow for tying to a preferred user.
}
@@ -51,6 +54,23 @@ export class Sessions {
return this._sessions.get(key)!;
}
/**
* Called when a session is modified, and any caching should be invalidated.
* Currently just removes all caching, if there is any. This caching is a bit
* of a weird corner of Grist, it is used in development for historic reasons
* but not in production.
* TODO: make more fine grained, or rethink.
*/
public clearCacheIfNeeded(options?: {
email?: string,
org?: string|null,
sessionID?: string,
}) {
if (!(process.env.GRIST_HOST || process.env.GRIST_HOSTED)) {
this._sessions.clear();
}
}
/**
* Returns the sessionId from the signed grist cookie.
*/

View File

@@ -0,0 +1,42 @@
import { GristLoginSystem, GristServer } from 'app/server/lib/GristServer';
import { Request } from 'express';
/**
* Return a login system for testing. Just enough to use the test/login endpoint
* available when GRIST_TEST_LOGIN=1 is set.
*/
export async function getTestLoginSystem(): Promise<GristLoginSystem> {
return {
async getMiddleware(gristServer: GristServer) {
async function getLoginRedirectUrl(req: Request, url: URL) {
// The "gristlogin" query parameter does nothing except make tests
// that expect hosted cognito happy (they check for gristlogin in url).
const target = new URL(gristServer.getHomeUrl(req, 'test/login?gristlogin=1'));
target.searchParams.append('next', url.href);
return target.href || url.href;
}
return {
getLoginRedirectUrl,
async getLogoutRedirectUrl(req: Request, url: URL) {
return url.href;
},
getSignUpRedirectUrl: getLoginRedirectUrl,
async addEndpoints() {
// Make sure support user has a test api key if needed.
if (process.env.TEST_SUPPORT_API_KEY) {
const dbManager = gristServer.getHomeDBManager();
const user = await dbManager.getUserByLogin('support@getgrist.com');
if (user) {
user.apiKey = process.env.TEST_SUPPORT_API_KEY;
await user.save();
}
}
return "test-login";
},
};
},
async deleteUser() {
// nothing to do
},
};
}

View File

@@ -78,7 +78,8 @@ export class TestingHooks implements ITestingHooks {
const sessionId = this._comm.getSessionIdFromCookie(gristSidCookie);
const scopedSession = this._comm.getOrCreateSession(sessionId, {org});
const req = {} as Request;
return await scopedSession.updateUserProfile(req, profile);
await scopedSession.updateUserProfile(req, profile);
this._server.getSessions().clearCacheIfNeeded({email: profile?.email, org});
}
public async setServerVersion(version: string|null): Promise<void> {

View File

@@ -4,6 +4,7 @@ import * as gutil from 'app/common/gutil';
import {DocScope, QueryResult, Scope} from 'app/gen-server/lib/HomeDBManager';
import {getUserId, RequestWithLogin} from 'app/server/lib/Authorizer';
import {RequestWithOrg} from 'app/server/lib/extractOrg';
import {RequestWithGrist} from 'app/server/lib/GristServer';
import * as log from 'app/server/lib/log';
import {Permit} from 'app/server/lib/Permit';
import {Request, Response} from 'express';
@@ -74,6 +75,7 @@ export function trustOrigin(req: Request, resp: Response): boolean {
// Note that the request origin is undefined for non-CORS requests.
const origin = req.get('origin');
if (!origin) { return true; } // Not a CORS request.
if (process.env.GRIST_HOST && req.hostname === process.env.GRIST_HOST) { return true; }
if (!allowHost(req, new URL(origin))) { return false; }
// For a request to a custom domain, the full hostname must match.
@@ -254,3 +256,16 @@ export function getOriginUrl(req: Request) {
const protocol = req.get("X-Forwarded-Proto") || req.protocol;
return `${protocol}://${host}`;
}
/**
* In some configurations, session information may be cached by the server.
* When session information changes, give the server a chance to clear its
* cache if needed.
*/
export function clearSessionCacheIfNeeded(req: Request, options?: {
email?: string,
org?: string|null,
sessionID?: string,
}) {
(req as RequestWithGrist).gristServer?.getSessions().clearCacheIfNeeded(options);
}

View File

@@ -4,9 +4,8 @@ import {FetchUrlOptions, FileUploadResult, UPLOAD_URL_PATH, UploadResult} from '
import {getAuthorizedUserId, getTransitiveHeaders, getUserId, isSingleUserMode,
RequestWithLogin} from 'app/server/lib/Authorizer';
import {expressWrap} from 'app/server/lib/expressWrap';
import {RequestWithGrist} from 'app/server/lib/FlexServer';
import {downloadFromGDrive, isDriveUrl} from 'app/server/lib/GoogleImport';
import {GristServer} from 'app/server/lib/GristServer';
import {GristServer, RequestWithGrist} from 'app/server/lib/GristServer';
import {guessExt} from 'app/server/lib/guessExt';
import * as log from 'app/server/lib/log';
import {optStringParam} from 'app/server/lib/requestUtils';