OAuth login often fails in new UI (4582) #3385
Open
+249
−47
Conversation
This file contains hidden or 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
This change addresses the timing issue which often leads to failed login attempts
This condition returns true, even though the token is not valid anymore.
PayPal triggers multiple page redirects after completing the login popup. This commit introduces a new state handler that will halt duplicate requests until the first request is finished
Narek13
reviewed
May 14, 2025
| sleep( 1 ); | ||
|
|
||
| // Get the current URL. | ||
| $current_url = add_query_arg( null, null ); |
There was a problem hiding this comment.
To be honest, I’m not a big fan of adding inline comments. I understand that they can make the code clearer, especially in this plugin where some logic is quite complex. However, in cases where the code is obvious like here, I would suggest not adding them. This isn't a strong opinion and open to discussion. Another point is that other devs aren’t adding them either, which leads to inconsistent styling.
Narek13
approved these changes
May 14, 2025
Dinamiko
approved these changes
May 19, 2025
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.
Description
This PR refactores the OAuth process in two ways:
I) Solve timing issues between REST and redirect
The initial REST call only stores the auth_code and ID, without attempting to resolve them directly. This ensures that the REST call finishes very fast.
The following page reload fetches the code/id from the DB and performs the API requests which were initially handled in the Ajax call. This consolidates the full authentication logic to the same HTTP request.
II) Handling concurrent page loads
After finishing the login popup, PayPal often starts TWO page reloads. While the first (detached) request handles the authentication correctly, the second request (which happens in the browser) would previously run into a “token already used” situation and display the login screen again.
A new state was introduced to mark the token as “currently processing” (max lifetime 10 seconds). When any follow-up request encounter this “currently processing” state, they will pause for 1 second and do a simple page reload, waiting for the authentication request to complete.