From 1517dca644052cb22919feafd185dcfd76298b1d Mon Sep 17 00:00:00 2001 From: Dmitry S Date: Fri, 1 Oct 2021 10:24:23 -0400 Subject: [PATCH] (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 --- app/client/aclui/ACLUsers.ts | 13 +-- app/client/ui/BillingPlanManagers.ts | 29 +++---- app/client/ui/UserItem.ts | 120 +++++++++++++++++++++++++++ app/client/ui/UserManager.ts | 107 +----------------------- app/client/ui/WelcomePage.ts | 48 ++++++++++- app/common/gristUrls.ts | 2 +- app/server/declarations.d.ts | 1 + app/server/lib/Authorizer.ts | 46 +++++++++- app/server/lib/BrowserSession.ts | 40 ++++++--- app/server/lib/DiscourseConnect.ts | 107 ++++++++++++++++++++++++ app/server/lib/FlexServer.ts | 15 +++- app/server/lib/ITestingHooks-ti.ts | 1 + app/server/lib/ITestingHooks.ts | 1 + app/server/lib/MinimalLogin.ts | 2 +- app/server/lib/SamlConfig.ts | 2 +- app/server/lib/TestingHooks.ts | 16 +++- app/server/lib/gristSessions.ts | 36 ++++---- test/nbrowser/testServer.ts | 2 + 18 files changed, 423 insertions(+), 165 deletions(-) create mode 100644 app/client/ui/UserItem.ts create mode 100644 app/server/lib/DiscourseConnect.ts diff --git a/app/client/aclui/ACLUsers.ts b/app/client/aclui/ACLUsers.ts index a342cec1..e1e7bf10 100644 --- a/app/client/aclui/ACLUsers.ts +++ b/app/client/aclui/ACLUsers.ts @@ -2,7 +2,8 @@ import {copyToClipboard} from 'app/client/lib/copyToClipboard'; import {DocPageModel} from 'app/client/models/DocPageModel'; import {urlState} from 'app/client/models/gristUrlState'; import {createUserImage} from 'app/client/ui/UserImage'; -import * as um from 'app/client/ui/UserManager'; +import {cssMemberImage, cssMemberListItem, cssMemberPrimary, + cssMemberSecondary, cssMemberText} from 'app/client/ui/UserItem'; import {basicButton, basicButtonLink} from 'app/client/ui2018/buttons'; import {colors, testId} from 'app/client/ui2018/cssVars'; import {icon} from 'app/client/ui2018/icons'; @@ -24,14 +25,14 @@ const roleNames: {[role: string]: string} = { function buildUserRow(user: UserAccessData, currentUser: FullUser|null, ctl: IOpenController) { const isCurrentUser = Boolean(currentUser && user.id === currentUser.id); return cssUserItem( - um.cssMemberImage( + cssMemberImage( createUserImage(user, 'large') ), - um.cssMemberText( - um.cssMemberPrimary(user.name || dom('span', user.email), + cssMemberText( + cssMemberPrimary(user.name || dom('span', user.email), cssRole('(', roleNames[user.access!] || user.access, ')', testId('acl-user-access')), ), - user.name ? um.cssMemberSecondary(user.email) : null + user.name ? cssMemberSecondary(user.email) : null ), basicButton(cssUserButton.cls(''), icon('Copy'), 'Copy Email', testId('acl-user-copy'), @@ -84,7 +85,7 @@ const cssUsers = styled('div', ` max-width: unset; `); -const cssUserItem = styled(um.cssMemberListItem, ` +const cssUserItem = styled(cssMemberListItem, ` width: auto; padding: 8px 16px; align-items: center; diff --git a/app/client/ui/BillingPlanManagers.ts b/app/client/ui/BillingPlanManagers.ts index 01f48db0..fe81d0e9 100644 --- a/app/client/ui/BillingPlanManagers.ts +++ b/app/client/ui/BillingPlanManagers.ts @@ -1,7 +1,8 @@ import {getHomeUrl, reportError} from 'app/client/models/AppModel'; import {BillingModel} from 'app/client/models/BillingModel'; import {createUserImage} from 'app/client/ui/UserImage'; -import * as um from 'app/client/ui/UserManager'; +import {cssEmailInput, cssEmailInputContainer, cssMailIcon, cssMemberBtn, cssMemberImage, cssMemberListItem, + cssMemberPrimary, cssMemberSecondary, cssMemberText, cssRemoveIcon} from 'app/client/ui/UserItem'; import {bigPrimaryButton} from 'app/client/ui2018/buttons'; import {testId} from 'app/client/ui2018/cssVars'; import {normalizeEmail} from 'app/common/emails'; @@ -39,9 +40,9 @@ export class BillingPlanManagers extends Disposable { dom.forEach(this._managers, manager => this._buildManagerRow(manager)), ), cssEmailInputRow( - um.cssEmailInputContainer({style: `flex: 1 1 0; margin: 0 7px 0 0;`}, - um.cssMailIcon('Mail'), - this._emailElem = um.cssEmailInput(this._email, {onInput: true, isValid: this._isValid}, + cssEmailInputContainer({style: `flex: 1 1 0; margin: 0 7px 0 0;`}, + cssMailIcon('Mail'), + this._emailElem = cssEmailInput(this._email, {onInput: true, isValid: this._isValid}, {type: "email", placeholder: "Enter email address"}, dom.on('keyup', (e: KeyboardEvent) => { switch (e.keyCode) { @@ -51,8 +52,8 @@ export class BillingPlanManagers extends Disposable { }), dom.boolAttr('disabled', this._loading) ), - um.cssEmailInputContainer.cls('-green', enableAdd), - um.cssEmailInputContainer.cls('-disabled', this._loading), + cssEmailInputContainer.cls('-green', enableAdd), + cssEmailInputContainer.cls('-disabled', this._loading), testId('bpm-manager-new') ), bigPrimaryButton('Add Billing Contact', @@ -66,17 +67,17 @@ export class BillingPlanManagers extends Disposable { private _buildManagerRow(manager: FullUser) { const isCurrentUser = this._currentValidUser && manager.id === this._currentValidUser.id; - return um.cssMemberListItem({style: 'width: auto;'}, - um.cssMemberImage( + return cssMemberListItem({style: 'width: auto;'}, + cssMemberImage( createUserImage(manager, 'large') ), - um.cssMemberText( - um.cssMemberPrimary(manager.name || dom('span', manager.email, testId('bpm-email'))), - manager.name ? um.cssMemberSecondary(manager.email, testId('bpm-email')) : null + cssMemberText( + cssMemberPrimary(manager.name || dom('span', manager.email, testId('bpm-email'))), + manager.name ? cssMemberSecondary(manager.email, testId('bpm-email')) : null ), - um.cssMemberBtn( - um.cssRemoveIcon('Remove', testId('bpm-manager-delete')), - um.cssMemberBtn.cls('-disabled', (use) => Boolean(use(this._loading) || isCurrentUser)), + cssMemberBtn( + cssRemoveIcon('Remove', testId('bpm-manager-delete')), + cssMemberBtn.cls('-disabled', (use) => Boolean(use(this._loading) || isCurrentUser)), // Click handler. dom.on('click', () => this._loading.get() || isCurrentUser || this._remove(manager)) ), diff --git a/app/client/ui/UserItem.ts b/app/client/ui/UserItem.ts new file mode 100644 index 00000000..f5c85297 --- /dev/null +++ b/app/client/ui/UserItem.ts @@ -0,0 +1,120 @@ +import {colors, vars} from 'app/client/ui2018/cssVars'; +import {icon} from 'app/client/ui2018/icons'; +import {input, styled} from 'grainjs'; +import {cssMenuItem} from 'popweasel'; + +// Styled elements used for rendering a user, e.g. in the UserManager, Billing, etc. +// There is a general structure, but enough small variation that there is no helper at this point. +// +// cssMemberListItem( +// cssMemberImage( +// createUserImage(getFullUser(member), 'large') +// ), +// cssMemberText( +// cssMemberPrimary(NAME), +// cssMemberSecondary(EMAIL), +// ) +// ) + +export const cssMemberListItem = styled('div', ` + display: flex; + width: 460px; + height: 64px; + margin: 0 auto; + padding: 12px 0; +`); + +export const cssMemberImage = styled('div', ` + width: 40px; + height: 40px; + margin: 0 4px; + border-radius: 20px; + background-color: ${colors.lightGreen}; + background-size: cover; + + .${cssMemberListItem.className}-removed & { + opacity: 0.4; + } +`); + +export const cssMemberText = styled('div', ` + display: flex; + flex-direction: column; + justify-content: center; + margin: 2px 12px; + flex: 1 1 0; + min-width: 0px; + font-size: ${vars.mediumFontSize}; + + .${cssMemberListItem.className}-removed & { + opacity: 0.4; + } +`); + +export const cssMemberPrimary = styled('span', ` + font-weight: bold; + color: ${colors.dark}; + padding: 2px 0; + + .${cssMenuItem.className}-sel & { + color: white; + } +`); + +export const cssMemberSecondary = styled('span', ` + color: ${colors.slate}; + /* the following just undo annoying bootstrap styles that apply to all labels */ + margin: 0px; + font-weight: normal; + padding: 2px 0; + white-space: nowrap; + + .${cssMenuItem.className}-sel & { + color: white; + } +`); + +export const cssMemberBtn = styled('div', ` + width: 16px; + height: 16px; + cursor: pointer; + + &-disabled { + opacity: 0.3; + cursor: default; + } +`); + +export const cssRemoveIcon = styled(icon, ` + margin: 12px 0; +`); + +export const cssEmailInputContainer = styled('div', ` + position: relative; + display: flex; + height: 42px; + padding: 0 3px; + margin: 16px 63px; + border: 1px solid ${colors.darkGrey}; + border-radius: 3px; + font-size: ${vars.mediumFontSize}; + outline: none; + + &-green { + border: 1px solid ${colors.lightGreen}; + } +`); + +export const cssEmailInput = styled(input, ` + flex: 1 1 0; + line-height: 42px; + font-size: ${vars.mediumFontSize}; + font-family: ${vars.fontFamily}; + outline: none; + border: none; +`); + +export const cssMailIcon = styled(icon, ` + margin: 12px 8px 12px 13px; + background-color: ${colors.slate}; +`); diff --git a/app/client/ui/UserManager.ts b/app/client/ui/UserManager.ts index 87bf2e4c..a696782c 100644 --- a/app/client/ui/UserManager.ts +++ b/app/client/ui/UserManager.ts @@ -10,7 +10,7 @@ import * as roles from 'app/common/roles'; import {tbind} from 'app/common/tbind'; import {PermissionData, UserAPI} from 'app/common/UserAPI'; import {computed, Computed, Disposable, observable, Observable} from 'grainjs'; -import {dom, DomElementArg, input, styled} from 'grainjs'; +import {dom, DomElementArg, styled} from 'grainjs'; import {cssMenuItem} from 'popweasel'; import {copyToClipboard} from 'app/client/lib/copyToClipboard'; @@ -24,6 +24,8 @@ import {getResourceParent, ResourceType} from 'app/client/models/UserManagerMode import {shadowScroll} from 'app/client/ui/shadowScroll'; import {showTransientTooltip} from 'app/client/ui/tooltips'; import {createUserImage, cssUserImage} from 'app/client/ui/UserImage'; +import {cssEmailInput, cssEmailInputContainer, cssMailIcon, cssMemberBtn, cssMemberImage, cssMemberListItem, + cssMemberPrimary, cssMemberSecondary, cssMemberText, cssRemoveIcon} from 'app/client/ui/UserItem'; import {basicButton, bigBasicButton, bigPrimaryButton} from 'app/client/ui2018/buttons'; import {colors, testId, vars} from 'app/client/ui2018/cssVars'; import {icon} from 'app/client/ui2018/icons'; @@ -451,27 +453,6 @@ const cssOptionBtn = styled('span', ` cursor: pointer; `); -export const cssMemberListItem = styled('div', ` - display: flex; - width: 460px; - height: 64px; - margin: 0 auto; - padding: 12px 0; -`); - -export const cssMemberImage = styled('div', ` - width: 40px; - height: 40px; - margin: 0 4px; - border-radius: 20px; - background-color: ${colors.lightGreen}; - background-size: cover; - - .${cssMemberListItem.className}-removed & { - opacity: 0.4; - } -`); - const cssPublicMemberIcon = styled(icon, ` width: 32px; height: 32px; @@ -479,88 +460,6 @@ const cssPublicMemberIcon = styled(icon, ` --icon-color: ${colors.lightGreen}; `); -export const cssMemberText = styled('div', ` - display: flex; - flex-direction: column; - justify-content: center; - margin: 2px 12px; - flex: 1 1 0; - min-width: 0px; - font-size: ${vars.mediumFontSize}; - - .${cssMemberListItem.className}-removed & { - opacity: 0.4; - } -`); - -export const cssMemberPrimary = styled('span', ` - font-weight: bold; - color: ${colors.dark}; - padding: 2px 0; - - .${cssMenuItem.className}-sel & { - color: white; - } -`); - -export const cssMemberSecondary = styled('span', ` - color: ${colors.slate}; - /* the following just undo annoying bootstrap styles that apply to all labels */ - margin: 0px; - font-weight: normal; - padding: 2px 0; - white-space: nowrap; - - .${cssMenuItem.className}-sel & { - color: white; - } -`); - -export const cssEmailInputContainer = styled('div', ` - position: relative; - display: flex; - height: 42px; - padding: 0 3px; - margin: 16px 63px; - border: 1px solid ${colors.darkGrey}; - border-radius: 3px; - font-size: ${vars.mediumFontSize}; - outline: none; - - &-green { - border: 1px solid ${colors.lightGreen}; - } -`); - -export const cssMailIcon = styled(icon, ` - margin: 12px 8px 12px 13px; - background-color: ${colors.slate}; -`); - -export const cssEmailInput = styled(input, ` - flex: 1 1 0; - line-height: 42px; - font-size: ${vars.mediumFontSize}; - font-family: ${vars.fontFamily}; - outline: none; - border: none; -`); - -export const cssMemberBtn = styled('div', ` - width: 16px; - height: 16px; - cursor: pointer; - - &-disabled { - opacity: 0.3; - cursor: default; - } -`); - -export const cssRemoveIcon = styled(icon, ` - margin: 12px 0; -`); - const cssUndoIcon = styled(icon, ` margin: 12px 0; `); diff --git a/app/client/ui/WelcomePage.ts b/app/client/ui/WelcomePage.ts index 1303bedc..4368905e 100644 --- a/app/client/ui/WelcomePage.ts +++ b/app/client/ui/WelcomePage.ts @@ -8,7 +8,10 @@ import { AppHeader } from 'app/client/ui/AppHeader'; import * as BillingPageCss from "app/client/ui/BillingPageCss"; import * as forms from "app/client/ui/forms"; import { pagePanels } from "app/client/ui/PagePanels"; -import { bigBasicButton, bigBasicButtonLink, bigPrimaryButton, bigPrimaryButtonLink, +import { createUserImage } from 'app/client/ui/UserImage'; +import { cssMemberImage, cssMemberListItem, cssMemberPrimary, + cssMemberSecondary, cssMemberText } from 'app/client/ui/UserItem'; +import { basicButtonLink, bigBasicButton, bigBasicButtonLink, bigPrimaryButton, bigPrimaryButtonLink, cssButton } from "app/client/ui2018/buttons"; import { colors, mediaSmall, testId, vars } from "app/client/ui2018/cssVars"; import { getOrgName, Organization } from "app/common/UserAPI"; @@ -92,6 +95,7 @@ export class WelcomePage extends Disposable { state.welcome === 'user' ? dom.create(this._buildNameForm.bind(this)) : state.welcome === 'info' ? dom.create(this._buildInfoForm.bind(this)) : state.welcome === 'teams' ? dom.create(this._buildOrgPicker.bind(this)) : + state.welcome === 'select-account' ? dom.create(this._buildAccountPicker.bind(this)) : null )), )); @@ -336,8 +340,50 @@ export class WelcomePage extends Disposable { } }); } + + private _buildAccountPicker(): DomContents { + function addUserToLink(email: string): string { + const next = new URLSearchParams(location.search).get('next') || ''; + const url = new URL(next, location.href); + url.searchParams.set('user', email); + return url.toString(); + } + + return [ + cssParagraph( + "Select an account to continue with.", + ), + dom.maybe(this._appModel.topAppModel.users, users => + users.map(user => basicButtonLink( + cssUserItem.cls(''), + cssMemberListItem( + cssMemberImage( + createUserImage(user, 'large') + ), + cssMemberText( + cssMemberPrimary(user.name || dom('span', user.email, testId('select-email'))), + user.name ? cssMemberSecondary(user.email, testId('select-email')) : null + ), + ), + {href: addUserToLink(user.email)}, + testId('select-user'), + )), + ), + ]; + } } +const cssUserItem = styled('div', ` + margin: 0 0 8px; + align-items: center; + &:first-of-type { + margin-top: 16px; + } + &:hover { + background-color: ${colors.lightGrey}; + } +`); + const cssScrollContainer = styled('div', ` display: flex; overflow-y: auto; diff --git a/app/common/gristUrls.ts b/app/common/gristUrls.ts index cf7a23c7..b0f38170 100644 --- a/app/common/gristUrls.ts +++ b/app/common/gristUrls.ts @@ -17,7 +17,7 @@ export type IDocPage = number | 'new' | 'code' | 'acl' | 'GristDocTour'; export const HomePage = StringUnion('all', 'workspace', 'templates', 'trash'); export type IHomePage = typeof HomePage.type; -export const WelcomePage = StringUnion('user', 'info', 'teams', 'signup', 'verify'); +export const WelcomePage = StringUnion('user', 'info', 'teams', 'signup', 'verify', 'select-account'); export type WelcomePage = typeof WelcomePage.type; // Overall UI style. "full" is normal, "light" is a single page focused, panels hidden experience. diff --git a/app/server/declarations.d.ts b/app/server/declarations.d.ts index 3b99700b..327b87fd 100644 --- a/app/server/declarations.d.ts +++ b/app/server/declarations.d.ts @@ -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. diff --git a/app/server/lib/Authorizer.ts b/app/server/lib/Authorizer.ts index edb6ff3a..7f60c8be 100644 --- a/app/server/lib/Authorizer.ts +++ b/app/server/lib/Authorizer.ts @@ -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(); +} diff --git a/app/server/lib/BrowserSession.ts b/app/server/lib/BrowserSession.ts index 2c9179a6..50926325 100644 --- a/app/server/lib/BrowserSession.ts +++ b/app/server/lib/BrowserSession.ts @@ -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 { + public async updateUserProfile(req: Request, profile: UserProfile|null): Promise { 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): 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 { + public async clearScopedSession(req: Request, prev?: SessionObj): Promise { 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 { + private async _setSession(req: Request, session: SessionObj): Promise { 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 { + private async _updateScopedSession(req: Request, user: SessionUserObj, prev?: SessionObj): Promise { 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); } /** diff --git a/app/server/lib/DiscourseConnect.ts b/app/server/lib/DiscourseConnect.ts new file mode 100644 index 00000000..b517d41b --- /dev/null +++ b/app/server/lib/DiscourseConnect.ts @@ -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) + ); +} diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 75171581..ef52a567 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -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(`

Logged in as ${JSON.stringify(profile)}.

@@ -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[]) { diff --git a/app/server/lib/ITestingHooks-ti.ts b/app/server/lib/ITestingHooks-ti.ts index ede0abb3..13ebfa88 100644 --- a/app/server/lib/ITestingHooks-ti.ts +++ b/app/server/lib/ITestingHooks-ti.ts @@ -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 = { diff --git a/app/server/lib/ITestingHooks.ts b/app/server/lib/ITestingHooks.ts index d0710a75..0f3e205c 100644 --- a/app/server/lib/ITestingHooks.ts +++ b/app/server/lib/ITestingHooks.ts @@ -14,4 +14,5 @@ export interface ITestingHooks { flushAuthorizerCache(): Promise; getDocClientCounts(): Promise>; setActiveDocTimeout(seconds: number): Promise; + setDiscourseConnectVar(varName: string, value: string|null): Promise; } diff --git a/app/server/lib/MinimalLogin.ts b/app/server/lib/MinimalLogin.ts index 5ef0c1d1..32270a23 100644 --- a/app/server/lib/MinimalLogin.ts +++ b/app/server/lib/MinimalLogin.ts @@ -47,7 +47,7 @@ export async function getMinimalLoginSystem(): Promise { */ 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() })); } diff --git a/app/server/lib/SamlConfig.ts b/app/server/lib/SamlConfig.ts index f7abff76..b190806c 100644 --- a/app/server/lib/SamlConfig.ts +++ b/app/server/lib/SamlConfig.ts @@ -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, diff --git a/app/server/lib/TestingHooks.ts b/app/server/lib/TestingHooks.ts index b6831551..7f70f4af 100644 --- a/app/server/lib/TestingHooks.ts +++ b/app/server/lib/TestingHooks.ts @@ -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 { @@ -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 { + const key = varName as keyof typeof DiscourseConnectDeps; + const prev = DiscourseConnectDeps[key] || null; + if (value == null) { + delete DiscourseConnectDeps[key]; + } else { + DiscourseConnectDeps[key] = value; + } + return prev; + } } diff --git a/app/server/lib/gristSessions.ts b/app/server/lib/gristSessions.ts index 8a26c5db..8ec8509d 100644 --- a/app/server/lib/gristSessions.ts +++ b/app/server/lib/gristSessions.ts @@ -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; } diff --git a/test/nbrowser/testServer.ts b/test/nbrowser/testServer.ts index 677a5e5e..b82caf18 100644 --- a/test/nbrowser/testServer.ts +++ b/test/nbrowser/testServer.ts @@ -100,6 +100,8 @@ export class TestServerMerged implements IMochaServer { // Set low limits for uploads, for testing. GRIST_MAX_UPLOAD_IMPORT_MB: '1', GRIST_MAX_UPLOAD_ATTACHMENT_MB: '2', + // The following line only matters for testing with non-localhost URLs, which some tests do. + GRIST_SERVE_SAME_ORIGIN: 'true', APP_UNTRUSTED_URL : "http://localhost:18096", // Run with HOME_PORT, STATIC_PORT, DOC_PORT, DOC_WORKER_COUNT in the environment to override. ...(isCore ? {