(core) Implement DiscourseConnect to enable easy sign-in to community forum

Summary:
- Update cookie module, to support modern sameSite settings
- Add a new cookie, grist_sid_status with less-sensitive value, to let less-trusted subdomains know if user is signed in
- The new cookie is kept in-sync with the session cookie.
- For a user signed in once, allow auto-signin is appropriate.
- For a user signed in with multiple accounts, show a page to select which account to use.
- Move css stylings for rendering users to a separate module.

Test Plan: Added a test case with a simulated Discourse page to test redirects and account-selection page.

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D3047
This commit is contained in:
Dmitry S
2021-10-01 10:24:23 -04:00
parent b3b7410ede
commit 1517dca644
18 changed files with 423 additions and 165 deletions

View File

@@ -83,6 +83,7 @@ declare module "mime-types";
declare module "morgan";
declare module "cookie";
declare module "cookie-parser";
declare module "on-headers";
declare module "@gristlabs/express-session";
// Used for command line path tweaks.

View File

@@ -6,14 +6,17 @@ import {canEdit, canView, getWeakestRole, Role} from 'app/common/roles';
import {Document} from 'app/gen-server/entity/Document';
import {User} from 'app/gen-server/entity/User';
import {DocAuthKey, DocAuthResult, HomeDBManager} from 'app/gen-server/lib/HomeDBManager';
import {getSessionProfiles, getSessionUser, linkOrgWithEmail, SessionObj,
SessionUserObj} from 'app/server/lib/BrowserSession';
import {getSessionProfiles, getSessionUser, getSignInStatus, linkOrgWithEmail, SessionObj,
SessionUserObj, SignInStatus} from 'app/server/lib/BrowserSession';
import {RequestWithOrg} from 'app/server/lib/extractOrg';
import {COOKIE_MAX_AGE, getAllowedOrgForSessionID} from 'app/server/lib/gristSessions';
import {COOKIE_MAX_AGE, getAllowedOrgForSessionID, getCookieDomain,
cookieName as sessionCookieName} from 'app/server/lib/gristSessions';
import * as log from 'app/server/lib/log';
import {IPermitStore, Permit} from 'app/server/lib/Permit';
import {allowHost} from 'app/server/lib/requestUtils';
import * as cookie from 'cookie';
import {NextFunction, Request, RequestHandler, Response} from 'express';
import * as onHeaders from 'on-headers';
export interface RequestWithLogin extends Request {
sessionID: string;
@@ -189,7 +192,7 @@ export async function addRequestUser(dbManager: HomeDBManager, permitStore: IPer
// See if we have a profile linked with the active organization already.
// TODO: implement userSelector for rest API, to allow "sticky" user selection on pages.
let sessionUser: SessionUserObj|null = getSessionUser(session, mreq.org, '');
let sessionUser: SessionUserObj|null = getSessionUser(session, mreq.org, mreq.query.user || '');
if (!sessionUser) {
// No profile linked yet, so let's elect one.
@@ -497,3 +500,38 @@ export function getTransitiveHeaders(req: Request): {[key: string]: string} {
...(Origin ? { Origin } : undefined),
};
}
export const signInStatusCookieName = sessionCookieName + '_status';
// We expose a sign-in status in a cookie accessible to all subdomains, to assist in auto-signin.
// Its value is SignInStatus ("S", "M" or unset). This middleware keeps this cookie in sync with
// the session state.
//
// Note that this extra cookie isn't strictly necessary today: since it has similar settings to
// the session cookie, subdomains can infer status from that one. It is here in anticipation that
// we make sessions a host-only cookie, to avoid exposing it to externally-hosted subdomains of
// getgrist.com. In that case, the sign-in status cookie would remain a 2nd-level domain cookie.
export function signInStatusMiddleware(req: Request, resp: Response, next: NextFunction) {
const mreq = req as RequestWithLogin;
let origSignInStatus: SignInStatus = '';
if (req.headers.cookie) {
const cookies = cookie.parse(req.headers.cookie);
origSignInStatus = cookies[signInStatusCookieName] || '';
}
onHeaders(resp, () => {
const newSignInStatus = getSignInStatus(mreq.session);
if (newSignInStatus !== origSignInStatus) {
// If not signed-in any more, set a past date to delete this cookie.
const expires = (newSignInStatus && mreq.session.cookie.expires) || new Date(0);
resp.append('Set-Cookie', cookie.serialize(signInStatusCookieName, newSignInStatus, {
httpOnly: false, // make available to client-side scripts
expires,
domain: getCookieDomain(req),
sameSite: 'lax', // same setting as for grist-sid is fine here.
}));
}
});
next();
}

View File

@@ -2,6 +2,8 @@ import {normalizeEmail} from 'app/common/emails';
import {UserProfile} from 'app/common/LoginSessionAPI';
import {SessionStore} from 'app/server/lib/gristSessions';
import * as log from 'app/server/lib/log';
import {fromCallback} from 'app/server/lib/serverUtils';
import {Request} from 'express';
// Part of a session related to a single user.
export interface SessionUserObj {
@@ -47,6 +49,18 @@ export interface SessionObj {
alive?: boolean;
}
// We expose a sign-in status in a cookie accessible to all subdomains, to assist in auto-signin.
// The values are:
// - "S": the user is signed in once; in this case an automatic signin can be unambiguous and seamless.
// - "M": the user is signed in multiple times.
// - "": the user is not signed in.
export type SignInStatus = 'S'|'M'|'';
export function getSignInStatus(sessionObj: SessionObj|null): SignInStatus {
const length = sessionObj?.users?.length;
return !length ? "" : (length === 1 ? 'S' : 'M');
}
/**
* Extract the available user profiles from the session.
*
@@ -146,14 +160,14 @@ export class ScopedSession {
// email addresses. This will update the one with a matching email address, or add a new one.
// This is mainly used to know which emails are logged in in this session; fields like name and
// picture URL come from the database instead.
public async updateUserProfile(profile: UserProfile|null): Promise<void> {
public async updateUserProfile(req: Request, profile: UserProfile|null): Promise<void> {
if (profile) {
await this.operateOnScopedSession(async user => {
await this.operateOnScopedSession(req, async user => {
user.profile = profile;
return user;
});
} else {
await this.clearScopedSession();
await this.clearScopedSession(req);
}
}
@@ -169,16 +183,16 @@ export class ScopedSession {
* @return a pair [prev, current] with the state of the single user entry before and after the operation.
*
*/
public async operateOnScopedSession(op: (user: SessionUserObj) =>
public async operateOnScopedSession(req: Request, op: (user: SessionUserObj) =>
Promise<SessionUserObj>): Promise<[SessionUserObj, SessionUserObj]> {
const session = await this._getSession();
const user = await this.getScopedSession(session);
const oldUser = JSON.parse(JSON.stringify(user)); // Old version to compare against.
const newUser = await op(JSON.parse(JSON.stringify(user))); // Modify a scratch version.
if (Object.keys(newUser).length === 0) {
await this.clearScopedSession(session);
await this.clearScopedSession(req, session);
} else {
await this._updateScopedSession(newUser, session);
await this._updateScopedSession(req, newUser, session);
}
return [oldUser, newUser];
}
@@ -187,10 +201,10 @@ export class ScopedSession {
* This clears the current user entry from the session.
* @param prev: if supplied, this session object is used rather than querying the session again.
*/
public async clearScopedSession(prev?: SessionObj): Promise<void> {
public async clearScopedSession(req: Request, prev?: SessionObj): Promise<void> {
const session = prev || await this._getSession();
this._clearUser(session);
await this._setSession(session);
await this._setSession(req, session);
}
/**
@@ -206,10 +220,14 @@ export class ScopedSession {
/**
* Set the session to the supplied object.
*/
private async _setSession(session: SessionObj): Promise<void> {
private async _setSession(req: Request, session: SessionObj): Promise<void> {
try {
await this._sessionStore.setAsync(this._sessionId, session);
if (!this._live) { this._sessionCache = session; }
const reqSession = (req as any).session;
if (reqSession?.reload) {
await fromCallback(cb => reqSession.reload(cb));
}
} catch (e) {
// (I've copied this from old code, not sure if continuing after a session save error is
// something existing code depends on?)
@@ -224,7 +242,7 @@ export class ScopedSession {
* @param prev: if supplied, this session object is used rather than querying the session again.
*
*/
private async _updateScopedSession(user: SessionUserObj, prev?: SessionObj): Promise<void> {
private async _updateScopedSession(req: Request, user: SessionUserObj, prev?: SessionObj): Promise<void> {
const profile = user.profile;
if (!profile) {
throw new Error("No profile available");
@@ -242,7 +260,7 @@ export class ScopedSession {
if (index < 0) { index = session.users.length; }
session.orgToUser[this._org] = index;
session.users[index] = user;
await this._setSession(session);
await this._setSession(req, session);
}
/**

View File

@@ -0,0 +1,107 @@
/**
* Endpoint to support DiscourseConnect, to allow users to use their Grist logins for the
* Grist Community Forum.
*
* Adds one endpoint:
* - /discourse-connect: sends signed user info in a redirect to DISCOURSE_SITE.
*
* Expects environment variables:
* - DISCOURSE_SITE: URL of the Discourse site to which to redirect back.
* - DISCOURSE_CONNECT_SECRET: Secret for checking and adding signatures.
*
* This follows documentation at
* https://meta.discourse.org/t/discourseconnect-official-single-sign-on-for-discourse-sso/13045
* The recommended Discourse configuration includes:
* - enable discourse connect: true
* - discourse connect url: GRIST_SITE/discourse-connect
* - discourse connect secret: DISCOURSE_CONNECT_SECRET
* - logout redirect (in Users): GRIST_SITE/logout?next=DISCOURSE_SITE
*/
import type {Express, NextFunction, Request, RequestHandler, Response} from 'express';
import type {RequestWithLogin} from 'app/server/lib/Authorizer';
import {expressWrap} from 'app/server/lib/expressWrap';
import * as crypto from 'crypto';
const DISCOURSE_CONNECT_SECRET = process.env.DISCOURSE_CONNECT_SECRET;
const DISCOURSE_SITE = process.env.DISCOURSE_SITE;
// A hook for dependency injection. Allows tests to override these variables on the fly.
export const Deps = {DISCOURSE_CONNECT_SECRET, DISCOURSE_SITE};
// Calculate payload signature using the given secret.
function calcSignature(payload: string, secret: string) {
return crypto.createHmac('sha256', secret).update(payload).digest('hex');
}
// Check configuration and signature of the Discourse nonce in the request.
function checkParams(req: Request, resp: Response, next: NextFunction) {
if (!Deps.DISCOURSE_SITE || !Deps.DISCOURSE_CONNECT_SECRET) {
throw new Error('DiscourseConnect not configured');
}
const payload = String(req.query.sso || '');
const signature = String(req.query.sig || '');
if (calcSignature(payload, Deps.DISCOURSE_CONNECT_SECRET) !== signature) {
throw new Error('Invalid signature for Discourse SSO request');
}
const params = new URLSearchParams(Buffer.from(payload, 'base64').toString('utf8'));
const nonce = params.get('nonce');
if (!nonce) {
throw new Error('Invalid request for Discourse SSO');
}
(req as any).discourseConnectNonce = nonce;
next();
}
// Respond to the DiscourseConnect request by redirecting back to discourse, including the user
// info and a signature into the URL parameters.
function discourseConnect(req: Request, resp: Response) {
const mreq = req as RequestWithLogin;
const nonce: string|undefined = (req as any).discourseConnectNonce;
if (!nonce) {
throw new Error('Invalid request for Discourse SSO');
}
if (!mreq.userIsAuthorized || !mreq.user?.loginEmail) {
throw new Error('User is not authenticated');
}
if (!req.query.user && mreq.users && mreq.users.length > 1) {
const origUrl = new URL(req.originalUrl, `${req.protocol}://${req.get('host')}`);
const redirectUrl = new URL('/welcome/select-account', `${req.protocol}://${req.get('host')}`);
redirectUrl.searchParams.set('next', origUrl.toString());
return resp.redirect(redirectUrl.toString());
}
const responseObj: {[key: string]: string} = {
nonce,
email: mreq.user.loginEmail,
// We don't treat user IDs as secret, so use the same ID with Discourse directly.
external_id: String(mreq.user.id),
// We could specify the username (used for @ mentions), but let Discourse create one for us
// (it bases it on name or email). The user can change it within Discourse.
// username,
name: mreq.user.name,
...(mreq.user.picture ? {avatar_url: mreq.user.picture} : {}),
suppress_welcome_message: 'true',
};
const responseString = new URLSearchParams(responseObj).toString();
const responsePayload = Buffer.from(responseString, 'utf8').toString('base64');
const responseSignature = calcSignature(responsePayload, Deps.DISCOURSE_CONNECT_SECRET!);
const redirectUrl = new URL('/session/sso_login', Deps.DISCOURSE_SITE);
redirectUrl.search = new URLSearchParams({sso: responsePayload, sig: responseSignature}).toString();
return resp.redirect(redirectUrl.toString());
}
/**
* Attach the endpoint for /discourse-connect, as documented at
* https://meta.discourse.org/t/discourseconnect-official-single-sign-on-for-discourse-sso/13045
*/
export function addDiscourseConnectEndpoints(app: Express, options: {
userIdMiddleware: RequestHandler,
redirectToLogin: RequestHandler,
}) {
app.get('/discourse-connect',
expressWrap(checkParams), // Check early, to fail early if Discourse is misconfigured.
options.userIdMiddleware,
options.redirectToLogin,
expressWrap(discourseConnect)
);
}

View File

@@ -20,9 +20,10 @@ import {Usage} from 'app/gen-server/lib/Usage';
import {attachAppEndpoint} from 'app/server/lib/AppEndpoint';
import {addRequestUser, getUser, getUserId, isSingleUserMode,
redirectToLoginUnconditionally} from 'app/server/lib/Authorizer';
import {redirectToLogin, RequestWithLogin} from 'app/server/lib/Authorizer';
import {redirectToLogin, RequestWithLogin, signInStatusMiddleware} from 'app/server/lib/Authorizer';
import * as Comm from 'app/server/lib/Comm';
import {create} from 'app/server/lib/create';
import {addDiscourseConnectEndpoints} from 'app/server/lib/DiscourseConnect';
import {addDocApiRoutes} from 'app/server/lib/DocApi';
import {DocManager} from 'app/server/lib/DocManager';
import {DocStorageManager} from 'app/server/lib/DocStorageManager';
@@ -30,6 +31,7 @@ import {DocWorker} from 'app/server/lib/DocWorker';
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 {initGristSessions, SessionStore} from 'app/server/lib/gristSessions';
import {HostedStorageManager} from 'app/server/lib/HostedStorageManager';
@@ -63,7 +65,6 @@ import {AddressInfo} from 'net';
import fetch from 'node-fetch';
import * as path from 'path';
import * as serveStatic from "serve-static";
import { addGoogleAuthEndpoint } from "app/server/lib/GoogleAuth";
// Health checks are a little noisy in the logs, so we don't show them all.
// We show the first N health checks:
@@ -588,6 +589,7 @@ export class FlexServer implements GristServer {
// Create the sessionStore and related objects.
const {sessions, sessionMiddleware, sessionStore} = initGristSessions(this.instanceRoot, this);
this.app.use(sessionMiddleware);
this.app.use(signInStatusMiddleware);
// Create an endpoint for making cookies during testing.
this.app.get('/test/session', async (req, res) => {
@@ -833,7 +835,7 @@ export class FlexServer implements GristServer {
email: optStringParam(req.query.email) || 'chimpy@getgrist.com',
name: optStringParam(req.query.name) || 'Chimpy McBanana',
};
await scopedSession.updateUserProfile(profile);
await scopedSession.updateUserProfile(req, profile);
res.send(`<!doctype html>
<html><body>
<p>Logged in as ${JSON.stringify(profile)}.<p>
@@ -859,7 +861,7 @@ export class FlexServer implements GristServer {
// Express-session will save these changes.
const expressSession = (req as any).session;
if (expressSession) { expressSession.users = []; expressSession.orgToUser = {}; }
await scopedSession.clearScopedSession();
await scopedSession.clearScopedSession(req);
resp.redirect(redirectUrl);
}));
@@ -875,6 +877,11 @@ export class FlexServer implements GristServer {
const comment = await this._loginMiddleware.addEndpoints(this.app);
this.info.push(['loginMiddlewareComment', comment]);
addDiscourseConnectEndpoints(this.app, {
userIdMiddleware: this._userIdMiddleware,
redirectToLogin: this._redirectToLoginWithoutExceptionsMiddleware,
});
}
public async addTestingHooks(workerServers?: FlexServer[]) {

View File

@@ -18,6 +18,7 @@ export const ITestingHooks = t.iface([], {
"flushAuthorizerCache": t.func("void"),
"getDocClientCounts": t.func(t.array(t.tuple("string", "number"))),
"setActiveDocTimeout": t.func("number", t.param("seconds", "number")),
"setDiscourseConnectVar": t.func(t.union("string", "null"), t.param("varName", "string"), t.param("value", t.union("string", "null"))),
});
const exportedTypeSuite: t.ITypeSuite = {

View File

@@ -14,4 +14,5 @@ export interface ITestingHooks {
flushAuthorizerCache(): Promise<void>;
getDocClientCounts(): Promise<Array<[string, number]>>;
setActiveDocTimeout(seconds: number): Promise<number>;
setDiscourseConnectVar(varName: string, value: string|null): Promise<string|null>;
}

View File

@@ -47,7 +47,7 @@ export async function getMinimalLoginSystem(): Promise<GristLoginSystem> {
*/
async function setSingleUser(req: Request, gristServer: GristServer) {
const scopedSession = gristServer.getSessions().getOrCreateSessionFromRequest(req);
await scopedSession.operateOnScopedSession(async (user) => Object.assign(user, {
await scopedSession.operateOnScopedSession(req, async (user) => Object.assign(user, {
profile: getDefaultProfile()
}));
}

View File

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

View File

@@ -2,9 +2,11 @@ import * as net from 'net';
import {UserProfile} from 'app/common/LoginSessionAPI';
import {Deps as ActiveDocDeps} from 'app/server/lib/ActiveDoc';
import {Deps as DiscourseConnectDeps} from 'app/server/lib/DiscourseConnect';
import * as Comm from 'app/server/lib/Comm';
import * as log from 'app/server/lib/log';
import {IMessage, Rpc} from 'grain-rpc';
import {Request} from 'express';
import * as t from 'ts-interface-checker';
import {FlexServer} from './FlexServer';
import {ITestingHooks} from './ITestingHooks';
@@ -74,7 +76,8 @@ export class TestingHooks implements ITestingHooks {
log.info("TestingHooks.setLoginSessionProfile called with", gristSidCookie, profile, org);
const sessionId = this._comm.getSessionIdFromCookie(gristSidCookie);
const scopedSession = this._comm.getOrCreateSession(sessionId, {org});
return await scopedSession.updateUserProfile(profile);
const req = {} as Request;
return await scopedSession.updateUserProfile(req, profile);
}
public async setServerVersion(version: string|null): Promise<void> {
@@ -180,4 +183,15 @@ export class TestingHooks implements ITestingHooks {
return prev;
}
// Sets env vars for the DiscourseConnect module, and returns the previous value.
public async setDiscourseConnectVar(varName: string, value: string|null): Promise<string|null> {
const key = varName as keyof typeof DiscourseConnectDeps;
const prev = DiscourseConnectDeps[key] || null;
if (value == null) {
delete DiscourseConnectDeps[key];
} else {
DiscourseConnectDeps[key] = value;
}
return prev;
}
}

View File

@@ -100,22 +100,6 @@ export function initGristSessions(instanceRoot: string, server: GristServer) {
const sessionStoreCreator = createSessionStoreFactory(sessionsDB);
const sessionStore = sessionStoreCreator();
const adaptDomain = process.env.GRIST_ADAPT_DOMAIN === 'true';
const fixedDomain = process.env.GRIST_SESSION_DOMAIN || process.env.GRIST_DOMAIN;
const getCookieDomain = (req: express.Request) => {
const mreq = req as RequestWithOrg;
if (mreq.isCustomHost) {
// For custom hosts, omit the domain to make it a "host-only" cookie, to avoid it being
// included into subdomain requests (since we would not control all the subdomains).
return undefined;
}
if (adaptDomain) {
const reqDomain = parseSubdomain(req.get('host'));
if (reqDomain.base) { return reqDomain.base.split(':')[0]; }
}
return fixedDomain;
};
// Use a separate session IDs for custom domains than for native ones. Because a custom domain
// cookie could be stolen (with some effort) by the custom domain's owner, we limit the damage
// by only honoring custom-domain cookies for requests to that domain.
@@ -148,5 +132,23 @@ export function initGristSessions(instanceRoot: string, server: GristServer) {
const sessions = new Sessions(sessionSecret, sessionStore);
return {sessions, sessionSecret, sessionStore, sessionMiddleware, sessionStoreCreator};
return {sessions, sessionSecret, sessionStore, sessionMiddleware};
}
export function getCookieDomain(req: express.Request) {
const mreq = req as RequestWithOrg;
if (mreq.isCustomHost) {
// For custom hosts, omit the domain to make it a "host-only" cookie, to avoid it being
// included into subdomain requests (since we would not control all the subdomains).
return undefined;
}
const adaptDomain = process.env.GRIST_ADAPT_DOMAIN === 'true';
const fixedDomain = process.env.GRIST_SESSION_DOMAIN || process.env.GRIST_DOMAIN;
if (adaptDomain) {
const reqDomain = parseSubdomain(req.get('host'));
if (reqDomain.base) { return reqDomain.base.split(':')[0]; }
}
return fixedDomain;
}