When an invalid redirect_uri is detected, the framework is still redirecting to the invalid uri with the error message in the url. The RFC states:
Invalid Endpoint
If an authorization request fails validation due to a missing,
invalid, or mismatching redirection URI, the authorization server
SHOULD inform the resource owner of the error and MUST NOT
automatically redirect the user-agent to the invalid redirection URI.
I think it's kind of weird that the framework wants you to opt in / define a compliant redirect_uri check yourself (one that compares the scheme, authority and path), when the RFC states that the redirect_uri's must be validated against a whitelist when dealing with public clients or when using implicit grant, and nevertheless should be validated against a whitelist for everyone. Given most oauth providers are more often than not dealing with public clients and the fairly strong recommendation to perform validation in any case, why not make it an opt out feature? I may not even have noticed it if I didn't go out of my way to test for it. Some other devs may not even test for it and just assume the framework is implementing standard oauth behaviour.
I believe the callback should not be limited to the pair of redirect_uri's for comparison. What if I want to register multiple uri's? In the context of the callback, I have no way of accessing the already retrieved Client model - which is likely where I would store the multiple uri's. I am willing to make a PR to change the two places where this is called:
if err := m.validateURI(cli.GetDomain(), tgr.RedirectURI); err != nil {
would change to
if err := m.validateURI(cli, tgr.RedirectURI); err != nil {
3 issues relating to the redirect_uri:
I think it's kind of weird that the framework wants you to opt in / define a compliant redirect_uri check yourself (one that compares the scheme, authority and path), when the RFC states that the redirect_uri's must be validated against a whitelist when dealing with public clients or when using implicit grant, and nevertheless should be validated against a whitelist for everyone. Given most oauth providers are more often than not dealing with public clients and the fairly strong recommendation to perform validation in any case, why not make it an opt out feature? I may not even have noticed it if I didn't go out of my way to test for it. Some other devs may not even test for it and just assume the framework is implementing standard oauth behaviour.
I believe the callback should not be limited to the pair of redirect_uri's for comparison. What if I want to register multiple uri's? In the context of the callback, I have no way of accessing the already retrieved Client model - which is likely where I would store the multiple uri's. I am willing to make a PR to change the two places where this is called:
if err := m.validateURI(cli.GetDomain(), tgr.RedirectURI); err != nil {would change to
if err := m.validateURI(cli, tgr.RedirectURI); err != nil {Thanks