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 redirect_uri handling #163

Closed
davidkuridza opened this issue Jun 20, 2013 · 4 comments
Closed

Invalid redirect_uri handling #163

davidkuridza opened this issue Jun 20, 2013 · 4 comments

Comments

@davidkuridza
Copy link
Contributor

According to Access Token Request part of the RFC, redirect_uri is required only if it's not included in authorization request. This library is requesting it in token step regardless of whether it was included or not in authorization step. Problematic part of the code is below line:

diff --git a/src/OAuth2/Controller/AuthorizeController.php b/src/OAuth2/Controller/AuthorizeController.php
index a610a82..07c31b8 100644
--- a/src/OAuth2/Controller/AuthorizeController.php
+++ b/src/OAuth2/Controller/AuthorizeController.php
@@ -122,7 +122,7 @@ class AuthorizeController implements AuthorizeControllerInterface
                 $response->setError(400, 'invalid_uri', 'A redirect URI must be supplied when multiple redirect URIs are registered', '#section-3.1.2.3');
                 return false;
             }
-            $redirect_uri = $registered_redirect_uri;
+//            $redirect_uri = $registered_redirect_uri;
         }

         // Select the redirect URI

However, if it's commented out, a whole hell breaks loose :) By following example provided with the library, first thing that breaks is: PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'redirect_uri' cannot be null in oauth2-server-php/src/OAuth2/Storage/Pdo.php on line 141

At the same time enforce_redirect has to be set to false, otherwise it doesn't work. But then redirect doesn't work since there is no redirect_uri set at all. I tried looking into fixing it all together, but it seems it's not a simple fix and my knowledge of the library is not good enough yet to not break something else (will try to look into it more in depth next week though).

Is this what the behavior of the library should be even though the flow is not OAuth2 compliant? Was is done this way due to some security reasons?

@raymondjplante
Copy link

I ran into this issue as well. When requesting a code without supplying the redirect URI, the following request for the access token using the returned code should not require a redirect URI. It looks like whenever a Code is requested the redirect_uri is copied from the Client table if not supplied in the code request. When requesting the access token if the redirect_uri field isn't empty, it will fail on access_token request without a redirect API specified. I found a way around this:

  1. Request the code without the redirect uri
  2. in the oauth_authorization_codes table delete the redirect_uri value
  3. Make the access toke request
  4. Viola! It works....

I implemented my own storage that never stores the redirect URI in the authorization_code table... This isn't compliant, but I don't require my users to supply it and alway use the redirect url from the client table.

@bshaffer
Copy link
Owner

Thank you for reporting this. 
Sounds like we just need to change the DB setup query to make redirect_uri nullable and then add a conditional in TokenController to check if the redirect_uri was set in the auth code. 

Brent Shaffer

On Thu, Jun 20, 2013 at 7:43 AM, Raymond Plante notifications@github.com
wrote:

I ran into this issue as well. When requesting a code without supplying the redirect URI, the following request for the access token using the returned code should not require a redirect URI. It looks like whenever a Code is requested the redirect_uri is copied from the Client table. If this field isn't empty, it will fail on access_token request without a redirect API. I found a way around this:

  1. Request the code without the redirect uri
  2. in the oauth_authorization_codes table delete the redirect_uri value
  3. Make the access toke request

4. Viola! It works....

Reply to this email directly or view it on GitHub:
#163 (comment)

@davidkuridza
Copy link
Contributor Author

@raymondjplante I ended up doing the same thing, just wanted to double check whether I'm not doing something wrong :)

@bshaffer Sounds reasonable, will look into it after Wednesday next week when I get back to civilization (bad timing, but we had a trip planned for some time now). Unless it's already fixed by then of course :)

@davidkuridza
Copy link
Contributor Author

First thing I saw this morning when getting back was an email saying redirect_uri problem is fixed. I fetched the changes, tested them out and everything works as it should. Kudos to you, sir! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants