diff --git a/Modules/Sources/NetworkingCore/ApplicationPassword/ApplicationPasswordStorage.swift b/Modules/Sources/NetworkingCore/ApplicationPassword/ApplicationPasswordStorage.swift index f414377cd70..47170e3a9f3 100644 --- a/Modules/Sources/NetworkingCore/ApplicationPassword/ApplicationPasswordStorage.swift +++ b/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 diff --git a/Modules/Sources/NetworkingCore/ApplicationPassword/ApplicationPasswordUseCase.swift b/Modules/Sources/NetworkingCore/ApplicationPassword/ApplicationPasswordUseCase.swift index b88064cca0c..b0eafe8b3b6 100644 --- a/Modules/Sources/NetworkingCore/ApplicationPassword/ApplicationPasswordUseCase.swift +++ b/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,28 +46,21 @@ 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 @@ -74,9 +68,10 @@ final public class DefaultApplicationPasswordUseCase: ApplicationPasswordUseCase 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) } diff --git a/Modules/Sources/NetworkingCore/ApplicationPassword/OneTimeApplicationPasswordUseCase.swift b/Modules/Sources/NetworkingCore/ApplicationPassword/OneTimeApplicationPasswordUseCase.swift index b3cf0d46c63..bf360adfa4e 100644 --- a/Modules/Sources/NetworkingCore/ApplicationPassword/OneTimeApplicationPasswordUseCase.swift +++ b/Modules/Sources/NetworkingCore/ApplicationPassword/OneTimeApplicationPasswordUseCase.swift @@ -1,6 +1,12 @@ 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. /// @@ -8,18 +14,20 @@ 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 { @@ -27,12 +35,19 @@ final public class OneTimeApplicationPasswordUseCase: ApplicationPasswordUseCase 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) diff --git a/Modules/Sources/NetworkingCore/Mapper/ApplicationPassword/ApplicationPasswordNameAndUUIDMapper.swift b/Modules/Sources/NetworkingCore/Mapper/ApplicationPassword/ApplicationPasswordNameAndUUIDMapper.swift index 6f7bf922369..df49f488133 100644 --- a/Modules/Sources/NetworkingCore/Mapper/ApplicationPassword/ApplicationPasswordNameAndUUIDMapper.swift +++ b/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] +} diff --git a/Modules/Sources/Yosemite/Base/SessionManagerProtocol.swift b/Modules/Sources/Yosemite/Base/SessionManagerProtocol.swift index 20560e3f597..08e726b8d53 100644 --- a/Modules/Sources/Yosemite/Base/SessionManagerProtocol.swift +++ b/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) } } diff --git a/Modules/Sources/Yosemite/Model/Mocks/MockSessionManager.swift b/Modules/Sources/Yosemite/Model/Mocks/MockSessionManager.swift index 2f38060926a..7ea6cbcf4d7 100644 --- a/Modules/Sources/Yosemite/Model/Mocks/MockSessionManager.swift +++ b/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 } } diff --git a/Modules/Tests/NetworkingTests/ApplicationPassword/DefaultApplicationPasswordUseCaseTests.swift b/Modules/Tests/NetworkingTests/ApplicationPassword/DefaultApplicationPasswordUseCaseTests.swift index f270d44181d..dc6438acfd9 100644 --- a/Modules/Tests/NetworkingTests/ApplicationPassword/DefaultApplicationPasswordUseCaseTests.swift +++ b/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) + } } diff --git a/Modules/Tests/NetworkingTests/ApplicationPassword/MockApplicationPasswordStorage.swift b/Modules/Tests/NetworkingTests/ApplicationPassword/MockApplicationPasswordStorage.swift new file mode 100644 index 00000000000..812ecd42745 --- /dev/null +++ b/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 + } +} diff --git a/Modules/Tests/NetworkingTests/ApplicationPassword/MockURLSession.swift b/Modules/Tests/NetworkingTests/ApplicationPassword/MockURLSession.swift new file mode 100644 index 00000000000..3377fe361eb --- /dev/null +++ b/Modules/Tests/NetworkingTests/ApplicationPassword/MockURLSession.swift @@ -0,0 +1,56 @@ +import Foundation +@testable import NetworkingCore + +final class MockURLSession: URLSessionProtocol { + var responses: [String: (Data, URLResponse)] = [:] + var errors: [String: Error] = [:] + + private(set) var lastRequest: URLRequest? + private(set) var requestCount = 0 + + func data(for request: URLRequest) async throws -> (Data, URLResponse) { + lastRequest = request + requestCount += 1 + + let key = request.url?.absoluteString ?? "" + + if let error = errors[key] { + throw error + } + + if let response = responses[key] { + return response + } + + // Default success response + let data = Data() + let response = HTTPURLResponse( + url: request.url!, + statusCode: 200, + httpVersion: nil, + headerFields: nil + )! + return (data, response) + } + + func simulateResponse(for url: String, data: Data, statusCode: Int = 200) { + let response = HTTPURLResponse( + url: URL(string: url)!, + statusCode: statusCode, + httpVersion: nil, + headerFields: nil + )! + responses[url] = (data, response) + } + + func simulateError(for url: String, error: Error) { + errors[url] = error + } + + func reset() { + responses.removeAll() + errors.removeAll() + lastRequest = nil + requestCount = 0 + } +} diff --git a/Modules/Tests/NetworkingTests/ApplicationPassword/OneTimeApplicationPasswordUseCaseTests.swift b/Modules/Tests/NetworkingTests/ApplicationPassword/OneTimeApplicationPasswordUseCaseTests.swift new file mode 100644 index 00000000000..fbbb1c6fc43 --- /dev/null +++ b/Modules/Tests/NetworkingTests/ApplicationPassword/OneTimeApplicationPasswordUseCaseTests.swift @@ -0,0 +1,180 @@ +import XCTest +@testable import Networking +@testable import NetworkingCore +import KeychainAccess + +/// OneTimeApplicationPasswordUseCase Unit Tests +/// +final class OneTimeApplicationPasswordUseCaseTests: XCTestCase { + private var mockSession: MockURLSession! + private var storage: MockApplicationPasswordStorage! + private let siteAddress = "https://test.com" + + override func setUp() { + super.setUp() + mockSession = MockURLSession() + storage = MockApplicationPasswordStorage() + } + + override func tearDown() { + mockSession = nil + storage = nil + super.tearDown() + } + + // MARK: - Initialization Tests + + func test_init_with_application_password_saves_password_to_storage() { + // Given + let password = createTestPassword(username: "testuser", password: "secret123", uuid: "test-uuid") + + // When + let sut = createSUT(password: password) + + // Then + XCTAssertEqual(sut.applicationPassword?.wpOrgUsername, "testuser") + XCTAssertEqual(sut.applicationPassword?.uuid, "test-uuid") + XCTAssertEqual(storage.applicationPassword?.wpOrgUsername, "testuser") + XCTAssertEqual(storage.applicationPassword?.uuid, "test-uuid") + } + + func test_init_without_application_password_has_nil_password() { + // When + let sut = createSUT() + + // Then + XCTAssertNil(sut.applicationPassword) + } + + // MARK: - Generate Password Tests + + func test_generateNewPassword_throws_notSupported_error() async { + // Given + let sut = createSUT() + + // When/Then + do { + _ = try await sut.generateNewPassword() + XCTFail("Expected notSupported error to be thrown") + } catch { + XCTAssertEqual(error as? ApplicationPasswordUseCaseError, .notSupported) + } + } + + // MARK: - Delete Password Tests + + func test_deletePassword_locally_true_removes_from_storage_and_calls_api() async throws { + // Given + let deleteUUID = "fetched-uuid-456" + simulateIntrospectResponse(uuid: deleteUUID) + simulateDeleteResponse(for: deleteUUID) + + let password = createTestPassword(uuid: "original-uuid") + let sut = createSUT(password: password) + + // Verify password is initially present + XCTAssertNotNil(sut.applicationPassword) + + // When + try await sut.deletePassword(locally: true) + + // Then + XCTAssertNil(storage.applicationPassword) + XCTAssertEqual(mockSession.requestCount, 2) + XCTAssertEqual(mockSession.lastRequest?.url?.absoluteString, deleteURL(for: deleteUUID)) + XCTAssertEqual(mockSession.lastRequest?.httpMethod, "DELETE") + XCTAssertEqual(mockSession.lastRequest?.value(forHTTPHeaderField: "Authorization"), "Basic dGVzdHVzZXI6c2VjcmV0") + } + + func test_deletePassword_locally_false_fetches_uuid_from_introspect_endpoint_and_does_not_remove_from_storage() async throws { + // Given + let deleteUUID = "fetched-uuid-456" + simulateIntrospectResponse(uuid: deleteUUID) + simulateDeleteResponse(for: deleteUUID) + + let password = createTestPassword(uuid: "original-uuid") + let sut = createSUT(password: password) + + // When + try await sut.deletePassword(locally: false) + + // Then + XCTAssertNotNil(storage.applicationPassword) // Password should still be in storage + XCTAssertEqual(mockSession.requestCount, 2) + // Check introspect call + XCTAssertEqual(mockSession.responses.keys.contains(introspectURL()), true) + // Check delete call + XCTAssertEqual(mockSession.responses.keys.contains(deleteURL(for: deleteUUID)), true) + } + + // MARK: - Authentication Header Tests + + func test_deletePassword_sets_correct_authorization_header() async throws { + // Given + let username = "testuser" + let passwordValue = "testpassword" + let deleteUUID = "fetched-uuid-456" + simulateIntrospectResponse(uuid: deleteUUID) + simulateDeleteResponse(for: deleteUUID) + + let password = createTestPassword(username: username, password: passwordValue, uuid: "original-uuid") + let sut = createSUT(password: password) + + // When + try await sut.deletePassword(locally: true) + + // Then + let expectedAuth = Data("\(username):\(passwordValue)".utf8).base64EncodedString() + XCTAssertEqual(mockSession.lastRequest?.value(forHTTPHeaderField: "Authorization"), "Basic \(expectedAuth)") + XCTAssertEqual(mockSession.lastRequest?.value(forHTTPHeaderField: "Accept"), "application/json") + XCTAssertEqual(mockSession.lastRequest?.value(forHTTPHeaderField: "User-Agent"), UserAgent.defaultUserAgent) + XCTAssertEqual(mockSession.lastRequest?.httpShouldHandleCookies, false) + } +} + +// MARK: - Helper Methods +private extension OneTimeApplicationPasswordUseCaseTests { + func createSUT(password: ApplicationPassword? = nil, siteAddress: String? = nil) -> OneTimeApplicationPasswordUseCase { + return OneTimeApplicationPasswordUseCase( + applicationPassword: password, + siteAddress: siteAddress ?? self.siteAddress, + injectedStorage: storage, + session: mockSession + ) + } + + func simulateDeleteResponse(for uuid: String) { + let deleteURL = "\(siteAddress)/?rest_route=/wp/v2/users/me/application-passwords/\(uuid)" + let deleteResponse = """ + { + "data": { + "deleted": true + } + } + """.data(using: .utf8)! + mockSession.simulateResponse(for: deleteURL, data: deleteResponse) + } + + func simulateIntrospectResponse(uuid: String, name: String = "test-password") { + let introspectURL = "\(siteAddress)/?rest_route=/wp/v2/users/me/application-passwords/introspect" + let introspectResponse = """ + { + "uuid": "\(uuid)", + "name": "\(name)" + } + """.data(using: .utf8)! + mockSession.simulateResponse(for: introspectURL, data: introspectResponse) + } + + func createTestPassword(username: String = "testuser", password: String = "secret", uuid: String = "test-uuid") -> ApplicationPassword { + return ApplicationPassword(wpOrgUsername: username, password: .init(password), uuid: uuid) + } + + func deleteURL(for uuid: String) -> String { + return "\(siteAddress)/?rest_route=/wp/v2/users/me/application-passwords/\(uuid)" + } + + func introspectURL() -> String { + return "\(siteAddress)/?rest_route=/wp/v2/users/me/application-passwords/introspect" + } +} diff --git a/Modules/Tests/NetworkingTests/Mapper/ApplicationPasswordNameAndUUIDMapperTests.swift b/Modules/Tests/NetworkingTests/Mapper/ApplicationPasswordNameAndUUIDMapperTests.swift index dce021e6c55..ca1f1d1cf6a 100644 --- a/Modules/Tests/NetworkingTests/Mapper/ApplicationPasswordNameAndUUIDMapperTests.swift +++ b/Modules/Tests/NetworkingTests/Mapper/ApplicationPasswordNameAndUUIDMapperTests.swift @@ -17,6 +17,19 @@ final class ApplicationPasswordNameAndUUIDMapperTests: XCTestCase { XCTAssertEqual(password.uuid, "42467857-579d-4bf3-8cc7-88bfb701d3a7") XCTAssertEqual(password.name, "testest") } + + /// Verifies that GET application password response in data envelope is parsed properly + /// + func test_response_with_data_envelope_is_properly_parsed_when_loading_all_application_passwords() throws { + guard let passwords = mapGetApplicationPasswordsResponseWithDataEnvelope() else { + XCTFail() + return + } + + let password = try XCTUnwrap(passwords.first) + XCTAssertEqual(password.uuid, "8ffe00cb-f903-49f9-a3e7-7674fb90fd1b") + XCTAssertEqual(password.name, "test-name") + } } // MARK: - Private Methods. @@ -32,4 +45,15 @@ private extension ApplicationPasswordNameAndUUIDMapperTests { return try? ApplicationPasswordNameAndUUIDMapper().map(response: response) } + + /// Returns the ApplicationPasswordNameAndUUIDMapper output upon receiving success response + /// wrapped in data envelope + /// + func mapGetApplicationPasswordsResponseWithDataEnvelope() -> [ApplicationPasswordNameAndUUID]? { + guard let response = Loader.contentsOf("get-application-passwords-success-with-data") else { + return nil + } + + return try? ApplicationPasswordNameAndUUIDMapper().map(response: response) + } } diff --git a/Modules/Tests/NetworkingTests/Network/DefaultRequestAuthenticatorTests.swift b/Modules/Tests/NetworkingTests/Network/DefaultRequestAuthenticatorTests.swift index 720f3746091..07e61b6eb02 100644 --- a/Modules/Tests/NetworkingTests/Network/DefaultRequestAuthenticatorTests.swift +++ b/Modules/Tests/NetworkingTests/Network/DefaultRequestAuthenticatorTests.swift @@ -235,7 +235,7 @@ private final class MockApplicationPasswordUseCase: ApplicationPasswordUseCase { throw mockGenerationError ?? NetworkError.notFound() } - func deletePassword() async throws { + func deletePassword(locally: Bool) async throws { throw mockDeletionError ?? NetworkError.notFound() } } diff --git a/Modules/Tests/NetworkingTests/Responses/delete-application-password-success.json b/Modules/Tests/NetworkingTests/Responses/delete-application-password-success.json new file mode 100644 index 00000000000..7db68794fcb --- /dev/null +++ b/Modules/Tests/NetworkingTests/Responses/delete-application-password-success.json @@ -0,0 +1,5 @@ +{ + "data": { + "deleted": true + } +} diff --git a/Modules/Tests/NetworkingTests/Responses/get-application-passwords-success-with-data.json b/Modules/Tests/NetworkingTests/Responses/get-application-passwords-success-with-data.json new file mode 100644 index 00000000000..2774c3a773e --- /dev/null +++ b/Modules/Tests/NetworkingTests/Responses/get-application-passwords-success-with-data.json @@ -0,0 +1,6 @@ +{ "data": [ + { + "uuid": "8ffe00cb-f903-49f9-a3e7-7674fb90fd1b", + "name": "test-name", + } +]} diff --git a/WooCommerce/Classes/System/SessionManager.swift b/WooCommerce/Classes/System/SessionManager.swift index bfc2fd4259e..77b045e26c9 100644 --- a/WooCommerce/Classes/System/SessionManager.swift +++ b/WooCommerce/Classes/System/SessionManager.swift @@ -7,6 +7,7 @@ import protocol Networking.ApplicationPasswordUseCase import class Networking.OneTimeApplicationPasswordUseCase import class Networking.DefaultApplicationPasswordUseCase import class Kingfisher.ImageCache +import class Networking.AlamofireNetwork // MARK: - SessionManager Notifications // @@ -175,6 +176,10 @@ final class SessionManager: SessionManagerProtocol { } } + /// Keeps strong reference of the use case to keep the password deletion request alive + /// periphery: ignore + var applicationPasswordUseCase: ApplicationPasswordUseCase? + /// Designated Initializer. /// init(defaults: UserDefaults, @@ -200,7 +205,6 @@ final class SessionManager: SessionManagerProtocol { /// Nukes all of the known Session's properties. /// func reset() { - deleteApplicationPassword() defaultAccount = nil defaultCredentials = nil defaultStoreID = nil @@ -227,17 +231,23 @@ final class SessionManager: SessionManagerProtocol { /// Deletes application password /// - func deleteApplicationPassword(using credentials: Credentials?) { + func deleteApplicationPassword(using creds: Credentials?, locally: Bool) { let useCase: ApplicationPasswordUseCase? = { - switch credentials ?? loadCredentials() { + let credentials = creds ?? loadCredentials() + switch credentials { case let .wporg(username, password, siteAddress): return try? DefaultApplicationPasswordUseCase(username: username, password: password, - siteAddress: siteAddress, - keychain: keychain) + siteAddress: siteAddress) case let .applicationPassword(_, _, siteAddress): - return OneTimeApplicationPasswordUseCase(siteAddress: siteAddress, keychain: keychain) - default: + return OneTimeApplicationPasswordUseCase(siteAddress: siteAddress) + case .wpcom: + guard let siteID = defaultStoreID else { + return nil + } + let network = AlamofireNetwork(credentials: credentials) + return DefaultApplicationPasswordUseCase(type: .wpcom(siteID: siteID), network: network) + case .none: return nil } }() @@ -245,8 +255,10 @@ final class SessionManager: SessionManagerProtocol { return } - Task { - try await useCase.deletePassword() + applicationPasswordUseCase = useCase + Task { @MainActor in + try await useCase.deletePassword(locally: locally) + applicationPasswordUseCase = nil } } } diff --git a/WooCommerce/Classes/ViewRelated/JetpackSetup/JetpackSetupCoordinator.swift b/WooCommerce/Classes/ViewRelated/JetpackSetup/JetpackSetupCoordinator.swift index 8ae58038b7e..ae0efd0b632 100644 --- a/WooCommerce/Classes/ViewRelated/JetpackSetup/JetpackSetupCoordinator.swift +++ b/WooCommerce/Classes/ViewRelated/JetpackSetup/JetpackSetupCoordinator.swift @@ -255,7 +255,7 @@ private extension JetpackSetupCoordinator { guard let self else { return } switch result { case .success(let site): - self.stores.sessionManager.deleteApplicationPassword(using: previousCredentials) + self.stores.sessionManager.deleteApplicationPassword(using: previousCredentials, locally: true) self.stores.updateDefaultStore(storeID: site.siteID) self.stores.synchronizeEntities { [weak self] in self?.stores.updateDefaultStore(site) diff --git a/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift b/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift index cb9fb8b2726..b8c288b1434 100644 --- a/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift +++ b/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift @@ -220,7 +220,10 @@ class DefaultStoresManager: StoresManager { /// Prepares for changing the selected store and remains Authenticated. /// func removeDefaultStore() { - sessionManager.deleteApplicationPassword() + /// In case of store switching, new password might be saved locally before the deletion of old password is done. + /// Here we delete the password remotely only to avoid the race condition + /// when the new password is removed from the local storage by mistake. + sessionManager.deleteApplicationPassword(locally: false) ServiceLocator.analytics.refreshUserData() ZendeskProvider.shared.reset() ServiceLocator.pushNotesManager.unregisterForRemoteNotifications() @@ -250,6 +253,7 @@ class DefaultStoresManager: StoresManager { dispatch(resetAction) } + sessionManager.deleteApplicationPassword(locally: true) sessionManager.reset() state = DeauthenticatedState() diff --git a/WooCommerce/WooCommerceTests/Authentication/PostSiteCredentialLoginCheckerTests.swift b/WooCommerce/WooCommerceTests/Authentication/PostSiteCredentialLoginCheckerTests.swift index 771cec23fa8..f5fda777ce9 100644 --- a/WooCommerce/WooCommerceTests/Authentication/PostSiteCredentialLoginCheckerTests.swift +++ b/WooCommerce/WooCommerceTests/Authentication/PostSiteCredentialLoginCheckerTests.swift @@ -253,7 +253,7 @@ private final class MockApplicationPasswordUseCase: ApplicationPasswordUseCase { throw mockGenerationError ?? NetworkError.notFound() } - func deletePassword() async throws { + func deletePassword(locally: Bool) async throws { throw mockDeletionError ?? NetworkError.notFound() } } diff --git a/WooCommerce/WooCommerceTests/System/SessionManagerTests.swift b/WooCommerce/WooCommerceTests/System/SessionManagerTests.swift index 47ab4556fc9..22807667ded 100644 --- a/WooCommerce/WooCommerceTests/System/SessionManagerTests.swift +++ b/WooCommerce/WooCommerceTests/System/SessionManagerTests.swift @@ -84,50 +84,6 @@ final class SessionManagerTests: XCTestCase { XCTAssertEqual(retrieved, Settings.applicationPasswordCredentials) } - /// Verifies that application password is deleted upon calling `deleteApplicationPassword` - /// - func test_deleteApplicationPassword_deletes_password_from_keychain() { - // Given - manager.defaultCredentials = Settings.wporgCredentials - let storage = ApplicationPasswordStorage(keychain: Keychain(service: Settings.keychainServiceName)) - - // When - storage.saveApplicationPassword(applicationPassword) - - // Then - XCTAssertNotNil(storage.applicationPassword) - - // When - manager.deleteApplicationPassword() - - // Then - waitUntil { - storage.applicationPassword == nil - } - } - - /// Verifies that application password is deleted upon reset - /// - func test_application_password_is_deleted_upon_reset() { - // Given - manager.defaultCredentials = Settings.wporgCredentials - let storage = ApplicationPasswordStorage(keychain: Keychain(service: Settings.keychainServiceName)) - - // When - storage.saveApplicationPassword(applicationPassword) - - // Then - XCTAssertNotNil(storage.applicationPassword) - - // When - manager.reset() - - // Then - waitUntil { - storage.applicationPassword == nil - } - } - /// Verifies that `storePhoneNumber` is set to `nil` upon reset /// func test_storePhoneNumber_is_set_to_nil_upon_reset() throws { diff --git a/WooCommerce/WooCommerceTests/Yosemite/StoresManagerTests.swift b/WooCommerce/WooCommerceTests/Yosemite/StoresManagerTests.swift index 80682444bc8..b63edc27996 100644 --- a/WooCommerce/WooCommerceTests/Yosemite/StoresManagerTests.swift +++ b/WooCommerce/WooCommerceTests/Yosemite/StoresManagerTests.swift @@ -309,7 +309,20 @@ final class StoresManagerTests: XCTestCase { XCTAssertTrue(mockProductImageUploader.resetWasCalled) } - func test_removing_default_store_invokes_delete_application_password() { + func test_deauthenticate_invokes_delete_application_password() { + // Given + let mockSessionManager = MockSessionManager() + let sut = DefaultStoresManager(sessionManager: mockSessionManager) + + // When + sut.deauthenticate() + + // Then + XCTAssertTrue(mockSessionManager.deleteApplicationPasswordInvoked) + XCTAssertTrue(mockSessionManager.deleteApplicationPasswordLocally) + } + + func test_removingDefaultStore_invokes_delete_application_password() { // Given let mockSessionManager = MockSessionManager() let sut = DefaultStoresManager(sessionManager: mockSessionManager) @@ -319,6 +332,7 @@ final class StoresManagerTests: XCTestCase { // Then XCTAssertTrue(mockSessionManager.deleteApplicationPasswordInvoked) + XCTAssertFalse(mockSessionManager.deleteApplicationPasswordLocally) } func test_updating_default_storeID_sets_storePhoneNumber_to_nil() throws { @@ -500,6 +514,7 @@ final class MockAuthenticationManager: AuthenticationManager { final class MockSessionManager: SessionManagerProtocol { private(set) var deleteApplicationPasswordInvoked: Bool = false + private(set) var deleteApplicationPasswordLocally = false var defaultAccount: Yosemite.Account? = nil @@ -535,16 +550,13 @@ final class MockSessionManager: SessionManagerProtocol { // Do nothing } - func deleteApplicationPassword(using credentials: Credentials?) { - deleteApplicationPasswordInvoked = true - } - - func deleteApplicationPassword() { + func deleteApplicationPassword(using credentials: Credentials?, locally: Bool) { deleteApplicationPasswordInvoked = true + deleteApplicationPasswordLocally = locally } } -private class MockNotificationCenter: NotificationCenter { +private class MockNotificationCenter: NotificationCenter, @unchecked Sendable { static var testingInstance = MockNotificationCenter() }