diff --git a/docs/maintainers/session-cookie-transport.md b/docs/maintainers/session-cookie-transport.md index 744468f4..f9eda16c 100644 --- a/docs/maintainers/session-cookie-transport.md +++ b/docs/maintainers/session-cookie-transport.md @@ -1,32 +1,43 @@ # Session Cookie Transport Decision -**Date:** 2026-08-05 -**Status:** Production cookies require HTTPS. +**Date:** 2026-08-01 +**Status:** Compatibility decision; HTTPS migration remains follow-up work. SeerrNG uses an `express-session` cookie to carry the authenticated browser session, including the session created by Plex sign-in. The current -configuration uses a secure session cookie in production: +configuration uses `secure: 'auto'`: -- Production sets the `Secure` attribute unconditionally. When SeerrNG is - behind a trusted TLS terminator, `express-session` uses - `X-Forwarded-Proto: https` to validate the request transport. -- Development also sets `Secure`; local browser testing must use HTTPS (or a - test client that does not require a browser cookie). +- When SeerrNG is reached through HTTPS, or a trusted TLS terminator forwards + `X-Forwarded-Proto: https`, the session cookie receives the `Secure` + attribute. +- When SeerrNG is reached directly over HTTP, the browser can store and send + the cookie over HTTP. -## Why production requires HTTPS +## Why `secure: true` is not being enabled unconditionally -The production deployment uses `https://request.snape.tech` and has proxy -trust enabled for its TLS terminator. A browser must use that HTTPS endpoint -for production sign-in. Direct `http://kspls0:5055` access is a diagnostic -health-check path, not a supported browser authentication origin. +The direct deployment at `http://kspls0:5055` depends on the HTTP session +cookie. Making the cookie unconditionally secure causes the browser to reject +that cookie on the HTTP origin. The Plex OAuth result then cannot be resumed +in the SeerrNG browser session, so Plex sign-in to the app fails. The same +change would break other direct-HTTP and LAN deployments. -This prevents an authenticated session from being sent over clear-text HTTP. -The `js/clear-text-cookie` finding that motivated this change should resolve in -the next CodeQL scan; it must not be dismissed or hidden with a query -configuration. +This is a compatibility/security tradeoff, not a CodeQL false positive. The +`js/clear-text-cookie` finding at `server/index.ts:323` is accurate for direct +HTTP deployments. It must remain visible; it must not be dismissed, ignored, +or hidden with a CodeQL model or query configuration. ## Chosen behavior -All session cookies are secure, `httpOnly`, and use the existing CSRF-dependent -`SameSite` policy. The `development` flag only controls whether Express trusts -a forwarded proxy protocol. +Preserve `secure: 'auto'` so Plex sign-in and existing HTTP deployments keep +working, while ensuring HTTPS deployments receive secure session cookies. The +remaining risk is that an HTTP deployment exposes the authenticated session to +anyone able to observe or modify that network traffic. + +## Resolution path + +To remove the finding without breaking Plex sign-in, deploy SeerrNG behind a +working HTTPS endpoint and redirect HTTP to HTTPS. After that deployment is +verified, change the session cookie to `secure: true`, update the transport +tests, and remove the HTTP compatibility path. The TLS termination and +redirect must be deployed and tested first; changing the cookie setting alone +breaks authentication. diff --git a/release-notes/fixed-plex-login-session.md b/release-notes/fixed-plex-login-session.md new file mode 100644 index 00000000..0418f777 --- /dev/null +++ b/release-notes/fixed-plex-login-session.md @@ -0,0 +1,8 @@ +--- +category: fixed +audience: users, operators +area: authentication +action: none +breaking: false +--- +Plex sign-in now completes on direct HTTP/LAN deployments again, while HTTPS deployments continue to receive Secure session cookies; failed session handoffs now show an error instead of spinning indefinitely. diff --git a/server/index.ts b/server/index.ts index 367b2599..4383b874 100644 --- a/server/index.ts +++ b/server/index.ts @@ -321,11 +321,7 @@ app secret: settings.sessionSecret, resave: false, saveUninitialized: false, - cookie: { - ...sessionTransportOptions.cookie, - // Keep the clear-text transport guard explicit at the session sink. - secure: true, - }, + cookie: sessionTransportOptions.cookie, proxy: sessionTransportOptions.proxy, ...(sessionStore ? { store: sessionStore } : {}), }) diff --git a/server/utils/sessionCookie.test.ts b/server/utils/sessionCookie.test.ts index 65b2616b..55d0d2bd 100644 --- a/server/utils/sessionCookie.test.ts +++ b/server/utils/sessionCookie.test.ts @@ -5,19 +5,14 @@ import { describe, it } from 'node:test'; import request from 'supertest'; import { getSessionTransportOptions } from './sessionCookie'; -const createApp = () => { +const createApp = (development = false) => { const app = express(); app.use( session({ secret: '01234567890123456789012345678901', resave: false, saveUninitialized: false, - cookie: { - httpOnly: true, - sameSite: 'strict', - secure: true, - }, - proxy: true, + ...getSessionTransportOptions(development, true), }) ); app.get('/', (req, res) => { @@ -28,14 +23,17 @@ const createApp = () => { }; describe('getSessionTransportOptions', () => { - it('requires secure session cookies in every runtime', () => { - assert.equal(getSessionTransportOptions(false, true).cookie.secure, true); - assert.equal(getSessionTransportOptions(false, false).cookie.secure, true); + it('uses automatic transport-aware security for session cookies', () => { + assert.equal(getSessionTransportOptions(false, true).cookie.secure, 'auto'); + assert.equal( + getSessionTransportOptions(false, false).cookie.secure, + 'auto' + ); assert.equal(getSessionTransportOptions(false, true).proxy, true); }); it('keeps the remaining cookie protections in development and production', () => { - assert.equal(getSessionTransportOptions(true, true).cookie.secure, true); + assert.equal(getSessionTransportOptions(true, true).cookie.secure, 'auto'); assert.equal( getSessionTransportOptions(true, true).cookie.sameSite, 'strict' @@ -52,7 +50,13 @@ describe('getSessionTransportOptions', () => { assert.equal(getSessionTransportOptions(true, true).proxy, false); }); - it('emits a secure production cookie from a TLS terminator without global proxy trust', async () => { + it('matches the cookie security to the forwarded request transport', async () => { + const directResponse = await request(createApp()).get('/'); + assert.doesNotMatch( + directResponse.get('Set-Cookie')?.[0] ?? '', + /; Secure(?:;|$)/ + ); + const response = await request(createApp()) .get('/') .set('X-Forwarded-Proto', 'https'); diff --git a/server/utils/sessionCookie.ts b/server/utils/sessionCookie.ts index f146b984..cb1df102 100644 --- a/server/utils/sessionCookie.ts +++ b/server/utils/sessionCookie.ts @@ -10,7 +10,9 @@ export const getSessionTransportOptions = ( maxAge: SESSION_MAX_AGE_MS, httpOnly: true, sameSite: csrfProtection ? 'strict' : 'lax', - secure: true, + // Let express-session add Secure for HTTPS requests while preserving + // compatibility with direct HTTP/LAN deployments. + secure: 'auto', }, // This option is scoped to express-session's transport check. It lets a // TLS terminator's X-Forwarded-Proto=https authorize a Secure cookie diff --git a/src/components/Login/index.tsx b/src/components/Login/index.tsx index cdc8dae9..6e477264 100644 --- a/src/components/Login/index.tsx +++ b/src/components/Login/index.tsx @@ -61,12 +61,29 @@ const Login = ({ initialBackdrops }: { initialBackdrops?: string[] }) => { { signal: controller.signal } ); - if (active && response.data?.id) { - void revalidate(); + if (active) { + if (!response.data?.id) { + throw new Error('Unable to complete Plex sign-in.'); + } + + const authenticatedUser = await revalidate(); + if (!authenticatedUser) { + throw new Error( + 'Plex sign-in succeeded, but Seerr could not establish a browser session. Check that you are using the HTTPS URL or that direct HTTP session cookies are enabled.' + ); + } } } catch (e) { if (active && !axios.isCancel(e)) { - setError(axios.isAxiosError(e) ? e.response?.data?.message : ''); + const message = axios.isAxiosError(e) + ? e.response?.data?.message + : e instanceof Error + ? e.message + : undefined; + setError(message || 'Unable to complete Plex sign-in.'); + } + } finally { + if (active) { setAuthToken(undefined); setProcessing(false); }