fix(session-cookie): update secure cookie handling to prevent issues in local deployments #307
Closed
polaris-dxz wants to merge 2 commits intobuilderz-labs:mainfrom
Closed
fix(session-cookie): update secure cookie handling to prevent issues in local deployments #307polaris-dxz wants to merge 2 commits intobuilderz-labs:mainfrom
polaris-dxz wants to merge 2 commits intobuilderz-labs:mainfrom
Conversation
…in local deployments - Adjusted the logic for determining the 'secure' flag in getMcSessionCookieOptions to prioritize explicit environment overrides and request security, while removing the fallback to NODE_ENV. This change addresses potential issues with HTTP-only local deployments where browsers may drop 'Secure' cookies.
Member
|
Thanks for the contribution @polaris-dxz! This fix was already merged in Your PR adds explanatory comments to the same line but the functional change is already in |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When running Mission Control with
pnpm startorbash install.sh --local(production build over HTTP), login returns 200 but the user stays on the login page. The session cookie was being set withSecurebecause we defaulted tosecure = truewhenNODE_ENV === 'production'. Browsers do not store or sendSecurecookies over HTTP, so the cookie was never persisted and the middleware redirected back to/login.Change: In
getMcSessionCookieOptions(src/lib/session-cookie.ts), stop usingNODE_ENV === 'production'to set the cookiesecureflag. Use only (1) explicitMC_COOKIE_SECUREenv, or (2) actual request security (x-forwarded-proto: httpsorhttps:). Default tofalseso HTTP-only local deployments work without extra config. HTTPS deployments and reverse-proxy setups are unchanged when the request is seen as secure or whenMC_COOKIE_SECURE=trueis set.Risk Level
Low
Tests
pnpm build && pnpm start— opened http://localhost:3000/login, logged in with admin credentials; redirect to/and session persisted (no redirect back to login).pnpm dev— login still works as before.Set-Cookieno longer includesSecurewhen request is HTTP; cookie ismc-sessionwithHttpOnly; SameSite=strictonly.Contribution Checklist
Notes
x-forwarded-proto: https),secureremains true and behavior is unchanged. Operators who want to forceSecureon HTTP (e.g. for testing) can setMC_COOKIE_SECURE=true.