From 057ea078f1aa7e2504c083870ab02b894a7ccb89 Mon Sep 17 00:00:00 2001 From: Michael Uray Date: Tue, 23 Jun 2026 06:11:11 +0000 Subject: [PATCH 1/7] fix(authProviders): instrument OIDC callback + redirect on fail (not 500) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production-blocking: every OIDC callback (bogus or real code) returned 500 Internal Server Error, even with valid sessions. No log was emitted between `try auth via openid` and the 500 — passport strategy threw silently before `handleProviderAuth` could log. Per Codex plan-review amendments: - Switch to explicit-callback passport.authenticate variant so strategy errors surface as err/info/status args instead of being swallowed. - Privacy-safe diagnostic shape (no raw code/state/tokens/user). Fields: stage, errMessage/Name/Stack, hasUser, infoSummary, statusCode, hasSession + sessionKeys (names only), cookie + state + code PRESENCE (lengths only), host, forwardedProto. - Permanent invalid-callback guard: ALL failures → 302 to /login with ?error=oidc_callback_failed, never 500. Same wrap covers handleProviderAuth throws. Image-tag: hardcoreeng/account:integration-wac-2026-06-21-codex-r7. Next step (P0-T2): user attempts login, we capture the actual error from the diagnostic, then ship a surgical fix in r8. Signed-off-by: Michael Uray Signed-off-by: Michael Uray --- pods/authProviders/src/openid.ts | 90 +++++++++++++++++++++++++++----- 1 file changed, 78 insertions(+), 12 deletions(-) diff --git a/pods/authProviders/src/openid.ts b/pods/authProviders/src/openid.ts index 0a19d99b288..6bfa5e56fa7 100644 --- a/pods/authProviders/src/openid.ts +++ b/pods/authProviders/src/openid.ts @@ -74,17 +74,61 @@ export function registerOpenid ( })(ctx, next) }) - router.get( - redirectURL, - async (ctx, next) => { - const state = safeParseAuthState(ctx.query?.state) - const branding = getBranding(brandings, state?.branding) - - await passport.authenticate('oidc', { - failureRedirect: concatLink(branding?.front ?? frontUrl, '/login') - })(ctx, next) - }, - async (ctx, next) => { + router.get(redirectURL, async (ctx, next) => { + const state = safeParseAuthState(ctx.query?.state) + const branding = getBranding(brandings, state?.branding) + const loginUrl = concatLink(branding?.front ?? frontUrl, '/login') + + try { + // INSTRUMENTATION (Codex-approved): explicit-callback variant captures + // err/info/status that would otherwise be swallowed by the strategy. + // PRIVACY: never log raw code, raw state, tokens, or full ctx.state.user. + await new Promise((resolve) => { + passport.authenticate( + 'oidc', + { failureRedirect: loginUrl }, + (err: any, user: any, info: any, status: any) => { + const diag = { + stage: 'oidc_callback', + hasErr: err != null, + errMessage: err?.message, + errName: err?.name, + errStack: err?.stack, + hasUser: user != null, + infoSummary: info?.message ?? String(info ?? ''), + statusCode: status, + // session + cookie diagnostics (no raw values) + hasSession: ctx.session != null, + sessionKeys: ctx.session != null ? Object.keys(ctx.session) : [], + hasCookieHeader: ctx.request.headers.cookie != null, + cookieHeaderLen: ctx.request.headers.cookie?.length ?? 0, + // request shape + host: ctx.request.headers.host, + forwardedProto: ctx.request.headers['x-forwarded-proto'], + // state presence (length only, not value) + statePresent: typeof ctx.query?.state === 'string', + stateLength: typeof ctx.query?.state === 'string' ? (ctx.query.state as string).length : 0, + codePresent: typeof ctx.query?.code === 'string' + } + if (err != null || user == null) { + measureCtx.error('OIDC callback failed', diag) + } else { + measureCtx.info('OIDC callback succeeded — entering handleProviderAuth', { + hasSession: diag.hasSession + }) + ctx.state.user = user + } + resolve() + } + )(ctx, async () => {}) + }) + + if (ctx.state.user == null) { + // Strategy failed; redirect explicitly so we never bubble a 500. + ctx.redirect(loginUrl + '?error=oidc_callback_failed') + return + } + const email = ctx.state.user.email const verifiedEmail = (ctx.state.user.email_verified as boolean) ? email : '' const nameParts = (ctx.state.user.name ?? ctx.state.user.username ?? '').split(' ') @@ -112,8 +156,30 @@ export function registerOpenid ( } await next() + } catch (err: any) { + // Permanent invalid-callback guard: ANY failure → 302 to /login, never 500. + measureCtx.error('OIDC callback failed', { + stage: 'oidc_callback', + hasErr: true, + errMessage: err?.message, + errName: err?.name, + errStack: err?.stack, + hasUser: ctx.state?.user != null, + infoSummary: '', + statusCode: undefined, + hasSession: ctx.session != null, + sessionKeys: ctx.session != null ? Object.keys(ctx.session) : [], + hasCookieHeader: ctx.request.headers.cookie != null, + cookieHeaderLen: ctx.request.headers.cookie?.length ?? 0, + host: ctx.request.headers.host, + forwardedProto: ctx.request.headers['x-forwarded-proto'], + statePresent: typeof ctx.query?.state === 'string', + stateLength: typeof ctx.query?.state === 'string' ? (ctx.query.state as string).length : 0, + codePresent: typeof ctx.query?.code === 'string' + }) + ctx.redirect(loginUrl + '?error=oidc_callback_failed') } - ) + }) return { name, displayName } } From 0b6fc909c3c49dc23d528269045dd770aa2277c7 Mon Sep 17 00:00:00 2001 From: Michael Uray Date: Tue, 23 Jun 2026 06:11:17 +0000 Subject: [PATCH 2/7] fix(account): admin.ts trim+lowercase on both env-set AND lookup Codex plan-review Critical: server/account/src/admin.ts:15-18 built the ADMIN_EMAILS set without normalizing, AND isAdminEmail() didn't normalize the input. Today value is clean so isAdmin works, but a future env-value like ADMIN_EMAILS=" Michael@Uray.io " would silently break admin detection (Set lookup would miss the lowercased input). Mongo backend already has this hardening (collections/mongo.ts:1141-1147). Postgres + this helper didn't. Now BOTH sides normalize: env split -> trim -> toLowerCase -> filter non-empty + warn on no-@ entries (Codex Optional: warn not reject -- too sharp for a latent fix). Lookup also trim().toLowerCase(). Standalone upstream-PR-able per Codex Q-I3. Tests cover whitespace, case-insensitive env, case-insensitive lookup, invalid-shape warn, empty/unset env, multi-entry. Signed-off-by: Michael Uray Signed-off-by: Michael Uray --- .../account/src/__tests__/adminEmails.test.ts | 125 ++++++++++++++++++ server/account/src/admin.ts | 45 ++++++- 2 files changed, 167 insertions(+), 3 deletions(-) create mode 100644 server/account/src/__tests__/adminEmails.test.ts diff --git a/server/account/src/__tests__/adminEmails.test.ts b/server/account/src/__tests__/adminEmails.test.ts new file mode 100644 index 00000000000..74f370367d7 --- /dev/null +++ b/server/account/src/__tests__/adminEmails.test.ts @@ -0,0 +1,125 @@ +// +// Copyright © 2026 Hardcore Engineering Inc. +// +// Licensed under the Eclipse Public License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. You may +// obtain a copy of the License at https://www.eclipse.org/legal/epl-2.0 +// +// SPDX-License-Identifier: EPL-2.0 +// +// Coverage for admin.ts hardening: both env-set (parseAdminEmails) AND +// lookup (isAdminEmail) must trim+lowercase. Today ADMIN_EMAILS is clean, +// but a future env value like " Michael@Uray.io " would silently break +// admin detection without normalization on the env-side. Mongo backend +// already has this hardening (collections/mongo.ts:1141-1147); this test +// covers the equivalent for the shared helper. +// + +import type * as AdminModuleType from '../admin' + +type AdminModule = typeof AdminModuleType + +/** + * Re-import admin.ts with a controlled ADMIN_EMAILS env value. + * admin.ts evaluates ADMIN_EMAILS at module-load, so each test scenario + * needs an isolated module load. + */ +function loadAdmin (envValue: string | undefined): AdminModule { + let mod: AdminModule | undefined + jest.isolateModules(() => { + if (envValue === undefined) { + delete process.env.ADMIN_EMAILS + } else { + process.env.ADMIN_EMAILS = envValue + } + mod = jest.requireActual('../admin') + }) + if (mod === undefined) throw new Error('admin module did not load') + return mod +} + +describe('admin.ts — env parsing + lookup normalization', () => { + const originalEnv = process.env.ADMIN_EMAILS + + afterAll(() => { + if (originalEnv === undefined) { + delete process.env.ADMIN_EMAILS + } else { + process.env.ADMIN_EMAILS = originalEnv + } + }) + + test('1. whitespace tolerance — env entry with leading/trailing spaces', () => { + const { isAdminEmail } = loadAdmin(' Michael@Uray.io ') + expect(isAdminEmail('michael@uray.io')).toBe(true) + }) + + test('2. case-insensitive env — uppercase env entry matches lowercase lookup', () => { + const { isAdminEmail } = loadAdmin('MICHAEL@URAY.IO') + expect(isAdminEmail('michael@uray.io')).toBe(true) + }) + + test('3. case-insensitive lookup — lowercase env matches uppercase lookup input', () => { + const { isAdminEmail } = loadAdmin('michael@uray.io') + expect(isAdminEmail('MICHAEL@URAY.IO')).toBe(true) + }) + + test('4. invalid-shape entries WARN (not reject) — no-@ entries kept for backwards compatibility', () => { + const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}) + try { + const { isAdminEmail } = loadAdmin('foo,bar@@,baz') + // All entries kept; warn lists no-@ entries + expect(isAdminEmail('foo')).toBe(true) + expect(isAdminEmail('baz')).toBe(true) + expect(isAdminEmail('bar@@')).toBe(true) + expect(warnSpy).toHaveBeenCalledWith( + 'ADMIN_EMAILS contains entries without "@" (kept for backwards compatibility)', + { entries: ['foo', 'baz'] } + ) + } finally { + warnSpy.mockRestore() + } + }) + + test('4b. invalid-shape WARN via injected logger — all entries kept', () => { + // Direct parseAdminEmails call with custom logger — proves the + // injection point works (admin.ts has no MeasureContext at load time). + const { parseAdminEmails } = loadAdmin('') + const warn = jest.fn() + const set = parseAdminEmails('foo,alice@example.com,baz', { warn }) + expect(set.has('foo')).toBe(true) + expect(set.has('alice@example.com')).toBe(true) + expect(set.has('baz')).toBe(true) + expect(set.size).toBe(3) + expect(warn).toHaveBeenCalledWith( + 'ADMIN_EMAILS contains entries without "@" (kept for backwards compatibility)', + { entries: ['foo', 'baz'] } + ) + }) + + test('5. empty env string — empty set, all lookups return false', () => { + const { isAdminEmail } = loadAdmin('') + expect(isAdminEmail('michael@uray.io')).toBe(false) + expect(isAdminEmail('')).toBe(false) + }) + + test('6. unset env — empty set, all lookups return false', () => { + const { isAdminEmail } = loadAdmin(undefined) + expect(isAdminEmail('michael@uray.io')).toBe(false) + }) + + test('7. multiple entries with mixed whitespace + case all normalized', () => { + const { isAdminEmail } = loadAdmin('alice@example.com, bob@example.org , Carol@example.io') + expect(isAdminEmail('alice@example.com')).toBe(true) + expect(isAdminEmail('bob@example.org')).toBe(true) + expect(isAdminEmail('carol@example.io')).toBe(true) + // Lookup also normalizes + expect(isAdminEmail(' Bob@Example.ORG ')).toBe(true) + }) + + test('8. null/undefined lookup input — return false (no throw)', () => { + const { isAdminEmail } = loadAdmin('admin@example.com') + expect(isAdminEmail(null)).toBe(false) + expect(isAdminEmail(undefined)).toBe(false) + }) +}) diff --git a/server/account/src/admin.ts b/server/account/src/admin.ts index c5199e4b30d..b1a614025e8 100644 --- a/server/account/src/admin.ts +++ b/server/account/src/admin.ts @@ -12,8 +12,47 @@ // See the License for the specific language governing permissions and // limitations under the License. // -const ADMIN_EMAILS = new Set(process.env.ADMIN_EMAILS?.split(',') ?? []) -export function isAdminEmail (email: string): boolean { - return ADMIN_EMAILS.has(email.trim()) +interface AdminEmailsLogger { + warn?: (msg: string, attrs?: Record) => void +} + +/** + * Parse the ADMIN_EMAILS env value into a normalized Set. + * + * - split on ',' + * - trim() each entry + * - toLowerCase() each entry + * - drop empty entries + * - entries without '@' are kept (backwards compatibility with deployments + * that use login-id style ADMIN_EMAILS values, e.g. ADMIN_EMAILS=admin) + * but a warning is emitted listing them (Codex Optional: warn-don't-reject). + */ +export function parseAdminEmails (envValue: string | undefined, logger?: AdminEmailsLogger): Set { + if (envValue == null || envValue === '') return new Set() + const entries = envValue + .split(',') + .map((s) => s.trim().toLowerCase()) + .filter((s) => s.length > 0) + const invalid: string[] = [] + for (const entry of entries) { + if (!entry.includes('@')) invalid.push(entry) + } + if (invalid.length > 0) { + const warn = + logger?.warn ?? + ((msg: string, attrs?: Record) => { + // eslint-disable-next-line no-console + console.warn(msg, attrs) + }) + warn('ADMIN_EMAILS contains entries without "@" (kept for backwards compatibility)', { entries: invalid }) + } + return new Set(entries) +} + +const ADMIN_EMAILS = parseAdminEmails(process.env.ADMIN_EMAILS) + +export function isAdminEmail (email: string | null | undefined): boolean { + if (email == null || email === '') return false + return ADMIN_EMAILS.has(email.trim().toLowerCase()) } From bc55078f203fad8ed8b19eb2829d0ff96c6ee3f2 Mon Sep 17 00:00:00 2001 From: Michael Uray Date: Tue, 23 Jun 2026 06:11:20 +0000 Subject: [PATCH 3/7] chore(authProviders): drop cookieHeaderLen from OIDC diagnostics (E4 amendment) Codex E4 amendment (optional, low-priority): hasCookieHeader is sufficient to diagnose missing session-cookie issues; the cookieHeaderLen byte count adds no operational value and is one more PII-adjacent surface to keep an eye on. Drop it from both diagnostic paths in pods/authProviders/src/openid.ts. Signed-off-by: Michael Uray Signed-off-by: Michael Uray --- pods/authProviders/src/openid.ts | 68 +++++++++---------- .../account/src/__tests__/adminEmails.test.ts | 11 +-- 2 files changed, 37 insertions(+), 42 deletions(-) diff --git a/pods/authProviders/src/openid.ts b/pods/authProviders/src/openid.ts index 6bfa5e56fa7..25a5330a225 100644 --- a/pods/authProviders/src/openid.ts +++ b/pods/authProviders/src/openid.ts @@ -84,43 +84,38 @@ export function registerOpenid ( // err/info/status that would otherwise be swallowed by the strategy. // PRIVACY: never log raw code, raw state, tokens, or full ctx.state.user. await new Promise((resolve) => { - passport.authenticate( - 'oidc', - { failureRedirect: loginUrl }, - (err: any, user: any, info: any, status: any) => { - const diag = { - stage: 'oidc_callback', - hasErr: err != null, - errMessage: err?.message, - errName: err?.name, - errStack: err?.stack, - hasUser: user != null, - infoSummary: info?.message ?? String(info ?? ''), - statusCode: status, - // session + cookie diagnostics (no raw values) - hasSession: ctx.session != null, - sessionKeys: ctx.session != null ? Object.keys(ctx.session) : [], - hasCookieHeader: ctx.request.headers.cookie != null, - cookieHeaderLen: ctx.request.headers.cookie?.length ?? 0, - // request shape - host: ctx.request.headers.host, - forwardedProto: ctx.request.headers['x-forwarded-proto'], - // state presence (length only, not value) - statePresent: typeof ctx.query?.state === 'string', - stateLength: typeof ctx.query?.state === 'string' ? (ctx.query.state as string).length : 0, - codePresent: typeof ctx.query?.code === 'string' - } - if (err != null || user == null) { - measureCtx.error('OIDC callback failed', diag) - } else { - measureCtx.info('OIDC callback succeeded — entering handleProviderAuth', { - hasSession: diag.hasSession - }) - ctx.state.user = user - } - resolve() + passport.authenticate('oidc', { failureRedirect: loginUrl }, (err: any, user: any, info: any, status: any) => { + const diag = { + stage: 'oidc_callback', + hasErr: err != null, + errMessage: err?.message, + errName: err?.name, + errStack: err?.stack, + hasUser: user != null, + infoSummary: info?.message ?? String(info ?? ''), + statusCode: status, + // session + cookie diagnostics (no raw values) + hasSession: ctx.session != null, + sessionKeys: ctx.session != null ? Object.keys(ctx.session) : [], + hasCookieHeader: ctx.request.headers.cookie != null, + // request shape + host: ctx.request.headers.host, + forwardedProto: ctx.request.headers['x-forwarded-proto'], + // state presence (length only, not value) + statePresent: typeof ctx.query?.state === 'string', + stateLength: typeof ctx.query?.state === 'string' ? (ctx.query.state as string).length : 0, + codePresent: typeof ctx.query?.code === 'string' } - )(ctx, async () => {}) + if (err != null || user == null) { + measureCtx.error('OIDC callback failed', diag) + } else { + measureCtx.info('OIDC callback succeeded — entering handleProviderAuth', { + hasSession: diag.hasSession + }) + ctx.state.user = user + } + resolve() + })(ctx, async () => {}) }) if (ctx.state.user == null) { @@ -170,7 +165,6 @@ export function registerOpenid ( hasSession: ctx.session != null, sessionKeys: ctx.session != null ? Object.keys(ctx.session) : [], hasCookieHeader: ctx.request.headers.cookie != null, - cookieHeaderLen: ctx.request.headers.cookie?.length ?? 0, host: ctx.request.headers.host, forwardedProto: ctx.request.headers['x-forwarded-proto'], statePresent: typeof ctx.query?.state === 'string', diff --git a/server/account/src/__tests__/adminEmails.test.ts b/server/account/src/__tests__/adminEmails.test.ts index 74f370367d7..a683bda78d6 100644 --- a/server/account/src/__tests__/adminEmails.test.ts +++ b/server/account/src/__tests__/adminEmails.test.ts @@ -74,7 +74,9 @@ describe('admin.ts — env parsing + lookup normalization', () => { expect(isAdminEmail('bar@@')).toBe(true) expect(warnSpy).toHaveBeenCalledWith( 'ADMIN_EMAILS contains entries without "@" (kept for backwards compatibility)', - { entries: ['foo', 'baz'] } + { + entries: ['foo', 'baz'] + } ) } finally { warnSpy.mockRestore() @@ -91,10 +93,9 @@ describe('admin.ts — env parsing + lookup normalization', () => { expect(set.has('alice@example.com')).toBe(true) expect(set.has('baz')).toBe(true) expect(set.size).toBe(3) - expect(warn).toHaveBeenCalledWith( - 'ADMIN_EMAILS contains entries without "@" (kept for backwards compatibility)', - { entries: ['foo', 'baz'] } - ) + expect(warn).toHaveBeenCalledWith('ADMIN_EMAILS contains entries without "@" (kept for backwards compatibility)', { + entries: ['foo', 'baz'] + }) }) test('5. empty env string — empty set, all lookups return false', () => { From 33637a6913e30c92b9f101ed91600edf2eb98902 Mon Sep 17 00:00:00 2001 From: Michael Uray Date: Wed, 8 Jul 2026 13:08:30 +0000 Subject: [PATCH 4/7] auth(oidc): gate verbose callback diagnostics behind OIDC_DEBUG (L-AUTH-2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bei jedem fehlgeschlagenen OIDC-Callback wurde ein umfangreicher Diag-Block (errStack/sessionKeys/host/…) auf error-Level geloggt -> anonymes Log- Flooding via wiederholter invalider Callbacks. Normalbetrieb loggt jetzt nur eine knappe Fehlerzeile; der volle Diag-Block nur mit OIDC_DEBUG=true. Signed-off-by: Michael Uray --- pods/authProviders/src/openid.ts | 77 +++++++++++++++++++------------- 1 file changed, 46 insertions(+), 31 deletions(-) diff --git a/pods/authProviders/src/openid.ts b/pods/authProviders/src/openid.ts index 25a5330a225..83159c0df09 100644 --- a/pods/authProviders/src/openid.ts +++ b/pods/authProviders/src/openid.ts @@ -85,32 +85,38 @@ export function registerOpenid ( // PRIVACY: never log raw code, raw state, tokens, or full ctx.state.user. await new Promise((resolve) => { passport.authenticate('oidc', { failureRedirect: loginUrl }, (err: any, user: any, info: any, status: any) => { - const diag = { + // L-AUTH-2: keep only a terse error line in normal operation; the + // verbose diagnostics (errStack/sessionKeys/host/…) are enabled by + // OIDC_DEBUG so anonymous repeated invalid callbacks cannot flood logs. + const baseDiag = { stage: 'oidc_callback', hasErr: err != null, - errMessage: err?.message, errName: err?.name, - errStack: err?.stack, - hasUser: user != null, - infoSummary: info?.message ?? String(info ?? ''), + errMessage: err?.message, statusCode: status, - // session + cookie diagnostics (no raw values) - hasSession: ctx.session != null, - sessionKeys: ctx.session != null ? Object.keys(ctx.session) : [], - hasCookieHeader: ctx.request.headers.cookie != null, - // request shape - host: ctx.request.headers.host, - forwardedProto: ctx.request.headers['x-forwarded-proto'], - // state presence (length only, not value) - statePresent: typeof ctx.query?.state === 'string', - stateLength: typeof ctx.query?.state === 'string' ? (ctx.query.state as string).length : 0, - codePresent: typeof ctx.query?.code === 'string' + hasUser: user != null } + const diag = + process.env.OIDC_DEBUG === 'true' + ? { + ...baseDiag, + errStack: err?.stack, + infoSummary: info?.message ?? String(info ?? ''), + hasSession: ctx.session != null, + sessionKeys: ctx.session != null ? Object.keys(ctx.session) : [], + hasCookieHeader: ctx.request.headers.cookie != null, + host: ctx.request.headers.host, + forwardedProto: ctx.request.headers['x-forwarded-proto'], + statePresent: typeof ctx.query?.state === 'string', + stateLength: typeof ctx.query?.state === 'string' ? (ctx.query.state as string).length : 0, + codePresent: typeof ctx.query?.code === 'string' + } + : baseDiag if (err != null || user == null) { measureCtx.error('OIDC callback failed', diag) } else { measureCtx.info('OIDC callback succeeded — entering handleProviderAuth', { - hasSession: diag.hasSession + hasSession: ctx.session != null }) ctx.state.user = user } @@ -153,24 +159,33 @@ export function registerOpenid ( await next() } catch (err: any) { // Permanent invalid-callback guard: ANY failure → 302 to /login, never 500. - measureCtx.error('OIDC callback failed', { + // L-AUTH-2: verbose fields behind OIDC_DEBUG (see the callback diag above). + const baseDiag = { stage: 'oidc_callback', hasErr: true, - errMessage: err?.message, errName: err?.name, - errStack: err?.stack, - hasUser: ctx.state?.user != null, - infoSummary: '', + errMessage: err?.message, statusCode: undefined, - hasSession: ctx.session != null, - sessionKeys: ctx.session != null ? Object.keys(ctx.session) : [], - hasCookieHeader: ctx.request.headers.cookie != null, - host: ctx.request.headers.host, - forwardedProto: ctx.request.headers['x-forwarded-proto'], - statePresent: typeof ctx.query?.state === 'string', - stateLength: typeof ctx.query?.state === 'string' ? (ctx.query.state as string).length : 0, - codePresent: typeof ctx.query?.code === 'string' - }) + hasUser: ctx.state?.user != null + } + measureCtx.error( + 'OIDC callback failed', + process.env.OIDC_DEBUG === 'true' + ? { + ...baseDiag, + errStack: err?.stack, + infoSummary: '', + hasSession: ctx.session != null, + sessionKeys: ctx.session != null ? Object.keys(ctx.session) : [], + hasCookieHeader: ctx.request.headers.cookie != null, + host: ctx.request.headers.host, + forwardedProto: ctx.request.headers['x-forwarded-proto'], + statePresent: typeof ctx.query?.state === 'string', + stateLength: typeof ctx.query?.state === 'string' ? (ctx.query.state as string).length : 0, + codePresent: typeof ctx.query?.code === 'string' + } + : baseDiag + ) ctx.redirect(loginUrl + '?error=oidc_callback_failed') } }) From 5983f86b63b534c3243c973940c088087e6ad348 Mon Sep 17 00:00:00 2001 From: Michael Uray Date: Wed, 8 Jul 2026 13:08:40 +0000 Subject: [PATCH 5/7] account(admin): drop non-email ADMIN_EMAILS entries unless explicitly allowed (L-AUTH-3) Ein Tippfehler wie ADMIN_EMAILS=admin,michel@... legte bislang einen unbeabsichtigten Admin-Identifier an (Eintrag ohne @ wurde still behalten). Jetzt werden @-lose Eintraege per Default verworfen (fail-closed) und nur bei ADMIN_EMAILS_ALLOW_LOGIN_ID=true als Login-ID akzeptiert; Warnung stets. Signed-off-by: Michael Uray --- .../account/src/__tests__/adminEmails.test.ts | 71 +++++++++++++------ server/account/src/admin.ts | 24 +++++-- 2 files changed, 68 insertions(+), 27 deletions(-) diff --git a/server/account/src/__tests__/adminEmails.test.ts b/server/account/src/__tests__/adminEmails.test.ts index a683bda78d6..15cac826384 100644 --- a/server/account/src/__tests__/adminEmails.test.ts +++ b/server/account/src/__tests__/adminEmails.test.ts @@ -64,38 +64,67 @@ describe('admin.ts — env parsing + lookup normalization', () => { expect(isAdminEmail('MICHAEL@URAY.IO')).toBe(true) }) - test('4. invalid-shape entries WARN (not reject) — no-@ entries kept for backwards compatibility', () => { + test('4. L-AUTH-3: no-@ entries are DROPPED by default (fail-closed) + warn', () => { const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}) + const prevFlag = process.env.ADMIN_EMAILS_ALLOW_LOGIN_ID + delete process.env.ADMIN_EMAILS_ALLOW_LOGIN_ID try { const { isAdminEmail } = loadAdmin('foo,bar@@,baz') - // All entries kept; warn lists no-@ entries - expect(isAdminEmail('foo')).toBe(true) - expect(isAdminEmail('baz')).toBe(true) + // No-@ entries dropped; only the '@'-bearing entry survives. + expect(isAdminEmail('foo')).toBe(false) + expect(isAdminEmail('baz')).toBe(false) expect(isAdminEmail('bar@@')).toBe(true) - expect(warnSpy).toHaveBeenCalledWith( - 'ADMIN_EMAILS contains entries without "@" (kept for backwards compatibility)', - { - entries: ['foo', 'baz'] - } - ) + expect(warnSpy).toHaveBeenCalledWith('ADMIN_EMAILS contains entries without "@" (DROPPED)', { + entries: ['foo', 'baz'] + }) } finally { warnSpy.mockRestore() + if (prevFlag === undefined) delete process.env.ADMIN_EMAILS_ALLOW_LOGIN_ID + else process.env.ADMIN_EMAILS_ALLOW_LOGIN_ID = prevFlag } }) - test('4b. invalid-shape WARN via injected logger — all entries kept', () => { + test('4b. L-AUTH-3: injected logger sees DROPPED no-@ entries by default', () => { // Direct parseAdminEmails call with custom logger — proves the // injection point works (admin.ts has no MeasureContext at load time). - const { parseAdminEmails } = loadAdmin('') - const warn = jest.fn() - const set = parseAdminEmails('foo,alice@example.com,baz', { warn }) - expect(set.has('foo')).toBe(true) - expect(set.has('alice@example.com')).toBe(true) - expect(set.has('baz')).toBe(true) - expect(set.size).toBe(3) - expect(warn).toHaveBeenCalledWith('ADMIN_EMAILS contains entries without "@" (kept for backwards compatibility)', { - entries: ['foo', 'baz'] - }) + const prevFlag = process.env.ADMIN_EMAILS_ALLOW_LOGIN_ID + delete process.env.ADMIN_EMAILS_ALLOW_LOGIN_ID + try { + const { parseAdminEmails } = loadAdmin('') + const warn = jest.fn() + const set = parseAdminEmails('foo,alice@example.com,baz', { warn }) + expect(set.has('foo')).toBe(false) + expect(set.has('alice@example.com')).toBe(true) + expect(set.has('baz')).toBe(false) + expect(set.size).toBe(1) + expect(warn).toHaveBeenCalledWith('ADMIN_EMAILS contains entries without "@" (DROPPED)', { + entries: ['foo', 'baz'] + }) + } finally { + if (prevFlag === undefined) delete process.env.ADMIN_EMAILS_ALLOW_LOGIN_ID + else process.env.ADMIN_EMAILS_ALLOW_LOGIN_ID = prevFlag + } + }) + + test('4c. L-AUTH-3: no-@ entries kept when ADMIN_EMAILS_ALLOW_LOGIN_ID=true (opt-in)', () => { + const prevFlag = process.env.ADMIN_EMAILS_ALLOW_LOGIN_ID + process.env.ADMIN_EMAILS_ALLOW_LOGIN_ID = 'true' + try { + const { parseAdminEmails } = loadAdmin('') + const warn = jest.fn() + const set = parseAdminEmails('foo,alice@example.com,baz', { warn }) + expect(set.has('foo')).toBe(true) + expect(set.has('alice@example.com')).toBe(true) + expect(set.has('baz')).toBe(true) + expect(set.size).toBe(3) + expect(warn).toHaveBeenCalledWith( + 'ADMIN_EMAILS contains entries without "@" (kept (ADMIN_EMAILS_ALLOW_LOGIN_ID=true))', + { entries: ['foo', 'baz'] } + ) + } finally { + if (prevFlag === undefined) delete process.env.ADMIN_EMAILS_ALLOW_LOGIN_ID + else process.env.ADMIN_EMAILS_ALLOW_LOGIN_ID = prevFlag + } }) test('5. empty env string — empty set, all lookups return false', () => { diff --git a/server/account/src/admin.ts b/server/account/src/admin.ts index b1a614025e8..a4c4185d67d 100644 --- a/server/account/src/admin.ts +++ b/server/account/src/admin.ts @@ -24,19 +24,30 @@ interface AdminEmailsLogger { * - trim() each entry * - toLowerCase() each entry * - drop empty entries - * - entries without '@' are kept (backwards compatibility with deployments - * that use login-id style ADMIN_EMAILS values, e.g. ADMIN_EMAILS=admin) - * but a warning is emitted listing them (Codex Optional: warn-don't-reject). + * - L-AUTH-3: entries without '@' are DROPPED by default (a typo like + * ADMIN_EMAILS=admin,michel@… would otherwise mint an unintended admin + * identifier). Deployments that intentionally use login-id style values must + * opt in with ADMIN_EMAILS_ALLOW_LOGIN_ID=true; a warning is always emitted + * listing the affected entries. */ export function parseAdminEmails (envValue: string | undefined, logger?: AdminEmailsLogger): Set { if (envValue == null || envValue === '') return new Set() + const allowLoginId = process.env.ADMIN_EMAILS_ALLOW_LOGIN_ID === 'true' const entries = envValue .split(',') .map((s) => s.trim().toLowerCase()) .filter((s) => s.length > 0) const invalid: string[] = [] + const kept: string[] = [] for (const entry of entries) { - if (!entry.includes('@')) invalid.push(entry) + if (entry.includes('@')) { + kept.push(entry) + } else if (allowLoginId) { + kept.push(entry) + invalid.push(entry) // kept, but still warn for visibility + } else { + invalid.push(entry) // fail-closed: dropped + } } if (invalid.length > 0) { const warn = @@ -45,9 +56,10 @@ export function parseAdminEmails (envValue: string | undefined, logger?: AdminEm // eslint-disable-next-line no-console console.warn(msg, attrs) }) - warn('ADMIN_EMAILS contains entries without "@" (kept for backwards compatibility)', { entries: invalid }) + const action = allowLoginId ? 'kept (ADMIN_EMAILS_ALLOW_LOGIN_ID=true)' : 'DROPPED' + warn(`ADMIN_EMAILS contains entries without "@" (${action})`, { entries: invalid }) } - return new Set(entries) + return new Set(kept) } const ADMIN_EMAILS = parseAdminEmails(process.env.ADMIN_EMAILS) From 8f39da662963b4ff52a4e8ddf16837741d5f4b10 Mon Sep 17 00:00:00 2001 From: Michael Uray Date: Wed, 8 Jul 2026 13:08:57 +0000 Subject: [PATCH 6/7] auth(oidc): redirect to /login when provider auth yields no account (L-AUTH-4) handleProviderAuth liefert '' bei einem nicht aufloesbaren Login (z.B. kein Account + Signup disabled). Bisher erfolgte dann kein Redirect und next() lief ins Leere -> haengende/leere Antwort. Jetzt fail-closed: explizites 302 auf /login?error=oidc_no_account. Signed-off-by: Michael Uray --- pods/authProviders/src/openid.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pods/authProviders/src/openid.ts b/pods/authProviders/src/openid.ts index 83159c0df09..da927c940d0 100644 --- a/pods/authProviders/src/openid.ts +++ b/pods/authProviders/src/openid.ts @@ -154,6 +154,11 @@ export function registerOpenid ( if (redirectUrl !== '') { ctx.redirect(redirectUrl) + } else { + // L-AUTH-4: handleProviderAuth signals an unresolvable login with '' (e.g. + // no account + signup disabled). Terminate fail-closed with an explicit + // 302 to /login instead of leaving a hanging/empty response. + ctx.redirect(loginUrl + '?error=oidc_no_account') } await next() From bfcb6c88e93626df4a9b491b74961a8843e23063 Mon Sep 17 00:00:00 2001 From: Michael Uray Date: Thu, 9 Jul 2026 07:18:09 +0000 Subject: [PATCH 7/7] test(account): use example.com fixtures in adminEmails tests Signed-off-by: Michael Uray --- .../account/src/__tests__/adminEmails.test.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/server/account/src/__tests__/adminEmails.test.ts b/server/account/src/__tests__/adminEmails.test.ts index 15cac826384..305536e2ad4 100644 --- a/server/account/src/__tests__/adminEmails.test.ts +++ b/server/account/src/__tests__/adminEmails.test.ts @@ -9,7 +9,7 @@ // // Coverage for admin.ts hardening: both env-set (parseAdminEmails) AND // lookup (isAdminEmail) must trim+lowercase. Today ADMIN_EMAILS is clean, -// but a future env value like " Michael@Uray.io " would silently break +// but a future env value like " Admin@Example.com " would silently break // admin detection without normalization on the env-side. Mongo backend // already has this hardening (collections/mongo.ts:1141-1147); this test // covers the equivalent for the shared helper. @@ -50,18 +50,18 @@ describe('admin.ts — env parsing + lookup normalization', () => { }) test('1. whitespace tolerance — env entry with leading/trailing spaces', () => { - const { isAdminEmail } = loadAdmin(' Michael@Uray.io ') - expect(isAdminEmail('michael@uray.io')).toBe(true) + const { isAdminEmail } = loadAdmin(' Admin@Example.com ') + expect(isAdminEmail('admin@example.com')).toBe(true) }) test('2. case-insensitive env — uppercase env entry matches lowercase lookup', () => { - const { isAdminEmail } = loadAdmin('MICHAEL@URAY.IO') - expect(isAdminEmail('michael@uray.io')).toBe(true) + const { isAdminEmail } = loadAdmin('ADMIN@EXAMPLE.COM') + expect(isAdminEmail('admin@example.com')).toBe(true) }) test('3. case-insensitive lookup — lowercase env matches uppercase lookup input', () => { - const { isAdminEmail } = loadAdmin('michael@uray.io') - expect(isAdminEmail('MICHAEL@URAY.IO')).toBe(true) + const { isAdminEmail } = loadAdmin('admin@example.com') + expect(isAdminEmail('ADMIN@EXAMPLE.COM')).toBe(true) }) test('4. L-AUTH-3: no-@ entries are DROPPED by default (fail-closed) + warn', () => { @@ -129,13 +129,13 @@ describe('admin.ts — env parsing + lookup normalization', () => { test('5. empty env string — empty set, all lookups return false', () => { const { isAdminEmail } = loadAdmin('') - expect(isAdminEmail('michael@uray.io')).toBe(false) + expect(isAdminEmail('admin@example.com')).toBe(false) expect(isAdminEmail('')).toBe(false) }) test('6. unset env — empty set, all lookups return false', () => { const { isAdminEmail } = loadAdmin(undefined) - expect(isAdminEmail('michael@uray.io')).toBe(false) + expect(isAdminEmail('admin@example.com')).toBe(false) }) test('7. multiple entries with mixed whitespace + case all normalized', () => {