Bug fixes & generic OIDC/SSO provider support

backend/index.js

@@ -3,7 +3,9 @@
 import app from "./app.js";
 import internalCertificate from "./internal/certificate.js";
 import internalIpRanges from "./internal/ip_ranges.js";
+import { logFileConfigAudit } from "./internal/oidc.js";
 import { global as logger } from "./logger.js";
+import { loadFileConfig } from "./lib/oidc-file-config.js";
 import { migrateUp } from "./migrate.js";
 import { getCompiledSchema } from "./schema/index.js";
 import setup from "./setup.js";
@@ -14,6 +16,12 @@ async function appStart() {
 	return migrateUp()
 		.then(setup)
 		.then(getCompiledSchema)
+		.then(() => {
+			// Eagerly load file-based OIDC config so errors surface at startup,
+			// not on the first authentication attempt.
+			loadFileConfig();
+			return logFileConfigAudit();
+		})
 		.then(() => {
 			if (!IP_RANGES_FETCH_ENABLED) {
 				logger.info("IP Ranges fetch is disabled by environment variable");

backend/internal/2fa.js

@@ -6,6 +6,25 @@ import authModel from "../models/auth.js";
 import internalUser from "./user.js";
 
 const APP_NAME = "Nginx Proxy Manager";
+
+/**
+ * Guard that throws if the user has no password auth record (i.e. OIDC-only).
+ * 2FA is tied to password auth and should not be available for externally
+ * authenticated accounts — their IdP manages security.
+ */
+const requirePasswordAuth = async (userId) => {
+	const auth = await authModel
+		.query()
+		.where("user_id", userId)
+		.andWhere("type", "password")
+		.first();
+	if (!auth) {
+		throw new errs.ValidationError(
+			"Two-factor authentication is not available for externally authenticated accounts",
+		);
+	}
+	return auth;
+};
 const BACKUP_CODE_COUNT = 8;
 
 /**
@@ -33,8 +52,13 @@ const internal2fa = {
 	 * @returns {Promise<boolean>}
 	 */
 	isEnabled: async (userId) => {
-		const auth = await internal2fa.getUserPasswordAuth(userId);
-		return auth?.meta?.totp_enabled === true;
+		try {
+			const auth = await internal2fa.getUserPasswordAuth(userId);
+			return auth?.meta?.totp_enabled === true;
+		} catch {
+			// No password auth record exists (e.g. OIDC-only user) — 2FA not possible
+			return false;
+		}
 	},
 
 	/**
@@ -46,7 +70,7 @@ const internal2fa = {
 	getStatus: async (access, userId) => {
 		await access.can("users:password", userId);
 		await internalUser.get(access, { id: userId });
-		const auth = await internal2fa.getUserPasswordAuth(userId);
+		const auth = await requirePasswordAuth(userId);
 		const enabled = auth?.meta?.totp_enabled === true;
 		let backup_codes_remaining = 0;
 
@@ -77,7 +101,7 @@ const internal2fa = {
 			label: user.email,
 			secret: secret,
 		});
-		const auth = await internal2fa.getUserPasswordAuth(userId);
+		const auth = await requirePasswordAuth(userId);
 
 		// ensure user isn't already setup for 2fa
 		const enabled = auth?.meta?.totp_enabled === true;
@@ -109,7 +133,7 @@ const internal2fa = {
 	enable: async (access, userId, code) => {
 		await access.can("users:password", userId);
 		await internalUser.get(access, { id: userId });
-		const auth = await internal2fa.getUserPasswordAuth(userId);
+		const auth = await requirePasswordAuth(userId);
 		const secret = auth?.meta?.totp_pending_secret || false;
 
 		if (!secret) {
@@ -153,7 +177,7 @@ const internal2fa = {
 	disable: async (access, userId, code) => {
 		await access.can("users:password", userId);
 		await internalUser.get(access, { id: userId });
-		const auth = await internal2fa.getUserPasswordAuth(userId);
+		const auth = await requirePasswordAuth(userId);
 
 		const enabled = auth?.meta?.totp_enabled === true;
 		if (!enabled) {
@@ -194,7 +218,13 @@ const internal2fa = {
 	 * @returns {Promise<boolean>}
 	 */
 	verifyForLogin: async (userId, token) => {
-		const auth = await internal2fa.getUserPasswordAuth(userId);
+		let auth;
+		try {
+			auth = await internal2fa.getUserPasswordAuth(userId);
+		} catch {
+			// No password auth record (OIDC-only user) — 2FA not applicable
+			return false;
+		}
 		const secret = auth?.meta?.totp_secret || false;
 
 		if (!secret) {
@@ -254,7 +284,7 @@ const internal2fa = {
 	regenerateBackupCodes: async (access, userId, token) => {
 		await access.can("users:password", userId);
 		await internalUser.get(access, { id: userId });
-		const auth = await internal2fa.getUserPasswordAuth(userId);
+		const auth = await requirePasswordAuth(userId);
 		const enabled = auth?.meta?.totp_enabled === true;
 		const secret = auth?.meta?.totp_secret || false;
 

backend/internal/nginx.js

@@ -83,10 +83,7 @@ const internalNginx = {
 								meta: combined_meta,
 							})
 							.then(() => {
-								internalNginx.renameConfigAsError(host_type, host);
-							})
-							.then(() => {
-								return internalNginx.deleteConfig(host_type, host, true);
+								return internalNginx.renameConfigAsError(host_type, host);
 							});
 					});
 			})
@@ -375,8 +372,10 @@ const internalNginx = {
 		const config_file_err = `${config_file}.err`;
 
 		return new Promise((resolve /*, reject*/) => {
-			fs.unlink(config_file, () => {
-				// ignore result, continue
+			// Remove any pre-existing .err file first, then rename the current
+			// config so it is preserved for debugging instead of being lost.
+			fs.unlink(config_file_err, () => {
+				// ignore result — the .err file may not exist
 				fs.rename(config_file, config_file_err, () => {
 					// also ignore result, as this is a debugging informative file anyway
 					resolve();
@@ -419,7 +418,15 @@ const internalNginx = {
 	 * @param   {string}  config
 	 * @returns {boolean}
 	 */
-	advancedConfigHasDefaultLocation: (cfg) => !!cfg.match(/^(?:.*;)?\s*?location\s*?\/\s*?{/im),
+	advancedConfigHasDefaultLocation: (cfg) => {
+		// Strip comment lines so they don't interfere with detection
+		const stripped = cfg.replace(/^\s*#.*$/gm, "");
+		// Match location blocks that resolve to "/" (root path), including
+		// nginx modifiers like "=", "~", "~*", "^~" and optional whitespace.
+		// The (?:^|;) prefix with \s* handles both start-of-line and the rare
+		// case where a directive and a location block share a line.
+		return !!stripped.match(/(?:^|;)\s*location\s+(?:[=~^~*]{1,2}\s+)?\/\s*\{/im);
+	},
 
 	/**
 	 * @returns {boolean}