Invalid logic during redirect_uri validation

[KminekMatej]

Hey ya all, shouldnt this statement be simply empty($authCodePayload->redirect_uri) instead of empty($authCodePayload->redirect_uri) === false ?

https://github.com/thephpleague/oauth2-server/blob/0c2f32cd766861f3add4b95c49c5fcef3427e133/src/Grant/AuthCodeGrant.php#L214

Comparing that to false simply negates the check for nonempty. Now, when I have redirect_uri stored in payload and not supplied in request, validation fails, which, in my opinion, is a bug. Almost looks like some unfinished refactoring.

I can send PR, not sure if its not on purpose though..

[Sephster]

So it looks like we are only throwing an error if the auth code payload's redirect URI is set and no redirect URI has been passed in the request. There are situations where you don't need to specify a redirect uri so this check makes sense as in some cases, it is valid for it to be empty. Hope that makes sense but don't think there is anything that needs changing here. Thanks for raising this for discussion though

[KminekMatej]

But that actually makes $authCodePayload->redirect_uri meaningless, doesnt it?

[Sephster]

I'm not sure what you mean. It is an optional field but still important

[KminekMatej]

I misundestood the flow - thought you can just specify redirect_uri in the auth code and keep it empty in request, but that violates the RFC, where it clearly states that if the auth code redirect_uri has been filled, request redirect_uri must be filled as well and they must be equal.

So this issue was just my misunderstanding, sorry about that and thanks for a good work