Skip to content

Request object not given to confirm_redirect_uri#504

Merged
skion merged 2 commits intooauthlib:masterfrom
jimmy-lt:fix/504-confirm-redirect-uri-request
Apr 13, 2018
Merged

Request object not given to confirm_redirect_uri#504
skion merged 2 commits intooauthlib:masterfrom
jimmy-lt:fix/504-confirm-redirect-uri-request

Conversation

@jimmy-lt
Copy link
Copy Markdown
Contributor

It seems that the request object is never given to the confirm_redirect_uri.

$ git grep -l confirm_redirect_uri oauthlib
oauthlib/oauth2/rfc6749/grant_types/authorization_code.py
oauthlib/oauth2/rfc6749/request_validator.py
$ grep -A 1 confirm_redirect_uri oauthlib/oauth2/rfc6749/grant_types/authorization_code.py
        if not self.request_validator.confirm_redirect_uri(request.client_id, request.code,
                                                           request.redirect_uri, request.client):

This is an issue since I enclose the database session within the request object. That means that I cannot proceed with the query to fulfill the request.

@skion
Copy link
Copy Markdown
Member

skion commented Dec 12, 2017

I see no reason not to do this: It aligns the validator signature with most of the others without a breaking change. 👍

@skion skion added this to the 2.1.0 milestone Mar 18, 2018
Copy link
Copy Markdown
Member

@JonathanHuot JonathanHuot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the report.
It seems the be the only method which doesn't have request. Odd!

LGTM

@skion skion self-requested a review March 29, 2018 18:03
@skion skion merged commit d49b9f0 into oauthlib:master Apr 13, 2018
skion pushed a commit to skion/oauthlib that referenced this pull request Apr 23, 2018
@skion skion mentioned this pull request May 8, 2018
@skion skion mentioned this pull request May 21, 2018
@JonathanHuot JonathanHuot mentioned this pull request Dec 3, 2018
23 tasks
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.

3 participants