This repository was archived by the owner on Jun 24, 2025. It is now read-only.
refactor(cookiePath): remove non-working cookiePath option#1686
Merged
eliandoran merged 10 commits intodevelopfrom Apr 13, 2025
Merged
refactor(cookiePath): remove non-working cookiePath option#1686eliandoran merged 10 commits intodevelopfrom
eliandoran merged 10 commits intodevelopfrom
Conversation
|
|
this option will currently not work => the cookie will never be set by the server, if you use a different path other than "/" in order for this to work we would need to introduce some kind of "custom route prefix", that would make express serve the routes with the custom prefix — but that kinda falls more into a reverse proxy job territory. So let's remove this feature for now and amend the docs on how to correctly handle the cookies per instance via the reverse proxy.
1952d94 to
324223f
Compare
eliandoran
approved these changes
Apr 13, 2025
pano9000
added a commit
that referenced
this pull request
Apr 14, 2025
…g-sample.ini not sure how I managed to forget to commit this as well, this should've been part of (the now already approved & merged) PR #1686 as well
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Hi,
this PR basically reverts TriliumNext/Trilium#5473
After some further investigation I found out that this option will currently never work out of the box – and in my testing it only worked due to some special reverse-proxy config in combination I hadn't considered… 🙈 Sorry about that!
Under normal circumstances the cookie will never be set by the server, if you use a different path other than "/", because express never serves that path and hence the cookie is never set or even read.
In order for this to work we would need to introduce some kind of "custom route prefix", that would make express serve the routes with the custom prefix — but that kinda falls more into a reverse proxy job territory and IMHO probably out of scope for Trilium Server itself (correct me if I'm wrong though).
So let's remove this feature for now and amend the docs on how to correctly handle the cookies per instance via the reverse proxy.
For nginx e.g. the correct way to handle this is to set the
proxy_cookie_pathdirective, that will rewrite the cookiePath for us and allow us to have different cookies per instance:just need to figure out where in the Docs this info fits the best and I will amend them accordingly and set the PR to "Ready for review".
closes TriliumNext/Trilium#5632