-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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-4877: Only allow replacing passkeys for the same userhandle #9804
base: main
Are you sure you want to change the base?
Conversation
New Issues
Fixed Issues
|
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.
Tests failing?
@coroiu Ah, I ran the tests locally but mistakenly only ran those in the /apps/browser folder. I've now fixed the failing platform test, but in github I get this error which I don't understand. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9804 +/- ##
==========================================
+ Coverage 29.30% 29.53% +0.22%
==========================================
Files 2532 2537 +5
Lines 73825 74158 +333
Branches 13783 13857 +74
==========================================
+ Hits 21636 21903 +267
- Misses 50569 50596 +27
- Partials 1620 1659 +39 โ View full report in Codecov by Sentry. |
How did a rebase trigger all of that crap. Sorry everyone that got pinged. |
@coroiu Tests passing |
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.
Looks good! Thanks for working on this @abergs
I wonder why the mac os desktop build keeps failing |
* @param userHandle | ||
*/ | ||
private hasNoOtherPasskeys(cipher: CipherView, userHandle: string): boolean { | ||
if (cipher.login.fido2Credentials == null || cipher.login.fido2Credentials.length === 0) { |
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.
Need re-review because I was using === null
which caused an error. Thanks to @justindbaur who told me to fix that in another PR i knew right away what to do!
๐๏ธ Tracking
https://bitwarden.atlassian.net/browse/PM-4877
๐ Objective
๐ธ Screenshots
Demo after these changes:
https://share.cleanshot.com/8j29tzPK
Demo before these changes:
https://share.cleanshot.com/xfLJ11Hd
โฐ Reminders before review
๐ฆฎ 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 confirmed issue 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