-
Notifications
You must be signed in to change notification settings - Fork 46
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
[PM-9527] Add PIN validation support #912
Conversation
No New Or Fixed Issues Found |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #912 +/- ##
==========================================
+ Coverage 57.92% 58.07% +0.15%
==========================================
Files 193 194 +1
Lines 13120 13194 +74
==========================================
+ Hits 7600 7663 +63
- Misses 5520 5531 +11 ☔ View full report in Codecov by Sentry. |
|
||
match login_method { | ||
UserLoginMethod::Username { email, kdf, .. } | ||
| UserLoginMethod::ApiKey { email, kdf, .. } => { |
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.
question: Do we expect Api key logins to use pin?
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 so, and I'm not sure if they would need password validation either. I think this is mostly a case of copying over the match
to get email
and kdf
. We can return an error for ApiKey
if you prefer though.
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-9527
📔 Objective
Fede mentioned on Slack some time ago that we currently don't have a simple way to validate the user's PIN like we do with the password. Instead the way to do it was to call
init_crypto
and check that the result is not an error.This requires more data than needed and does a lot of unnecessary operations, so I think it makes sense to expose a simpler way to do PIN verification.
In this PR we're decrypting the PIN protected user key and comparing it with the stored user key, similar to what we're doing with
validate_password_user_key
.⏰ Reminders before review
team
🦮 Reviewer guidelines
:+1:
) or similar for great changes:memo:
) or ℹ️ (:information_source:
) for notes or general info:question:
) for questions:thinking:
) or 💭 (:thought_balloon:
) for more open inquiry that's not quite a confirmedissue and could potentially benefit from discussion
:art:
) for suggestions / improvements:x:
) or:warning:
) for more significant problems or concerns needing attention:seedling:
) or ♻️ (:recycle:
) for future improvements or indications of technical debt:pick:
) for minor or nitpick changes