Auth/PM-8113 - 2FA Components Consolidation and UI Refresh #12087
Conversation
…of UnauthenticatedExtensionUIRefresh flag
…ed import is used a dialog.
…s-ui-refresh + merge conflict fixes
|
New Issues (4)Checkmarx found the following issues in this Pull Request
|
…ead of libs/angular/src/auth
…en top level page component and child components
…tor-auth and move all logic into single component - WIP
…s-ui-refresh + merge confliction resolution
…nt imports on other clients.
…ulLoginNavigate (3) after successful login we always loginEmailService.clearValues()
…ork for extension
JaredSnider-Bitwarden
changed the title
…ructure for auth component client logic services.
|
@rr-bw , I've added TDE offboarding support to the flows. Here's my tests: Extension: Desktop: |
|
| </ng-container> | ||
|
|
||
| <!-- Buttons --> | ||
| <div class="tw-flex tw-flex-col tw-space-y-3 tw-mb-3"> |
There was a problem hiding this comment.
| <div class="tw-flex tw-flex-col tw-space-y-3 tw-mb-3"> | |
| <div class="tw-flex tw-flex-col tw-space-y-3"> |
AnonLayout already has padding built-in.
| loading = true; | ||
|
|
||
| orgSsoIdentifier: string | undefined = undefined; | ||
| inSsoFlow = false; |
There was a problem hiding this comment.
Is inSsoFlow used anywhere? It's assigned in ngOnInit() but I don't think it's ever used.
| // than usual to avoid cutting off the dialog. | ||
| const isLinux = await this.isLinux(); | ||
| if (selected2faProviderType === TwoFactorProviderType.WebAuthn && isLinux) { | ||
| document.body.classList.add("linux-webauthn"); |
There was a problem hiding this comment.
.linux-webauthn is an SCSS class. Do we want this in a Tailwind file?
| ItemModule, | ||
| IconModule, | ||
| ], | ||
| providers: [], | ||
| }) | ||
| export class TwoFactorOptionsComponent implements OnInit { | ||
| @Output() onProviderSelected = new EventEmitter<TwoFactorProviderType>(); | ||
| @Output() onRecoverSelected = new EventEmitter(); |
There was a problem hiding this comment.
I don't think the onProviderSelected and onRecoverSelected outputs are actually being used in the new flow. Also, I'd think you can remove the concept of "recover" entirely from this component (and the TwoFactorOptionsDialogResult enum) since the recovery code button is now being shown as an option directly on the 2FA screen. If I'm correct, address also the reference to TwoFactorOptionsDialogResult in selectOtherTwoFactorMethod() on TwoFactorAuthComponent.
| ItemModule, | ||
| IconModule, | ||
| ], | ||
| providers: [], | ||
| }) | ||
| export class TwoFactorOptionsComponent implements OnInit { | ||
| @Output() onProviderSelected = new EventEmitter<TwoFactorProviderType>(); | ||
| @Output() onRecoverSelected = new EventEmitter(); | ||
|
|
||
| providers: any[] = []; |
There was a problem hiding this comment.
Is there a reason for the 4 uses of any in this file?
| } | ||
| }; | ||
|
|
||
| async selectOtherTwofactorMethod() { |
There was a problem hiding this comment.
| async selectOtherTwofactorMethod() { | |
| async selectOtherTwoFactorMethod() { |
| bitButton | ||
| bitFormButton | ||
| *ngIf="twoFactorProviders?.size > 1" | ||
| (click)="selectOtherTwofactorMethod()" |
There was a problem hiding this comment.
| (click)="selectOtherTwofactorMethod()" | |
| (click)="selectOtherTwoFactorMethod()" |
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-8113
Associated Server PR: bitwarden/server#5120
📔 Objective
To consolidate all the cross client 2FA components into single implementation components delegating all client specific logic into services while updating the UIs to match the new design.
This PR is unfortunately quite large, but a ton of the file changes are renames or moves.
📸 Screenshots
To avoid slowing down the PR loading massively, each of the below videos are linked using a thumbnail which downloads the videos instead of directly rendering them inline.
Web
Web - Standard MP Login Scenarios
Web - SSO Scenarios
Note: remember me does not currently work with SSO, but that is being addressed separately.
Desktop
Desktop - Standard MP Login Scenarios
Extension
Extension - Chrome - Standard MP Login Scenarios
Note: WebAuthn is now done inline in Chrome instead of in a new tab like we have to do for other Browsers until we rebuild the WebAuthn flows with better support.
Extension - Firefox - Standard MP Login Scenarios
⏰ 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