Skip to content
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

Optionally sign or encrypt cookie #140

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

rchekaluk
Copy link

This change fixes #48 by optionally signing or encrypting the cookie. Although #48 primarily concerns signing, I also added encryption support for completeness (though as mentioned here, this is technically not necessary).

Two new configuration parameters are defined: sign_cookie, encrypt_cookie. Both default to false for backwards compatibility.

sign_cookie encrypt_cookie Behavior
false false Cleartext cookie
false true Encrypted cookie
true false Signed cookie
true true Encrypted cookie

The tests succeed under Rails 4, 5, and 6.

@sikachu
Copy link
Member

sikachu commented Feb 26, 2019

I have yet to review the code yet, but just want to give you a heads up that I'll be cutting a new patch version first since we're overdue for one, and this feature can go with the next minor version.

@rchekaluk
Copy link
Author

Ping @sikachu ?

@sikachu
Copy link
Member

sikachu commented Jan 27, 2020

@rafaelfranca what do you think about this feature? I forgot that it's here for about a year now ...

Do you think this is a good feature to have?

@rchekaluk
Copy link
Author

Ping @sikachu ? This seems like a solid addition to the gem.

@rchekaluk
Copy link
Author

Hi @sikachu @rafaelfranca this PR has been updated for all rubies and Rails declared in its GitHub Actions workflow.

If possible, it would be appreciated if you would let us know if you intend to accept this PR or not.

@hibachrach
Copy link

hibachrach commented Aug 20, 2024

This patch helps block a security footgun. If anything, it should default to signing and/or encrypting values. Anything I can do to help move this forward?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Why are cookies not being signed?
3 participants