From ae9b5d29b99d0f4fe24f70fed82bf7bde1d7b78e Mon Sep 17 00:00:00 2001 From: Sai Venkat Desu Date: Thu, 2 May 2024 17:12:30 +0530 Subject: [PATCH 1/9] fix: removed the possibility of concurrent webauth transactions to handle continuation misuse --- Auth0/TransactionStore.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Auth0/TransactionStore.swift b/Auth0/TransactionStore.swift index 260cae8b..8d2512c1 100644 --- a/Auth0/TransactionStore.swift +++ b/Auth0/TransactionStore.swift @@ -14,7 +14,10 @@ class TransactionStore { return isResumed } + + /// Calling store would cancel existing transactions if any, and then would set the supplied transaction as the current one. func store(_ transaction: AuthTransaction) { + self.cancel() self.current = transaction } From b8aa270a089bd1a55f5c785ef89120a97b9f4b32 Mon Sep 17 00:00:00 2001 From: Sai Venkat Desu Date: Mon, 6 May 2024 13:08:50 +0530 Subject: [PATCH 2/9] test: updated tests in TransactionStoreSpec to cancel any previous transaction when a new transaction is added --- Auth0Tests/TransactionStoreSpec.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Auth0Tests/TransactionStoreSpec.swift b/Auth0Tests/TransactionStoreSpec.swift index 25db4543..2e23c417 100644 --- a/Auth0Tests/TransactionStoreSpec.swift +++ b/Auth0Tests/TransactionStoreSpec.swift @@ -29,10 +29,10 @@ class TransactionStoreSpec: QuickSpec { expect(storage.current).to(beNil()) } - it("should not cancel current transaction") { + it("should cancel previous transaction when new transaction is added to store") { storage.store(transaction) storage.store(SpyTransaction()) - expect(transaction.isCancelled) == false + expect(transaction.isCancelled) == true } } From 2a413d793d0524b33fb9997efc09e104d0f52ede Mon Sep 17 00:00:00 2001 From: Sai Venkat Desu Date: Wed, 8 May 2024 12:10:20 +0530 Subject: [PATCH 3/9] fix: updated to cancel any new webauth flows initiated when a webauth flow is active already --- Auth0/Auth0WebAuth.swift | 10 ++++++++++ Auth0/AuthenticationError.swift | 2 +- Auth0/TransactionStore.swift | 3 --- Auth0/WebAuthError.swift | 2 ++ 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/Auth0/Auth0WebAuth.swift b/Auth0/Auth0WebAuth.swift index f211a38f..c6e65174 100644 --- a/Auth0/Auth0WebAuth.swift +++ b/Auth0/Auth0WebAuth.swift @@ -158,6 +158,11 @@ final class Auth0WebAuth: WebAuth { } func start(_ callback: @escaping (WebAuthResult) -> Void) { + + if let currentTransaction = self.storage.current { + return callback(.failure(WebAuthError(code: .transactionActiveAlready))) + } + guard let redirectURL = self.redirectURL else { return callback(.failure(WebAuthError(code: .noBundleIdentifier))) } @@ -207,6 +212,11 @@ final class Auth0WebAuth: WebAuth { } func clearSession(federated: Bool, callback: @escaping (WebAuthResult) -> Void) { + + if let currentTransaction = self.storage.current { + return callback(.failure(WebAuthError(code: .transactionActiveAlready))) + } + let endpoint = federated ? URL(string: "v2/logout?federated", relativeTo: self.url)! : URL(string: "v2/logout", relativeTo: self.url)! diff --git a/Auth0/AuthenticationError.swift b/Auth0/AuthenticationError.swift index 66b24c15..bfe09b1d 100644 --- a/Auth0/AuthenticationError.swift +++ b/Auth0/AuthenticationError.swift @@ -184,7 +184,7 @@ extension AuthenticationError { return "Received error with code \(self.code)." } - + } // MARK: - Equatable diff --git a/Auth0/TransactionStore.swift b/Auth0/TransactionStore.swift index 8d2512c1..260cae8b 100644 --- a/Auth0/TransactionStore.swift +++ b/Auth0/TransactionStore.swift @@ -14,10 +14,7 @@ class TransactionStore { return isResumed } - - /// Calling store would cancel existing transactions if any, and then would set the supplied transaction as the current one. func store(_ transaction: AuthTransaction) { - self.cancel() self.current = transaction } diff --git a/Auth0/WebAuthError.swift b/Auth0/WebAuthError.swift index 9be4a9af..ab216b41 100644 --- a/Auth0/WebAuthError.swift +++ b/Auth0/WebAuthError.swift @@ -6,6 +6,7 @@ public struct WebAuthError: Auth0Error { enum Code: Equatable { case noBundleIdentifier + case transactionActiveAlready case invalidInvitationURL(String) case userCancelled case noAuthorizationCode([String: String]) @@ -79,6 +80,7 @@ extension WebAuthError { switch self.code { case .noBundleIdentifier: return "Unable to retrieve the bundle identifier from Bundle.main.bundleIdentifier," + " or it could not be used to build a valid URL." + case .transactionActiveAlready: return "Failed to start this transaction, as there is an active transaction at the moment" case .invalidInvitationURL(let url): return "The invitation URL (\(url)) is missing the 'invitation' and/or" + " the 'organization' query parameters." case .userCancelled: return "The user cancelled the Web Auth operation." From 33a91a12a356b5aea30ff825b09e126c39e9d4e7 Mon Sep 17 00:00:00 2001 From: Sai Venkat Desu Date: Wed, 8 May 2024 14:07:37 +0530 Subject: [PATCH 4/9] chore: minor improvements --- Auth0/Auth0WebAuth.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Auth0/Auth0WebAuth.swift b/Auth0/Auth0WebAuth.swift index c6e65174..1fce14a0 100644 --- a/Auth0/Auth0WebAuth.swift +++ b/Auth0/Auth0WebAuth.swift @@ -158,8 +158,8 @@ final class Auth0WebAuth: WebAuth { } func start(_ callback: @escaping (WebAuthResult) -> Void) { - - if let currentTransaction = self.storage.current { + + if self.storage.current != nil { return callback(.failure(WebAuthError(code: .transactionActiveAlready))) } @@ -212,8 +212,8 @@ final class Auth0WebAuth: WebAuth { } func clearSession(federated: Bool, callback: @escaping (WebAuthResult) -> Void) { - - if let currentTransaction = self.storage.current { + + if self.storage.current != nil { return callback(.failure(WebAuthError(code: .transactionActiveAlready))) } From 24dced0a6c31b8034e94e7175ee7910958ed347d Mon Sep 17 00:00:00 2001 From: Sai Venkat Desu Date: Wed, 8 May 2024 14:08:32 +0530 Subject: [PATCH 5/9] test: updated units test to not cancel current transaction when a new transaction is added --- Auth0Tests/TransactionStoreSpec.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Auth0Tests/TransactionStoreSpec.swift b/Auth0Tests/TransactionStoreSpec.swift index 2e23c417..25db4543 100644 --- a/Auth0Tests/TransactionStoreSpec.swift +++ b/Auth0Tests/TransactionStoreSpec.swift @@ -29,10 +29,10 @@ class TransactionStoreSpec: QuickSpec { expect(storage.current).to(beNil()) } - it("should cancel previous transaction when new transaction is added to store") { + it("should not cancel current transaction") { storage.store(transaction) storage.store(SpyTransaction()) - expect(transaction.isCancelled) == true + expect(transaction.isCancelled) == false } } From 5b8d38aaefffb87c8bbc59f4171855b05e9f3736 Mon Sep 17 00:00:00 2001 From: Sai Venkat Desu Date: Wed, 8 May 2024 14:09:23 +0530 Subject: [PATCH 6/9] test: updated WebAuthSpec login and logout tests to clear transaction store before each run to start clean --- Auth0Tests/WebAuthSpec.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Auth0Tests/WebAuthSpec.swift b/Auth0Tests/WebAuthSpec.swift index cfe8f094..54b6e362 100644 --- a/Auth0Tests/WebAuthSpec.swift +++ b/Auth0Tests/WebAuthSpec.swift @@ -475,6 +475,7 @@ class WebAuthSpec: QuickSpec { beforeEach { auth = newWebAuth() + TransactionStore.shared.clear() } it("should start the supplied provider") { @@ -600,6 +601,7 @@ class WebAuthSpec: QuickSpec { beforeEach { auth = newWebAuth() + TransactionStore.shared.clear() } it("should start the supplied provider") { From 20acabcfbd17744c99e74a5cff2b5967c7f75fe5 Mon Sep 17 00:00:00 2001 From: Sai Venkat Desu Date: Wed, 8 May 2024 15:19:04 +0530 Subject: [PATCH 7/9] chore: removed whitespaces --- Auth0/Auth0WebAuth.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Auth0/Auth0WebAuth.swift b/Auth0/Auth0WebAuth.swift index 1fce14a0..752b93dd 100644 --- a/Auth0/Auth0WebAuth.swift +++ b/Auth0/Auth0WebAuth.swift @@ -158,7 +158,7 @@ final class Auth0WebAuth: WebAuth { } func start(_ callback: @escaping (WebAuthResult) -> Void) { - + if self.storage.current != nil { return callback(.failure(WebAuthError(code: .transactionActiveAlready))) } @@ -212,7 +212,7 @@ final class Auth0WebAuth: WebAuth { } func clearSession(federated: Bool, callback: @escaping (WebAuthResult) -> Void) { - + if self.storage.current != nil { return callback(.failure(WebAuthError(code: .transactionActiveAlready))) } From 8b3db6394631c2a79b2633728920034b90db4b5f Mon Sep 17 00:00:00 2001 From: Sai Venkat Desu Date: Wed, 8 May 2024 15:47:20 +0530 Subject: [PATCH 8/9] chore: minor improvements for the .transactionActiveAlready WebAuthError message --- Auth0/WebAuthError.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Auth0/WebAuthError.swift b/Auth0/WebAuthError.swift index ab216b41..7552261d 100644 --- a/Auth0/WebAuthError.swift +++ b/Auth0/WebAuthError.swift @@ -80,7 +80,8 @@ extension WebAuthError { switch self.code { case .noBundleIdentifier: return "Unable to retrieve the bundle identifier from Bundle.main.bundleIdentifier," + " or it could not be used to build a valid URL." - case .transactionActiveAlready: return "Failed to start this transaction, as there is an active transaction at the moment" + case .transactionActiveAlready: return "Failed to start this transaction, as there is an active transaction at the" + + " moment." case .invalidInvitationURL(let url): return "The invitation URL (\(url)) is missing the 'invitation' and/or" + " the 'organization' query parameters." case .userCancelled: return "The user cancelled the Web Auth operation." From 69f1f85cd07ea660540ff4f5eebe40a0eab19a82 Mon Sep 17 00:00:00 2001 From: Sai Venkat Desu Date: Wed, 8 May 2024 15:47:54 +0530 Subject: [PATCH 9/9] test: updated WebAuthErrorSpec to include tests for .transactionActiveAlready --- Auth0Tests/WebAuthErrorSpec.swift | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Auth0Tests/WebAuthErrorSpec.swift b/Auth0Tests/WebAuthErrorSpec.swift index 4e3c31f1..782ae3ba 100644 --- a/Auth0Tests/WebAuthErrorSpec.swift +++ b/Auth0Tests/WebAuthErrorSpec.swift @@ -86,6 +86,13 @@ class WebAuthErrorSpec: QuickSpec { expect(error.localizedDescription) == message } + it("should return message for transaction active already") { + let message = "Failed to start this transaction, as there is an active transaction at the" + + " moment." + let error = WebAuthError(code: .transactionActiveAlready) + expect(error.localizedDescription) == message + } + it("should return message for invalid invitation URL") { let url = "https://samples.auth0.com" let message = "The invitation URL (\(url)) is missing the 'invitation' and/or"