8000 `confirm_redirect_uri` does not get the default redirect URI · Issue #445 · oauthlib/oauthlib · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

confirm_redirect_uri does not get the default redirect URI #445

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.

Sign up for GitHub

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

Jump to bottom
Closed
remko opened this issue Nov 5, 2016 · 3 comments
Closed

confirm_redirect_uri does not get the default redirect URI #445

remko opened this issue Nov 5, 2016 · 3 comments
Assignees
@JonathanHuot
Labels
Bug Discussion
Milestone
3.0.0

Comments

@remko
Copy link
remko commented Nov 5, 2016

When a client doesn't request a redirect_uri in an authorization request, oauthlib calls get_default_redirect_uri, and passes this to save_authorization_code. So far, so good.

However, when the redirect is performed, confirm_redirect_uri is called with None as redirect_uri. This doesn't match the redirect_uri saved earlier. Is this intended behavior? It felt wrong for me to have to check for None myself, and repeat getting the default URI from the client datastruct again.
@sladebot
Copy link

+1

@JonathanHuot
Copy link
Member

Hi @remko, thanks for the bug report. Indeed, this part is not handled correctly.

This check should be part of the oauthlib framework.

@JonathanHuot
Copy link
Member

redirect_uri management in oauthlib for authorization code is as below:

Browser's request response_type=code:

See RFC's section:

redirect_uri
     OPTIONAL

Implementation (pseudo code):

if redirect_uri in request:
    validate_redirect_uri(request.redirect_uri)
else
    get_default_redirect_uri()

Client's request grant_type=authorization_code:

See RFC's section:

redirect_uri
     REQUIRED, if the "redirect_uri" parameter was included in the
     authorization request as described in Section 4.1.1, and their
     values MUST be identical.

Current implementation (oauthlib 2.x)

if redirect_uri in request:
    confirm_redirect_uri(request.redirect_uri)
else
    confirm_redirect_uri(None)

Proposed implementation (oauthlib 3.x)

if redirect_uri in request:
    confirm_redirect_uri(request.redirect_uri)
else
    confirm_redirect_uri(get_default_redirect_uri())

What do you think ? It will call get_default_redirect_uri() also in the grant_type=authorization_code request, but it doesn't break the RFC because at the end, the implementation will verify that their values MUST be identical.

@JonathanHuot JonathanHuot mentioned this issue Jul 30, 2018
Merged
@JonathanHuot JonathanHuot self-assigned this Aug 12, 2018
@JonathanHuot JonathanHuot added this to the 3.0.0 milestone Dec 3, 2018
@JonathanHuot JonathanHuot mentioned this issue Dec 3, 2018
Closed
23 tasks
JonathanHuot added a commit that referenced this issue Jan 8, 2019
@JonathanHuot
Fix flow of authcode/redirect_uri since #445 is merged.
332c2a4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JonathanHuot JonathanHuot

Labels
Bug Discussion
Projects
None yet
Milestone
3.0.0
Development

No branches or pull requests
3 participants
@remko @JonathanHuot @sladebot
0