PM- 2059 Update Two factor webauthn dialog #9009

KiruthigaManivannan · 2024-05-02T08:13:32Z

Type of change

- [ ] Bug fix
- [ ] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [x] Other

Objective

Update Two factor webauthn dialog component to use the component library

Code changes

two-factor-webauthn.component.html: Converted modal to dialog compoenents
two-factor-webauthn.component.ts: Added dialog open method. Data passed to dialog is typed.
two-factor-setup.component.ts: Added Web authn dialog logic and passing the result of two factor verify dialog in the switch statement of mange method call.

Screenshots

Two-step.login._Webauth_.Bitwarden.Web.vault.-.Google.Chrome.2024-05-02.13-42-28.mp4

…059-update-two-factor-webauthn-dialog

KiruthigaManivannan · 2024-05-02T08:16:28Z

@rr-bw When I click the read key button , it is not opening the windows authentication using PIN like in QA vault. That may be due to that im running using local host. Because I get the following err ..

This error is due to the rp value in publicKeyCredentials passed to navigator. So I think this will work in qa vault

codecov · 2024-05-02T08:18:50Z

Codecov Report

Attention: Patch coverage is 13.04348% with 20 lines in your changes missing coverage. Please review.

Project coverage is 28.16%. Comparing base (419c107) to head (44f1e65).

Files	Patch %	Lines
...app/auth/settings/two-factor-webauthn.component.ts	14.28%	17 Missing and 1 partial ⚠️
...rc/app/auth/settings/two-factor-setup.component.ts	0.00%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #9009   +/-   ##
=======================================
  Coverage   28.15%   28.16%           
=======================================
  Files        2432     2432           
  Lines       71563    71575   +12     
  Branches    13372    13373    +1     
=======================================
+ Hits        20146    20156   +10     
- Misses      49830    49831    +1     
- Partials     1587     1588    +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2024-05-02T08:43:22Z

Checkmarx One – Scan Summary & Details – e81efbab-5923-4ec0-9995-5b81842e692d

New Issues

Severity	Issue	Source File / Package	Checkmarx Insight
	Client_Privacy_Violation	/apps/browser/src/background/main.background.ts: [1073](https://github.com/bitwarden/clients/blob/PM-2059-update-two-factor-webauthn-dialog//apps/browser/src/background/main.background.ts# L1073)	Attack Vector

rr-bw

When I click "Save" I get the following error, are you able to track this down? Everything works fine on main. Let me know if you need help debugging.

…059-update-two-factor-webauthn-dialog

KiruthigaManivannan · 2024-05-21T00:00:05Z

When I click "Save" I get the following error, are you able to track this down? Everything works fine on main. Let me know if you need help debugging.

@rr-bw Actually the issue is when we give name in the name input field , it should read key. Since we already have issue in reading key in local host , webAuthnResponse is null. And actual workflow is if webAuthnResponse is null Save button should be disabled. It should not be enabled and user is unable to click. But here even if the webAuthnResponse is null save is getting enabled. Yes please help me in debugging.

KiruthigaManivannan · 2024-05-22T15:59:31Z

When I click "Save" I get the following error, are you able to track this down? Everything works fine on main. Let me know if you need help debugging.

@rr-bw Actually the issue is when we give name in the name input field , it should read key. Since we already have issue in reading key in local host , webAuthnResponse is null. And actual workflow is if webAuthnResponse is null Save button should be disabled. It should not be enabled and user is unable to click. But here even if the webAuthnResponse is null save is getting enabled. Yes please help me in debugging.

@rr-bw Please advise on the above

rr-bw

It won't let me comment on the specific line, but line 39 of the html file needs updated to tw-text-muted.

apps/web/src/app/auth/settings/two-factor-setup.component.ts

rr-bw · 2024-05-23T08:24:12Z

apps/web/src/app/auth/settings/two-factor-webauthn.component.ts

@@ -31,6 +33,7 @@ interface Key {
  templateUrl: "two-factor-webauthn.component.html",
 })
 export class TwoFactorWebAuthnComponent extends TwoFactorBaseComponent {
+  @Output() onChangeStatus = new EventEmitter<boolean>();
  type = TwoFactorProviderType.WebAuthn;
  name: string;


Do will still need name? Where do we access this property?

@rr-bw Yes this is used in setup component , to immediately send enabled status to set up component , the enabled tick mark should appear before we close the dialog . so we are using this event emitter.

apps/web/src/app/auth/settings/two-factor-webauthn.component.ts

apps/web/src/app/auth/settings/two-factor-webauthn.component.html

rr-bw · 2024-05-23T08:40:22Z

apps/web/src/app/auth/settings/two-factor-webauthn.component.html

+        <li>{{ "twoFactorU2fTouchButton" | i18n }}</li>
+        <li>{{ "twoFactorU2fSaveForm" | i18n }}</li>
+      </ol>
+      <div class="tw-flex tw-flex-row">


I think tw-flex-row is unnecessary because it is the default.

apps/web/src/app/auth/settings/two-factor-webauthn.component.html

rr-bw · 2024-05-23T09:03:42Z

When I click "Save" I get the following error, are you able to track this down? Everything works fine on main. Let me know if you need help debugging.

@rr-bw Actually the issue is when we give name in the name input field , it should read key. Since we already have issue in reading key in local host , webAuthnResponse is null. And actual workflow is if webAuthnResponse is null Save button should be disabled. It should not be enabled and user is unable to click. But here even if the webAuthnResponse is null save is getting enabled. Yes please help me in debugging.

@jlf0dev or @JaredSnider-Bitwarden Would either of you have capacity to help in debugging this? I can't seem to track down the issue. The "save" button is somehow getting enabled when you simply start typing in the name field. It's supposed to remain disabled as long as webAuthnResponse is null. It's almost like form status changes are overriding our [disabled]="!webAuthnResponse" on the "save" button. One odd thing I noticed is that if I remove bitFormButton from the "save" button, it will in fact remain disabled until you Read Key. (But we still then have the problem where clicking "save" gives an error).

…059-update-two-factor-webauthn-dialog

rr-bw · 2024-06-05T17:04:40Z

@KiruthigaManivannan Reading the key should now work, and the "save" button will also remain disabled until the appropriate time. Could you now fix the remaining issues when clicking "deactivate all keys". The dialog should close, a toast should appear, and the green check mark next to "FIDO2 WebAuthn" should get removed. Here is a video of the correct deactivate flow on main:

deactivate.mov

KiruthigaManivannan · 2024-06-06T15:23:10Z

@KiruthigaManivannan Reading the key should now work, and the "save" button will also remain disabled until the appropriate time. Could you now fix the remaining issues when clicking "deactivate all keys". The dialog should close, a toast should appear, and the green check mark next to "FIDO2 WebAuthn" should get removed. Here is a video of the correct deactivate flow on main:

deactivate.mov

@willmartian As you said , save button is working as expected now. But clicking on read key button does not do anything.

This occurs not only in this branch , but also in main while running in localhost

KiruthigaManivannan added 2 commits April 30, 2024 14:50

PM-2059 Update Two Factor Webauth Dialog

c260814

Merge branch 'main' of https://github.com/bitwarden/clients into PM-2…

f1d1956

…059-update-two-factor-webauthn-dialog

KiruthigaManivannan requested a review from a team as a code owner May 2, 2024 08:13

KiruthigaManivannan requested a review from rr-bw May 2, 2024 08:13

github-actions bot added the needs-qa Marks a PR as requiring QA approval label May 2, 2024

rr-bw requested changes May 17, 2024

View reviewed changes

KiruthigaManivannan added 3 commits May 20, 2024 15:40

Merge branch 'main' of https://github.com/bitwarden/clients into PM-2…

5c36657

…059-update-two-factor-webauthn-dialog

PM-2059 Added event emitter for enabled status

f706738

Merge branch 'main' of https://github.com/bitwarden/clients into PM-2…

24f74f1

…059-update-two-factor-webauthn-dialog

KiruthigaManivannan requested a review from rr-bw May 20, 2024 22:43

rr-bw requested changes May 23, 2024

View reviewed changes

KiruthigaManivannan added 2 commits May 23, 2024 17:45

Merge branch 'main' of https://github.com/bitwarden/clients into PM-2…

b6f995f

…059-update-two-factor-webauthn-dialog

PM-2059 Addressed review comments

d5a6d60

KiruthigaManivannan requested a review from rr-bw May 23, 2024 12:44

rr-bw added 2 commits June 5, 2024 09:39

Merge branch 'main' into PM-2059-update-two-factor-webauthn-dialog

d85d40d

convert to arrow function

44f1e65

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PM- 2059 Update Two factor webauthn dialog #9009

PM- 2059 Update Two factor webauthn dialog #9009

KiruthigaManivannan commented May 2, 2024

KiruthigaManivannan commented May 2, 2024

codecov bot commented May 2, 2024 •

edited

github-actions bot commented May 2, 2024 •

edited

rr-bw left a comment

KiruthigaManivannan commented May 21, 2024

KiruthigaManivannan commented May 22, 2024

rr-bw left a comment

rr-bw May 23, 2024

KiruthigaManivannan May 23, 2024

rr-bw May 23, 2024

rr-bw commented May 23, 2024 •

edited

rr-bw commented Jun 5, 2024 •

edited

KiruthigaManivannan commented Jun 6, 2024

PM- 2059 Update Two factor webauthn dialog #9009

Are you sure you want to change the base?

PM- 2059 Update Two factor webauthn dialog #9009

Conversation

KiruthigaManivannan commented May 2, 2024

Type of change

Objective

Code changes

Screenshots

KiruthigaManivannan commented May 2, 2024

codecov bot commented May 2, 2024 • edited

Codecov Report

github-actions bot commented May 2, 2024 • edited

New Issues

rr-bw left a comment

Choose a reason for hiding this comment

KiruthigaManivannan commented May 21, 2024

KiruthigaManivannan commented May 22, 2024

rr-bw left a comment

Choose a reason for hiding this comment

rr-bw May 23, 2024

Choose a reason for hiding this comment

KiruthigaManivannan May 23, 2024

Choose a reason for hiding this comment

rr-bw May 23, 2024

Choose a reason for hiding this comment

rr-bw commented May 23, 2024 • edited

rr-bw commented Jun 5, 2024 • edited

KiruthigaManivannan commented Jun 6, 2024

codecov bot commented May 2, 2024 •

edited

github-actions bot commented May 2, 2024 •

edited

rr-bw commented May 23, 2024 •

edited

rr-bw commented Jun 5, 2024 •

edited