Skip to content

Commit 09438f3

Browse files
itsmeichigoitsmeichigo
authored
Handle deletion of app password when authenticated with WPCom (#16046)
2 parents c657687 + f302200 commit 09438f3

20 files changed

+477
-107
lines changed

‎Modules/Sources/NetworkingCore/ApplicationPassword/ApplicationPasswordStorage.swift‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,18 @@
11
import Foundation
22
import KeychainAccess
33

4-
public struct ApplicationPasswordStorage {
4+
public protocol ApplicationPasswordStorageType {
5+
/// Returns the saved application password if available
6+
var applicationPassword: ApplicationPassword? { get }
7+
8+
/// Saves application password into keychain
9+
func saveApplicationPassword(_ password: ApplicationPassword)
10+
11+
/// Removes the currently saved password from storage
12+
func removeApplicationPassword()
13+
}
14+
15+
public struct ApplicationPasswordStorage: ApplicationPasswordStorageType {
516
/// Stores the application password
617
///
718
private let keychain: Keychain

‎Modules/Sources/NetworkingCore/ApplicationPassword/ApplicationPasswordUseCase.swift‎

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,10 @@ public protocol ApplicationPasswordUseCase {
2929

3030
/// Deletes the application password
3131
///
32-
/// Deletes locally and also sends an API request to delete it from the site
32+
/// - Parameter locally: Determines whether to remove the password from the local storage
33+
/// or only sends an API request to delete it from the site.
3334
///
34-
func deletePassword() async throws
35+
func deletePassword(locally: Bool) async throws
3536
}
3637

3738
final public class DefaultApplicationPasswordUseCase: ApplicationPasswordUseCase {
@@ -45,38 +46,32 @@ final public class DefaultApplicationPasswordUseCase: ApplicationPasswordUseCase
4546

4647
/// To store application password
4748
///
48-
private let storage: ApplicationPasswordStorage
49+
private let storage: ApplicationPasswordStorageType
4950

5051
/// Used to name the password in wpadmin.
5152
///
52-
private var applicationPasswordName: String {
53-
#if !os(watchOS)
54-
let bundleIdentifier = Bundle.main.bundleIdentifier ?? "Unknown"
55-
let model = UIDevice.current.model
56-
let identifierForVendor = UIDevice.current.identifierForVendor?.uuidString ?? ""
57-
return "\(bundleIdentifier).ios-app-client.\(model).\(identifierForVendor)"
58-
#else
59-
fatalError("Unexpected error: Application password should not be generated through watch app")
60-
#endif
61-
}
53+
private let applicationPasswordName: String
6254

6355
/// Internal initializer
64-
init(type: AuthenticationType,
65-
network: Network,
66-
keychain: Keychain = Keychain(service: WooConstants.keychainServiceName)) {
56+
public init(type: AuthenticationType,
57+
network: Network,
58+
passwordName: String? = nil,
59+
storage: ApplicationPasswordStorageType? = nil) {
6760
self.authenticationType = type
68-
self.storage = ApplicationPasswordStorage(keychain: keychain)
61+
self.storage = storage ?? ApplicationPasswordStorage(keychain: Keychain(service: WooConstants.keychainServiceName))
6962
self.network = network
63+
self.applicationPasswordName = passwordName ?? Self.createPasswordName()
7064
}
7165

7266
/// Public initializer for wporg authentication
7367
public init(username: String,
7468
password: String,
7569
siteAddress: String,
7670
network: Network? = nil,
77-
keychain: Keychain = Keychain(service: WooConstants.keychainServiceName)) throws {
71+
storage: ApplicationPasswordStorageType? = nil) throws {
7872
self.authenticationType = .wporg(username: username, password: password, siteAddress: siteAddress)
79-
self.storage = ApplicationPasswordStorage(keychain: keychain)
73+
self.storage = storage ?? ApplicationPasswordStorage(keychain: Keychain(service: WooConstants.keychainServiceName))
74+
self.applicationPasswordName = Self.createPasswordName()
8075

8176
if let network {
8277
self.network = network
@@ -113,7 +108,7 @@ final public class DefaultApplicationPasswordUseCase: ApplicationPasswordUseCase
113108
return try await createApplicationPassword()
114109
} catch ApplicationPasswordUseCaseError.duplicateName {
115110
do {
116-
try await deletePassword()
111+
try await deletePassword(locally: true)
117112
} catch ApplicationPasswordUseCaseError.unableToFindPasswordUUID {
118113
// No password found with the `applicationPasswordName`
119114
// We can proceed to the creation step
@@ -130,12 +125,14 @@ final public class DefaultApplicationPasswordUseCase: ApplicationPasswordUseCase
130125
///
131126
/// Deletes locally and also sends an API request to delete it from the site
132127
///
133-
public func deletePassword() async throws {
128+
public func deletePassword(locally: Bool) async throws {
134129
// Get the uuid before removing the password from storage
135-
let uuidFromLocalPassword = applicationPassword?.uuid
130+
let uuidFromLocalPassword = locally ? storage.applicationPassword?.uuid : nil
136131

137-
// Remove password from storage
138-
storage.removeApplicationPassword()
132+
if locally {
133+
// Remove password from storage
134+
storage.removeApplicationPassword()
135+
}
139136

140137
let uuidToBeDeleted = try await {
141138
if let uuidFromLocalPassword {
@@ -149,6 +146,18 @@ final public class DefaultApplicationPasswordUseCase: ApplicationPasswordUseCase
149146
}
150147

151148
private extension DefaultApplicationPasswordUseCase {
149+
/// Helper method to create password name from device
150+
static func createPasswordName() -> String {
151+
#if !os(watchOS)
152+
let bundleIdentifier = Bundle.main.bundleIdentifier ?? "Unknown"
153+
let model = UIDevice.current.model
154+
let identifierForVendor = UIDevice.current.identifierForVendor?.uuidString ?? ""
155+
return "\(bundleIdentifier).ios-app-client.\(model).\(identifierForVendor)"
156+
#else
157+
fatalError("Unexpected error: Application password should not be generated through watch app")
158+
#endif
159+
}
160+
152161
/// Helper method to construct network requests either directly with the remote site
153162
/// or through Jetpack proxy.
154163
func constructRequest(method: HTTPMethod, path: String, parameters: [String: Any]? = nil) -> Request {
@@ -294,7 +303,7 @@ private extension DefaultApplicationPasswordUseCase {
294303
}
295304

296305
extension DefaultApplicationPasswordUseCase {
297-
enum AuthenticationType {
306+
public enum AuthenticationType {
298307
case wporg(username: String, password: String, siteAddress: String)
299308
case wpcom(siteID: Int64)
300309
}

‎Modules/Sources/NetworkingCore/ApplicationPassword/OneTimeApplicationPasswordUseCase.swift‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,53 @@
11
import Foundation
22
import KeychainAccess
33

4+
public protocol URLSessionProtocol {
5+
func data(for request: URLRequest) async throws -> (Data, URLResponse)
6+
}
7+
8+
extension URLSession: URLSessionProtocol {}
9+
410
/// Use case to save application password generated from web view;
511
/// The password will not be re-generated because no cookie authentication is available.
612
///
713
final public class OneTimeApplicationPasswordUseCase: ApplicationPasswordUseCase {
814
public let applicationPassword: ApplicationPassword?
915

1016
private let siteAddress: String
11-
private let session: URLSession
17+
private let session: URLSessionProtocol
18+
private let storage: ApplicationPasswordStorageType
1219

1320
public init(applicationPassword: ApplicationPassword? = nil,
1421
siteAddress: String,
15-
keychain: Keychain = Keychain(service: WooConstants.keychainServiceName)) {
16-
let storage = ApplicationPasswordStorage(keychain: keychain)
22+
injectedStorage: ApplicationPasswordStorageType? = nil,
23+
session: URLSessionProtocol = URLSession(configuration: .default)) {
24+
self.storage = injectedStorage ?? ApplicationPasswordStorage(keychain: Keychain(service: WooConstants.keychainServiceName))
1725
if let applicationPassword {
1826
storage.saveApplicationPassword(applicationPassword)
1927
}
2028
self.applicationPassword = storage.applicationPassword
2129
self.siteAddress = siteAddress
22-
self.session = URLSession(configuration: .default)
30+
self.session = session
2331
}
2432

2533
public func generateNewPassword() async throws -> ApplicationPassword {
2634
/// We don't support generating new password for this use case.
2735
throw ApplicationPasswordUseCaseError.notSupported
2836
}
2937

30-
public func deletePassword() async throws {
38+
public func deletePassword(locally: Bool) async throws {
39+
/// Always fetch UUID because the one in storage was generated locally only.
40+
/// Check `ApplicationPasswordAuthorizationWebViewController` for more details.
3141
guard let uuid = try await fetchApplicationPasswordUUID(),
3242
let url = URL(string: siteAddress + Path.applicationPasswords + uuid) else {
3343
return
3444
}
3545

46+
if locally {
47+
// Remove password from storage
48+
storage.removeApplicationPassword()
49+
}
50+
3651
let request = try URLRequest(url: url, method: .delete)
3752
let authenticatedRequest = authenticateRequest(request: request)
3853
_ = try await session.data(for: authenticatedRequest)

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,18 @@ import Foundation
33
struct ApplicationPasswordNameAndUUIDMapper: Mapper {
44
func map(response: Data) throws -> [ApplicationPasswordNameAndUUID] {
55
let decoder = JSONDecoder()
6-
return try decoder.decode([ApplicationPasswordNameAndUUID].self, from: response)
6+
if hasDataEnvelope(in: response) {
7+
return try decoder.decode(ApplicationPasswordNameAndUUIDEnvelope.self, from: response).data
8+
} else {
9+
return try decoder.decode([ApplicationPasswordNameAndUUID].self, from: response)
10+
}
711
}
812
}
13+
14+
/// ApplicationPasswordNameAndUUID Disposable Entity:
15+
/// When retrieving application password with Jetpack proxy, the result is returned within the `data` key.
16+
/// This entity allows us to do parse data with JSONDecoder.
17+
///
18+
private struct ApplicationPasswordNameAndUUIDEnvelope: Decodable {
19+
let data: [ApplicationPasswordNameAndUUID]
20+
}

‎Modules/Sources/Yosemite/Base/SessionManagerProtocol.swift‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,13 @@ public protocol SessionManagerProtocol {
5151

5252
/// Deletes application password
5353
///
54-
func deleteApplicationPassword(using credentials: Credentials?)
54+
func deleteApplicationPassword(using credentials: Credentials?, locally: Bool)
5555
}
5656

5757
/// Helper methods
5858
public extension SessionManagerProtocol {
5959
/// Let the session manager figure out the credentials by itself
60-
func deleteApplicationPassword() {
61-
deleteApplicationPassword(using: nil)
60+
func deleteApplicationPassword(locally: Bool) {
61+
deleteApplicationPassword(using: nil, locally: locally)
6262
}
6363
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ public struct MockSessionManager: SessionManagerProtocol {
4444
// Do nothing
4545
}
4646

47-
public func deleteApplicationPassword(using credentials: Credentials?) {
47+
/// periphery: ignore
48+
public func deleteApplicationPassword(using credentials: Credentials?, locally: Bool) {
4849
// Do nothing
4950
}
5051
}

‎Modules/Tests/NetworkingTests/ApplicationPassword/DefaultApplicationPasswordUseCaseTests.swift‎

Lines changed: 60 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import XCTest
22
@testable import Networking
33
@testable import NetworkingCore
44
import Alamofire
5+
import KeychainAccess
56

67
/// DefaultApplicationPasswordUseCase Unit Tests
78
///
@@ -13,10 +14,12 @@ final class DefaultApplicationPasswordUseCaseTests: XCTestCase {
1314
/// URL suffixes
1415
///
1516
private enum URLSuffix {
16-
static let generateApplicationPassword = "users/me/application-passwords"
17+
static let applicationPassword = "users/me/application-passwords"
1718
static let userDetails = "wp/v2/users/me"
1819
}
1920

21+
private static let keychainServiceName = "com.automattic.woocommerce.tests"
22+
2023
override func setUp() {
2124
super.setUp()
2225
network = MockNetwork(useResponseQueue: true)
@@ -29,7 +32,7 @@ final class DefaultApplicationPasswordUseCaseTests: XCTestCase {
2932

3033
func test_password_is_generated_with_correct_values_upon_success_response() async throws {
3134
// Given
32-
network.simulateResponse(requestUrlSuffix: URLSuffix.generateApplicationPassword,
35+
network.simulateResponse(requestUrlSuffix: URLSuffix.applicationPassword,
3336
filename: "generate-application-password-using-wporg-creds-success")
3437
let username = "demo"
3538
let siteAddress = "https://test.com"
@@ -49,7 +52,7 @@ final class DefaultApplicationPasswordUseCaseTests: XCTestCase {
4952
func test_applicationPasswordsDisabled_error_is_thrown_if_generating_password_fails_with_501_error() async throws {
5053
// Given
5154
let error = AFError.responseValidationFailed(reason: .unacceptableStatusCode(code: 501))
52-
network.simulateError(requestUrlSuffix: URLSuffix.generateApplicationPassword, error: error)
55+
network.simulateError(requestUrlSuffix: URLSuffix.applicationPassword, error: error)
5356
let username = "demo"
5457
let siteAddress = "https://test.com"
5558
let sut = try DefaultApplicationPasswordUseCase(username: username,
@@ -72,7 +75,7 @@ final class DefaultApplicationPasswordUseCaseTests: XCTestCase {
7275
func test_unauthorizedRequest_error_is_thrown_if_generating_password_fails_with_401_error() async throws {
7376
// Given
7477
let error = AFError.responseValidationFailed(reason: .unacceptableStatusCode(code: 401))
75-
network.simulateError(requestUrlSuffix: URLSuffix.generateApplicationPassword, error: error)
78+
network.simulateError(requestUrlSuffix: URLSuffix.applicationPassword, error: error)
7679
let username = "demo"
7780
let siteAddress = "https://test.com"
7881
let sut = try DefaultApplicationPasswordUseCase(username: username,
@@ -94,7 +97,7 @@ final class DefaultApplicationPasswordUseCaseTests: XCTestCase {
9497

9598
func test_password_is_generated_with_correct_values_upon_success_response_when_authenticated_with_wpcom() async throws {
9699
// Given
97-
network.simulateResponse(requestUrlSuffix: URLSuffix.generateApplicationPassword,
100+
network.simulateResponse(requestUrlSuffix: URLSuffix.applicationPassword,
98101
filename: "generate-application-password-using-wporg-creds-success")
99102
network.simulateResponse(requestUrlSuffix: URLSuffix.userDetails, filename: "user-complete")
100103
let sut = DefaultApplicationPasswordUseCase(type: .wpcom(siteID: 123), network: network)
@@ -110,7 +113,7 @@ final class DefaultApplicationPasswordUseCaseTests: XCTestCase {
110113
func test_applicationPasswordsDisabled_error_is_thrown_if_generating_password_fails_with_501_error_when_authenticated_with_wpcom() async throws {
111114
// Given
112115
let error = AFError.responseValidationFailed(reason: .unacceptableStatusCode(code: 501))
113-
network.simulateError(requestUrlSuffix: URLSuffix.generateApplicationPassword, error: error)
116+
network.simulateError(requestUrlSuffix: URLSuffix.applicationPassword, error: error)
114117
network.simulateResponse(requestUrlSuffix: URLSuffix.userDetails, filename: "user-complete")
115118
let sut = DefaultApplicationPasswordUseCase(type: .wpcom(siteID: 123), network: network)
116119

@@ -129,7 +132,7 @@ final class DefaultApplicationPasswordUseCaseTests: XCTestCase {
129132
func test_unauthorizedRequest_error_is_thrown_if_generating_password_fails_with_401_error_when_authenticated_with_wpcom() async throws {
130133
// Given
131134
let error = AFError.responseValidationFailed(reason: .unacceptableStatusCode(code: 401))
132-
network.simulateError(requestUrlSuffix: URLSuffix.generateApplicationPassword, error: error)
135+
network.simulateError(requestUrlSuffix: URLSuffix.applicationPassword, error: error)
133136
network.simulateResponse(requestUrlSuffix: URLSuffix.userDetails, filename: "user-complete")
134137
let sut = DefaultApplicationPasswordUseCase(type: .wpcom(siteID: 123), network: network)
135138

@@ -144,4 +147,54 @@ final class DefaultApplicationPasswordUseCaseTests: XCTestCase {
144147
// Then
145148
XCTAssertTrue(failure == .unauthorizedRequest)
146149
}
150+
151+
func test_delete_application_password_with_locally_false_does_not_clear_storage() async throws {
152+
// Given
153+
let storage = MockApplicationPasswordStorage()
154+
let sut = DefaultApplicationPasswordUseCase(
155+
type: .wpcom(siteID: 123),
156+
network: network,
157+
passwordName: "test-name",
158+
storage: storage
159+
)
160+
161+
let uuid = "8ffe00cb-f903-49f9-a3e7-7674fb90fd1b"
162+
let password = ApplicationPassword(wpOrgUsername: "test", password: .init("secret"), uuid: uuid)
163+
storage.saveApplicationPassword(password)
164+
165+
network.simulateResponse(
166+
requestUrlSuffix: URLSuffix.applicationPassword,
167+
filename: "get-application-passwords-success-with-data"
168+
)
169+
network.simulateResponse(
170+
requestUrlSuffix: URLSuffix.applicationPassword + "/" + uuid,
171+
filename: "delete-application-password-success"
172+
)
173+
174+
// When
175+
try await sut.deletePassword(locally: false)
176+
177+
// Then
178+
XCTAssertEqual(storage.applicationPassword, password)
179+
}
180+
181+
func test_delete_application_password_with_locally_true_clears_storage() async throws {
182+
// Given
183+
let storage = MockApplicationPasswordStorage()
184+
let sut = DefaultApplicationPasswordUseCase(type: .wpcom(siteID: 123), network: network, storage: storage)
185+
186+
let uuid = "4567-8901-2345-6789"
187+
storage.saveApplicationPassword(ApplicationPassword(wpOrgUsername: "testuser", password: .init("password123"), uuid: uuid))
188+
189+
network.simulateResponse(
190+
requestUrlSuffix: "users/me/application-passwords/\(uuid)",
191+
filename: "delete-application-password-success"
192+
)
193+
194+
// When
195+
try await sut.deletePassword(locally: true)
196+
197+
// Then
198+
XCTAssertNil(storage.applicationPassword)
199+
}
147200
}

‎Modules/Tests/NetworkingTests/ApplicationPassword/MockApplicationPasswordStorage.swift‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import Foundation
2+
@testable import NetworkingCore
3+
4+
final class MockApplicationPasswordStorage: ApplicationPasswordStorageType {
5+
private(set) var applicationPassword: ApplicationPassword?
6+
7+
func saveApplicationPassword(_ password: ApplicationPassword) {
8+
applicationPassword = password
9+
}
10+
11+
func removeApplicationPassword() {
12+
applicationPassword = nil
13+
}
14+
}

0 commit comments

Comments
 (0)