Handle deletion of app password when authenticated with WPCom

Modules/Sources/NetworkingCore/ApplicationPassword/ApplicationPasswordStorage.swift

@@ -1,7 +1,18 @@
 import Foundation
 import KeychainAccess
 
-public struct ApplicationPasswordStorage {
+public protocol ApplicationPasswordStorageType {
+    /// Returns the saved application password if available
+    var applicationPassword: ApplicationPassword? { get }
+
+    /// Saves application password into keychain
+    func saveApplicationPassword(_ password: ApplicationPassword)
+
+    /// Removes the currently saved password from storage
+    func removeApplicationPassword()
+}
+
+public struct ApplicationPasswordStorage: ApplicationPasswordStorageType {
     /// Stores the application password
     ///
     private let keychain: Keychain

Modules/Sources/NetworkingCore/ApplicationPassword/ApplicationPasswordUseCase.swift

@@ -29,9 +29,10 @@ public protocol ApplicationPasswordUseCase {
 
     /// Deletes the application password
     ///
-    ///  Deletes locally and also sends an API request to delete it from the site
+    /// - Parameter locally: Determines whether to remove the password from the local storage
+    /// or only sends an API request to delete it from the site.
     ///
-    func deletePassword() async throws
+    func deletePassword(locally: Bool) async throws
 }
 
 final public class DefaultApplicationPasswordUseCase: ApplicationPasswordUseCase {
@@ -45,38 +46,32 @@ final public class DefaultApplicationPasswordUseCase: ApplicationPasswordUseCase
 
     /// To store application password
     ///
-    private let storage: ApplicationPasswordStorage
+    private let storage: ApplicationPasswordStorageType
 
     /// Used to name the password in wpadmin.
     ///
-    private var applicationPasswordName: String {
-#if !os(watchOS)
-        let bundleIdentifier = Bundle.main.bundleIdentifier ?? "Unknown"
-        let model = UIDevice.current.model
-        let identifierForVendor = UIDevice.current.identifierForVendor?.uuidString ?? ""
-        return "\(bundleIdentifier).ios-app-client.\(model).\(identifierForVendor)"
-#else
-        fatalError("Unexpected error: Application password should not be generated through watch app")
-#endif
-    }
+    private let applicationPasswordName: String
 
     /// Internal initializer
-    init(type: AuthenticationType,
-         network: Network,
-         keychain: Keychain = Keychain(service: WooConstants.keychainServiceName)) {
+    public init(type: AuthenticationType,
+                network: Network,
+                passwordName: String? = nil,
+                storage: ApplicationPasswordStorageType? = nil) {
         self.authenticationType = type
-        self.storage = ApplicationPasswordStorage(keychain: keychain)
+        self.storage = storage ?? ApplicationPasswordStorage(keychain: Keychain(service: WooConstants.keychainServiceName))
         self.network = network
+        self.applicationPasswordName = passwordName ?? Self.createPasswordName()
     }
 
     /// Public initializer for wporg authentication
     public init(username: String,
                 password: String,
                 siteAddress: String,
                 network: Network? = nil,
-                keychain: Keychain = Keychain(service: WooConstants.keychainServiceName)) throws {
+                storage: ApplicationPasswordStorageType? = nil) throws {
         self.authenticationType = .wporg(username: username, password: password, siteAddress: siteAddress)
-        self.storage = ApplicationPasswordStorage(keychain: keychain)
+        self.storage = storage ?? ApplicationPasswordStorage(keychain: Keychain(service: WooConstants.keychainServiceName))
+        self.applicationPasswordName = Self.createPasswordName()
 
         if let network {
             self.network = network
@@ -113,7 +108,7 @@ final public class DefaultApplicationPasswordUseCase: ApplicationPasswordUseCase
                 return try await createApplicationPassword()
             } catch ApplicationPasswordUseCaseError.duplicateName {
                 do {
-                    try await deletePassword()
+                    try await deletePassword(locally: true)
                 } catch ApplicationPasswordUseCaseError.unableToFindPasswordUUID {
                     // No password found with the `applicationPasswordName`
                     // We can proceed to the creation step
@@ -130,12 +125,14 @@ final public class DefaultApplicationPasswordUseCase: ApplicationPasswordUseCase
     ///
     ///  Deletes locally and also sends an API request to delete it from the site
     ///
-    public func deletePassword() async throws {
+    public func deletePassword(locally: Bool) async throws {
         // Get the uuid before removing the password from storage
-        let uuidFromLocalPassword = applicationPassword?.uuid
+        let uuidFromLocalPassword = locally ? storage.applicationPassword?.uuid : nil
 
-        // Remove password from storage
-        storage.removeApplicationPassword()
+        if locally {
+            // Remove password from storage
+            storage.removeApplicationPassword()
+        }
 
         let uuidToBeDeleted = try await {
             if let uuidFromLocalPassword {
@@ -149,6 +146,18 @@ final public class DefaultApplicationPasswordUseCase: ApplicationPasswordUseCase
 }
 
 private extension DefaultApplicationPasswordUseCase {
+    /// Helper method to create password name from device
+    static func createPasswordName() -> String {
+        #if !os(watchOS)
+        let bundleIdentifier = Bundle.main.bundleIdentifier ?? "Unknown"
+        let model = UIDevice.current.model
+        let identifierForVendor = UIDevice.current.identifierForVendor?.uuidString ?? ""
+        return "\(bundleIdentifier).ios-app-client.\(model).\(identifierForVendor)"
+        #else
+        fatalError("Unexpected error: Application password should not be generated through watch app")
+        #endif
+    }
+
     /// Helper method to construct network requests either directly with the remote site
     /// or through Jetpack proxy.
     func constructRequest(method: HTTPMethod, path: String, parameters: [String: Any]? = nil) -> Request {
@@ -294,7 +303,7 @@ private extension DefaultApplicationPasswordUseCase {
 }
 
 extension DefaultApplicationPasswordUseCase {
-    enum AuthenticationType {
+    public enum AuthenticationType {
         case wporg(username: String, password: String, siteAddress: String)
         case wpcom(siteID: Int64)
     }

Modules/Sources/NetworkingCore/ApplicationPassword/OneTimeApplicationPasswordUseCase.swift

@@ -1,38 +1,53 @@
 import Foundation
 import KeychainAccess
 
+public protocol URLSessionProtocol {
+    func data(for request: URLRequest) async throws -> (Data, URLResponse)
+}
+
+extension URLSession: URLSessionProtocol {}
+
 /// Use case to save application password generated from web view;
 /// The password will not be re-generated because no cookie authentication is available.
 ///
 final public class OneTimeApplicationPasswordUseCase: ApplicationPasswordUseCase {
     public let applicationPassword: ApplicationPassword?
 
     private let siteAddress: String
-    private let session: URLSession
+    private let session: URLSessionProtocol
+    private let storage: ApplicationPasswordStorageType
 
     public init(applicationPassword: ApplicationPassword? = nil,
                 siteAddress: String,
-                keychain: Keychain = Keychain(service: WooConstants.keychainServiceName)) {
-        let storage = ApplicationPasswordStorage(keychain: keychain)
+                injectedStorage: ApplicationPasswordStorageType? = nil,
+                session: URLSessionProtocol = URLSession(configuration: .default)) {
+        self.storage = injectedStorage ?? ApplicationPasswordStorage(keychain: Keychain(service: WooConstants.keychainServiceName))
         if let applicationPassword {
             storage.saveApplicationPassword(applicationPassword)
         }
         self.applicationPassword = storage.applicationPassword
         self.siteAddress = siteAddress
-        self.session = URLSession(configuration: .default)
+        self.session = session
     }
 
     public func generateNewPassword() async throws -> ApplicationPassword {
         /// We don't support generating new password for this use case.
         throw ApplicationPasswordUseCaseError.notSupported
     }
 
-    public func deletePassword() async throws {
+    public func deletePassword(locally: Bool) async throws {
+        /// Always fetch UUID because the one in storage was generated locally only.
+        /// Check `ApplicationPasswordAuthorizationWebViewController` for more details.
         guard let uuid = try await fetchApplicationPasswordUUID(),
               let url = URL(string: siteAddress + Path.applicationPasswords + uuid) else {
             return
         }
 
+        if locally {
+            // Remove password from storage
+            storage.removeApplicationPassword()
+        }
+
         let request = try URLRequest(url: url, method: .delete)
         let authenticatedRequest = authenticateRequest(request: request)
         _ = try await session.data(for: authenticatedRequest)

Modules/Sources/NetworkingCore/Mapper/ApplicationPassword/ApplicationPasswordNameAndUUIDMapper.swift

@@ -3,6 +3,18 @@ import Foundation
 struct ApplicationPasswordNameAndUUIDMapper: Mapper {
     func map(response: Data) throws -> [ApplicationPasswordNameAndUUID] {
         let decoder = JSONDecoder()
-        return try decoder.decode([ApplicationPasswordNameAndUUID].self, from: response)
+        if hasDataEnvelope(in: response) {
+            return try decoder.decode(ApplicationPasswordNameAndUUIDEnvelope.self, from: response).data
+        } else {
+            return try decoder.decode([ApplicationPasswordNameAndUUID].self, from: response)
+        }
     }
 }
+
+/// ApplicationPasswordNameAndUUID Disposable Entity:
+/// When retrieving application password with Jetpack proxy, the result is returned within the `data` key.
+/// This entity allows us to do parse data with JSONDecoder.
+///
+private struct ApplicationPasswordNameAndUUIDEnvelope: Decodable {
+    let data: [ApplicationPasswordNameAndUUID]
+}

Modules/Sources/Yosemite/Base/SessionManagerProtocol.swift

@@ -51,13 +51,13 @@ public protocol SessionManagerProtocol {
 
     /// Deletes application password
     ///
-    func deleteApplicationPassword(using credentials: Credentials?)
+    func deleteApplicationPassword(using credentials: Credentials?, locally: Bool)
 }
 
 /// Helper methods
 public extension SessionManagerProtocol {
     /// Let the session manager figure out the credentials by itself
-    func deleteApplicationPassword() {
-        deleteApplicationPassword(using: nil)
+    func deleteApplicationPassword(locally: Bool) {
+        deleteApplicationPassword(using: nil, locally: locally)
     }
 }

Modules/Sources/Yosemite/Model/Mocks/MockSessionManager.swift

@@ -44,7 +44,8 @@ public struct MockSessionManager: SessionManagerProtocol {
         // Do nothing
     }
 
-    public func deleteApplicationPassword(using credentials: Credentials?) {
+    /// periphery: ignore
+    public func deleteApplicationPassword(using credentials: Credentials?, locally: Bool) {
         // Do nothing
     }
 }

Modules/Tests/NetworkingTests/ApplicationPassword/DefaultApplicationPasswordUseCaseTests.swift

@@ -2,6 +2,7 @@ import XCTest
 @testable import Networking
 @testable import NetworkingCore
 import Alamofire
+import KeychainAccess
 
 /// DefaultApplicationPasswordUseCase Unit Tests
 ///
@@ -13,10 +14,12 @@ final class DefaultApplicationPasswordUseCaseTests: XCTestCase {
     /// URL suffixes
     ///
     private enum URLSuffix {
-        static let generateApplicationPassword = "users/me/application-passwords"
+        static let applicationPassword = "users/me/application-passwords"
         static let userDetails = "wp/v2/users/me"
     }
 
+    private static let keychainServiceName = "com.automattic.woocommerce.tests"
+
     override func setUp() {
         super.setUp()
         network = MockNetwork(useResponseQueue: true)
@@ -29,7 +32,7 @@ final class DefaultApplicationPasswordUseCaseTests: XCTestCase {
 
     func test_password_is_generated_with_correct_values_upon_success_response() async throws {
         // Given
-        network.simulateResponse(requestUrlSuffix: URLSuffix.generateApplicationPassword,
+        network.simulateResponse(requestUrlSuffix: URLSuffix.applicationPassword,
                                  filename: "generate-application-password-using-wporg-creds-success")
         let username = "demo"
         let siteAddress = "https://test.com"
@@ -49,7 +52,7 @@ final class DefaultApplicationPasswordUseCaseTests: XCTestCase {
     func test_applicationPasswordsDisabled_error_is_thrown_if_generating_password_fails_with_501_error() async throws {
         // Given
         let error = AFError.responseValidationFailed(reason: .unacceptableStatusCode(code: 501))
-        network.simulateError(requestUrlSuffix: URLSuffix.generateApplicationPassword, error: error)
+        network.simulateError(requestUrlSuffix: URLSuffix.applicationPassword, error: error)
         let username = "demo"
         let siteAddress = "https://test.com"
         let sut = try DefaultApplicationPasswordUseCase(username: username,
@@ -72,7 +75,7 @@ final class DefaultApplicationPasswordUseCaseTests: XCTestCase {
     func test_unauthorizedRequest_error_is_thrown_if_generating_password_fails_with_401_error() async throws {
         // Given
         let error = AFError.responseValidationFailed(reason: .unacceptableStatusCode(code: 401))
-        network.simulateError(requestUrlSuffix: URLSuffix.generateApplicationPassword, error: error)
+        network.simulateError(requestUrlSuffix: URLSuffix.applicationPassword, error: error)
         let username = "demo"
         let siteAddress = "https://test.com"
         let sut = try DefaultApplicationPasswordUseCase(username: username,
@@ -94,7 +97,7 @@ final class DefaultApplicationPasswordUseCaseTests: XCTestCase {
 
     func test_password_is_generated_with_correct_values_upon_success_response_when_authenticated_with_wpcom() async throws {
         // Given
-        network.simulateResponse(requestUrlSuffix: URLSuffix.generateApplicationPassword,
+        network.simulateResponse(requestUrlSuffix: URLSuffix.applicationPassword,
                                  filename: "generate-application-password-using-wporg-creds-success")
         network.simulateResponse(requestUrlSuffix: URLSuffix.userDetails, filename: "user-complete")
         let sut = DefaultApplicationPasswordUseCase(type: .wpcom(siteID: 123), network: network)
@@ -110,7 +113,7 @@ final class DefaultApplicationPasswordUseCaseTests: XCTestCase {
     func test_applicationPasswordsDisabled_error_is_thrown_if_generating_password_fails_with_501_error_when_authenticated_with_wpcom() async throws {
         // Given
         let error = AFError.responseValidationFailed(reason: .unacceptableStatusCode(code: 501))
-        network.simulateError(requestUrlSuffix: URLSuffix.generateApplicationPassword, error: error)
+        network.simulateError(requestUrlSuffix: URLSuffix.applicationPassword, error: error)
         network.simulateResponse(requestUrlSuffix: URLSuffix.userDetails, filename: "user-complete")
         let sut = DefaultApplicationPasswordUseCase(type: .wpcom(siteID: 123), network: network)
 
@@ -129,7 +132,7 @@ final class DefaultApplicationPasswordUseCaseTests: XCTestCase {
     func test_unauthorizedRequest_error_is_thrown_if_generating_password_fails_with_401_error_when_authenticated_with_wpcom() async throws {
         // Given
         let error = AFError.responseValidationFailed(reason: .unacceptableStatusCode(code: 401))
-        network.simulateError(requestUrlSuffix: URLSuffix.generateApplicationPassword, error: error)
+        network.simulateError(requestUrlSuffix: URLSuffix.applicationPassword, error: error)
         network.simulateResponse(requestUrlSuffix: URLSuffix.userDetails, filename: "user-complete")
         let sut = DefaultApplicationPasswordUseCase(type: .wpcom(siteID: 123), network: network)
 
@@ -144,4 +147,54 @@ final class DefaultApplicationPasswordUseCaseTests: XCTestCase {
         // Then
         XCTAssertTrue(failure == .unauthorizedRequest)
     }
+
+    func test_delete_application_password_with_locally_false_does_not_clear_storage() async throws {
+        // Given
+        let storage = MockApplicationPasswordStorage()
+        let sut = DefaultApplicationPasswordUseCase(
+            type: .wpcom(siteID: 123),
+            network: network,
+            passwordName: "test-name",
+            storage: storage
+        )
+
+        let uuid = "8ffe00cb-f903-49f9-a3e7-7674fb90fd1b"
+        let password = ApplicationPassword(wpOrgUsername: "test", password: .init("secret"), uuid: uuid)
+        storage.saveApplicationPassword(password)
+
+        network.simulateResponse(
+            requestUrlSuffix: URLSuffix.applicationPassword,
+            filename: "get-application-passwords-success-with-data"
+        )
+        network.simulateResponse(
+            requestUrlSuffix: URLSuffix.applicationPassword + "/" + uuid,
+            filename: "delete-application-password-success"
+        )
+
+        // When
+        try await sut.deletePassword(locally: false)
+
+        // Then
+        XCTAssertEqual(storage.applicationPassword, password)
+    }
+
+    func test_delete_application_password_with_locally_true_clears_storage() async throws {
+        // Given
+        let storage = MockApplicationPasswordStorage()
+        let sut = DefaultApplicationPasswordUseCase(type: .wpcom(siteID: 123), network: network, storage: storage)
+
+        let uuid = "4567-8901-2345-6789"
+        storage.saveApplicationPassword(ApplicationPassword(wpOrgUsername: "testuser", password: .init("password123"), uuid: uuid))
+
+        network.simulateResponse(
+            requestUrlSuffix: "users/me/application-passwords/\(uuid)",
+            filename: "delete-application-password-success"
+        )
+
+        // When
+        try await sut.deletePassword(locally: true)
+
+        // Then
+        XCTAssertNil(storage.applicationPassword)
+    }
 }

Modules/Tests/NetworkingTests/ApplicationPassword/MockApplicationPasswordStorage.swift

@@ -0,0 +1,14 @@
+import Foundation
+@testable import NetworkingCore
+
+final class MockApplicationPasswordStorage: ApplicationPasswordStorageType {
+    private(set) var applicationPassword: ApplicationPassword?
+
+    func saveApplicationPassword(_ password: ApplicationPassword) {
+        applicationPassword = password
+    }
+
+    func removeApplicationPassword() {
+        applicationPassword = nil
+    }
+}