thephpleague / oauth2-server

A spec compliant, secure by default PHP OAuth 2.0 Server
https://oauth2.thephpleague.com
MIT License
6.53k stars 1.12k forks source link

No redirect for some AuthCodeGrant::validateAuthorizationRequest errors #1039

Open davispuh opened 5 years ago

davispuh commented 5 years ago

Most error cases in AuthCodeGrant::validateAuthorizationRequest doesn't redirect back to client even when client_id is specified.

For example when using "Authorization code grant" and client sends query with response_type=code&client_id=existing&redirect_uri=https://... I would expect that it would get redirected back with error so client would know they're doing something wrong.

But for most errors there are no redirects, also even with response_type=invalid&client_id=existing I would want it to be redirected back.

See https://www.oauth.com/oauth2-servers/server-side-apps/possible-errors/

Sephster commented 5 years ago

There are only certain instances where we would use the redirect uri in an error state. If the client issues an invalid, missing, or mismatching redirect uri, we do not use it. Similarly if the client ID is missing or invalid, we again ignore the redirect URI. This is a security precaution.

If you know of any specific instances where you believe we are not adhering to the spec, please let me know and I will reopen this ticket. Thanks for getting in touch.

davispuh commented 5 years ago

If the client issues an invalid, missing, or mismatching redirect uri, we do not use it. Similarly if the client ID is missing or invalid, we again ignore the redirect URI. This is a security precaution.

That is correct but this library doesn't redirect in some cases even when that's not true.

If you know of any specific instances where you believe we are not adhering to the spec, please let me know and I will reopen this ticket. Thanks for getting in touch.

That's why I created this issue, because there are such cases, which can be seen by looking at code.

Examples I noticed

Maybe there's others but these 2 I just checked.

Based from that article, it doesn't follow this

If one or more parameters are invalid, such as a required value is missing, or the response_type parameter is wrong, the server will redirect to the redirect URL and include query string parameters describing the problem.

Sephster commented 5 years ago

Thanks. I will take a look at this over the weekend.

Sephster commented 5 years ago

Confirmed as an issue for at least the first scenario. Needs resolving.