From 091cd7a18a9fade1fd31c1ddf72b3fea97fcc3db Mon Sep 17 00:00:00 2001 From: Elian Doran Date: Fri, 6 Jun 2025 16:17:21 +0300 Subject: [PATCH] fix(server): totp asked even if no authentication is enabled --- apps/server/src/services/auth.spec.ts | 73 +++++++++++++++++++++++++++ apps/server/src/services/auth.ts | 15 ++++-- 2 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 apps/server/src/services/auth.spec.ts diff --git a/apps/server/src/services/auth.spec.ts b/apps/server/src/services/auth.spec.ts new file mode 100644 index 000000000..df0d49a7c --- /dev/null +++ b/apps/server/src/services/auth.spec.ts @@ -0,0 +1,73 @@ +import supertest from "supertest"; +import options from "./options"; +import cls from "./cls"; +import { Application } from "express"; +import config from "./config"; +import { refreshAuth } from "./auth"; + +let app: Application; + +describe("Auth", () => { + beforeAll(async () => { + const buildApp = (await (import("../../src/app.js"))).default; + app = await buildApp(); + }); + + describe("Auth", () => { + beforeAll(() => { + config.General.noAuthentication = false; + refreshAuth(); + }); + + it("goes to login and asks for TOTP if enabled", async () => { + cls.init(() => { + options.setOption("mfaEnabled", "true"); + options.setOption("mfaMethod", "totp"); + options.setOption("totpVerificationHash", "hi"); + }); + const response = await supertest(app) + .get("/") + .redirects(1) + .expect(200); + expect(response.text).toContain(`id="totpToken"`); + }); + + it("goes to login and doesn't ask for TOTP is disabled", async () => { + cls.init(() => { + options.setOption("mfaEnabled", "false"); + }); + const response = await supertest(app) + .get("/") + .redirects(1) + .expect(200) + expect(response.text).not.toContain(`id="totpToken"`); + }); + }); + + describe("No auth", () => { + beforeAll(() => { + config.General.noAuthentication = true; + refreshAuth(); + }); + + it("doesn't ask for authentication when disabled, even if TOTP is enabled", async () => { + cls.init(() => { + options.setOption("mfaEnabled", "true"); + options.setOption("mfaMethod", "totp"); + options.setOption("totpVerificationHash", "hi"); + }); + await supertest(app) + .get("/") + .expect(200); + }); + + it("doesn't ask for authentication when disabled, with TOTP disabled", async () => { + cls.init(() => { + options.setOption("mfaEnabled", "false"); + }); + await supertest(app) + .get("/") + .expect(200); + }); + }); +}, 60_000); diff --git a/apps/server/src/services/auth.ts b/apps/server/src/services/auth.ts index b076c61c5..df18fba49 100644 --- a/apps/server/src/services/auth.ts +++ b/apps/server/src/services/auth.ts @@ -11,7 +11,8 @@ import options from "./options.js"; import attributes from "./attributes.js"; import type { NextFunction, Request, Response } from "express"; -const noAuthentication = config.General && config.General.noAuthentication === true; +let noAuthentication = false; +refreshAuth(); function checkAuth(req: Request, res: Response, next: NextFunction) { if (!sqlInit.isDbInitialized()) { @@ -22,7 +23,7 @@ function checkAuth(req: Request, res: Response, next: NextFunction) { const currentSsoStatus = openID.isOpenIDEnabled(); const lastAuthState = req.session.lastAuthState || { totpEnabled: false, ssoEnabled: false }; - if (isElectron) { + if (isElectron || noAuthentication) { next(); return; } else if (currentTotpStatus !== lastAuthState.totpEnabled || currentSsoStatus !== lastAuthState.ssoEnabled) { @@ -58,7 +59,15 @@ function checkAuth(req: Request, res: Response, next: NextFunction) { } } - +/** + * Rechecks whether authentication is needed or not by re-reading the config. + * The value is cached to avoid reading at every request. + * + * Generally this method should only be called during tests. + */ +export function refreshAuth() { + noAuthentication = (config.General && config.General.noAuthentication === true); +} // for electron things which need network stuff // currently, we're doing that for file upload because handling form data seems to be difficult