diff --git a/README.md b/README.md index 0d481acd..27e711e8 100644 --- a/README.md +++ b/README.md @@ -183,7 +183,6 @@ GRIST_HOME_INCLUDE_STATIC | if set, home server also serves static resources GRIST_HOST | hostname to use when listening on a port. GRIST_ID_PREFIX | for subdomains of form o-*, expect or produce o-${GRIST_ID_PREFIX}*. GRIST_INST_DIR | path to Grist instance configuration files, for Grist server. -GRIST_LOGIN_REDIRECT_HOST | host of cognito-based login helper, if applicable (usually login.getgrist.com). GRIST_MANAGED_WORKERS | if set, Grist can assume that if a url targeted at a doc worker returns a 404, that worker is gone GRIST_MAX_UPLOAD_ATTACHMENT_MB | max allowed size for attachments (0 or empty for unlimited). GRIST_MAX_UPLOAD_IMPORT_MB | max allowed size for imports (except .grist files) (0 or empty for unlimited). diff --git a/app/client/models/gristUrlState.ts b/app/client/models/gristUrlState.ts index 833f837f..e1be9441 100644 --- a/app/client/models/gristUrlState.ts +++ b/app/client/models/gristUrlState.ts @@ -24,7 +24,8 @@ */ import {unsavedChanges} from 'app/client/components/UnsavedChanges'; import {UrlState} from 'app/client/lib/UrlState'; -import {decodeUrl, encodeUrl, getSlugIfNeeded, GristLoadConfig, IGristUrlState} from 'app/common/gristUrls'; +import {decodeUrl, encodeUrl, getSlugIfNeeded, GristLoadConfig, IGristUrlState, + parseFirstUrlPart} from 'app/common/gristUrls'; import {addOrgToPath} from 'app/common/urlUtils'; import {Document} from 'app/common/UserAPI'; import isEmpty = require('lodash/isEmpty'); @@ -64,39 +65,40 @@ export function getMainOrgUrl(): string { return urlState().makeUrl({}); } // When on a document URL, returns the URL with just the doc ID, omitting other bits (like page). export function getCurrentDocUrl(): string { return urlState().makeUrl({docPage: undefined}); } -// Get url for the login page, which will then redirect to nextUrl (current page by default). +// Get url for the login page, which will then redirect to `nextUrl` (current page by default). export function getLoginUrl(nextUrl: string | null = _getCurrentUrl()): string { - return _getLoginLogoutUrl('login', nextUrl ?? undefined); + return _getLoginLogoutUrl('login', nextUrl); } -// Get url for the signup page, which will then redirect to nextUrl (current page by default). +// Get url for the signup page, which will then redirect to `nextUrl` (current page by default). export function getSignupUrl(nextUrl: string = _getCurrentUrl()): string { return _getLoginLogoutUrl('signup', nextUrl); } -// Get url for the logout page, which will then redirect to nextUrl (signed-out page by default). -export function getLogoutUrl(nextUrl: string = getSignedOutUrl()): string { - return _getLoginLogoutUrl('logout', nextUrl); +// Get url for the logout page. +export function getLogoutUrl(): string { + return _getLoginLogoutUrl('logout'); } -// Get url for the login page, which will then redirect to nextUrl (current page by default). +// Get url for the signin page, which will then redirect to `nextUrl` (current page by default). export function getLoginOrSignupUrl(nextUrl: string = _getCurrentUrl()): string { return _getLoginLogoutUrl('signin', nextUrl); } -// Returns the URL for the "you are signed out" page. -export function getSignedOutUrl(): string { return getMainOrgUrl() + "signed-out"; } - -// Helper which returns the URL of the current page, except when it's the "/signed-out" page, in -// which case returns the org URL. This is a good URL to use for a post-login redirect. +// Returns the relative URL (i.e. path) of the current page, except when it's the +// "/signed-out" page, in which case it returns the home page ("/"). +// This is a good URL to use for a post-login redirect. function _getCurrentUrl(): string { - return window.location.pathname.endsWith("/signed-out") ? getMainOrgUrl() : window.location.href; + if (window.location.pathname.endsWith('/signed-out')) { return '/'; } + + const {pathname, search} = new URL(window.location.href); + return parseFirstUrlPart('o', pathname).path + search; } -// Helper for getLoginUrl()/getLogoutUrl(). -function _getLoginLogoutUrl(method: 'login'|'logout'|'signin'|'signup', nextUrl?: string): string { +// Returns the URL for the given login page, with 'next' param optionally set. +function _getLoginLogoutUrl(page: 'login'|'logout'|'signin'|'signup', nextUrl?: string | null): string { const startUrl = new URL(window.location.href); - startUrl.pathname = addOrgToPath('', window.location.href, true) + '/' + method; + startUrl.pathname = addOrgToPath('', window.location.href, true) + '/' + page; if (nextUrl) { startUrl.searchParams.set('next', nextUrl); } return startUrl.href; } diff --git a/app/client/ui/BillingPage.ts b/app/client/ui/BillingPage.ts index 4705d6ab..68bd1c23 100644 --- a/app/client/ui/BillingPage.ts +++ b/app/client/ui/BillingPage.ts @@ -1,7 +1,7 @@ import {beaconOpenMessage} from 'app/client/lib/helpScout'; import {AppModel, reportError} from 'app/client/models/AppModel'; import {BillingModel, BillingModelImpl, ISubscriptionModel} from 'app/client/models/BillingModel'; -import {getLoginUrl, getMainOrgUrl, urlState} from 'app/client/models/gristUrlState'; +import {getLoginUrl, urlState} from 'app/client/models/gristUrlState'; import {AppHeader} from 'app/client/ui/AppHeader'; import {BillingForm, IFormData} from 'app/client/ui/BillingForm'; import * as css from 'app/client/ui/BillingPageCss'; @@ -499,7 +499,7 @@ export class BillingPage extends Disposable { // If the user is not logged in and selects the free plan, provide a login link that // redirects back to the free org. return css.upgradeBtn('Sign up', - {href: getLoginUrl(getMainOrgUrl())}, + {href: getLoginUrl()}, testId('plan-btn') ); } else if ((!selectedPlan && plan.amount === 0) || (selectedPlan && plan.id === selectedPlan.id)) { diff --git a/app/client/ui/WelcomePage.ts b/app/client/ui/WelcomePage.ts index f3c40935..12362237 100644 --- a/app/client/ui/WelcomePage.ts +++ b/app/client/ui/WelcomePage.ts @@ -98,7 +98,7 @@ export class WelcomePage extends Disposable { `If you already have a Grist account as `, dom('b', email.get()), ` you can just `, - dom('a', {href: getLoginUrl(urlState().makeUrl({}))}, 'log in'), + dom('a', {href: getLoginUrl()}, 'log in'), ` now. Otherwise, please pick a password.` ), cssSeparatedLabel('The email address you activated Grist with:'), diff --git a/app/common/LoginState.ts b/app/common/LoginState.ts deleted file mode 100644 index e842edb2..00000000 --- a/app/common/LoginState.ts +++ /dev/null @@ -1,31 +0,0 @@ -import {parseSubdomain} from 'app/common/gristUrls'; - -// This interface is used by the standalone login-connect tool for knowing where to redirect to, -// by Client.ts to construct this info, and by CognitoClient to decide what to do. - -export interface LoginState { - // Locally-running Grist uses localPort, while hosted uses subdomain. Login-connect uses this to - // redirect back to the localhost or to the subdomain. - localPort?: number; - subdomain?: string; - baseDomain?: string; // the domain with the (left-most) subdomain removed, e.g. ".getgrist.com". - // undefined on localhost. - - // Standalone version sets clientId, used later to find the LoginSession. Hosted and dev - // versions rely on the browser cookies instead, specifically on the session cookie. - clientId?: string; - - // Hosted and dev versions set redirectUrl and redirect to it when login or logout completes. - // Standalone version omits redirectUrl, and serves a page which closes the window. - redirectUrl?: string; -} - -/// Allowed localhost addresses. -export const localhostRegex = /^localhost(?::(\d+))?$/i; - -export function getLoginState(reqHost: string): LoginState|null { - const {org, base} = parseSubdomain(reqHost); - const matchPort = localhostRegex.exec(reqHost); - return org ? {subdomain: org, baseDomain: base} : - matchPort ? {localPort: matchPort[1] ? parseInt(matchPort[1], 10) : 80} : null; -} diff --git a/app/common/gristUrls.ts b/app/common/gristUrls.ts index 19b9ac4b..47357881 100644 --- a/app/common/gristUrls.ts +++ b/app/common/gristUrls.ts @@ -2,7 +2,6 @@ import {BillingPage, BillingSubPage, BillingTask} from 'app/common/BillingAPI'; import {OpenDocMode} from 'app/common/DocListAPI'; import {EngineCode} from 'app/common/DocumentSettings'; import {encodeQueryParams, isAffirmative} from 'app/common/gutil'; -import {localhostRegex} from 'app/common/LoginState'; import {LocalPlugin} from 'app/common/plugin'; import {StringUnion} from 'app/common/StringUnion'; import {UIRowId} from 'app/common/UIRowId'; @@ -34,7 +33,7 @@ export type WelcomePage = typeof WelcomePage.type; export const AccountPage = StringUnion('account'); export type AccountPage = typeof AccountPage.type; -export const LoginPage = StringUnion('signup', 'verified', 'forgot-password'); +export const LoginPage = StringUnion('signup', 'login', 'verified', 'forgot-password'); export type LoginPage = typeof LoginPage.type; // Overall UI style. "full" is normal, "light" is a single page focused, panels hidden experience. @@ -89,7 +88,7 @@ export interface IGristUrlState { billingPlan?: string; billingTask?: BillingTask; embed?: boolean; - next?: string; + state?: string; style?: InterfaceStyle; compare?: string; linkParameters?: Record; // Parameters to pass as 'user.Link' in granular ACLs. @@ -302,13 +301,16 @@ export function decodeUrl(gristConfig: Partial, location: Locat if (map.has('signup')) { state.login = 'signup'; + } else if (map.has('login')) { + state.login = 'login'; } else if (map.has('verified')) { state.login = 'verified'; } else if (map.has('forgot-password')) { state.login = 'forgot-password'; } - - if (sp.has('next')) { state.params!.next = sp.get('next')!; } + if (sp.has('state')) { + state.params!.state = sp.get('state')!; + } if (sp.has('style')) { state.params!.style = InterfaceStyle.parse(sp.get('style')); @@ -407,6 +409,9 @@ export function parseSubdomain(host: string|undefined): {org?: string, base?: st return {}; } +// Allowed localhost addresses. +const localhostRegex = /^localhost(?::(\d+))?$/i; + /** * Like parseSubdomain, but throws an error if neither of these cases apply: * - host can be parsed into a valid subdomain and a valid base domain. @@ -566,7 +571,7 @@ export function isOrgInPathOnly(host?: string): boolean { const gristConfig: GristLoadConfig = (window as any).gristConfig; return (gristConfig && gristConfig.pathOnly) || false; } else { - if (host && host.match(/^localhost(:[0-9]+)?$/)) { return true; } + if (host && host.match(localhostRegex)) { return true; } return (process.env.GRIST_ORG_IN_PATH === 'true'); } } diff --git a/app/common/gutil.ts b/app/common/gutil.ts index 3eac1127..afa64952 100644 --- a/app/common/gutil.ts +++ b/app/common/gutil.ts @@ -921,3 +921,12 @@ export function getDistinctValues(values: readonly T[], count: number = Infin } return distinct; } + +/** + * Asserts that variable `name` has a non-nullish `value`. + */ +export function assertIsDefined(name: string, value: T): asserts value is NonNullable { + if (value === undefined || value === null) { + throw new Error(`Expected '${name}' to be defined, but received ${value}`); + } +} diff --git a/app/gen-server/lib/DocWorkerMap.ts b/app/gen-server/lib/DocWorkerMap.ts index 8f852aba..b4fbe5e8 100644 --- a/app/gen-server/lib/DocWorkerMap.ts +++ b/app/gen-server/lib/DocWorkerMap.ts @@ -93,6 +93,9 @@ class DummyDocWorkerMap implements IDocWorkerMap { }, async close(): Promise { _permits.clear(); + }, + getKeyPrefix() { + return formatPermitKey('', prefix); } }; this._permitStores.set(prefix, store); @@ -461,6 +464,9 @@ export class DocWorkerMap implements IDocWorkerMap { }, async close() { // nothing to do + }, + getKeyPrefix() { + return formatPermitKey('', prefix); } }; } diff --git a/app/server/lib/BrowserSession.ts b/app/server/lib/BrowserSession.ts index 3ffdd789..b21b1013 100644 --- a/app/server/lib/BrowserSession.ts +++ b/app/server/lib/BrowserSession.ts @@ -282,7 +282,9 @@ export class ScopedSession { const session = prev || await this._getSession(); if (!session.users) { session.users = []; } if (!session.orgToUser) { session.orgToUser = {}; } - let index = session.users.findIndex(u => Boolean(u.profile && u.profile.email === profile.email)); + let index = session.users.findIndex(u => { + return Boolean(u.profile && normalizeEmail(u.profile.email) === normalizeEmail(profile.email)); + }); if (index < 0) { index = session.users.length; } session.orgToUser[this._org] = index; session.users[index] = user; diff --git a/app/server/lib/Client.ts b/app/server/lib/Client.ts index 2c809b6a..ed46f56c 100644 --- a/app/server/lib/Client.ts +++ b/app/server/lib/Client.ts @@ -2,7 +2,6 @@ import {ApiError} from 'app/common/ApiError'; import {BrowserSettings} from 'app/common/BrowserSettings'; import {ErrorWithCode} from 'app/common/ErrorWithCode'; import {UserProfile} from 'app/common/LoginSessionAPI'; -import {getLoginState, LoginState} from 'app/common/LoginState'; import {ANONYMOUS_USER_EMAIL} from 'app/common/UserAPI'; import {User} from 'app/gen-server/entity/User'; import {HomeDBManager} from 'app/gen-server/lib/HomeDBManager'; @@ -75,7 +74,6 @@ export class Client { private _destroyTimer: NodeJS.Timer|null = null; private _destroyed: boolean = false; private _websocket: any; - private _loginState: LoginState|null = null; private _org: string|null = null; private _profile: UserProfile|null = null; private _userId: number|null = null; @@ -95,9 +93,6 @@ export class Client { public toString() { return `Client ${this.clientId} #${this._counter}`; } - // Returns the LoginState object that's encoded and passed via login pages to login-connect. - public getLoginState(): LoginState|null { return this._loginState; } - public setCounter(counter: string) { this._counter = counter; } @@ -112,8 +107,6 @@ export class Client { public setConnection(websocket: any, reqHost: string, browserSettings: BrowserSettings) { this._websocket = websocket; - // Set this._loginState, used by CognitoClient to construct login/logout URLs. - this._loginState = getLoginState(reqHost); this.browserSettings = browserSettings; } diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 1abd13d2..182962a6 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -1,4 +1,3 @@ -import {ApiError} from 'app/common/ApiError'; import {BillingTask} from 'app/common/BillingAPI'; import {delay} from 'app/common/delay'; import {DocCreationInfo} from 'app/common/DocListAPI'; @@ -305,8 +304,7 @@ export class FlexServer implements GristServer { if (process.env.GRIST_LOG_SKIP_HTTP) { return; } // Add a timestamp token that matches exactly the formatting of non-morgan logs. morganLogger.token('logTime', (req: Request) => log.timestamp()); - // Add an optional gristInfo token that can replace the url, if the url is sensitive - // (this is the case for some cognito login urls). + // Add an optional gristInfo token that can replace the url, if the url is sensitive. morganLogger.token('gristInfo', (req: RequestWithGristInfo) => req.gristInfo || req.originalUrl || req.url); morganLogger.token('host', (req: express.Request) => req.get('host')); @@ -850,49 +848,32 @@ export class FlexServer implements GristServer { // should be factored out of it. this.addComm(); - /** - * Gets the URL to redirect back to after successful sign-up, login, or logout. - * - * Note that in the test env, this will redirect further. - */ - const getNextUrl = async (mreq: RequestWithLogin): Promise => { - const next = optStringParam(mreq.query.next); - - // If a "next" query param isn't present, return the URL of the request org. - if (next === undefined) { return getOrgUrl(mreq); } - - // Check that the "next" param has a valid host (native or custom) before returning it. - if (!(await this._hosts.isSafeRedirectUrl(next))) { - throw new ApiError('Invalid redirect URL', 400); - } - - return next; - }; - async function redirectToLoginOrSignup( this: FlexServer, signUp: boolean|null, req: express.Request, resp: express.Response, ) { const mreq = req as RequestWithLogin; // This will ensure that express-session will set our cookie if it hasn't already - - // we'll need it when we come back from Cognito. + // we'll need it when we redirect back. forceSessionChange(mreq.session); + // Redirect to the requested URL after successful login. + const nextPath = optStringParam(req.query.next); + const nextUrl = new URL(getOrgUrl(req, nextPath)); 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. signUp = (mreq.session.users === undefined); } const getRedirectUrl = signUp ? this._getSignUpRedirectUrl : this._getLoginRedirectUrl; - resp.redirect(await getRedirectUrl(req, new URL(await getNextUrl(mreq)))); + resp.redirect(await getRedirectUrl(req, nextUrl)); } - const middleware = this._loginMiddleware.getLoginOrSignUpMiddleware ? + const signinMiddleware = this._loginMiddleware.getLoginOrSignUpMiddleware ? this._loginMiddleware.getLoginOrSignUpMiddleware() : []; - - this.app.get('/login', ...middleware, expressWrap(redirectToLoginOrSignup.bind(this, false))); - this.app.get('/signup', ...middleware, expressWrap(redirectToLoginOrSignup.bind(this, true))); - this.app.get('/signin', ...middleware, expressWrap(redirectToLoginOrSignup.bind(this, null))); + this.app.get('/login', ...signinMiddleware, expressWrap(redirectToLoginOrSignup.bind(this, false))); + this.app.get('/signup', ...signinMiddleware, expressWrap(redirectToLoginOrSignup.bind(this, true))); + this.app.get('/signin', ...signinMiddleware, expressWrap(redirectToLoginOrSignup.bind(this, null))); if (allowTestLogin()) { // This is an endpoint for the dev environment that lets you log in as anyone. @@ -943,15 +924,18 @@ export class FlexServer implements GristServer { })); } - this.app.get('/logout', expressWrap(async (req, resp) => { - const mreq = req as RequestWithLogin; - const scopedSession = this._sessions.getOrCreateSessionFromRequest(mreq); - const redirectUrl = await this._getLogoutRedirectUrl(req, new URL(await getNextUrl(mreq))); + const logoutMiddleware = this._loginMiddleware.getLogoutMiddleware ? + this._loginMiddleware.getLogoutMiddleware() : + []; + this.app.get('/logout', ...logoutMiddleware, expressWrap(async (req, resp) => { + const scopedSession = this._sessions.getOrCreateSessionFromRequest(req); + const signedOutUrl = new URL(getOrgUrl(req) + 'signed-out'); + const redirectUrl = await this._getLogoutRedirectUrl(req, signedOutUrl); // Clear session so that user needs to log in again at the next request. // SAML logout in theory uses userSession, so clear it AFTER we compute the URL. // Express-session will save these changes. - const expressSession = mreq.session; + const expressSession = (req as RequestWithLogin).session; if (expressSession) { expressSession.users = []; expressSession.orgToUser = {}; } await scopedSession.clearScopedSession(req); // TODO: limit cache clearing to specific user. @@ -959,8 +943,8 @@ export class FlexServer implements GristServer { resp.redirect(redirectUrl); })); - // Add a static "signed-out" page. This is where logout typically lands (after redirecting - // through Cognito or SAML). + // Add a static "signed-out" page. This is where logout typically lands (e.g. after redirecting + // through SAML). this.app.get('/signed-out', expressWrap((req, resp) => this._sendAppPage(req, resp, {path: 'error.html', status: 200, config: {errPage: 'signed-out'}}))); diff --git a/app/server/lib/GristServer.ts b/app/server/lib/GristServer.ts index e2a15c77..9b14c67c 100644 --- a/app/server/lib/GristServer.ts +++ b/app/server/lib/GristServer.ts @@ -51,10 +51,10 @@ export interface GristLoginMiddleware { getLoginRedirectUrl(req: express.Request, target: URL): Promise; getSignUpRedirectUrl(req: express.Request, target: URL): Promise; getLogoutRedirectUrl(req: express.Request, nextUrl: URL): Promise; - // Optional middleware for the GET /login, /signup, and /signin routes. getLoginOrSignUpMiddleware?(): express.RequestHandler[]; - + // Optional middleware for the GET /logout route. + getLogoutMiddleware?(): express.RequestHandler[]; // Returns arbitrary string for log. addEndpoints(app: express.Express): Promise; } diff --git a/app/server/lib/MinimalLogin.ts b/app/server/lib/MinimalLogin.ts index 9fff97d6..f289fbda 100644 --- a/app/server/lib/MinimalLogin.ts +++ b/app/server/lib/MinimalLogin.ts @@ -1,7 +1,7 @@ -import { UserProfile } from 'app/common/UserAPI'; -import { GristLoginSystem, GristServer } from 'app/server/lib/GristServer'; -import { fromCallback } from 'app/server/lib/serverUtils'; -import { Request } from 'express'; +import {UserProfile} from 'app/common/UserAPI'; +import {GristLoginSystem, GristServer} from 'app/server/lib/GristServer'; +import {fromCallback} from 'app/server/lib/serverUtils'; +import {Request} from 'express'; /** * Return a login system that supports a single hard-coded user. diff --git a/app/server/lib/Permit.ts b/app/server/lib/Permit.ts index 001017c2..13c70b1b 100644 --- a/app/server/lib/Permit.ts +++ b/app/server/lib/Permit.ts @@ -49,6 +49,9 @@ export interface IPermitStore { // Close down the permit store. close(): Promise; + + // Get the permit key prefix. + getKeyPrefix(): string; } export interface IPermitStores { diff --git a/app/server/lib/TestLogin.ts b/app/server/lib/TestLogin.ts index 3462890b..a5deb9ba 100644 --- a/app/server/lib/TestLogin.ts +++ b/app/server/lib/TestLogin.ts @@ -1,5 +1,5 @@ -import { GristLoginSystem, GristServer } from 'app/server/lib/GristServer'; -import { Request } from 'express'; +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 @@ -9,9 +9,7 @@ export async function getTestLoginSystem(): Promise { 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')); + const target = new URL(gristServer.getHomeUrl(req, 'test/login')); target.searchParams.append('next', url.href); return target.href || url.href; } diff --git a/app/server/lib/extractOrg.ts b/app/server/lib/extractOrg.ts index b03478db..9e53c8af 100644 --- a/app/server/lib/extractOrg.ts +++ b/app/server/lib/extractOrg.ts @@ -131,27 +131,6 @@ export class Hosts { return this._redirectHost.bind(this); } - /** - * Returns true if `url` either points to a native Grist host (e.g. foo.getgrist.com), - * or a custom host for an existing Grist org. - */ - public async isSafeRedirectUrl(url: string): Promise { - const {host, protocol} = new URL(url); - if (!['http:', 'https:'].includes(protocol)) { return false; } - - switch (this._getHostType(host)) { - case 'native': { return true; } - case 'custom': { - const org = await mapGetOrSet(this._host2org, host, async () => { - const o = await this._dbManager.connection.manager.findOne(Organization, {host}); - return o?.domain ?? undefined; - }); - return org !== undefined; - } - default: { return false; } - } - } - public close() { this._host2org.clear(); this._org2host.clear(); diff --git a/app/server/lib/requestUtils.ts b/app/server/lib/requestUtils.ts index 144cb88a..7cf93243 100644 --- a/app/server/lib/requestUtils.ts +++ b/app/server/lib/requestUtils.ts @@ -68,8 +68,8 @@ export function addOrgToPath(req: RequestWithOrg, path: string): string { /** * Get url to the org associated with the request. */ -export function getOrgUrl(req: Request) { - return req.protocol + '://' + req.get('host') + addOrgToPathIfNeeded(req, '/'); +export function getOrgUrl(req: Request, path: string = '/') { + return req.protocol + '://' + req.get('host') + addOrgToPathIfNeeded(req, path); } /** diff --git a/package.json b/package.json index 1b5ce20f..1814ebb4 100644 --- a/package.json +++ b/package.json @@ -66,6 +66,7 @@ "mocha-webdriver": "0.2.9", "moment-locales-webpack-plugin": "^1.2.0", "nodemon": "^2.0.4", + "otplib": "12.0.1", "selenium-webdriver": "3.6.0", "source-map-loader": "^0.2.4", "stats-webpack-plugin": "^0.7.0", diff --git a/test/gen-server/seed.ts b/test/gen-server/seed.ts index 6d287402..736f67f9 100644 --- a/test/gen-server/seed.ts +++ b/test/gen-server/seed.ts @@ -210,6 +210,8 @@ const exampleUsers: {[user: string]: {[org: string]: string}} = { Fish: 'viewers', Abyss: 'owners', }, + // User Ham has two-factor authentication enabled on staging/prod. + Ham: {}, // User support@ owns a workspace "Examples & Templates" in its personal org. It can be shared // with everyone@ to let all users see it (this is not done here to avoid impacting all tests). Support: { Supportland: 'owners' }, diff --git a/test/nbrowser/ActionLog.ts b/test/nbrowser/ActionLog.ts index 214bc61f..98f2ec79 100644 --- a/test/nbrowser/ActionLog.ts +++ b/test/nbrowser/ActionLog.ts @@ -24,7 +24,7 @@ describe('ActionLog', function() { } before(async function() { - const session = await gu.session().login(); + const session = await gu.session().user('user4').login(); await session.tempDoc(cleanup, 'Hello.grist'); await gu.dismissWelcomeTourIfNeeded(); }); diff --git a/test/nbrowser/HomeIntro.ts b/test/nbrowser/HomeIntro.ts index 6cbacdc2..d0e1e3a8 100644 --- a/test/nbrowser/HomeIntro.ts +++ b/test/nbrowser/HomeIntro.ts @@ -27,9 +27,9 @@ describe('HomeIntro', function() { const signUp = await driver.findContent('.test-welcome-text a', 'sign up'); assert.include(await signUp.getAttribute('href'), '/signin'); - // Check that the link takes us to a login page (either Cognito or Grist, depending on session). + // Check that the link takes us to a Grist login page. await signUp.click(); - await gu.checkSigninPage(); + await gu.checkLoginPage(); await driver.navigate().back(); await gu.waitForDocMenuToLoad(); }); diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts index d3a9754f..b770f77f 100644 --- a/test/nbrowser/gristUtils.ts +++ b/test/nbrowser/gristUtils.ts @@ -41,9 +41,9 @@ export const simulateLogin = homeUtil.simulateLogin.bind(homeUtil); export const removeLogin = homeUtil.removeLogin.bind(homeUtil); export const setValue = homeUtil.setValue.bind(homeUtil); export const isOnLoginPage = homeUtil.isOnLoginPage.bind(homeUtil); +export const isOnGristLoginPage = homeUtil.isOnLoginPage.bind(homeUtil); export const checkLoginPage = homeUtil.checkLoginPage.bind(homeUtil); export const checkGristLoginPage = homeUtil.checkGristLoginPage.bind(homeUtil); -export const checkSigninPage = homeUtil.checkSigninPage.bind(homeUtil); export const fixturesRoot: string = testUtils.fixturesRoot; @@ -1333,6 +1333,7 @@ export enum TestUserEnum { user1 = 'chimpy', user2 = 'charon', user3 = 'kiwi', + user4 = 'ham', owner = 'chimpy', anon = 'anon', support = 'support', diff --git a/test/nbrowser/homeUtil.ts b/test/nbrowser/homeUtil.ts index 77abdcc5..23705852 100644 --- a/test/nbrowser/homeUtil.ts +++ b/test/nbrowser/homeUtil.ts @@ -6,6 +6,7 @@ import * as fse from 'fs-extra'; import defaults = require('lodash/defaults'); import {WebElement} from 'mocha-webdriver'; import fetch from 'node-fetch'; +import {authenticator} from 'otplib'; import * as path from 'path'; import {WebDriver} from 'selenium-webdriver'; @@ -46,7 +47,7 @@ export class HomeUtil { * (after having been created if necessary), so that their home api can be later * instantiated without page loads. * When testing against an external server, the simulated login is in fact genuine, - * done via Cognito. + * done via the Grist login page. */ public async simulateLogin(name: string, email: string, org: string = "", options: { loginMethod?: UserProfile['loginMethod'], @@ -75,26 +76,22 @@ export class HomeUtil { } // Make sure we revisit page in case login is changing. await this.driver.get('about:blank'); - // When running against an external server, we log in through Cognito. + // When running against an external server, we log in through the Grist login page. await this.driver.get(this.server.getUrl(org, "")); - if (!(await this.isOnSigninPage())) { + if (!await this.isOnLoginPage()) { // Explicitly click sign-in link if necessary. await this.driver.findWait('.test-user-signin', 4000).click(); await this.driver.findContentWait('.grist-floating-menu a', 'Sign in', 500).click(); } - // Check if we need to switch to Cognito login from the Grist sign-up page. - if (await this.isOnGristLoginPage()) { - await this.driver.findWait('a[href*="login?"]', 4000).click(); - } - // Fill the login form (either on /test/login or Cognito). + // Fill the login form (either test or Grist). if (await this.isOnTestLoginPage()) { await this.fillTestLoginForm(email, name); } else { - await this.fillLoginForm(email); + await this.fillGristLoginForm(email); } - if (!(await this.isWelcomePage()) && (options.freshAccount || options.isFirstLogin)) { + if (!await this.isWelcomePage() && (options.freshAccount || options.isFirstLogin)) { await this._recreateCurrentUser(email, org, name); } } @@ -148,29 +145,43 @@ export class HomeUtil { await form.find('input[name="signInSubmitButton"]').click(); } - // Fill up a Cognito login page. If on a signup page, switch to a login page. - // TEST_ACCOUNT_PASSWORD must be set, or a password provided. Should be on a Cognito - // login/signup page before calling this method. - public async fillLoginForm(email: string, password?: string) { + /** + * Fill up the Grist login page form, and submit. If called with a user that + * has TOTP-based 2FA enabled, TEST_ACCOUNT_TOTP_SECRET must be set for a valid + * code to be submitted on the following form. + * + * Should be on the Grist login or sign-up page before calling this method. If + * `password` is not passed in, TEST_ACCOUNT_PASSWORD must be set. + */ + public async fillGristLoginForm(email: string, password?: string) { if (!password) { password = process.env.TEST_ACCOUNT_PASSWORD; if (!password) { throw new Error('TEST_ACCOUNT_PASSWORD not set'); } } - await this.checkLoginPage(); + await this.checkGristLoginPage(); if ((await this.driver.getCurrentUrl()).match(/signup\?/)) { await this.driver.findWait('a[href*="login?"]', 4000).click(); } - // There are two login forms, one hidden, one shown. Pick the one that is shown. - const block = - (await this.driver.find('div.modal-content-desktop').isDisplayed()) ? - (await this.driver.find('div.modal-content-desktop')) : - (await this.driver.find('div.modal-content-mobile')); - await block.findWait('input[name="username"]', 4000); - await this.setValue(block.findWait('input[name="username"]', 4000), email); - await this.setValue(block.findWait('input[name="password"]', 4000), password); - await block.find('input[name="signInSubmitButton"]').click(); + + await this.driver.findWait('input[name="email"]', 4000).sendKeys(email); + await this.driver.find('input[name="password"]').sendKeys(password); + await this.driver.find('.test-lp-sign-in').click(); + await this.driver.wait(async () => !await this.isOnGristLoginPage(), 4000); + if (!await this.driver.findContent('.test-mfa-title', 'Almost there!').isPresent()) { + return; + } + + const secret = process.env.TEST_ACCOUNT_TOTP_SECRET; + if (!secret) { throw new Error('TEST_ACCOUNT_TOTP_SECRET not set'); } + + const code = authenticator.generate(secret); + await this.driver.find('input[name="verificationCode"]').sendKeys(code); + await this.driver.find('.test-mfa-submit').click(); + await this.driver.wait(async () => { + return !await this.driver.findContent('.test-mfa-title', 'Almost there!').isPresent(); + }, 4000); } /** @@ -275,18 +286,19 @@ export class HomeUtil { } /** - * Returns whether we are currently on the Cognito or test login page. + * Returns whether we are currently on any login page (including the test page). */ public async isOnLoginPage() { - const url = await this.driver.getCurrentUrl(); - return /^https:\/\/gristlogin/.test(url) || await this.isOnTestLoginPage(); + return await this.isOnGristLoginPage() || await this.isOnTestLoginPage(); } /** * Returns whether we are currently on a Grist login page. */ public async isOnGristLoginPage() { - return /^https:\/\/login(-s)?\.getgrist\.com/.test(await this.driver.getCurrentUrl()); + const isOnSignupPage = await this.driver.find('.test-sp-heading').isPresent(); + const isOnLoginPage = await this.driver.find('.test-lp-heading').isPresent(); + return isOnSignupPage || isOnLoginPage; } /** @@ -297,14 +309,7 @@ export class HomeUtil { } /** - * Returns whether we are currently on any sign-in page (e.g. Cognito, Grist, test). - */ - public async isOnSigninPage() { - return await this.isOnLoginPage() || await this.isOnGristLoginPage(); - } - - /** - * Waits for browser to navigate to the Cognito login page. + * Waits for browser to navigate to any login page (including the test page). */ public async checkLoginPage(waitMs: number = 2000) { await this.driver.wait(this.isOnLoginPage.bind(this), waitMs); @@ -317,13 +322,6 @@ export class HomeUtil { await this.driver.wait(this.isOnGristLoginPage.bind(this), waitMs); } - /** - * Waits for browser to navigate to any sign-in page (e.g. Cognito, Grist, test). - */ - public async checkSigninPage(waitMs: number = 4000) { - await this.driver.wait(this.isOnSigninPage.bind(this), waitMs); - } - /** * Delete and recreate the user, via the specified org. The specified user must be * currently logged in! @@ -335,11 +333,11 @@ export class HomeUtil { await this.driver.findWait('.test-user-signin', 4000).click(); await this.driver.findContentWait('.grist-floating-menu a', 'Sign in', 500).click(); await this.checkLoginPage(); - // Fill the login form (either on /test/login or Cognito). + // Fill the login form (either test or Grist). if (await this.isOnTestLoginPage()) { await this.fillTestLoginForm(email, name); } else { - await this.fillLoginForm(email); + await this.fillGristLoginForm(email); } } diff --git a/yarn.lock b/yarn.lock index fb576dd5..7143e6b2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -85,6 +85,44 @@ node-addon-api "2.0.0" node-pre-gyp "^0.11.0" +"@otplib/core@^12.0.1": + version "12.0.1" + resolved "https://registry.yarnpkg.com/@otplib/core/-/core-12.0.1.tgz#73720a8cedce211fe5b3f683cd5a9c098eaf0f8d" + integrity sha512-4sGntwbA/AC+SbPhbsziRiD+jNDdIzsZ3JUyfZwjtKyc/wufl1pnSIaG4Uqx8ymPagujub0o92kgBnB89cuAMA== + +"@otplib/plugin-crypto@^12.0.1": + version "12.0.1" + resolved "https://registry.yarnpkg.com/@otplib/plugin-crypto/-/plugin-crypto-12.0.1.tgz#2b42c624227f4f9303c1c041fca399eddcbae25e" + integrity sha512-qPuhN3QrT7ZZLcLCyKOSNhuijUi9G5guMRVrxq63r9YNOxxQjPm59gVxLM+7xGnHnM6cimY57tuKsjK7y9LM1g== + dependencies: + "@otplib/core" "^12.0.1" + +"@otplib/plugin-thirty-two@^12.0.1": + version "12.0.1" + resolved "https://registry.yarnpkg.com/@otplib/plugin-thirty-two/-/plugin-thirty-two-12.0.1.tgz#5cc9b56e6e89f2a1fe4a2b38900ca4e11c87aa9e" + integrity sha512-MtT+uqRso909UkbrrYpJ6XFjj9D+x2Py7KjTO9JDPhL0bJUYVu5kFP4TFZW4NFAywrAtFRxOVY261u0qwb93gA== + dependencies: + "@otplib/core" "^12.0.1" + thirty-two "^1.0.2" + +"@otplib/preset-default@^12.0.1": + version "12.0.1" + resolved "https://registry.yarnpkg.com/@otplib/preset-default/-/preset-default-12.0.1.tgz#cb596553c08251e71b187ada4a2246ad2a3165ba" + integrity sha512-xf1v9oOJRyXfluBhMdpOkr+bsE+Irt+0D5uHtvg6x1eosfmHCsCC6ej/m7FXiWqdo0+ZUI6xSKDhJwc8yfiOPQ== + dependencies: + "@otplib/core" "^12.0.1" + "@otplib/plugin-crypto" "^12.0.1" + "@otplib/plugin-thirty-two" "^12.0.1" + +"@otplib/preset-v11@^12.0.1": + version "12.0.1" + resolved "https://registry.yarnpkg.com/@otplib/preset-v11/-/preset-v11-12.0.1.tgz#4c7266712e7230500b421ba89252963c838fc96d" + integrity sha512-9hSetMI7ECqbFiKICrNa4w70deTUfArtwXykPUvSHWOdzOlfa9ajglu7mNCntlvxycTiOAXkQGwjQCzzDEMRMg== + dependencies: + "@otplib/core" "^12.0.1" + "@otplib/plugin-crypto" "^12.0.1" + "@otplib/plugin-thirty-two" "^12.0.1" + "@popperjs/core@2.3.3": version "2.3.3" resolved "https://registry.yarnpkg.com/@popperjs/core/-/core-2.3.3.tgz#8731722aeb7330e8fd9eb5d424be6b98dea7d6da" @@ -5293,6 +5331,15 @@ osenv@^0.1.4: os-homedir "^1.0.0" os-tmpdir "^1.0.0" +otplib@12.0.1: + version "12.0.1" + resolved "https://registry.yarnpkg.com/otplib/-/otplib-12.0.1.tgz#c1d3060ab7aadf041ed2960302f27095777d1f73" + integrity sha512-xDGvUOQjop7RDgxTQ+o4pOol0/3xSZzawTiPKRrHnQWAy0WjhNs/5HdIDJCrqC4MBynmjXgULc6YfioaxZeFgg== + dependencies: + "@otplib/core" "^12.0.1" + "@otplib/preset-default" "^12.0.1" + "@otplib/preset-v11" "^12.0.1" + p-cancelable@^1.0.0: version "1.1.0" resolved "https://registry.yarnpkg.com/p-cancelable/-/p-cancelable-1.1.0.tgz#d078d15a3af409220c886f1d9a0ca2e441ab26cc" @@ -6946,6 +6993,11 @@ thenify-all@^1.0.0: dependencies: any-promise "^1.0.0" +thirty-two@^1.0.2: + version "1.0.2" + resolved "https://registry.yarnpkg.com/thirty-two/-/thirty-two-1.0.2.tgz#4ca2fffc02a51290d2744b9e3f557693ca6b627a" + integrity sha1-TKL//AKlEpDSdEueP1V2k8prYno= + throttleit@0.0.2: version "0.0.2" resolved "https://registry.yarnpkg.com/throttleit/-/throttleit-0.0.2.tgz#cfedf88e60c00dd9697b61fdd2a8343a9b680eaf"