x1ddos / simpleauth

Simple authentication for Python on Google App Engine supporting OAuth 2.0, OAuth 1.0(a) and OpenID
https://simpleauth.appspot.com
327 stars 61 forks source link

Support multiple oauth 'scopes'. #3

Closed erichiggins closed 12 years ago

erichiggins commented 12 years ago

Google OAuth separates the users' email address from the rest of their profile information. In order to get it, you must request both scopes. https://developers.google.com/accounts/docs/OAuth2Login#scopeparameter

Currently, simpleauth expects the scope to be a string/single value. It should allow a tuple or list then add all defined scopes.

Some refactoring may be required, since simpleauth sends a dictionary mapping of parameters to urllib.urlencode. A list of tuples is also valid for the 'query' parameter, and would be more appropriate in this case. http://docs.python.org/library/urllib.html#urllib.urlencode

erichiggins commented 12 years ago

Actually, I just read that the scopes should be space-separated (Google did a poor job of documenting this, in my opinion) so simpleauth supports this just fine. I'm not sure about other providers though, so the issue may still be relevant. If not, feel free to close and ignore :)

x1ddos commented 12 years ago

A side note: there I was, checking a few days ago whether my google code issue tracker had email notifications enabled. Turns out I had github's disabled too.

So, yeah. Google's implementation requires scopes to be space-separated. Facebook separates them by comma, e.g. "user_about_me, email". Don't remember about others.

The fact that they are using different delimiter pushed me to use scope as is, whatever is provided by _get_consumer_info_for(). I didn't really like having scope to be supplied as a string but that seemed like a fast and easy implementation.

I'll keep this issue open though, for some future thoughts.

erichiggins commented 12 years ago

I think that it'd be fine to simply add that in the comments where people are sure to see it, and possibly point them to the appropriate docs for each auth mechanism. Less code to deal with on your end, but consumers of your code will be informed.

x1ddos commented 12 years ago

Partially added in 56a5576f1f60712f941bbeee3cbdc45b530ca857 but I think I could add more info so, leaving it open for now.

x1ddos commented 12 years ago

closed by e9d74bc05e2545f2b6e0bc9579e4bf88df87ccf3 but please reopen if/when more info is needed.