-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: Missing support for cookieSessionSecret in config file for multi-replica deployments
#3052
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: alpha
Are you sure you want to change the base?
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Co-authored-by: mtrezza <5673677+mtrezza@users.noreply.github.com>
…loyments Co-authored-by: mtrezza <5673677+mtrezza@users.noreply.github.com>
cookieSessionSecret in config file not supported for multi-replica deployments
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. |
cookieSessionSecret in config file not supported for multi-replica deploymentscookieSessionSecret in config file not supported for multi-replica deployments
You're correct - this is a bug fix, not a feature addition. The issue describes a regression where multi-replica deployments that worked before v8.x stopped working. The inconsistency I mentioned refers to the implementation detail: when The user's code was passing Commit: 5ec933e |
cookieSessionSecret in config file not supported for multi-replica deploymentscookieSessionSecret in config file for multi-replica deployments
cookieSessionSecret in config file for multi-replica deploymentscookieSessionSecret in config file for multi-replica deployments
|
@chadpav can you test this out? |
Signed-off-by: Manuel <5673677+mtrezza@users.noreply.github.com>
I was able to verify locally but my deployment to prod fails. I think it's because I'm referencing a github branch for the dependency and I'm using AWS Elastic Beanstalk which might not like that. It tries to run Looking at the changes though, they are very minimal. I'd approve this and try to deploy the alpha release. |
How did you verify locally? I tried setting
There should be no issue with this. Let's first get this working before we merge. |
|
@mtrezza I created a TEST environment in AWS by cloning PROD. After a lot of troubleshooting, I believe the reason I had trouble deploying the branch vs. the NPM assets is that the Dashboard requires a webpack build step and AWS EB did not like that. That's based on working with Claude and webpack isn't something I've done a lot with so it could be off. Anyway, I modified my TEST setup to allow me to npm install locally and include node_modules when i deploy and that got me around the issue. However, I'm still getting the original error message so we might need to keep digging. I'm looking into the root issue now. |
|
I asked Claude to review this Github PR, the code, other issues, etc to help with a root cause analysis. It showed me that there were a lot of PR's recently around this issue and it's analysis sounds correct to me but I wanted to get your feedback on it. TLDR; the architecture has changed and we might not be able to support the old environment variable option. The breaking change happened in v8.0.0 when the session middleware was switched from
The Potential fixes:
The PR #3052 ( |
|
Makes most sense, if not too complex:
Do you want to give it a try? |
|
I asked it about that and it points out that the cookie has moved from the client to the server with this change. So making a cookie-based session store is just going to use a shared secret to encrypt a unique cookie on each instance of the server. I also learned that I can reuse my existing mongo db infrastructure as a session store via |
|
Using mongo as a session store and redeploying with 8.1.0 has resolved my original issue. It's an alternative to restoring the original behavior and I'm good with it.
All Parse Server users are going to have a mongo or pg data layer and it's a pretty simple solution to configure a mongo/pg session store that doesn't require additional infrastructure.
|
|
@mtrezza I was curious so I checked to see if the 8.0 release notes mentioned this breaking change already. After digging around, I think this slipped through the cracks and was never mentioned. The switch from See this commit fbb5e6d which was about fixing something that seems unrelated This is a breaking change scheduled to affect 7.6.0, fyi. CC: @Moumouls It also means the 7.6.0 and 8.0.0 will only work in single-instance deployments since you weren't able to configure your own session store until 8.1.0. This is interesting reading. I even read the AI comments which correctly pointed out some issues:
P.S. - don't misread my intent here. I was just curious about how you guys are using AI to support this project so I was interested in what the AI was telling you. +1 for transparency. It's all good, this would be hard to catch unless you had a multi-instance system to test against. Running the project locally does not reproduce the issue. Also, the AI messages were mixed in with a bunch of other noise and I had the benefit of knowing exactly what the needle in the haystack looked like. |
New Pull Request Checklist
Issue Description
Multi-replica deployments behind load balancers fail with "CSRF token validation failed" errors. This is a regression from before v8.x where configurations that worked previously stopped functioning. When
cookieSessionStorewas added in v8.x with config file support,cookieSessionSecretwas not given equivalent config file support, creating an inconsistency that broke multi-replica deployments relying on config file configuration.Approach
This fix restores the ability to configure multi-replica deployments through config files by adding config file fallback for
cookieSessionSecretto matchcookieSessionStorebehavior:config.data.cookieSessionSecretwhen CLI/env not providedconfig.cookieSessionSecretandconfig.cookieSessionStorewhen options not providedPriority: CLI/env → options → config → random generation
Example config file:
{ "apps": [...], "users": [...], "cookieSessionSecret": "shared-secret-across-replicas" }Example programmatic:
TODOs before merging
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.