search

thephpleague / oauth2-server

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

Possible regression with `redirect_uri`? #1239

Closed boesing closed 3 years ago

boesing commented 3 years ago

Hey there,

I am actually working on https://github.com/mezzio/mezzio-authentication-oauth2/pull/34 I've realized tests are failing since this package in v8.3.0 where the RedirectUriValidator was introduced with its RedirectUriValidator#isLoopbackUri.

As I am not that familiar with these packages, I just wanted to drop this here.

Was it ever possible to pass just a path as redirect_uri? The integration tests in the mezzio package are passing /redirect as redirect_uri and thus, parse_url wont have scheme or host.

So in case that the integration tests are just invalid, I'd be happy to close this issue as invalid. If it was ever possible to pass non-FQDN redirect_uri parameters, the RedirectUriValidator#isLoopbackUri should verify if scheme or host keys are available.

Thanks in advance for any feedback.

boesing commented 3 years ago

This PR could be a fix, even tho, it might also disallow the usage of non FQDN-URIs (e.g. path only like /redirect). https://github.com/thephpleague/oauth2-server/pull/1237

Sephster commented 3 years ago

I believe /redirect should be treated as an invalid redirect_uri. We should always be specifying some kind of scheme, whether this is https for a web address or a custom scheme for a native app.