Ac/pm 26364 extension UI for auto confirm

[BTreston]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-26364

📔 Objective

This PR implements the new Extension UI for the Automatic User Confirmation (auto-confirm) feature.

1. New Auto-Confirm Library (libs/auto-confirm/)

• Created a standalone library
• Migrate existing auto-confirm services, models, and abstractions into this library
• Added README documenting the feature's purpose and security model

2. Browser Extension Admin Settings Page

• New component: admin-settings.component.ts (apps/browser/src/vault/popup/settings/)
• Provides UI toggle for enabling/disabling auto-confirm in the browser extension
• Shows warning dialog when enabling (with link to help documentation)
• Implements spotlight/nudge notification for first-time setup

3. Navigation & Routing

• Added "Admin Settings" link to browser extension settings page
• Integrated route in app-routing.module.ts with guard protection
• Updated settings navigation template

4. Auto-Confirm Nudge Service

• Added auto-confirm-nudge.service.ts
• Integrates with existing nudges system to show spotlight, berry, and badge nudges

5. Guard Refactoring

• Moved org-policy.guard.ts from admin-console to libs/angular/src/admin-console/guards/
• Created new automatic-user-confirmation-settings.guard.ts for route protection

6. Testing & Configuration

• Enhanced test coverage for auto-confirm service
• Updated Jest and ESLint configurations for new library

📸 Screenshots

DESIGN REVIEW

https://github.com/user-attachments/assets/3de994eb-6b01-401b-afcd-a9fdc1785e7f

https://github.com/user-attachments/assets/9dab3894-96ca-4a81-82e3-a49f1c9eff22

⏰ 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

[github-actions[bot]]

Checkmarx One – Scan Summary & Details – 0fa9418a-34c0-42be-888e-c729996601e0

Great job! No new security vulnerabilities introduced in this pull request

[codecov[bot]]

Codecov Report

:x: Patch coverage is 76.06838% with 28 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 41.85%. Comparing base (48941a3) to head (97f6757). :warning: Report is 3 commits behind head on main. :white_check_mark: All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...omponents/auto-confirm-warning-dialog.component.ts	0.00%	8 Missing :warning:
apps/browser/src/popup/services/services.module.ts	0.00%	3 Missing :warning:
apps/web/src/app/layouts/user-layout.component.ts	0.00%	3 Missing :warning:
apps/browser/src/popup/app-routing.module.ts	0.00%	2 Missing :warning:
...c/vault/popup/settings/admin-settings.component.ts	95.45%	2 Missing :warning:
libs/auto-confirm/src/index.ts	0.00%	2 Missing :warning:
.../src/tools/popup/settings/settings-v2.component.ts	85.71%	1 Missing :warning:
apps/web/src/app/oss-routing.module.ts	0.00%	1 Missing :warning:
.../src/app/vault/individual-vault/vault.component.ts	0.00%	1 Missing :warning:
libs/angular/src/admin-console/guards/index.ts	0.00%	1 Missing :warning:
... and 4 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #17258      +/-   ##
==========================================
+ Coverage   41.82%   41.85%   +0.03%     
==========================================
  Files        3589     3596       +7     
  Lines      104160   104264     +104     
  Branches    15707    15714       +7     
==========================================
+ Hits        43564    43642      +78     
- Misses      58748    58776      +28     
+ Partials     1848     1846       -2     

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

:rocket: New features to boost your workflow:

• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude[bot]]

Claude finished @BTreston's task —— View job

---

Code Review Complete

Summary

I've completed a comprehensive review of PR #17258 implementing the Auto-Confirm feature for the browser extension. The code is well-structured with good separation of concerns and comprehensive tests, but there are critical issues that must be addressed before merging.

---

Critical Issues (Must Fix ❌)

1. UserId not reactive to account switching (admin-settings.component.ts:75)

   • If users switch accounts while on the settings page, changes will be saved to the wrong user's configuration
   • Security and data integrity issue

2. Race condition with dialog confirmation (admin-settings.component.ts:85)

   • The async dialog operation doesn't properly block the observable pipeline
   • Could allow setting to be enabled even when user rejects the warning dialog

3. Unsafe array access (default-auto-confirm.service.ts:57)

   • Accessing organizations[0] without validation
   • Business rule states single organization required but isn't enforced

---

Important Issues (Should Fix ⚠️)

1. Double-nesting state pattern (default-auto-confirm.service.ts:35) - User-scoped provider + userId keys creates confusing pattern
2. Subscription without error handler (admin-settings.component.ts:83) - Form could permanently stop responding if any operation errors
3. No error context for security operation (default-auto-confirm.service.ts:67) - Auto-confirm operations lack audit context

---

Suggestions & Questions (🎨/❓)

1. Duplicate permission checks (settings-v2.component.ts:106) - Consider using shareReplay to avoid duplicate service calls
2. Property naming clarity (auto-confirm-nudge.service.ts:32) - showBrowserNotification === false logic could use clarifying comment

---

Positive Aspects ✅

• Comprehensive test coverage for services and components
• Proper use of route guards for authorization
• Clean separation of concerns with new library structure
• Warning dialog for security-sensitive feature
• Multiple layers of permission validation
• Proper memory management with takeUntilDestroyed
• Follows ADR-0025 (no TypeScript enums)

---

Architecture Compliance

• ✅ Observable Data Services (ADR-0003): Mostly followed
• ✅ No TypeScript Enums (ADR-0025): Compliant
• ⚠️ Angular Signals (ADR-0027): Uses signals appropriately, but component should be more reactive
• ✅ Proper library boundaries maintained
• ✅ No security violations (credentials/decrypted data handling)

---

Recommendation

Do not merge until critical issues are resolved. The race condition and non-reactive userId are serious bugs that could lead to data corruption and security issues. Once these are fixed, the PR will be in excellent shape.

I've posted inline comments on specific lines with detailed suggestions for fixes.

---

[Hinton]

Should the settings page be unowned? I'm not quite sure why it's owned by tools.

[bw-ghapp[bot]]

Changes in this PR impact the Autofill experience of the browser client

BIT has tested the core experience with these changes and the feature flag configuration used by vault.bitwarden.com.

✅ Fortunately, these BIT tests have passed! 🎉

[bw-ghapp[bot]]

Changes in this PR impact the Autofill experience of the browser client

BIT has tested the core experience with these changes and all feature flags disabled.

✅ Fortunately, these BIT tests have passed! 🎉