-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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-11502] Support Remember Email option when using SSO authentication #13391
base: main
Are you sure you want to change the base?
Conversation
…/bitwarden/clients into auth/pm-17751/remember-sso-email
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #13391 +/- ##
=======================================
Coverage 35.60% 35.61%
=======================================
Files 3141 3141
Lines 93078 93050 -28
Branches 16925 16916 -9
=======================================
- Hits 33144 33143 -1
+ Misses 57397 57371 -26
+ Partials 2537 2536 -1 ☔ View full report in Codecov by Sentry. |
Great job, no security vulnerabilities found in this Pull Request |
83e66fe
to
ed11203
Compare
|
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-11502
📔 Objective
The Remember Email option was not working correctly when using SSO authentication.
Previously, the Remember Email choice only took effect after authentication was complete. This necessitated a lot of extra code and complexity in the
LoginEmailService
in order to track the user's email, as well as whether they want to remember it (but don't remember it yet!) through the various authentication flows. The reason SSO didn't work is because that particular flow didn't handle it correctly after login.Rather than build on that complexity, this PR changes the underlying assumption of when the Remember Email choice takes affect. The Remember Email option now takes effect as soon as you perform some action on the Login screen. This adjustment has been verified as acceptable by Product. The choice to remember email now takes effect when:
This allowed the simplification of the
LoginEmailService
interface quite a bit. TheLoginEmailService
now has two pieces of state it manages:loginEmail
setLoginEmail
and exposed via theloginEmail$
observable.rememberedEmail
null
.setRememberEmailChoice
method and exposed via therememberedEmail$
observable.📸 Screenshots
Web
Demonstrating that the remembered email persists through all operations, including SSO
New UI
web_remember_email_persists.mov
Old UI
remember_email_v1_web.mov
Desktop
Demonstrating that adding an account does not remember email, but logging in does
New UI
desktop_add_account_not_remembered.mov
Old UI
remember_email_v1_desktop.mov
remember_mail_v1_desktop_new_account.mov
Extension
Demonstrating that adding an account does not remember email, but logging in does
New UI
extension_not_remembered_account_add.mov
Old UI
remember_email_v1_extension.mov
remember_email_v1_extension_add_account.mov
⏰ 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