Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Invalid authorization code": Access token is requested twice in a row, breaking the login flow #305

Closed
Kirill-TechHub opened this issue Apr 12, 2017 · 11 comments
Assignees
Milestone

Comments

@Kirill-TechHub
Copy link

I am using Wordpress Version 4.7.3, trying to install the official Auth0 plugin on it for the first time.

The problem is related to logging in as an Auth0 user. After a successful login Auth0 returns the user agent to https://site.com/index.php?auth0=1, which receives the auth code and uses it to get an access token - that's the usual pattern. In my case, however, the Wordpress site automatically redirects from /index.php to / (root), but only AFTER running the plugin token logic. So what actually happened was:

  1. User logs in, Auth0 redirects to /index.php?auth0=1
  2. Wordpress receives the code at /index.php?auth0=1, gets an access token, THEN it redirects to / (root).
  3. Wordpress receives the code again, at / (root) this time, tries to get a new access token, but fails because the auth code was used already.

I don't know if this is a WP bug, perhaps related to a newer version of WP, but it is what it is. I fixed this by replacing every occurance of "index.php" in the source of the plugin with emptiness, and fixing the callbacks in Auth0 to redirect to /?auth0=1 instead of /index.php?auth0=1.

Maybe you can find a cleaner fix, perhaps a switch or an input field in the dashboard that lets the site owner choose the callback destination.

@Worldfavor
Copy link

I have the same issue, can you share your code?

@Kirill-TechHub
Copy link
Author

I'm afraid I can't share my code because I've made other changes to it besides this. But as I said, for this issue all I've done was replace all text occurances of "index.php" with emptiness in the php files.

@joshcanhelp joshcanhelp self-assigned this Jan 24, 2018
@joshcanhelp
Copy link
Contributor

@Kirill-TechHub - Late reply here but wanted to get something up about this.

We made a number of changes recently (specifically #360) around how URLs are handled in the plugin. I don't know that it will specifically address this but version 3.5.0 of the plugin and later will include them.

Talking through what's going on here ... it sounds like you've got a redirect loading somewhere during WP processing that's causing this to happen. Your best bet is a server-level one, if you need one, or none at all. Just with an OOTB WordPress install (latest, 4.9.2), I can type this:

http:https://localhost:8080/index.php?auth0=1

... and I'll get to the right place (meaning: Auth0 does it's processing and the filename is gone at the final URL).

So, at this point, I'm not sure there's much we can do. We need to be specific about the links were using for callbacks and everything else, and we haven't seen this behavior in any testing.

@coperator
Copy link

I think these recent changes do not affect this problem. I still have the issue described above.
Maybe, I will find to investigate this further. Until then, my thoughts:
The redirect from /index.php?auth0=1 to /?auth0=1 is most probably due to redirect_canonical action from Wordpress core, I guess? But I currently have no idea, why the redirect happens after the auth0 processing and before the auth0 plugin redirects to the correct redirect url.
Maybe this is due to an strange interplay with other plugins, if it works with OOTB Wordpress.

@coperator
Copy link

I found the issue in our case:
In another plugin hooked to wp_login an unhandled exception was thrown. This exception was then "catched" by the wp-auth0 plugin in WP_Auth0_LoginManager.init_auth0() by doing nothing, so later the canonical redirect took effect.
The responsible code:

    try {
      if ( $this->query_vars( 'auth0' ) === 'implicit' ) {
        $this->implicit_login();
      } else {
        $this->redirect_login(); // calls do_action('wp_login')
      }

...

    } catch (Exception $e) {
    }

This empty catch block is maybe not the best idea. Especially, due to the fact that the missing redirect by auth0 causes the problem described above. I would propose to either leave the catch away or to error log and a proper redirect:

    } catch (Exception $e) {
      error_log($e);
      this->redirect(); // deals with state etc.
    }

@joshcanhelp
Copy link
Contributor

Good ... catch 😆

We don't want to blindly redirect like that, and in the case that something went wrong on our end, that's not going to work well. That said, there is clearly room for improvement here so I'll reopen, assign to myself, and take a look at it for an upcoming release.

If inspiration strikes, please feel free to submit a PR! Also, when you have a sec, let me know the plugin you're using so I can test the behavior out.

Thanks for the investigation and checking back in with what you found, we appreciate it!

@joshcanhelp joshcanhelp reopened this Feb 7, 2018
@Floppy
Copy link

Floppy commented Feb 21, 2018

Pretty sure I have the same thing going on. From my own notes:

Wordpress is requesting the auth token twice. We can see a successful exchange in the Auth0 logs, followed by a failed one. Upon login, a callback is made to https://mydomain.com/index.php?auth0=1&code=whevs. This probably triggers the first exchange. However, Wordpress is redirecting that to https://mydomain.com/?auth0=1&code=whevs, which triggers a second exchange which fails.
The Wordpress canonical URL redirection is happening after the exchange, but the parameters are preserved, so off it goes. Really the Auth0 plugin should have already done an on-successful-exchange redirect, but this one happens anyway.

Annoyingly, something else in my code is stopping me just removing the index.php from my URLs, but I'll stick at it and see if I can fix it that way.

@Floppy
Copy link

Floppy commented Feb 23, 2018

A bit more info: this is happening to me because after successful code exchange, there is an exception thrown during the JWT decoding (there's no string to decode on https://github.com/auth0/wp-auth0/blob/master/lib/WP_Auth0_LoginManager.php#L245, which may well be the root cause of my problem). The exception bubbles up and causes the rediirection to the canonical URL, at which point the code exchange happens again and fails.

@joshcanhelp
Copy link
Contributor

I appreciate all the additional information here. This is absolutely on my radar to fix in the next minor release (maintenance one coming out next week first).

@joshcanhelp joshcanhelp added this to the v3-Next milestone Feb 23, 2018
@Floppy
Copy link

Floppy commented Feb 23, 2018

Confirmed that @coperator's identification of the empty catch block is the culprit. If I wp_die inside there, I see a much more useful error :)

joshcanhelp added a commit that referenced this issue Mar 8, 2018
WP_Auth0_LoginManager was not processing errors well, was poorly
documented, and might have been improperly exposing error messages.
Incoming URL param errors from Auth0 and configuration issues are caught
earlier in the login process. Error message are not exposed to the
user; instead they are logged for an admin. Thrown errors are
standarized and listed in docblocks.

Fixes #305
joshcanhelp added a commit that referenced this issue Mar 12, 2018
WP_Auth0_LoginManager was not processing errors well, was poorly
documented, and might have been improperly exposing error messages.
Incoming URL param errors from Auth0 and configuration issues are caught
earlier in the login process. Error message are not exposed to the
user; instead they are logged for an admin. Thrown errors are
standarized and listed in docblocks.

Fixes #305
joshcanhelp added a commit that referenced this issue Mar 15, 2018
WP_Auth0_LoginManager was not processing errors well, was poorly
documented, and might have been improperly exposing error messages.
Incoming URL param errors from Auth0 and configuration issues are caught
earlier in the login process. Error message are not exposed to the
user; instead they are logged for an admin. Thrown errors are
standarized and listed in docblocks.

Fixes #305
joshcanhelp added a commit that referenced this issue Apr 13, 2018
WP_Auth0_LoginManager was not processing errors well, was poorly
documented, and might have been improperly exposing error messages.
Incoming URL param errors from Auth0 and configuration issues are caught
earlier in the login process. Error message are not exposed to the
user; instead they are logged for an admin. Thrown errors are
standarized and listed in docblocks.

Fixes #305
@joshcanhelp
Copy link
Contributor

Fixed in #409, merged into dev

joshcanhelp added a commit that referenced this issue Jun 5, 2018
WP_Auth0_LoginManager was not processing errors well, was poorly
documented, and might have been improperly exposing error messages.
Incoming URL param errors from Auth0 and configuration issues are caught
earlier in the login process. Error message are not exposed to the
user; instead they are logged for an admin. Thrown errors are
standarized and listed in docblocks.

Fixes #305
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants