From 210940221efca7e534867c022a013d0c3072af90 Mon Sep 17 00:00:00 2001 From: Jamil Date: Sat, 25 Jan 2025 21:40:08 -0800 Subject: [PATCH] feat(apple/iOS): Show errors related to granting notifications (#7857) On iOS, it's possible for the notification granting process to throw errors, though not very likely. This PR updates updates the plumbing for how we request user notifications on iOS and respond to the result, allowing for errors to be propagated back to the user in case something goes wrong. Note that unlike installing system extensions and VPN configurations, disallowing notifications _does not_ result in an error being thrown. Instead, we receive `false`, and we now track this Bool instead of the entire `UNAuthorizationStatus` for updating the UI with. By keeping that Bool `nil` until we load the authorization status, we fix #7617 as a bonus. Related: #7714 Fixes: #7617 --- .../Models/SessionNotification.swift | 47 +++++++------------ .../Sources/FirezoneKit/Stores/Store.swift | 31 ------------ .../Sources/FirezoneKit/Views/AppView.swift | 37 ++++++++++----- .../Views/GrantNotificationsView.swift | 36 +++++++++++--- 4 files changed, 73 insertions(+), 78 deletions(-) diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/SessionNotification.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/SessionNotification.swift index f0e234f17..039d3d61f 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/SessionNotification.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/SessionNotification.swift @@ -22,17 +22,13 @@ public enum NotificationIndentifier: String { } public class SessionNotification: NSObject { - public var signInHandler: (() -> Void)? = nil - @Published private(set) var decision: UNAuthorizationStatus + public var signInHandler = {} + private let notificationCenter = UNUserNotificationCenter.current() public override init() { - self.decision = .notDetermined - super.init() #if os(iOS) - let notificationCenter = UNUserNotificationCenter.current() - notificationCenter.delegate = self let signInAction = UNNotificationAction( @@ -53,32 +49,23 @@ public class SessionNotification: NSObject { options: []) notificationCenter.setNotificationCategories([certificateExpiryCategory]) - - notificationCenter.getNotificationSettings { notificationSettings in - self.decision = notificationSettings.authorizationStatus - } #endif } + func askUserForNotificationPermissions() async throws -> UNAuthorizationStatus { + // Ask the user for permission. + try await notificationCenter.requestAuthorization(options: [.sound, .alert]) + + // Retrieve the result + return await loadAuthorizationStatus() + } + + func loadAuthorizationStatus() async -> UNAuthorizationStatus { + let settings = await notificationCenter.notificationSettings() + + return settings.authorizationStatus + } #if os(iOS) - func askUserForNotificationPermissions() { - guard case .notDetermined = self.decision else { - Log.log("Already determined!") - return - } - - let notificationCenter = UNUserNotificationCenter.current() - notificationCenter.requestAuthorization(options: [.sound, .alert]) { _, _ in - notificationCenter.getNotificationSettings { notificationSettings in - self.decision = notificationSettings.authorizationStatus - } - } - } - - func loadAuthorizationStatus(handler: (UNAuthorizationStatus) -> Void) { - - } - // In iOS, use User Notifications. // This gets called from the tunnel side. public static func showSignedOutNotificationiOS() { @@ -119,7 +106,7 @@ public class SessionNotification: NSObject { let response = alert.runModal() if response == NSApplication.ModalResponse.alertFirstButtonReturn { Log.log("\(#function): 'Sign In' clicked in notification") - signInHandler?() + signInHandler() } } #endif @@ -134,7 +121,7 @@ extension SessionNotification: UNUserNotificationCenterDelegate { if categoryId == NotificationIndentifier.sessionEndedNotificationCategory.rawValue, actionId == NotificationIndentifier.signInNotificationAction.rawValue { // User clicked on 'Sign In' in the notification - signInHandler?() + signInHandler() } DispatchQueue.main.async { diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift index d893e9b2f..f8c6b239f 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift @@ -29,10 +29,6 @@ public final class Store: ObservableObject { @Published private(set) var systemExtensionStatus: SystemExtensionStatus? #endif - // This is not currently updated after it is initialized, but - // we could periodically update it if we need to. - @Published private(set) var decision: UNAuthorizationStatus - let vpnConfigurationManager: VPNConfigurationManager private var sessionNotification: SessionNotification private var cancellables: Set = [] @@ -40,36 +36,15 @@ public final class Store: ObservableObject { public init() { // Initialize all stored properties - self.decision = .authorized self.settings = Settings.defaultValue self.sessionNotification = SessionNotification() self.vpnConfigurationManager = VPNConfigurationManager() - - initNotifications() } public func internetResourceEnabled() -> Bool { self.vpnConfigurationManager.internetResourceEnabled } - private func initNotifications() { - // Finish initializing notification binding - sessionNotification.signInHandler = { - Task.detached { - do { try await WebAuthSession.signIn(store: self) } - catch { Log.error(error) } - } - } - - sessionNotification.$decision - .receive(on: DispatchQueue.main) - .sink(receiveValue: { [weak self] decision in - guard let self = self else { return } - self.decision = decision - }) - .store(in: &cancellables) - } - func bindToVPNConfigurationUpdates() async throws { // Load our existing VPN configuration and set an update handler try await self.vpnConfigurationManager.loadFromPreferences( @@ -166,12 +141,6 @@ public final class Store: ObservableObject { try await bindToVPNConfigurationUpdates() } - func requestNotifications() { - #if os(iOS) - sessionNotification.askUserForNotificationPermissions() - #endif - } - func authURL() -> URL? { return URL(string: settings.authBaseURL) } diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/AppView.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/AppView.swift index bae180880..15dd421c0 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/AppView.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/AppView.swift @@ -22,6 +22,7 @@ import UserNotifications public class AppViewModel: ObservableObject { let favorites: Favorites let store: Store + let sessionNotification: SessionNotification @Published private(set) var status: NEVPNStatus? @Published private(set) var decision: UNAuthorizationStatus? @@ -31,10 +32,25 @@ public class AppViewModel: ObservableObject { public init(favorites: Favorites, store: Store) { self.favorites = favorites self.store = store + self.sessionNotification = SessionNotification() + + self.sessionNotification.signInHandler = { + Task.detached { [weak self] in + guard let self else { return } + + do { try await WebAuthSession.signIn(store: self.store) } + catch { Log.error(error) } + } + } Task.detached { [weak self] in guard let self else { return } + // Load user's decision whether to allow / disallow notifications + let decision = await self.sessionNotification.loadAuthorizationStatus() + await updateNotificationDecision(to: decision) + + // Load VPN configuration and system extension status do { try await self.store.bindToVPNConfigurationUpdates() let vpnConfigurationStatus = await self.store.status @@ -70,16 +86,10 @@ public class AppViewModel: ObservableObject { self.status = status }) .store(in: &cancellables) + } - store.$decision - .receive(on: DispatchQueue.main) - .sink(receiveValue: { [weak self] decision in - guard let self = self else { return } - - Log.log("Decision: \(decision)") - self.decision = decision - }) - .store(in: &cancellables) + func updateNotificationDecision(to newStatus: UNAuthorizationStatus) async { + await MainActor.run { self.decision = newStatus } } } @@ -98,8 +108,13 @@ public struct AppView: View { ProgressView() case (.invalid, _): GrantVPNView(model: GrantVPNViewModel(store: model.store)) - case (.disconnected, .notDetermined): - GrantNotificationsView(model: GrantNotificationsViewModel(store: model.store)) + case (_, .notDetermined): + GrantNotificationsView(model: GrantNotificationsViewModel( + sessionNotification: model.sessionNotification, + onDecisionChanged: { decision in + await model.updateNotificationDecision(to: decision) + } + )) case (.disconnected, _): iOSNavigationView(model: model) { WelcomeView(model: WelcomeViewModel(store: model.store)) diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/GrantNotificationsView.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/GrantNotificationsView.swift index 7edb54e89..ac15816a9 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/GrantNotificationsView.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/GrantNotificationsView.swift @@ -7,22 +7,46 @@ import Combine import Foundation import SwiftUI +import UserNotifications @MainActor public final class GrantNotificationsViewModel: ObservableObject { - private var store: Store + private let sessionNotification: SessionNotification + private let onDecisionChanged: (UNAuthorizationStatus) async -> Void - init(store: Store) { - self.store = store + init( + sessionNotification: SessionNotification, + onDecisionChanged: @escaping (UNAuthorizationStatus) async -> Void + ) { + self.sessionNotification = sessionNotification + self.onDecisionChanged = onDecisionChanged } - func grantNotificationButtonTapped() { - store.requestNotifications() + func grantNotificationButtonTapped(errorHandler: GlobalErrorHandler) { + Task.detached { [weak self] in + guard let self else { return } + + do { + let decision = try await sessionNotification.askUserForNotificationPermissions() + await onDecisionChanged(decision) + } catch { + Log.error(error) + + await MainActor.run { + errorHandler.handle(ErrorAlert( + title: "Error granting notifications", + error: error + )) + } + } + } + } } struct GrantNotificationsView: View { @ObservedObject var model: GrantNotificationsViewModel + @EnvironmentObject var errorHandler: GlobalErrorHandler public var body: some View { VStack( @@ -46,7 +70,7 @@ struct GrantNotificationsView: View { .imageScale(.large) Spacer() Button("Grant Notification Permission") { - model.grantNotificationButtonTapped() + model.grantNotificationButtonTapped(errorHandler: errorHandler) } .buttonStyle(.borderedProminent) .controlSize(.large)