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

Add support for OAuth2 state parameter #63

Merged
merged 1 commit into from Dec 10, 2013

Conversation

@stanhu
Copy link
Contributor

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

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link
Contributor Author

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

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link
Contributor Author

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

This comment has been minimized.

Copy link
Owner

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

Merge pull request #63 from stanhu/master
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

1 check passed

default The Travis CI build passed
Details
@lepture

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link
Contributor Author

commented Dec 10, 2013

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.