Skip to content

[PM-33569] feat: Add notification service extension to update auth request notifications#2444

Open
matt-livefront wants to merge 3 commits intomainfrom
matt/PM-33569-push-notification-extension
Open

[PM-33569] feat: Add notification service extension to update auth request notifications#2444
matt-livefront wants to merge 3 commits intomainfrom
matt/PM-33569-push-notification-extension

Conversation

@matt-livefront
Copy link
Copy Markdown
Collaborator

@matt-livefront matt-livefront commented Mar 13, 2026

🎟️ Tracking

PM-33569

📔 Objective

We're switching from background push notifications to alert push notifications for "login with device" auth requests. This adds a UNNotificationServiceExtension that intercepts those alert notifications and updates the body with the requesting user's email before delivery.

  • Adds a new BitwardenNotificationExtension app extension target
  • Adds DefaultNotificationExtensionHelper in BitwardenShared which reads account state and updates the notification title/body for auth request notifications

The backend sends:

Title: Login requested
Message: Confirm login attempt

The notification extension updates this to:

Title: Login request (localized based on app language)
Message: Confirm login attempt for (localized and email added based on the user's ID)

This handles updating the content of the push notification. There will be a separate PR to handle processing this notification in the app when it's tapped.

@matt-livefront matt-livefront requested review from a team as code owners March 13, 2026 19:41
@matt-livefront matt-livefront added the ai-review Request a Claude code review label Mar 13, 2026
@github-actions github-actions bot added app:password-manager Bitwarden Password Manager app context app:authenticator Bitwarden Authenticator app context t:ci Change Type - Updates to automated workflows t:docs Change Type - Documentation t:feature labels Mar 13, 2026
@claude
Copy link
Copy Markdown

claude bot commented Mar 13, 2026

Claude finished @matt-livefront's task in 2m 49s —— View job


Overall Assessment: APPROVE

Re-reviewed the latest commit (6a6015fa2) which extracts the shared userInfo decoding logic into a PushNotificationData.init(userInfo:) initializer, addressing reviewer feedback. Both NotificationExtensionHelper and NotificationService now use this shared initializer, eliminating the code duplication. The new initializer and missingDataDictionary error case are well-tested with happy path, empty dictionary, invalid data, and missing key scenarios.

Code Review Details

No findings. The refactoring is straightforward and correctly preserves error handling behavior in both call sites. The minor behavioral change in NotificationService.decodePayload (now throws instead of returning nil for missing "data" key) is properly caught by the existing error handler at the call site.

@claude
Copy link
Copy Markdown

claude bot commented Mar 13, 2026

Overall Assessment: APPROVE

Reviewed the new BitwardenNotificationExtension target and DefaultNotificationExtensionHelper implementation. The extension correctly intercepts auth request push notifications and updates the notification body with the requesting user's email by reading account state from the shared app group UserDefaults. The extension entry point is intentionally thin with all logic in the fully-tested helper class in BitwardenShared.

Code Review Details

No findings. The implementation follows established codebase patterns consistently:

  • Error handling falls back to original notification content in all failure paths
  • Force unwrap on UserDefaults(suiteName:) matches existing ServiceContainer usage
  • Localization is configured via Resources.initialLanguageCode consistent with AppProcessor
  • CI workflows, Fastlane provisioning profiles, and XcodeGen project spec are correctly updated
  • Test coverage includes happy path, user-not-found, malformed payload, missing data key, and unhandled notification type scenarios

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 13, 2026

Logo
Checkmarx One – Scan Summary & Details4e0b708f-eb87-4231-9bab-223775442cbd

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

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 13, 2026

Codecov Report

❌ Patch coverage is 86.20690% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.72%. Comparing base (48115b8) to head (6a6015f).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
...cationExtension/NotificationServiceExtension.swift 0.00% 12 Missing ⚠️
...latform/Services/NotificationExtensionHelper.swift 80.95% 8 Missing ⚠️
...form/Models/Domain/PushNotificationDataTests.swift 90.90% 4 Missing ⚠️
...rm/Services/NotificationExtensionHelperTests.swift 95.74% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2444      +/-   ##
==========================================
- Coverage   86.86%   85.72%   -1.15%     
==========================================
  Files        1841     2077     +236     
  Lines      162244   177346   +15102     
==========================================
+ Hits       140941   152026   +11085     
- Misses      21303    25320    +4017     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@fedemkr fedemkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just some minor comments.

@matt-livefront matt-livefront removed t:ci Change Type - Updates to automated workflows t:docs Change Type - Documentation labels Mar 17, 2026
@vvolkgang
Copy link
Copy Markdown
Member

vvolkgang commented Mar 23, 2026

@matt-livefront Triggered runs to test the new provisioning profiles:

  1. BWPM - https://github.com/bitwarden/ios/actions/runs/23442727040
  2. BWA (double checking everything is ok) - https://github.com/bitwarden/ios/actions/runs/23442739178

edit: both passed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review app:authenticator Bitwarden Authenticator app context app:password-manager Bitwarden Password Manager app context t:feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants