From 39bf909c21489fddd6fc25f71290923356b764a7 Mon Sep 17 00:00:00 2001 From: uowis Date: Thu, 9 May 2024 16:04:04 +0200 Subject: [PATCH 1/7] Implement forcing mfa from OIDC IDP --- app/server/lib/OIDCConfig.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/app/server/lib/OIDCConfig.ts b/app/server/lib/OIDCConfig.ts index 86f78bce..999aa5fd 100644 --- a/app/server/lib/OIDCConfig.ts +++ b/app/server/lib/OIDCConfig.ts @@ -69,6 +69,7 @@ export class OIDCConfig { private _endSessionEndpoint: string; private _skipEndSessionEndpoint: boolean; private _ignoreEmailVerified: boolean; + private _forceMfa: boolean; public constructor() { } @@ -113,6 +114,11 @@ export class OIDCConfig { defaultValue: false, })!; + this._forceMfa = section.flag('forceMfa').readBool({ + envVar: 'GRIST_OIDC_SP_FORCE_MFA', + defaultValue: false, + })!; + const issuer = await Issuer.discover(issuerUrl); this._redirectUrl = new URL(CALLBACK_URL, spHost).href; this._client = new issuer.Client({ @@ -159,6 +165,15 @@ export class OIDCConfig { throw new Error(`OIDCConfig: email not verified for ${userInfo.email}`); } + const amr = tokenSet.claims().amr; + if (this._forceMfa && (!amr || !amr.includes("mfa"))) { + if (!amr) { + throw new Error('OIDCConfig: could not verify mfa status due to missing amr claim. Make sure your IDP returns it.'); + } else if (!amr.includes("mfa")) { + throw new Error(`OIDCConfig: multi-factor-authentication is not enabled for ${userInfo.email}.`); + } + } + const profile = this._makeUserProfileFromUserInfo(userInfo); log.info(`OIDCConfig: got OIDC response for ${profile.email} (${profile.name}) redirecting to ${targetUrl}`); From cdd0a5d1f925141b15898db0817222a0f94f97ae Mon Sep 17 00:00:00 2001 From: uowis Date: Thu, 9 May 2024 17:35:35 +0200 Subject: [PATCH 2/7] Introduce user facing error page for enabling mfa --- app/client/ui/errorPages.ts | 20 ++++++++++++++++++++ app/common/gristUrls.ts | 3 +++ app/server/lib/FlexServer.ts | 5 +++++ app/server/lib/OIDCConfig.ts | 11 ++++++++++- app/server/lib/sendAppPage.ts | 2 ++ 5 files changed, 40 insertions(+), 1 deletion(-) diff --git a/app/client/ui/errorPages.ts b/app/client/ui/errorPages.ts index b21ac106..a07e00a7 100644 --- a/app/client/ui/errorPages.ts +++ b/app/client/ui/errorPages.ts @@ -21,6 +21,7 @@ export function createErrPage(appModel: AppModel) { errPage === 'not-found' ? createNotFoundPage(appModel, errMessage) : errPage === 'access-denied' ? createForbiddenPage(appModel, errMessage) : errPage === 'account-deleted' ? createAccountDeletedPage(appModel) : + errPage === 'mfa-not-enabled' ? createMfaNotEnabledErrorPage(appModel) : createOtherErrorPage(appModel, errMessage); } @@ -81,6 +82,25 @@ export function createAccountDeletedPage(appModel: AppModel) { ]); } +/** + * Creates a page that show the user's account does not have multifactor authentication enabled, despite being needed. + */ +export function createMfaNotEnabledErrorPage(appModel: AppModel) { + document.title = t("Multi-factor authentication required{{suffix}}", {suffix: getPageTitleSuffix(getGristConfig())}); + + const searchParams = new URL(location.href).searchParams; + + return pagePanelsError(appModel, t("Multi-factor authentication required{{suffix}}", {suffix: ''}), [ + cssErrorText(t("Multi-factor-authentication is required for accessing this site, but it is not set up on your account. Please enable it and try again.")), + cssButtonWrap(bigPrimaryButtonLink( + t("Set up Multi-factor authentication"), {href: getGristConfig().mfaSettingsUrl, target: '_blank'}, testId('error-setup-mfa') + )), + cssButtonWrap(bigBasicButtonLink( + t("Try again"), {href: getSignupUrl({ nextUrl: searchParams.get("next") || "" })}, testId('error-signin') + )) + ]) +} + /** * Creates a "Page not found" page. */ diff --git a/app/common/gristUrls.ts b/app/common/gristUrls.ts index 8779d23a..f3d66958 100644 --- a/app/common/gristUrls.ts +++ b/app/common/gristUrls.ts @@ -793,6 +793,9 @@ export interface GristLoadConfig { // If backend has an email service for sending notifications. notifierEnabled?: boolean; + + // The URL to the external IDP, where the user can set up Multi-factor authentication + mfaSettingsUrl?: string; } export const Features = StringUnion( diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 17ac46a3..27f4f7b3 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -1273,6 +1273,11 @@ export class FlexServer implements GristServer { this.app.get('/signed-out', expressWrap((req, resp) => this._sendAppPage(req, resp, {path: 'error.html', status: 200, config: {errPage: 'signed-out'}}))); + // Add a static "mfa-not-enabled" page. This is where logout typically lands when GRIST_OIDC_SP_FORCE_MFA is true + // but the user hasn't configured multi-factor authentication. + this.app.get('/login/error/mfa-not-enabled', expressWrap((req, resp) => + this._sendAppPage(req, resp, {path: 'error.html', status: 401, config: {errPage: 'mfa-not-enabled'}}))); + const comment = await this._loginMiddleware.addEndpoints(this.app); this.info.push(['loginMiddlewareComment', comment]); diff --git a/app/server/lib/OIDCConfig.ts b/app/server/lib/OIDCConfig.ts index 999aa5fd..0bb5d82c 100644 --- a/app/server/lib/OIDCConfig.ts +++ b/app/server/lib/OIDCConfig.ts @@ -170,7 +170,16 @@ export class OIDCConfig { if (!amr) { throw new Error('OIDCConfig: could not verify mfa status due to missing amr claim. Make sure your IDP returns it.'); } else if (!amr.includes("mfa")) { - throw new Error(`OIDCConfig: multi-factor-authentication is not enabled for ${userInfo.email}.`); + log.error(`OIDCConfig: multi-factor-authentication is not enabled for ${userInfo.email}.`); + delete mreq.session.oidc; + + // Convert absolute URL into relative, since it will be prefixed further down the line + let targetURL = new URL(targetUrl as string); + let targetUrlRelative = targetURL.pathname; + if (targetURL.searchParams.toString()) targetUrlRelative += "?" + targetURL.searchParams.toString(); + + res.redirect(`/login/error/mfa-not-enabled?next=${targetUrlRelative}`); + return; } } diff --git a/app/server/lib/sendAppPage.ts b/app/server/lib/sendAppPage.ts index 8c5ebf92..ade3abc8 100644 --- a/app/server/lib/sendAppPage.ts +++ b/app/server/lib/sendAppPage.ts @@ -24,6 +24,7 @@ import * as handlebars from 'handlebars'; import jsesc from 'jsesc'; import * as path from 'path'; import difference = require('lodash/difference'); +import * as process from "node:process"; const translate = (req: express.Request, key: string, args?: any) => req.t(`sendAppPage.${key}`, args); @@ -98,6 +99,7 @@ export function makeGristConfig(options: MakeGristConfigOptions): GristLoadConfi canCloseAccount: isAffirmative(process.env.GRIST_ACCOUNT_CLOSE), experimentalPlugins: isAffirmative(process.env.GRIST_EXPERIMENTAL_PLUGINS), notifierEnabled: server?.hasNotifier(), + mfaSettingsUrl: process.env.GRIST_OIDC_SP_MFA_SETTINGS_URL, ...extra, }; } From b51bfe279be98f6041efe28171d21c58bd54bb65 Mon Sep 17 00:00:00 2001 From: uowis Date: Thu, 9 May 2024 17:40:23 +0200 Subject: [PATCH 3/7] add german translation --- static/locales/de.client.json | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/static/locales/de.client.json b/static/locales/de.client.json index 99331f95..273eaf06 100644 --- a/static/locales/de.client.json +++ b/static/locales/de.client.json @@ -948,7 +948,11 @@ "An unknown error occurred.": "Ein unbekannter Fehler ist aufgetreten.", "Powered by": "Angetrieben durch", "Build your own form": "Erstellen Sie Ihr eigenes Formular", - "Form not found": "Formular nicht gefunden" + "Form not found": "Formular nicht gefunden", + "Try again": "Erneut versuchen", + "Multi-factor authentication required{{suffix}}": "Multi-Faktor-Authentifizierung nötig", + "Set up Multi-factor authentication": "Multi-Faktor-Authentifizierung einrichten", + "Multi-factor-authentication is required for accessing this site, but it is not set up on your account. Please enable it and try again.": "Für den Zugriff auf diese Website ist Multi-Faktor-Authentifizierung erforderlich. Diese ist nicht in Ihrem Konto eingerichtet. Bitte richten Sie sie ein und versuchen Sie es erneut." }, "menus": { "* Workspaces are available on team plans. ": "* Arbeitsbereiche sind in Teamplänen verfügbar. ", From 7f2e3edcdea023ec171dd24f7532530121b76d22 Mon Sep 17 00:00:00 2001 From: uowis Date: Thu, 9 May 2024 17:43:41 +0200 Subject: [PATCH 4/7] fix german translation, add english "translation" --- static/locales/de.client.json | 6 +++--- static/locales/en.client.json | 6 +++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/static/locales/de.client.json b/static/locales/de.client.json index 273eaf06..6b461b89 100644 --- a/static/locales/de.client.json +++ b/static/locales/de.client.json @@ -949,10 +949,10 @@ "Powered by": "Angetrieben durch", "Build your own form": "Erstellen Sie Ihr eigenes Formular", "Form not found": "Formular nicht gefunden", - "Try again": "Erneut versuchen", - "Multi-factor authentication required{{suffix}}": "Multi-Faktor-Authentifizierung nötig", + "Multi-factor authentication required{{suffix}}": "Multi-Faktor-Authentifizierung nötig{{suffix}}", + "Multi-factor-authentication is required for accessing this site, but it is not set up on your account. Please enable it and try again.": "Für den Zugriff auf diese Website ist Multi-Faktor-Authentifizierung erforderlich. Diese ist nicht in Ihrem Konto eingerichtet. Bitte richten Sie sie ein und versuchen Sie es erneut.", "Set up Multi-factor authentication": "Multi-Faktor-Authentifizierung einrichten", - "Multi-factor-authentication is required for accessing this site, but it is not set up on your account. Please enable it and try again.": "Für den Zugriff auf diese Website ist Multi-Faktor-Authentifizierung erforderlich. Diese ist nicht in Ihrem Konto eingerichtet. Bitte richten Sie sie ein und versuchen Sie es erneut." + "Try again": "Erneut versuchen" }, "menus": { "* Workspaces are available on team plans. ": "* Arbeitsbereiche sind in Teamplänen verfügbar. ", diff --git a/static/locales/en.client.json b/static/locales/en.client.json index c26932fc..a322fba0 100644 --- a/static/locales/en.client.json +++ b/static/locales/en.client.json @@ -888,7 +888,11 @@ "An unknown error occurred.": "An unknown error occurred.", "Build your own form": "Build your own form", "Form not found": "Form not found", - "Powered by": "Powered by" + "Powered by": "Powered by", + "Multi-factor authentication required{{suffix}}": "Multi-factor authentication required{{suffix}}", + "Multi-factor-authentication is required for accessing this site, but it is not set up on your account. Please enable it and try again.": "Multi-factor-authentication is required for accessing this site, but it is not set up on your account. Please enable it and try again.", + "Set up Multi-factor authentication": "Set up Multi-factor authentication", + "Try again": "Try again" }, "menus": { "* Workspaces are available on team plans. ": "* Workspaces are available on team plans. ", From 1d5383ed876b248062b429de436c84a4905af041 Mon Sep 17 00:00:00 2001 From: uowis Date: Thu, 9 May 2024 17:50:53 +0200 Subject: [PATCH 5/7] add documentation on new env vars --- app/server/lib/OIDCConfig.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/server/lib/OIDCConfig.ts b/app/server/lib/OIDCConfig.ts index 0bb5d82c..a812ca2f 100644 --- a/app/server/lib/OIDCConfig.ts +++ b/app/server/lib/OIDCConfig.ts @@ -35,6 +35,14 @@ * env GRIST_OIDC_SP_IGNORE_EMAIL_VERIFIED * If set to "true", the user will be allowed to login even if the email is not verified by the IDP. * Defaults to false. + * env GRIST_OIDC_SP_FORCE_MFA + * If set to "true", the user will be forced to have multi-factor authentication enabled. The state of MFA will + * be determined by OIDC's amr claim: It must include "mfa". Make sure that the IDP returns the amr claim + * correctly, otherwise authentication will fail. + * env GRIST_OIDC_SP_MFA_SETTINGS_URL + * This is needed when GRIST_OIDC_SP_FORCE_MFA is set to true. Enter the URL where the user will be able to + * configure Multi-factor authentication on their account. This will be shown in the UI if the user does not have + * MFA enabled. * * This version of OIDCConfig has been tested with Keycloak OIDC IdP following the instructions * at: From 95b387f1b11d39d5d5e91f9d824902f24582f01a Mon Sep 17 00:00:00 2001 From: uowis Date: Thu, 9 May 2024 18:00:15 +0200 Subject: [PATCH 6/7] fix linting issue --- app/server/lib/OIDCConfig.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/server/lib/OIDCConfig.ts b/app/server/lib/OIDCConfig.ts index a812ca2f..d4414c72 100644 --- a/app/server/lib/OIDCConfig.ts +++ b/app/server/lib/OIDCConfig.ts @@ -182,7 +182,7 @@ export class OIDCConfig { delete mreq.session.oidc; // Convert absolute URL into relative, since it will be prefixed further down the line - let targetURL = new URL(targetUrl as string); + const targetURL = new URL(targetUrl as string); let targetUrlRelative = targetURL.pathname; if (targetURL.searchParams.toString()) targetUrlRelative += "?" + targetURL.searchParams.toString(); From a113681f1f05fd145aa5fe9e43a6f15d859a9310 Mon Sep 17 00:00:00 2001 From: uowis Date: Thu, 9 May 2024 18:04:34 +0200 Subject: [PATCH 7/7] fix linting warning --- app/server/lib/OIDCConfig.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/server/lib/OIDCConfig.ts b/app/server/lib/OIDCConfig.ts index d4414c72..05c92fac 100644 --- a/app/server/lib/OIDCConfig.ts +++ b/app/server/lib/OIDCConfig.ts @@ -176,7 +176,7 @@ export class OIDCConfig { const amr = tokenSet.claims().amr; if (this._forceMfa && (!amr || !amr.includes("mfa"))) { if (!amr) { - throw new Error('OIDCConfig: could not verify mfa status due to missing amr claim. Make sure your IDP returns it.'); + throw new Error('OIDCConfig: could not verify mfa status due to missing amr claim.'); } else if (!amr.includes("mfa")) { log.error(`OIDCConfig: multi-factor-authentication is not enabled for ${userInfo.email}.`); delete mreq.session.oidc; @@ -184,7 +184,9 @@ export class OIDCConfig { // Convert absolute URL into relative, since it will be prefixed further down the line const targetURL = new URL(targetUrl as string); let targetUrlRelative = targetURL.pathname; - if (targetURL.searchParams.toString()) targetUrlRelative += "?" + targetURL.searchParams.toString(); + if (targetURL.searchParams.toString()) { + targetUrlRelative += "?" + targetURL.searchParams.toString(); + } res.redirect(`/login/error/mfa-not-enabled?next=${targetUrlRelative}`); return;