From 95abe247985cd4f7e5c1b7ce1171c6ce22520ea3 Mon Sep 17 00:00:00 2001 From: LukeGus Date: Wed, 23 Sep 2026 07:17:03 -0500 Subject: [PATCH] fix: gate second factors after SSO/LDAP behind an admin setting --- packages/plugin-sdk/ARCHITECTURE.md | 9 +- packages/plugin-sdk/src/backend.ts | 8 ++ src/backend/auth/core-auth.ts | 15 ++ src/backend/auth/legacy-providers.ts | 2 + src/backend/auth/login-pipeline.ts | 22 ++- src/backend/database/routes/users.ts | 128 ++++++++++++++++++ src/backend/tests/auth/legacy-methods.test.ts | 26 +++- src/backend/tests/auth/login-pipeline.test.ts | 34 ++++- src/ui/api/user-management-api.ts | 27 ++++ src/ui/locales/en.json | 3 + src/ui/main-axios.ts | 2 + src/ui/sidebar/AdminSettingsPanel.tsx | 23 ++++ src/ui/sidebar/AdminSettingsSections.tsx | 13 ++ 13 files changed, 307 insertions(+), 5 deletions(-) diff --git a/packages/plugin-sdk/ARCHITECTURE.md b/packages/plugin-sdk/ARCHITECTURE.md index 1e350c62e..9e8110f94 100644 --- a/packages/plugin-sdk/ARCHITECTURE.md +++ b/packages/plugin-sdk/ARCHITECTURE.md @@ -610,6 +610,7 @@ ctx.auth.registerLoginMethod({ id: "corp-sso", labelKey: "signIn", kind: "redirect", + external: true, // governs the "second factor after external logins" setting describe: async () => [{ id: "main", label: "Corp", enabled: true }], start: async (request, instanceId) => ({ redirectUrl }), callback: async (request) => ({ @@ -648,7 +649,13 @@ or an external identity. Core then: 3. runs second factors: every row in `user_second_factors` plus any factor that reports itself enrolled. **Fail closed:** a row whose plugin is disabled or gone refuses the login ("contact an admin"). A trusted device - or a method that already proved a factor (a verified passkey) skips the step; + or a method that already proved a factor (a verified passkey) skips the step. + This step is skipped entirely for a login method whose registration sets + `external: true` (OIDC, LDAP; a plugin declares it the same way) unless the + admin setting "ask for a second factor after external logins" is on. Off + by default, matching the behaviour before second factors applied to every + method: a local method (password, a passkey, a plugin's own local form) + always runs them; 4. issues the JWT, cookie, trusted device and the `login` audit line. Routes, all under `/users/auth`: `GET methods` (public), `GET :methodId/start`, diff --git a/packages/plugin-sdk/src/backend.ts b/packages/plugin-sdk/src/backend.ts index 05e122573..10dd43117 100644 --- a/packages/plugin-sdk/src/backend.ts +++ b/packages/plugin-sdk/src/backend.ts @@ -697,6 +697,14 @@ export interface PluginLoginMethod { labelKey: string; icon?: string; kind: "redirect" | "form"; + /** + * True for a method that authenticates against something other than a + * password or key this server holds: SSO, an external directory, a + * federated identity provider. Governs whether the "ask for a second + * factor after external logins" admin setting applies to it. Leave unset + * for a local method (password, a passkey, a plugin's own local form). + */ + external?: boolean; /** Enabled instances, shown on the login screen. No secrets. */ describe?: () => Promise; /** Redirect methods: where to send the browser. */ diff --git a/src/backend/auth/core-auth.ts b/src/backend/auth/core-auth.ts index 2b8aaf05f..54b84f250 100644 --- a/src/backend/auth/core-auth.ts +++ b/src/backend/auth/core-auth.ts @@ -36,6 +36,21 @@ export function isPasswordLoginSettingOn(): boolean { } } +/** + * Off by default, matching 2.8: second factors only ran after password and + * other local form logins. On, they also run after external methods (SSO, + * LDAP), which needs the pending-cookie/temp_token step those methods' + * clients (Termix-Mobile included) may not handle yet. + */ +export function isSecondFactorAfterExternalLoginEnabled(): boolean { + try { + const value = getCurrentSettingValue("second_factor_after_external_login"); + return value === "true"; + } catch { + return false; + } +} + /** * Whether anyone could still sign in without a password: trusted proxy auth, * or a login method reporting an enabled instance (an SSO provider). diff --git a/src/backend/auth/legacy-providers.ts b/src/backend/auth/legacy-providers.ts index 67c762596..d23f225d2 100644 --- a/src/backend/auth/legacy-providers.ts +++ b/src/backend/auth/legacy-providers.ts @@ -317,6 +317,7 @@ const oidcLoginMethod: LoginMethod = { labelKey: "auth.loginWithSso", icon: "key-round", kind: "redirect", + external: true, describe: async () => (await listPublicProviders(["oidc", "github", "google"])).map( (provider) => ({ @@ -345,6 +346,7 @@ const ldapLoginMethod: LoginMethod = { labelKey: "auth.loginWithLdap", icon: "server", kind: "form", + external: true, describe: async () => (await listPublicProviders(["ldap"])).map((provider) => ({ id: String(provider.id), diff --git a/src/backend/auth/login-pipeline.ts b/src/backend/auth/login-pipeline.ts index 677b00bb9..869bf5a1a 100644 --- a/src/backend/auth/login-pipeline.ts +++ b/src/backend/auth/login-pipeline.ts @@ -25,9 +25,13 @@ import { createCurrentUserRepository, } from "../database/repositories/factory.js"; import type { UserRecord } from "../database/repositories/user-repository.js"; -import { ensureCoreLoginProviders } from "./core-auth.js"; +import { + ensureCoreLoginProviders, + isSecondFactorAfterExternalLoginEnabled, +} from "./core-auth.js"; import { findOrProvisionExternalUser } from "./provisioning.js"; import { + getLoginMethod, getSecondFactor, listSecondFactors, type SecondFactor, @@ -112,6 +116,18 @@ export async function evaluateSecondFactors( return { required: [...required.values()], blocked }; } +/** + * Whether second factors should even be asked for this login. Off, they only + * run for password and other local form logins, matching 2.8: the setting + * governs external methods (SSO, LDAP) alone. + */ +function shouldRunSecondFactors(methodId: string): boolean { + ensureCoreLoginProviders(); + const method = getLoginMethod(methodId); + if (!method?.external) return true; + return isSecondFactorAfterExternalLoginEnabled(); +} + async function isTrustedDevice(req: Request, userId: string): Promise { const deviceInfo = parseUserAgent(req); const fingerprint = generateDeviceFingerprint(deviceInfo, getDeviceId(req)); @@ -221,7 +237,9 @@ export async function runLogin( `${context.methodId}_role_shared_credentials`, ); - const factors = await evaluateSecondFactors(user.id); + const factors = shouldRunSecondFactors(context.methodId) + ? await evaluateSecondFactors(user.id) + : { required: [], blocked: [] }; if (factors.blocked.length > 0) { authLogger.warn("Login refused: an enrolled second factor is unavailable", { operation: "second_factor_unavailable", diff --git a/src/backend/database/routes/users.ts b/src/backend/database/routes/users.ts index fb105da8f..893c15791 100644 --- a/src/backend/database/routes/users.ts +++ b/src/backend/database/routes/users.ts @@ -1378,6 +1378,18 @@ router.patch("/registration-allowed", authenticateJWT, async (req, res) => { } }); +/** + * @openapi + * /users/oidc-auto-provision: + * get: + * summary: Get OIDC auto-provision status + * description: Whether a new user account is created automatically on first SSO sign-in. + * tags: + * - Users + * responses: + * 200: + * description: OIDC auto-provision status. + */ router.get("/oidc-auto-provision", async (_req, res) => { try { res.json({ @@ -1394,6 +1406,33 @@ router.get("/oidc-auto-provision", async (_req, res) => { } }); +/** + * @openapi + * /users/oidc-auto-provision: + * patch: + * summary: Set OIDC auto-provision status + * description: Enables or disables automatic account creation on first SSO sign-in. + * tags: + * - Users + * requestBody: + * required: true + * content: + * application/json: + * schema: + * type: object + * properties: + * enabled: + * type: boolean + * responses: + * 200: + * description: OIDC auto-provision status updated. + * 400: + * description: Invalid value for enabled. + * 403: + * description: Not authorized. + * 500: + * description: Failed to set OIDC auto-provision setting. + */ router.patch("/oidc-auto-provision", authenticateJWT, async (req, res) => { const userId = (req as AuthenticatedRequest).userId; try { @@ -1416,6 +1455,95 @@ router.patch("/oidc-auto-provision", authenticateJWT, async (req, res) => { } }); +/** + * @openapi + * /users/second-factor-after-external-login: + * get: + * summary: Get the external-login second-factor setting + * description: Whether an enrolled second factor is asked for after an external login method (SSO, LDAP), in addition to password and other local logins. + * tags: + * - Users + * responses: + * 200: + * description: External-login second-factor setting. + */ +router.get("/second-factor-after-external-login", async (_req, res) => { + try { + res.json({ + enabled: await createCurrentSettingsRepository().getBoolean( + "second_factor_after_external_login", + false, + ), + }); + } catch (err) { + authLogger.error( + "Failed to get second-factor-after-external-login setting", + err, + ); + res.status(500).json({ + error: "Failed to get second-factor-after-external-login setting", + }); + } +}); + +/** + * @openapi + * /users/second-factor-after-external-login: + * patch: + * summary: Set the external-login second-factor setting + * description: Enables or disables asking for an enrolled second factor after an external login method (SSO, LDAP). + * tags: + * - Users + * requestBody: + * required: true + * content: + * application/json: + * schema: + * type: object + * properties: + * enabled: + * type: boolean + * responses: + * 200: + * description: External-login second-factor setting updated. + * 400: + * description: Invalid value for enabled. + * 403: + * description: Not authorized. + * 500: + * description: Failed to set second-factor-after-external-login setting. + */ +router.patch( + "/second-factor-after-external-login", + authenticateJWT, + async (req, res) => { + const userId = (req as AuthenticatedRequest).userId; + try { + const user = await requireCurrentAdmin(userId); + if (!user) { + return res.status(403).json({ error: "Not authorized" }); + } + const { enabled } = req.body; + if (typeof enabled !== "boolean") { + return res.status(400).json({ error: "Invalid value for enabled" }); + } + await createCurrentSettingsRepository().set( + "second_factor_after_external_login", + enabled ? "true" : "false", + ); + res.json({ enabled }); + } catch (err) { + authLogger.error( + "Failed to set second-factor-after-external-login setting", + err, + ); + res.status(500).json({ + error: "Failed to set second-factor-after-external-login setting", + }); + } + }, +); + /** * @openapi * /users/oidc-silent-login-default: diff --git a/src/backend/tests/auth/legacy-methods.test.ts b/src/backend/tests/auth/legacy-methods.test.ts index b16f06800..19850c69d 100644 --- a/src/backend/tests/auth/legacy-methods.test.ts +++ b/src/backend/tests/auth/legacy-methods.test.ts @@ -358,7 +358,7 @@ describe("LDAP through the pipeline", () => { ).rejects.toMatchObject({ status: 401 }); }); - it("runs TOTP for an LDAP user who enrolled", async () => { + it("skips TOTP for an LDAP user who enrolled when the external-login setting is off", async () => { h.providerConfig = providerConfig; h.state.users.set("u-bob", { id: "u-bob", @@ -378,6 +378,30 @@ describe("LDAP through the pipeline", () => { methodId: "ldap", rememberMe: false, }); + expect(result.kind).toBe("session"); + }); + + it("runs TOTP for an LDAP user who enrolled when the external-login setting is on", async () => { + h.providerConfig = providerConfig; + h.state.settings.set("second_factor_after_external_login", "true"); + h.state.users.set("u-bob", { + id: "u-bob", + username: "Bob Builder", + passwordHash: "", + isAdmin: false, + isOidc: true, + oidcIdentifier: "ldap:4:bob", + totpEnabled: true, + totpSecret: "JBSWY3DPEHPK3PXP", + }); + const identity = await verifyLdapLogin({ + body: { providerId: 4, username: "bob", password: "hunter2" }, + ip: "10.0.0.3", + }); + const result = await runLogin(fakeRequest() as never, identity, { + methodId: "ldap", + rememberMe: false, + }); expect(result.kind).toBe("second-factor"); }); }); diff --git a/src/backend/tests/auth/login-pipeline.test.ts b/src/backend/tests/auth/login-pipeline.test.ts index a989da9ac..a284bc287 100644 --- a/src/backend/tests/auth/login-pipeline.test.ts +++ b/src/backend/tests/auth/login-pipeline.test.ts @@ -318,6 +318,13 @@ describe("second factors", () => { expect(h.state.users.get("u1")!.totpBackupCodes).not.toContain("BACKUP01"); }); + it("still stops password login for TOTP when the external-login setting is off", async () => { + enrolTotp(); + h.state.settings.set("second_factor_after_external_login", "false"); + const res = await passwordLogin({ username: "alice", password: PASSWORD }); + expect(res.body).toMatchObject({ requires_totp: true }); + }); + it("rejects a wrong code without a session", async () => { enrolTotp(); const first = await passwordLogin({ @@ -549,7 +556,31 @@ describe("external identities", () => { expect(h.state.users.get("u9")!.isAdmin).toBe(false); }); - it("runs enrolled second factors for SSO users too", async () => { + it("skips enrolled second factors for SSO users when the setting is off", async () => { + addUser({ + id: "u9", + username: "Alice", + isOidc: true, + passwordHash: "", + totpEnabled: true, + totpSecret, + }); + h.state.identities.push({ + id: 1, + userId: "u9", + providerId: "3", + subject: "sub-1", + email: null, + }); + const result = await runLogin(fakeRequest() as never, external, { + methodId: "oidc", + rememberMe: false, + }); + expect(result.kind).toBe("session"); + }); + + it("runs enrolled second factors for SSO users when the setting is on", async () => { + h.state.settings.set("second_factor_after_external_login", "true"); addUser({ id: "u9", username: "Alice", @@ -621,6 +652,7 @@ describe("redirect logins", () => { }); it("sends a second-factor step back with a pending cookie", async () => { + h.state.settings.set("second_factor_after_external_login", "true"); addUser({ id: "u9", username: "Alice", diff --git a/src/ui/api/user-management-api.ts b/src/ui/api/user-management-api.ts index 39955774c..2a1a93785 100644 --- a/src/ui/api/user-management-api.ts +++ b/src/ui/api/user-management-api.ts @@ -256,6 +256,33 @@ export async function updateOidcAutoProvision( } } +export async function getSecondFactorAfterExternalLogin(): Promise<{ + enabled: boolean; +}> { + try { + const response = await authApi.get( + "/users/second-factor-after-external-login", + ); + return response.data; + } catch (error) { + handleApiError(error, "check second factor after external login status"); + } +} + +export async function updateSecondFactorAfterExternalLogin( + enabled: boolean, +): Promise> { + try { + const response = await authApi.patch( + "/users/second-factor-after-external-login", + { enabled }, + ); + return response.data; + } catch (error) { + handleApiError(error, "update second factor after external login"); + } +} + export async function getOidcSilentLoginDefault(): Promise<{ enabled: boolean; locked?: boolean; diff --git a/src/ui/locales/en.json b/src/ui/locales/en.json index f55072ff5..2c8e79be7 100644 --- a/src/ui/locales/en.json +++ b/src/ui/locales/en.json @@ -2884,6 +2884,8 @@ "allowPasswordLoginDesc": "Username/password login", "oidcAutoProvision": "OIDC Auto-Provision", "oidcAutoProvisionDesc": "Auto-create accounts for OIDC/SSO users on first login (independent of the registration toggle)", + "secondFactorAfterExternalLogin": "Ask for a second factor after external logins (SSO, GitHub, Google, LDAP)", + "secondFactorAfterExternalLoginDesc": "Off, an enrolled second factor is only asked for after password and other local logins, same as before. On, it is asked for after every login method, including SSO and LDAP.", "oidcSilentLoginDefault": "Silent OIDC Login by Default", "oidcSilentLoginDefaultDesc": "Automatically redirect to OIDC login on every visit, skipping the login form entirely", "oidcSilentLoginDefaultLockedDesc": "This setting is locked by the OIDC_SILENT_LOGIN_DEFAULT environment variable and cannot be changed here.", @@ -3054,6 +3056,7 @@ "updatePasswordLoginFailed": "Failed to update password login setting", "cannotDisablePasswordLoginWithTotp": "Cannot disable password login while 2FA is enabled for one or more users. Disable 2FA first.", "updateOidcAutoProvisionFailed": "Failed to update OIDC auto-provision setting", + "updateSecondFactorAfterExternalLoginFailed": "Failed to update second factor setting", "updateOidcSilentLoginDefaultFailed": "Failed to update silent OIDC login setting", "updatePasswordResetFailed": "Failed to update password reset setting", "sessionTimeoutRange2": "Session timeout must be between 1 and 720 hours", diff --git a/src/ui/main-axios.ts b/src/ui/main-axios.ts index 470a72010..919bdcb6a 100644 --- a/src/ui/main-axios.ts +++ b/src/ui/main-axios.ts @@ -2054,6 +2054,8 @@ export { updateRegistrationAllowed, getOidcAutoProvision, updateOidcAutoProvision, + getSecondFactorAfterExternalLogin, + updateSecondFactorAfterExternalLogin, getOidcSilentLoginDefault, updateOidcSilentLoginDefault, updatePasswordLoginAllowed, diff --git a/src/ui/sidebar/AdminSettingsPanel.tsx b/src/ui/sidebar/AdminSettingsPanel.tsx index d5a812d7f..b425f0dc8 100644 --- a/src/ui/sidebar/AdminSettingsPanel.tsx +++ b/src/ui/sidebar/AdminSettingsPanel.tsx @@ -37,6 +37,8 @@ import { updateGuacamoleSettings, getOidcAutoProvision, updateOidcAutoProvision, + getSecondFactorAfterExternalLogin, + updateSecondFactorAfterExternalLogin, getOidcSilentLoginDefault, updateOidcSilentLoginDefault, getCommandHistoryEnabled, @@ -223,6 +225,8 @@ export function AdminSettingsPanel({ // SSO / auto-provision state const [oidcAutoProvision, setOidcAutoProvision] = useState(false); + const [secondFactorAfterExternalLogin, setSecondFactorAfterExternalLogin] = + useState(false); const [oidcSilentLoginDefault, setOidcSilentLoginDefault] = useState(false); const [ssoProviders, setSsoProviders] = useState([]); const [ssoDialogOpen, setSsoDialogOpen] = useState(false); @@ -394,6 +398,7 @@ export function AdminSettingsPanel({ level, guac, oidcProv, + secondFactorExternal, oidcSilent, cmdHistory, analytics, @@ -413,6 +418,7 @@ export function AdminSettingsPanel({ getLogLevel(), getGuacamoleSettings(), getOidcAutoProvision(), + getSecondFactorAfterExternalLogin(), getOidcSilentLoginDefault(), getCommandHistoryEnabled(), getAnalyticsEnabled(), @@ -435,6 +441,8 @@ export function AdminSettingsPanel({ } if (oidcProv.status === "fulfilled") setOidcAutoProvision(oidcProv.value.enabled); + if (secondFactorExternal.status === "fulfilled") + setSecondFactorAfterExternalLogin(secondFactorExternal.value.enabled); if (oidcSilent.status === "fulfilled") { setOidcSilentLoginDefault(oidcSilent.value.enabled); setOidcSilentLoginDefaultLocked(oidcSilent.value.locked ?? false); @@ -555,6 +563,17 @@ export function AdminSettingsPanel({ } } + async function handleToggleSecondFactorAfterExternalLogin() { + const newVal = !secondFactorAfterExternalLogin; + setSecondFactorAfterExternalLogin(newVal); + try { + await updateSecondFactorAfterExternalLogin(newVal); + } catch { + setSecondFactorAfterExternalLogin(!newVal); + toast.error(t("admin.updateSecondFactorAfterExternalLoginFailed")); + } + } + async function handleToggleOidcSilentLoginDefault() { if (oidcSilentLoginDefaultLocked) return; const newVal = !oidcSilentLoginDefault; @@ -1233,6 +1252,10 @@ export function AdminSettingsPanel({ handleTogglePasswordLogin={handleTogglePasswordLogin} oidcAutoProvision={oidcAutoProvision} handleToggleOidcAutoProvision={handleToggleOidcAutoProvision} + secondFactorAfterExternalLogin={secondFactorAfterExternalLogin} + handleToggleSecondFactorAfterExternalLogin={ + handleToggleSecondFactorAfterExternalLogin + } oidcSilentLoginDefault={oidcSilentLoginDefault} oidcSilentLoginDefaultLocked={oidcSilentLoginDefaultLocked} handleToggleOidcSilentLoginDefault={handleToggleOidcSilentLoginDefault} diff --git a/src/ui/sidebar/AdminSettingsSections.tsx b/src/ui/sidebar/AdminSettingsSections.tsx index bdac60158..05191cc9d 100644 --- a/src/ui/sidebar/AdminSettingsSections.tsx +++ b/src/ui/sidebar/AdminSettingsSections.tsx @@ -47,6 +47,8 @@ type GeneralSettingsSectionProps = { handleTogglePasswordLogin: () => void; oidcAutoProvision: boolean; handleToggleOidcAutoProvision: () => void; + secondFactorAfterExternalLogin: boolean; + handleToggleSecondFactorAfterExternalLogin: () => void; oidcSilentLoginDefault: boolean; oidcSilentLoginDefaultLocked: boolean; handleToggleOidcSilentLoginDefault: () => void; @@ -98,6 +100,8 @@ export function AdminGeneralSettingsSection({ handleTogglePasswordLogin, oidcAutoProvision, handleToggleOidcAutoProvision, + secondFactorAfterExternalLogin, + handleToggleSecondFactorAfterExternalLogin, oidcSilentLoginDefault, oidcSilentLoginDefaultLocked, handleToggleOidcSilentLoginDefault, @@ -301,6 +305,15 @@ export function AdminGeneralSettingsSection({ onToggle={handleToggleOidcAutoProvision} /> + + +