Issue with redirect_uri in Authorization Code Grant

[ls-sean-fraser]

Using version 9.0.0, using authorization code grant without specifying a redirect_uri in both requests does not seem to be accepted. The spec indicates these are only required if used in both places.

redirect_uri
         REQUIRED, if the "redirect_uri" parameter was included in the
         authorization request as described in Section 4.1.1, and their
         values MUST be identical.

When using a default Authorization code entity...

class OauthAuthorizationCode implements AuthCodeEntityInterface
{
    use EntityTrait;
    use TokenEntityTrait;
    use AuthCodeTrait;
}

If the redirect_uri is omitted from the authorize request, the authorization code contains a redirect_uri of null.

The access token call then fail here, As the value is null, but it is checking for empty string

https://github.com/thephpleague/oauth2-server/blob/master/src/Grant/AuthCodeGrant.php#L220-L224

if ($authCodePayload->redirect_uri !== '' && $redirectUri === null) {
     throw OAuthServerException::invalidRequest('redirect_uri');
}

Forcing the null redirect_uri to be empty string in the entity doesn't resolve the issue as the subsequent check which will fail to compare redirect_uri of the code and the request, as '' !== null.

I suspect that the check above should be changed to be:

if ($authCodePayload->redirect_uri !== null && $redirectUri === null) { 

[Sephster]

Have you got a redirect Uri registered against your client?

[ls-sean-fraser]

Yes, my ClientEntityInterface implementation returns a value for the client. The redirect itself works fine, it is only validation of it during the code exchange that is the problem

[Sephster]

Thanks for reporting this. I think I have a solution figured out. I'm going to change the code so we validate that if a redirect URI is passed, it must be a valid URI as per the specs. That negates the need to check for non empty strings as we shouldn't be accepting these in the first place.

I'll finalise this tomorrow and submit a fix. Thank you

[ls-sean-fraser]

Hi @Sephster, wondering if you might have an update on this task. Hoping to see this bug addressed before we rollout this library, so we don't need to force our partners to specify the redirect_uri every time

[Sephster]

Sorry @ls-sean-fraser I got a virus and then had a secondary infection caused by that which has had me out of action for nearly a fortnight. Aim to get back to this this week as slowly feeling better. Sorry for the delay

[ls-sean-fraser]

Sorry to hear that, @Sephster. I went ahead and put up a PR to address this if that helps. I hope you feel better soon.