Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support redirect_uri's with registered query param(s) #1053

Merged

Conversation

f3ndot
Copy link
Contributor

@f3ndot f3ndot commented Mar 15, 2018

This amends the URIChecker to allow authorization for requests that contain query params. The query params all must be defined in the client app's #redirect_uri and all defined params must be present. Their order does not matter.

If there is no query param registered in the app's #redirect_uri then the previous behaviour occurs: stripping the query param from the request and comparing the URLs without it.

Fixes #1050

This amends the URIChecker to allow authorization for requests that contain query params. The query params all must be defined in the client app's #redirect_uri and all defined params must be present. Their order does not matter.

If there is no query param registered in the app's #redirect_uri then the previous behaviour occurs: stripping the query param from the request and comparing the URLs without it.

Fixes doorkeeper-gem#1050
@f3ndot
Copy link
Contributor Author

f3ndot commented Mar 15, 2018

Somewhat hilariously I just ran into this issue while trying to integrate Amazon Alexa into my Doorkeeper app: they require a ?vendorId=XXXX in the redirect URI

@nbulaj nbulaj modified the milestones: 4.3.2, 5.0 Mar 16, 2018
@nbulaj nbulaj merged commit e130f8c into doorkeeper-gem:master Mar 17, 2018
@nbulaj
Copy link
Member

nbulaj commented Mar 17, 2018

Whooops, once again no changelog entry :) Can you add it please?

@f3ndot f3ndot deleted the support-redirect_uris-with-query branch March 17, 2018 15:16
f3ndot added a commit to f3ndot/doorkeeper that referenced this pull request Mar 17, 2018
@f3ndot f3ndot mentioned this pull request Mar 17, 2018
nbulaj added a commit that referenced this pull request Mar 17, 2018
nbulaj pushed a commit that referenced this pull request Mar 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants