Skip to content
New issue

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

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

Already on GitHub? # to your account

Add support for OAuth2 state parameter #63

Merged
merged 1 commit into from
Dec 10, 2013
Merged

Conversation

stanhu
Copy link
Contributor

@stanhu stanhu commented Dec 9, 2013

The state parameter is useful for relaying application-specific parameters to the code. It's hard-coded to None right now, but it is useful to have this option in the authorize callback since the value can change.

For more details:

https://developers.google.com/accounts/docs/OAuth2InstalledApp

lepture added a commit that referenced this pull request Dec 10, 2013
@lepture
Copy link
Owner

lepture commented Dec 10, 2013

@stanhu Thanks for your suggestion. But I'd like to implement it the other way.

In fact, that you can set a state in request_token_params, but if you want to generate a random state, here is the solution now:

{
  'request_token_params': {
    'state': generate_state,
  }
}

@lepture lepture closed this Dec 10, 2013
@stanhu
Copy link
Contributor Author

stanhu commented Dec 10, 2013

I considered that approach, but I didn't like it because it would require changing the remote app's request_token_params each time the callback needed to be initiated and also caused the OAuth2 client to retain information it shouldn't need to keep. For example, what if storing the CSRF token in memory on the OAuth2 is a security hole?

@lepture lepture reopened this Dec 10, 2013
@lepture
Copy link
Owner

lepture commented Dec 10, 2013

  1. the state can be a string:
{
  'request_token_params': {
    'state': 'a string',
  }
}
  1. it can be a function. a function that can do anything properly, not only generate a random string.

@stanhu
Copy link
Contributor Author

stanhu commented Dec 10, 2013

Yes, I realize it can be a string or a function, but I think my use case is a bit different. Let's say, for example, a CSRF token comes from an outside HTTP request which is simply forwarded along by the OAuth2 client. In this case, the OAuth2 client is not generating the token or have any control what it is. With this implementation, the client is forced to store data that it should not need in-memory. Additionally, this state value can change quite often, so it may introduce other bugs for clients using the same OAuth2 app.

@lepture
Copy link
Owner

lepture commented Dec 10, 2013

@stanhu I am considering it, coz the change of API.

lepture added a commit that referenced this pull request Dec 10, 2013
Add support for OAuth2 state parameter
lepture added a commit that referenced this pull request Dec 10, 2013
@lepture lepture merged commit d2a496c into lepture:master Dec 10, 2013
@lepture
Copy link
Owner

lepture commented Dec 10, 2013

@stanhu Merged now. I will release it some day. Maybe I should release it in version 0.5 rather than 0.4 since the API changes.

@stanhu
Copy link
Contributor Author

stanhu commented Dec 10, 2013

Thanks!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants