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 client authentication for authorization-code grant type #86

Closed
wants to merge 1 commit into from
Closed

Conversation

stianpr
Copy link
Contributor

@stianpr stianpr commented Mar 21, 2014

When using a confidential client it should be possible to obtain a token through authorization code grant type with a valid code.

In some cases you want to authenticate the client when using authorization code grant type. Though as it is implemented now you to have the client_id in the body. Which is not really required when a token was obtained with client_credentials... I'm not 100% sure on how this should be, but at least authorization_code is working with non-confidential clients also with this implementation.

https://github.com/idan/oauthlib/blob/master/oauthlib/oauth2/rfc6749/request_validator.py#L22

@lepture
Copy link
Owner

lepture commented Mar 21, 2014

Thanks for this patch. I am not sure whether this is the right way. I'll look into the documentation in another day.

But firstly, could you send me a test case for this issue?

@stianpr
Copy link
Contributor Author

stianpr commented Mar 22, 2014

Sorry for the missing tests! I have added them now =)

@stianpr
Copy link
Contributor Author

stianpr commented Mar 25, 2014

Any progress? =)

@lepture
Copy link
Owner

lepture commented Mar 25, 2014

@stianpr I am sorry. I am busy these days for my project. Do you need it badly right now?

@stianpr
Copy link
Contributor Author

stianpr commented Mar 25, 2014

@lepture Well we need it within the week, so in a couple of days would be great!

@lepture
Copy link
Owner

lepture commented Mar 25, 2014

@stianpr Your test cases are useless. You test cases will pass even without the changes.

@stianpr
Copy link
Contributor Author

stianpr commented Mar 26, 2014

Sorry for creating some stupid test... The test are now correct, but in order to have it working I had to add some code in authenticate_client(). I'm not sure how this should be fixed but I have tried to explain the situation in a comment above the code and with some suggestions.

Hmm the last commit should probably have been split into two commits for the code in client_authentication_required() and authenticate_client(). I'm not 100% sure if the code in client_authentication_required() should be in that way...

I will clean up things after we agree on how it should be working. And please let me know if anything is unclear!

Thank you!

@stianpr
Copy link
Contributor Author

stianpr commented Apr 3, 2014

Okay. I removed the comment from the previous code and actually implemented the code that needed to be there in order for the whole thing to work. Hopefully it's even more clear now!

"""
auth_required = ('password', 'authorization_code', 'refresh_token')
return 'Authorization' in request.headers and\
request.grant_type in auth_required
Copy link
Owner

Choose a reason for hiding this comment

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

I think password grant type will always need to be verified with Authorization header.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah so you want it to be stricter?

Copy link
Owner

Choose a reason for hiding this comment

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

I think it should be:

if request.grant_type == 'password':
    return True
return 'Authorization' in ....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All right.

When using a confidential client it should be possible to obtain
a token through authorization code grant type with a valid code.

This makes `OAuth2RequestValidator.validate_code()` and
`OAuth2RequestValidator.confirm_redirect_uri)` support retrieving
client from either request.client or request.client_id.
@lepture
Copy link
Owner

lepture commented Apr 5, 2014

Merged at 2d711e9

@lepture lepture closed this Apr 5, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants