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

Always sign cookies, also in development mode for classic style apps #1245

Merged
merged 1 commit into from Jan 30, 2017

Conversation

Projects
None yet
2 participants
@stomar
Contributor

stomar commented Jan 29, 2017

Currently there is a surprising or at least undocumented difference in the behavior of modular and classic style apps: cookies are not signed for classic style applications in development mode, while for modular applications they are always signed, regardless of the environment.

The patch removes the redefinition of the session_secret for classic style applications that caused this behavior.

In case the current behavior is intended, at least the docs should be updated to reflect this. Right now the README generally states (see "Using Sessions"):

To improve security, the session data in the cookie is signed with a session secret. A random secret is generated for you by Sinatra.

Short tale of woe: I wanted to create a simple example for cookies and how they work on the HTTP level, and noticed they were not signed; then went to the docs to find out how to activate the signing of cookies; unnecessarily used Rack::Session::Cookie as suggested in the FAQ ("If you need to set additional parameters for sessions, like expiration date, use Rack::Session::Cookie directly"), before finding out how to set the session_secret directly; still wondering, I finally read the source. And the irony: the actual example app for my students is in modular style...

Use same session_secret setting for classic and modular apps
Use the same session_secret setting for classic style applications
as for modular apps using Sinatra::Base and always sign cookies.
Before, cookies were not signed for classic style applications
in development mode.
@zzak

This comment has been minimized.

Show comment
Hide comment
@zzak

zzak Jan 30, 2017

Member

I'd consider this a bug, not sure why the behavior would be different between classic/modular in this case.

Member

zzak commented Jan 30, 2017

I'd consider this a bug, not sure why the behavior would be different between classic/modular in this case.

@zzak zzak added this to the 2.0.0 milestone Jan 30, 2017

@zzak zzak added the bug label Jan 30, 2017

@zzak

This comment has been minimized.

Show comment
Hide comment
@zzak

zzak Jan 30, 2017

Member

/cc #1216

Member

zzak commented Jan 30, 2017

/cc #1216

@zzak zzak merged commit f6395dc into sinatra:master Jan 30, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@stomar stomar deleted the stomar:always-sign-cookies branch Jan 31, 2017

@stomar

This comment has been minimized.

Show comment
Hide comment
@stomar

stomar Jan 31, 2017

Contributor

thanks!

Contributor

stomar commented Jan 31, 2017

thanks!

zzak added a commit that referenced this pull request Mar 4, 2017

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