-
-
Notifications
You must be signed in to change notification settings - Fork 91
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
Case insensitive tokens #249
base: master
Are you sure you want to change the base?
Case insensitive tokens #249
Conversation
app/models/passwordless/session.rb
Outdated
def token=(token) | ||
Passwordless.config.case_insensitive_tokens ? modified_token = token.upcase : modified_token = token | ||
self.token_digest = Passwordless.digest(modified_token) | ||
@token = (modified_token) |
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 don't think we should do anything at write time. Rather, let's just upcase in SQL when searching for tokens
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.
def token=(token) | |
Passwordless.config.case_insensitive_tokens ? modified_token = token.upcase : modified_token = token | |
self.token_digest = Passwordless.digest(modified_token) | |
@token = (modified_token) | |
def token=(plaintext) | |
self.token_digest = Passwordless.digest(plaintext) | |
@token = (plaintext) |
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 think we have to do this at write time - we're persisting the token digest, not the token, so I believe we cannot upcase the token digest when searching for tokens.
Co-authored-by: Mikkel Malmberg <[email protected]>
closes #248