-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Encrypting user/email from cookie #120
Encrypting user/email from cookie #120
Conversation
@JoelSpeed, Just pushed the code for encrypting the user and email for the new json encoding of the cookie. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of minor bits but generally looks pretty good, thanks for sorting this one 😄
providers/session_state.go
Outdated
@@ -62,6 +62,19 @@ func (s *SessionState) EncodeSessionState(c *cookie.Cipher) (string, error) { | |||
} else { | |||
ss = *s | |||
var err error | |||
// Encrypt also Email and User when cipher is provided |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this comment is necessary, personally I would drop it but happy to leave if you feel strongly about it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I guess from this point forward there is no different treatment in encrypting the email and user versus the other fields. Removing the comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, thanks for the fix! 👍
Correct label in NOTES.txt
Description
This change does one thing:
if a cypher is provided, it encrypts also the email and user with the same logic and algorithm as the rest of the fields from the session state.
Motivation and Context
This is the PR for the issue #60 .
Don't expose any information part of the cookie, information that can be easily retrieved if not encrypted.
How Has This Been Tested?
All the test from session_state_test are passing.
Testing in our environments.
Checklist: