Skip to content

Introduce an env variable for the session cookie path#2573

Merged
ssddanbrown merged 1 commit intoBookStackApp:masterfrom
ckleemann:master
Mar 16, 2021
Merged

Introduce an env variable for the session cookie path#2573
ssddanbrown merged 1 commit intoBookStackApp:masterfrom
ckleemann:master

Conversation

@ckleemann
Copy link
Contributor

The session cookie path parameter can not configured by an env file. There are scenarios where you want to set the cookie path e.g. multi App Setups under the same domain. With this PR a new env variable SESSION_COOKIE_PATH is introduced.

@ssddanbrown
Copy link
Member

Hi @ckleemann,
Thanks for the PR.

Would you be able to technically explain what this enables you to do or achieve? From what I can understand, modification of this option would make the system more restricted and work against your example use case.

@ckleemann
Copy link
Contributor Author

Hi @ssddanbrown,

sure: In our setup we run several apps under the same domain. e.g. example.net/appA, example.net/bookstack and so on. If the cookie path for bookstack is set to / the cookie is send by the browser for every request for the domain. So if we request a page of appA e.g. example.net/appA/ the browser also sends the cookies from bookstack. But there is no need to send them because appA can not make any use of them.
By setting the cookie path to /bookstack the browser only sends the cookie if he requests a path which has the prefix /bookstack/. As result the browser will not send the bookstack cookies if he requests a path of a different app.

In our setup the different apps are run by different teams. Most of the apps are based on Django and Python. In the case of a bug Django sends a email with the stacktrace and the cookies received to the developers. The list of cookies send by the browser for a user is quite long. It might contain information which are not required to see for the team running this app. By setting the cookie path correctly we are able to reduce the number of cookies. In the result only cookies needed are send to each app.

I think there is not much complexity added: If you do not know how to set the cookie path you simply use the default value. This also makes the change backwards compatible. But if you have a need to set the cookie path the change allows you to do so without getting in trouble on the next update.

Some background:
The pattern matching algorithm of the browser is descried in RFC6265 Section 5.1.4. Paths and Path-Match. If the cookie path does not end with an / it is added by the browser before for the pattern matching. So cookie path /app does note match /appA but /app, /app/ and /app/view etc.

@ssddanbrown ssddanbrown merged commit 3d0e1bc into BookStackApp:master Mar 16, 2021
@ssddanbrown
Copy link
Member

Thanks for explaining this further @ckleemann.

I've made some further changes to the implementation in 1420f23.

Instead of this being an additional option that's set this will instead be automatic based on the set APP_URL value. Reduces the amount of options and that helps prevent the chance of mis-alignment of those.

This will be part of the next feature release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants