quexten commented 3 months ago

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-7084 https://bitwarden.atlassian.net/browse/PM-9098 https://bitwarden.atlassian.net/browse/PM-9024 https://bitwarden.atlassian.net/browse/PM-9056 https://bitwarden.atlassian.net/browse/PM-9057

📔 Objective

This PR introduced a feature flagged shared two-factor-auth component along with the first shared authentication method - TOTP. Tests are ported from the old shared two-factor.component.

[!IMPORTANT]
STACKED PR: This set of changes is split into a set of smaller PR's that can be reviewed independently, in parallel. Merging still should be done in order once all PR's are reviewed. Merging can be done incrementally as the feature is feature flagged.

9767 - Shared two-fa options component

9768 - Shared two-fa orchestrator component & totp authenticator component

9769 - Shared two-fa yubikey component

9770 - Shared two-fa email component

9771 - Shared two-fa webauthn component

9772 - Shared two-fa duo component

📸 Screenshots

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Protected functional changes with optionality (feature flags)
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation (Confluence, contributing docs) or informed the documentation 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 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

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 30.51948% with 214 lines in your changes missing coverage. Please review.

Project coverage is 30.08%. Comparing base (053e255) to head (4a8809a). Report is 2 commits behind head on main.

Files	Patch %	Lines
...rowser/src/auth/popup/two-factor-auth.component.ts	0.00%	68 Missing :warning:
...nents/two-factor-auth/two-factor-auth.component.ts	57.04%	46 Missing and 18 partials :warning:
apps/web/src/app/auth/two-factor-auth.component.ts	0.00%	49 Missing :warning:
apps/desktop/src/auth/two-factor-auth.component.ts	0.00%	17 Missing :warning:
.../utils/two-factor-component-refactor-route-swap.ts	0.00%	9 Missing :warning:
apps/browser/src/popup/app-routing.module.ts	0.00%	2 Missing :warning:
apps/desktop/src/app/app-routing.module.ts	0.00%	2 Missing :warning:
apps/web/src/app/oss-routing.module.ts	0.00%	2 Missing :warning:
...or-auth/two-factor-auth-authenticator.component.ts	88.88%	1 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #9768 +/- ## ========================================== + Coverage 29.82% 30.08% +0.25% ========================================== Files 2552 2558 +6 Lines 74670 74978 +308 Branches 13950 14026 +76 ========================================== + Hits 22273 22554 +281 + Misses 50706 50680 -26 - Partials 1691 1744 +53 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

github-actions[bot] commented 3 months ago

Checkmarx One – Scan Summary & Details – 857750cd-1133-4ae5-b821-540fb5fb4a7d

bitwarden / clients

[PM-7084] 2/6: Add shared two-factor-auth orchestrator component, and TOTP two-factor component #9768