Application password experiment: Handle errors when generating password fails (#16057)

Author: itsmeichigo

Modules/Sources/NetworkingCore/ApplicationPassword/RequestAuthenticator.swift

@@ -10,6 +10,8 @@ protocol RequestAuthenticator {
     ///
     var credentials: Credentials? { get }
 
+    var jetpackSiteID: Int64? { get }
+
     /// Authenticates the provided urlRequest using the `credentials`
     ///
     /// - Parameter urlRequest: `URLRequest` to authenticate
@@ -21,6 +23,10 @@ protocol RequestAuthenticator {
     ///
     func generateApplicationPassword() async throws
 
+    /// Delete existing application password remotely
+    ///
+    func deleteApplicationPassword() async throws
+
     /// Checks whether the given URLRequest is eligible for retyring
     ///
     func shouldRetry(_ urlRequest: URLRequest) -> Bool
@@ -33,6 +39,10 @@ public struct DefaultRequestAuthenticator: RequestAuthenticator {
     ///
     let credentials: Credentials?
 
+    /// ID of current site if Jetpack site
+    ///
+    let jetpackSiteID: Int64?
+
     /// The use case to handle authentication with application passwords.
     ///
     private let applicationPasswordUseCase: ApplicationPasswordUseCase?
@@ -82,6 +92,8 @@ public struct DefaultRequestAuthenticator: RequestAuthenticator {
                 return selectedSite?.siteAddress
             }
         }()
+
+        jetpackSiteID = selectedSite?.siteID
     }
 
     /// Authenticates the provided urlRequest using the `credentials`
@@ -107,6 +119,13 @@ public struct DefaultRequestAuthenticator: RequestAuthenticator {
         return
     }
 
+    func deleteApplicationPassword() async throws {
+        guard let applicationPasswordUseCase else {
+            throw RequestAuthenticatorError.applicationPasswordUseCaseNotAvailable
+        }
+        try await applicationPasswordUseCase.deletePassword(locally: false)
+    }
+
     /// Checks whether the given URLRequest is eligible for retyring
     ///
     func shouldRetry(_ urlRequest: URLRequest) -> Bool {

Modules/Sources/NetworkingCore/ApplicationPassword/RequestProcessor.swift

@@ -1,6 +1,15 @@
 import Alamofire
 import Foundation
 
+enum AppPasswordFailureReason {
+    case notSupported
+    case unknown
+}
+
+protocol RequestProcessorDelegate: AnyObject {
+    func didFailToAuthenticateRequestWithAppPassword(siteID: Int64, reason: AppPasswordFailureReason)
+}
+
 /// Authenticates and retries requests
 ///
 final class RequestProcessor: RequestInterceptor {
@@ -12,6 +21,10 @@ final class RequestProcessor: RequestInterceptor {
 
     private let notificationCenter: NotificationCenter
 
+    private var currentSiteID: Int64?
+
+    weak var delegate: RequestProcessorDelegate?
+
     init(requestAuthenticator: RequestAuthenticator,
          notificationCenter: NotificationCenter = .default) {
         self.requestAuthenticator = requestAuthenticator
@@ -20,6 +33,7 @@ final class RequestProcessor: RequestInterceptor {
 
     func updateAuthenticator(_ authenticator: RequestAuthenticator) {
         requestAuthenticator = authenticator
+        currentSiteID = authenticator.jetpackSiteID
     }
 }
 
@@ -57,7 +71,7 @@ extension RequestProcessor: RequestRetrier {
 //
 private extension RequestProcessor {
     func generateApplicationPassword() {
-        Task(priority: .medium) {
+        Task(priority: .medium) { @MainActor in
             do {
                 let _ = try await requestAuthenticator.generateApplicationPassword()
                 isAuthenticating = false
@@ -67,16 +81,63 @@ private extension RequestProcessor {
 
                 completeRequests(true)
             } catch {
-                isAuthenticating = false
 
                 // Post a notification for tracking
                 notificationCenter.post(name: .ApplicationPasswordsGenerationFailed, object: error, userInfo: nil)
 
-                completeRequests(false)
+                let shouldRetry = await checkIfRetryingGenerationIsNeeded(for: error)
+                if shouldRetry {
+                    generateApplicationPassword()
+                } else {
+                    isAuthenticating = false
+                    completeRequests(false)
+                    if let currentSiteID {
+                        notifyFailure(error, for: currentSiteID)
+                    }
+                }
             }
         }
     }
 
+    /// Checks error code to retry or mark site as unsupported for app password.
+    /// Returns whether retry is needed.
+    @MainActor
+    func checkIfRetryingGenerationIsNeeded(for error: Error) async -> Bool {
+        guard currentSiteID != nil else {
+            return false
+        }
+        switch error {
+        case NetworkError.unacceptableStatusCode(let statusCode, _) where statusCode == 409:
+            /// Password with the same name already exists. Request deletion remotely and retry.
+            do {
+                try await requestAuthenticator.deleteApplicationPassword()
+                return true
+            } catch {
+                return false
+            }
+        default:
+            return false
+        }
+    }
+
+    func notifyFailure(_ error: Error, for siteID: Int64) {
+        let reason: AppPasswordFailureReason = {
+            switch error {
+            case NetworkError.notFound:
+                return .notSupported
+            case let networkError as NetworkError:
+                if let code = networkError.errorCode,
+                   AppPasswordConstants.disabledCodes.contains(code) {
+                    return .notSupported
+                }
+                return .unknown
+            default:
+                return .unknown
+            }
+        }()
+        delegate?.didFailToAuthenticateRequestWithAppPassword(siteID: siteID, reason: reason)
+    }
+
     func shouldRetry(_ error: Error) -> Bool {
         switch error {
         case RequestAuthenticatorError.applicationPasswordNotAvailable,

Modules/Sources/NetworkingCore/Network/AlamofireNetwork.swift

@@ -35,6 +35,8 @@ public class AlamofireNetwork: Network {
 
     private let selectedSite: AnyPublisher<JetpackSite?, Never>?
 
+    private let userDefaults: UserDefaults
+
     /// Converter to convert Jetpack tunnel requests into REST API requests if applicable
     ///
     private var requestConverter: RequestConverter
@@ -47,6 +49,9 @@ public class AlamofireNetwork: Network {
 
     private var subscription: AnyCancellable?
 
+    /// Keeps track of failure counts for each site when switching to direct requests
+    private var appPasswordFailures: [Int64: Int] = [:]
+
     /// Public Initializer
     ///
     /// - Parameters:
@@ -57,10 +62,12 @@ public class AlamofireNetwork: Network {
     ///     Defaults to false for backward compatibility. Set to true when making concurrent requests immediately after initialization.
     public required init(credentials: Credentials?,
                          selectedSite: AnyPublisher<JetpackSite?, Never>? = nil,
+                         userDefaults: UserDefaults = .standard,
                          sessionManager: Alamofire.Session? = nil,
                          ensuresSessionManagerIsInitialized: Bool = false) {
         self.credentials = credentials
         self.selectedSite = selectedSite
+        self.userDefaults = userDefaults
         self.requestConverter = {
             let siteAddress: String? = {
                 switch credentials {
@@ -89,6 +96,7 @@ public class AlamofireNetwork: Network {
         } else {
             requestConverter = RequestConverter(siteAddress: nil)
             requestAuthenticator.updateAuthenticator(DefaultRequestAuthenticator(credentials: credentials))
+            requestAuthenticator.delegate = nil
         }
     }
 
@@ -202,11 +210,14 @@ private extension AlamofireNetwork {
     func observeSelectedSite(_ selectedSite: AnyPublisher<JetpackSite?, Never>) {
         subscription = selectedSite
             .removeDuplicates()
-            .sink { [weak self] site in
+            .combineLatest(userDefaults.publisher(for: \.applicationPasswordUnsupportedList))
+            .sink { [weak self] site, unsupportedList in
                 guard let self else { return }
-                guard let site, site.applicationPasswordAvailable else {
+                guard let site, site.applicationPasswordAvailable,
+                      unsupportedList.contains(site.siteID) == false else {
                     requestConverter = RequestConverter(siteAddress: nil)
                     requestAuthenticator.updateAuthenticator(DefaultRequestAuthenticator(credentials: credentials))
+                    requestAuthenticator.delegate = nil
                     return
                 }
                 requestConverter = RequestConverter(siteAddress: site.siteAddress)
@@ -215,10 +226,42 @@ private extension AlamofireNetwork {
                     selectedSite: site,
                     network: self
                 ))
+                requestAuthenticator.delegate = self
+                appPasswordFailures.removeValue(forKey: site.siteID) // reset failure count
+            }
+    }
+}
+
+// MARK: `RequestProcessorDelegate` conformance
+//
+extension AlamofireNetwork: RequestProcessorDelegate {
+    func didFailToAuthenticateRequestWithAppPassword(siteID: Int64, reason: AppPasswordFailureReason) {
+        switch reason {
+        case .notSupported:
+            let currentList = userDefaults.applicationPasswordUnsupportedList
+            userDefaults.applicationPasswordUnsupportedList = currentList + [siteID]
+        case .unknown:
+            let currentFailureCount = appPasswordFailures[siteID] ?? 0
+            let updatedCount = currentFailureCount + 1
+            if updatedCount == AppPasswordConstants.requestFailureThreshold {
+                let currentList = userDefaults.applicationPasswordUnsupportedList
+                userDefaults.applicationPasswordUnsupportedList = currentList + [siteID]
             }
+            appPasswordFailures[siteID] = updatedCount
+        }
     }
 }
 
+// MARK: - Constants for direct request error handling
+enum AppPasswordConstants {
+    // flag site as disabled after threshold is reached
+    static let requestFailureThreshold = 10
+    static let disabledCodes = [
+        "application_passwords_disabled",
+        "application_passwords_disabled_for_user"
+    ]
+}
+
 private extension DataRequest {
     /// Validates only for `RESTRequest`
     ///
@@ -260,3 +303,16 @@ extension Alamofire.DataResponse {
         }
     }
 }
+
+// MARK: - Helper extension to save internal flag for app password availability
+//
+extension UserDefaults {
+    @objc dynamic var applicationPasswordUnsupportedList: [Int64] {
+        get { value(forKey: Key.applicationPasswordUnsupportedList.rawValue) as? [Int64] ?? [] }
+        set { setValue(newValue, forKey: Key.applicationPasswordUnsupportedList.rawValue) }
+    }
+
+    enum Key: String {
+        case applicationPasswordUnsupportedList
+    }
+}

Modules/Sources/NetworkingCore/Network/NetworkError.swift

@@ -49,6 +49,16 @@ public enum NetworkError: Error, Equatable {
             return nil
         }
     }
+
+    /// Content of the `code` field in the response if available
+    var errorCode: String? {
+        guard let response else { return nil }
+        let decoder = JSONDecoder()
+        guard let decodedResponse = try? decoder.decode(NetworkErrorResponse.self, from: response) else {
+            return nil
+        }
+        return decodedResponse.code
+    }
 }
 
 
@@ -121,3 +131,25 @@ extension NetworkError: CustomStringConvertible {
         }
     }
 }
+
+struct NetworkErrorResponse: Decodable {
+    let code: String?
+
+    init(from decoder: Decoder) throws {
+        let container = try decoder.container(keyedBy: CodingKeys.self)
+        self.code = try {
+            let errorValue = try container.decodeIfPresent(String.self, forKey: .error)
+            if let errorValue {
+                return errorValue
+            }
+            return try container.decodeIfPresent(String.self, forKey: .code)
+        }()
+    }
+
+    /// Coding Keys
+    ///
+    private enum CodingKeys: String, CodingKey {
+        case error
+        case code
+    }
+}