fix: removed the possibility of concurrent webauth transactions to handle continuation misuse #848
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ndle continuation misuse
6 tasks
…ansaction when a new transaction is added
… flow is active already
… transaction is added
… store before each run to start clean
Widcket
reviewed
May 8, 2024
Auth0/WebAuthError.swift
Outdated
| @@ -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" | |||
There was a problem hiding this comment.
Suggested change
| 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." |
There was a problem hiding this comment.
Updated the text description for the WebAuthError .transactionActiveAlready as per description
Widcket
reviewed
May 8, 2024
| @@ -6,6 +6,7 @@ public struct WebAuthError: Auth0Error { | |||
|
|
|||
| enum Code: Equatable { | |||
| case noBundleIdentifier | |||
| case transactionActiveAlready | |||
There was a problem hiding this comment.
There are tests for WebAuthError, those should be updated as well: https://github.com/auth0/Auth0.swift/blob/master/Auth0Tests/WebAuthErrorSpec.swift
There was a problem hiding this comment.
Thanks for pointing it out, included tests for it now.
Widcket
approved these changes
May 8, 2024
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📋 Changes
login&logoutin WebAuthSpec to clearTransactionStorebefore each run to start clean and not get failed by the un-cleared transaction in memory by previous test runs.📎 References
auth0/Auth0.swift#827
Checklist
🎯 Testing
Tested by trying to start the below defined two flows at exact same time via both callbacks & async/await approach and found that the code is working as per expectations by cancelling the newly triggering web auth flow and ensuring the existing web auth flow executes completely.
google-oauth2connection usingASWebAuthenticationSessionUniversal LoginusingSafariViewController