Notes on making OAuth work with Google
Marc Hedlund
Thanks for Tornado. I like it a lot.
I just worked through setting up Tornado to pull data from Google Analytics using OAuth (*not* OpenID / OAuth). It was a little hairy. Here are my notes. I've sent in one pull request with a fix (noted below) and will follow up with more pull requests unless someone says "no thanks."
Note that the last change I mention below is a security-related change (preventing the session fixation attack described here: http://oauth.net/advisories/2009-1/). That said, I can hardly get all worked up about this as a vulnerability since it looks like nearly every OAuth library and provider other than Google is still vulnerable to the attack. Still, that change at least is probably worth making sometime soon.
My notes:
* tornado.auth.GoogleMixin covers OpenID / OAuth, a.k.a. Federated Login, but not other auth methods Google supports (two-legged or three-legged OAuth, AuthSub, ClientLogin). While I doubt all of those are needed, I have and can see other people wanting to use GData APIs without using Federated Login. Assuming you were to add support for three- and/or two-legged OAuth, the name "GoogleMixin" would be a little confusing. I'd suggest renaming this GoogleFederatedMixin to make it easier to add other auth methods as separate mixins.
* Google OAuth requires that you send a scope parameter with the initial token request. The OAuth spec lets any provider ask for additional parameters (see http://oauth.net/core/1.0a/#auth_step1, section 6.1.1, "Additional parameters.") I didn't find an easy way to add this, so I added an optional extra_params argument to tornado.auth.OAuthMixin.authorize_redirect. I sent in a pull request for the change (http://github.com/precipice/tornado/commit/95c5b53f5dcb31049c974ce677c2222c2a165771).
* While I was trying to get things going, I had some failures and noticed that this doc comment on authorize_redirect is wrong:
This method sets a cookie called _oauth_request_token which is
subsequently used (and cleared) in get_authenticated_user for
security purposes.
The cookie is set but isn't cleared. I added some clear_cookie calls to remove it when the final access token is received, and in the various error handlers along the way.
* I got hell-of-confused by OAuthMixin._oauth_get_user and _on_oauth_get_user. I get that these are used in the FriendFeed and Twitter mixins, but they don't really have anything to do with OAuth and I'd suggest leaving them in the subclasses but not in the abstract class. If you have no need for a get_user step they're just confusing and unecessary (why is a private method complaining that it wasn't implemented?). If anyone else hits this, here's a stub method that satisfies the call chain:
def _oauth_get_user(self, access_token, callback):
temp_user = {"username":"temp_user"}
callback(temp_user)
return
You'd then look for user["access_token"] in your _on_auth callback (as shown in the doc comments at http://github.com/facebook/tornado/blob/master/tornado/auth.py#L33).
* Once I got through the above, I noticed that Google was showing a security warning on the page they show a user to confirm my access to their data: "This website is
registered with Google to make authorization requests, but has not been configured to send requests securely." I didn't want that security warning to be there. In the AuthSub world you get rid of this message by using RSA-SHA1 to sign requests, so I naively figured I had to do that with OAuth, too. Setting up the OAuth mixin to use RSA-SHA1 (instead of HMAC-SHA1) was kind of a pain, since the base string and signing methods are combined. Here's a monkeypatch that will let you do it, though:
def _oauth_rsa_signature(consumer_token, method, url, parameters={}, token=None):
parameters['oauth_signature_method'] = 'RSA-SHA1'
parts = urlparse.urlparse(url)
scheme, netloc, path = parts[:3]
normalized_url = scheme.lower() + "://" + netloc.lower() + path
base_elems = []
base_elems.append(method.upper())
base_elems.append(normalized_url)
base_elems.append("&".join("%s=%s" % (k, tornado.auth._oauth_escape(str(v)))
for k, v in sorted(parameters.items())))
base_string = "&".join(tornado.auth._oauth_escape(e) for e in base_elems)
keytxt = open(os.path.join(os.path.dirname(__file__), "keys", "oauthkey.pem")).read()
privatekey = keyfactory.parsePrivateKey(keytxt)
signature = privatekey.hashAndSign(base_string)
return base64.b64encode(signature)
Put that in your tornado file (top-level def, not in a class) and then add the following at the top of your get() handler for the OAuth process:
def get(self):
# Monkeypatch the default signature method so we can use RSA-SHA1 instead.
tornado.auth._oauth_signature = _oauth_rsa_signature
Then create a directory called 'keys' in your tornado directory and put your RSA key in there as 'oauthkey.pem'. It would take a bit to unwind the HMAC-SHA1 pieces of the OAuthMixin and let you use either method, but I'd be happy to do that if the Tornado folks want to support that.
* But: alas, RSA-SHA1 did not solve the security warning. I did some more digging and found this: http://groups.google.com/group/google-accounts-api/msg/2316216053d01d1c
Essentially what this says is that Google is taking the OAuth session fixation vulnerability (http://oauth.net/advisories/2009-1/) seriously and is warning users with a message about security if the OAuth consumer uses OAuth 1.0 (which is vulnerable) instead of 1.0a (which isn't known to be vulnerable). Tornado's OAuthMixin uses OAuth 1.0. So the way to get rid of the security warning is to upgrade to OAuth 1.0a, which fortunately isn't that hard and can be done so it is backwards-compatible with 1.0.
The changes needed are:
- send oauth_callback with the initial token request
- when receiving a message at the oauth_callback URL, record the oauth_verifier query parameter
- when upgrading to an access token, send the oauth_verifier as a component of the base string
Here are the methods I used (these were just in my subclass) to do that. Note that they depend on my extra_params change (bullet two, above).
def get_authenticated_user(self, callback):
request_key = self.get_argument("oauth_token")
oauth_verifier = self.get_argument("oauth_verifier")
request_cookie = self.get_cookie("_oauth_request_token")
if not request_cookie:
logging.warning("Missing OAuth request token cookie")
callback(None)
return
self.clear_cookie("_oauth_request_token")
cookie_key, cookie_secret = request_cookie.split("|")
if cookie_key != request_key:
logging.warning("Request token does not match cookie")
callback(None)
return
token = dict(key=cookie_key, secret=cookie_secret, verifier=oauth_verifier)
http = tornado.httpclient.AsyncHTTPClient()
http.fetch(self._oauth_access_token_url(token), self.async_callback(
self._on_access_token, callback))
def _oauth_access_token_url(self, request_token):
consumer_token = self._oauth_consumer_token()
url = self._OAUTH_ACCESS_TOKEN_URL
args = dict(
oauth_consumer_key=consumer_token["key"],
oauth_token=request_token["key"],
oauth_verifier=request_token["verifier"],
oauth_signature_method="HMAC-SHA1",
oauth_timestamp=str(int(time.time())),
oauth_nonce=binascii.b2a_hex(uuid.uuid4().bytes),
oauth_version="1.0a",
)
signature = _oauth_signature(consumer_token, "GET", url, args, request_token)
args["oauth_signature"] = signature
return url + "?" + urllib.urlencode(args)
With those in place the security warning went away, and everything worked.
Hope this is helpful to someone else. Like I say, I'll work on more pull requests for the suggestions above unless I hear otherwise. Thanks again for a great tool.
-Marc Hedlund
Elias Torres
Thanks for your detailed post. This is exactly what I was trying to do
tonight. I figured I'd better ask for an update on your patches before
trying to do this myself. Would you mind updating us on your final
solution and if there are any patches still left to apply to Tornado
that could be helpful to everyone?
-Elias
Marc Hedlund
I'm really glad the notes were helpful.
I said I would keep working on patches unless someone said "no
thanks," but instead I never heard anything back. I should have kept
going on my patches but since (at the time) my experience was the same
as some others who had sent pull requests, I didn't keep working on
it. Also, since my changes would have involved a pretty big revision
to tornado.auth, I wasn't sure it was worth the effort unless others
agreed the changes were right (upgrading to OAuth 1.0a, and removing
the _oauth_get_user and _on_oauth_get_user methods from the OAuth base
class).
Like I say, ideally I would have just kept going on it and at least
advertised that my patches existed, but I didn't do that due to other
pressing deadlines.
If people are interested, or even just if you are, I'd be happy to
pick it up again. It seems like more pull requests are getting
reviewed these days, which is great.
-M
On Apr 4, 6:03 pm, Elias Torres <el...@torrez.us> wrote:
> Marc,
>
> Thanks for your detailed post. This is exactly what I was trying to do
> tonight. I figured I'd better ask for an update on your patches before
> trying to do this myself. Would you mind updating us on your final
> solution and if there are any patches still left to apply to Tornado
> that could be helpful to everyone?
>
> -Elias
>
> On Sat, Jan 23, 2010 at 7:44 PM, Marc Hedlund <marcprecip...@gmail.com> wrote:
> > Hey,
>
> > Thanks for Tornado. I like it a lot.
>
> > I just worked through setting up Tornado to pull data from Google Analytics using OAuth (*not* OpenID / OAuth). It was a little hairy. Here are my notes. I've sent in one pull request with a fix (noted below) and will follow up with more pull requests unless someone says "no thanks."
>
> > Note that the last change I mention below is a security-related change (preventing the session fixation attack described here:http://oauth.net/advisories/2009-1/). That said, I can hardly get all worked up about this as a vulnerability since it looks like nearly every OAuth library and provider other than Google is still vulnerable to the attack. Still, that change at least is probably worth making sometime soon.
>
> > My notes:
>
> > * tornado.auth.GoogleMixin covers OpenID / OAuth, a.k.a. Federated Login, but not other auth methods Google supports (two-legged or three-legged OAuth, AuthSub, ClientLogin). While I doubt all of those are needed, I have and can see other people wanting to use GData APIs without using Federated Login. Assuming you were to add support for three- and/or two-legged OAuth, the name "GoogleMixin" would be a little confusing. I'd suggest renaming this GoogleFederatedMixin to make it easier to add other auth methods as separate mixins.
>
> > * Google OAuth requires that you send a scope parameter with the initial token request. The OAuth spec lets any provider ask for additional parameters (seehttp://oauth.net/core/1.0a/#auth_step1, section 6.1.1, "Additional parameters.") I didn't find an easy way to add this, so I added an optional extra_params argument to tornado.auth.OAuthMixin.authorize_redirect. I sent in a pull request for the change (http://github.com/precipice/tornado/commit/95c5b53f5dcb31049c974ce677...).
>
> > * While I was trying to get things going, I had some failures and noticed that this doc comment on authorize_redirect is wrong:
>
> > This method sets a cookie called _oauth_request_token which is
> > subsequently used (and cleared) in get_authenticated_user for
> > security purposes.
>
> > The cookie is set but isn't cleared. I added some clear_cookie calls to remove it when the final access token is received, and in the various error handlers along the way.
>
> > * I got hell-of-confused by OAuthMixin._oauth_get_user and _on_oauth_get_user. I get that these are used in the FriendFeed and Twitter mixins, but they don't really have anything to do with OAuth and I'd suggest leaving them in the subclasses but not in the abstract class. If you have no need for a get_user step they're just confusing and unecessary (why is a private method complaining that it wasn't implemented?). If anyone else hits this, here's a stub method that satisfies the call chain:
>
> > def _oauth_get_user(self, access_token, callback):
> > temp_user = {"username":"temp_user"}
> > callback(temp_user)
> > return
>
> > You'd then look for user["access_token"] in your _on_auth callback (as shown in the doc comments athttp://github.com/facebook/tornado/blob/master/tornado/auth.py#L33).
Elias Torres
> Hi, Elias,
>
> I'm really glad the notes were helpful.
>
> I said I would keep working on patches unless someone said "no
> thanks," but instead I never heard anything back. I should have kept
> going on my patches but since (at the time) my experience was the same
> as some others who had sent pull requests, I didn't keep working on
> it. Also, since my changes would have involved a pretty big revision
> to tornado.auth, I wasn't sure it was worth the effort unless others
> agreed the changes were right (upgrading to OAuth 1.0a, and removing
> the _oauth_get_user and _on_oauth_get_user methods from the OAuth base
> class).
>
> Like I say, ideally I would have just kept going on it and at least
> advertised that my patches existed, but I didn't do that due to other
> pressing deadlines.
>
> If people are interested, or even just if you are, I'd be happy to
> pick it up again. It seems like more pull requests are getting
> reviewed these days, which is great.
Yes, Ben Darnell is definitely on the ball with external patches. If
you had something coded against master/head, I'd be willing to be the
guinea pig. Since I too would like to embed Google Analytics reports
within my dashboard and would love to avoid having to store passwords
in settings.
-Elias
> --
> To unsubscribe, reply using "remove me" as the subject.
>
Marc Hedlund
-M
On Apr 4, 7:49 pm, Elias Torres <el...@torrez.us> wrote:
> ...
>
> read more »
Joe Bowman
main branch? I'd actually moved to giving node.js a try, but with the
fact I'd have to rewrite all the auth stuff tornado actually provides,
I'd probably come back to tornado if this got added. Basically, I have
my app probably 80% done in tornado, and then ran into issues with
authentication against sources like Google's Youtube.
On Apr 4, 11:42 pm, Marc Hedlund <marcprecip...@gmail.com> wrote:
> Okay, I'll start sending patches again.
>
> -M
>
> On Apr 4, 7:49 pm, Elias Torres <el...@torrez.us> wrote:
>
>
>
> > On Sun, Apr 4, 2010 at 9:32 PM, Marc Hedlund <marcprecip...@gmail.com> wrote:
> > > Hi, Elias,
>
> > > I'm really glad the notes were helpful.
>
> > > I said I would keep working on patches unless someone said "no
> > > thanks," but instead I never heard anything back. I should have kept
> > > going on my patches but since (at the time) my experience was the same
> > > as some others who had sent pull requests, I didn't keep working on
> > > it. Also, since my changes would have involved a pretty big revision
> > > to tornado.auth, I wasn't sure it was worth the effort unless others
>
> > >> > Note that the last change I mention below is a security-related change (preventing the session fixation attack described here:http://oauth.net/advisories/2009-1/). That said, I can hardly get all worked up about this as a vulnerability since it looks like nearly everyOAuthlibrary and provider other than Google is still vulnerable to the attack. Still, that change at least is probably worth making sometime soon.
>
> > >> > My notes:
>
> > >> > * tornado.auth.GoogleMixin covers OpenID /OAuth, a.k.a. Federated Login, but not other auth methods Google supports (two-legged or three-leggedOAuth, AuthSub, ClientLogin). While I doubt all of those are needed, I have and can see other people wanting to use GData APIs without using Federated Login. Assuming you were to add support for three- and/or two-leggedOAuth, the name "GoogleMixin" would be a little confusing. I'd suggest renaming this GoogleFederatedMixin to make it easier to add other auth methods as separate mixins.
>
> > >> > * GoogleOAuthrequires that you send a scope parameter with the initial token request. TheOAuthspec lets any provider ask for additional parameters (seehttp://oauth.net/core/1.0a/#auth_step1, section 6.1.1, "Additional parameters.") I didn't find an easy way to add this, so I added an optional extra_params argument to tornado.auth.OAuthMixin.authorize_redirect. I sent in a pull request for the change (http://github.com/precipice/tornado/commit/95c5b53f5dcb31049c974ce677...).
> > >> > * While I was trying to get things going, I had some failures and noticed that this doc comment on authorize_redirect is wrong:
>
> > >> > This method sets a cookie called _oauth_request_token which is
> > >> > subsequently used (and cleared) in get_authenticated_user for
> > >> > security purposes.
>
> > >> > The cookie is set but isn't cleared. I added some clear_cookie calls to remove it when the final access token is received, and in the various error handlers along the way.
>
> > >> > def _oauth_get_user(self, access_token, callback):
> > >> > temp_user = {"username":"temp_user"}
> > >> > callback(temp_user)
> > >> > return
>
> > >> > You'd then look for user["access_token"] in your _on_auth callback (as shown in the doc comments athttp://github.com/facebook/tornado/blob/master/tornado/auth.py#L33).
>
> > >> > * Once I got through the above, I noticed that Google was showing a security warning on the page they show a user to confirm my access to their data: "This website is
> > >> > def _oauth_rsa_signature(consumer_token, method, url, parameters={}, token=None):
> > >> > parameters['oauth_signature_method'] = 'RSA-SHA1'
> > >> > parts = urlparse.urlparse(url)
> > >> > scheme, netloc, path = parts[:3]
> > >> > normalized_url = scheme.lower() + "://" + netloc.lower() + path
>
> > >> > base_elems = []
> > >> > base_elems.append(method.upper())
> > >> > base_elems.append(normalized_url)
> > >> > base_elems.append("&".join("%s=%s" % (k, tornado.auth._oauth_escape(str(v)))
> > >> > for k, v in sorted(parameters.items())))
> > >> > base_string = "&".join(tornado.auth._oauth_escape(e) for e in base_elems)
>
> > >> > keytxt = open(os.path.join(os.path.dirname(__file__), "keys", "oauthkey.pem")).read()
> > >> > privatekey = keyfactory.parsePrivateKey(keytxt)
> > >> > signature = privatekey.hashAndSign(base_string)
> > >> > return base64.b64encode(signature)
>
> > >> > Put that in your tornado file (top-level def, not in a class) and then add the following at the top of your get() handler for theOAuthprocess:
>
> > >> > def get(self):
> > >> > # Monkeypatch the default signature method so we can use RSA-SHA1 instead.
> > >> > tornado.auth._oauth_signature = _oauth_rsa_signature
>
> > >> > Then create a directory called 'keys' in your tornado directory and put your RSA key in there as 'oauthkey.pem'. It would take a bit to unwind the HMAC-SHA1 pieces of the OAuthMixin and let you use either method, but I'd be happy to do that if the Tornado folks want to support that.
>
> > >> > * But: alas, RSA-SHA1 did not solve the security warning. I did some more digging and found this:http://groups.google.com/group/google-accounts-api/msg/2316216053d01d1c
>
> > >> > The changes needed are:
> > >> > - send oauth_callback with the initial token request
> > >> > - when receiving a message at the oauth_callback URL, record the oauth_verifier query parameter
> > >> > - when upgrading to an access token, send the oauth_verifier as a component of the base string
>
> > >> > Here are the methods I used (these were just in my subclass) to do that. Note that they depend on my extra_params change (bullet two, above).
>
> > >> > def get_authenticated_user(self, callback):
> > >> > request_key = self.get_argument("oauth_token")
> > >> > oauth_verifier = self.get_argument("oauth_verifier")
> > >> > request_cookie = self.get_cookie("_oauth_request_token")
> > >> > if not request_cookie:
> > >> > return
> > >> > self.clear_cookie("_oauth_request_token")
> > >> > cookie_key, cookie_secret = request_cookie.split("|")
> > >> > if cookie_key != request_key:
> > >> > logging.warning("Request token does not match cookie")
> > >> > callback(None)
> > >> > return
> > >> > token = dict(key=cookie_key, secret=cookie_secret, verifier=oauth_verifier)
> > >> > http = tornado.httpclient.AsyncHTTPClient()
> > >> > http.fetch(self._oauth_access_token_url(token), self.async_callback(
> > >> > self._on_access_token, callback))
>
> > >> > def _oauth_access_token_url(self, request_token):
> > >> > consumer_token = self._oauth_consumer_token()
> > >> >
>
>
> read more »
Marc Hedlund
does). :)
I sent one very small pull request but I haven't gotten back to this
yet. It will probably be a week or so until I can.
If someone else is interested I can send code snippets showing how I
implemented what I did, although I think a few of them require some
work before they live up to the surrounding code.
-M
>
> read more »
Joe Bowman
maybe after I wrap that up I could look at your snippets. Not sure
when that would be though. Making some pretty big changes to code I
haven't looked at in months.
>
> read more »
Joe Bowman
>
> read more »
Elias Torres
-Elias
Joe Bowman
greener on the other side and there's no precanned javascript
implementations of oauth 1.0a on the way, and with more providers
moving to oauth 2.0, I think I'm better off finishing with Tornado. I
prefer it's framework, templates, and I can hack back in my mongodb
sessions or just continue to use the memcached solution I already
wrote.
I need to get it caught up with the current tornado branch, but I
already have a branch I'm working with on github - http://github.com/joerussbowman/tornado.
I'll probably refork, do my changes, and then work from there and
offer pull requests to the main branch. Marc, I'll try to get this
done in the next few days, if you're interested in pushing your
patches into that branch I can give you write access.
>
> read more »
Marc Hedlund
only three things that require more work, from my initial list:
1. Adding a clean OAuth 1.0a implementation that falls back to 1.0 (I
can help explain how to do this if needed)
2. Adding a clean way for an implementer to choose between HMAC-SHA1
or RSA-SHA1
3. Pushing the OAuthMixin._oauth_get_user and _on_oauth_get_user
methods down to the FriendFeed and Twitter mixins, and removing them
from the superclass
#3 might be controversial; if anyone wants to argue for keeping things
as-is, speak up.
Joe or Elias, want to take any of the three bullets above? I can send
what I did for #1 and #2 as a starting point.
-M
On Apr 22, 10:00 am, Joe Bowman <bowman.jos...@gmail.com> wrote:
> I'll probably be back to taking a look at this too... grass isn't so
> greener on the other side and there's no precanned javascript
> implementations of oauth 1.0a on the way, and with more providers
> moving to oauth 2.0, I think I'm better off finishing with Tornado. I
> prefer it's framework, templates, and I can hack back in my mongodb
> sessions or just continue to use the memcached solution I already
> wrote.
>
> I need to get it caught up with the current tornado branch, but I
>
> read more »
--
Subscription settings: http://groups.google.com/group/python-tornado/subscribe?hl=en
Joe Bowman
I'll try again just in case.
I think I have an idea to handle 1 and 2.
I think we should not modify the OauthMixin, rather, create
OauthMixin10a. There are a few reasons for this:
- Less code complexity. There's no need for conditionals within the
mixin to determine which version should be used.
- We already know Oauth2 is coming, and have already seen that even a
minor version change can introduce breaking changes.
For #3, in the main class OAuthMixin._oauth_get_user is a stub, and if
you're using oauth for authentication you're going to need to need
something like that. I think it enforces a standard, while the actual
method needs to be written to meet the requirements of the provider.
I'm for keeping it.
However, I will make the change to the TwitterMixin to use the correct
endpoint for user validation.
I'll probably spend most of this evening working on getting my own
repo up to current tornado, and then will work on getting a pull
request for the TwitterMixin, and also a fix for allowing body content
on PUT requests.
Elias Torres
> I'm not sure if my attempted post from my mobile device worked, so
> I'll try again just in case.
>
> I think I have an idea to handle 1 and 2.
>
> I think we should not modify the OauthMixin, rather, create
> OauthMixin10a. There are a few reasons for this:
forking an auth contrib module (with unit tests possibly!) :)
>
> - Less code complexity. There's no need for conditionals within the
> mixin to determine which version should be used.
> - We already know Oauth2 is coming, and have already seen that even a
> minor version change can introduce breaking changes.
>
> For #3, in the main class OAuthMixin._oauth_get_user is a stub, and if
> you're using oauth for authentication you're going to need to need
> something like that. I think it enforces a standard, while the actual
> method needs to be written to meet the requirements of the provider.
> I'm for keeping it.
Joe Bowman
can see on this repository
http://github.com/joerussbowman/tornado
I'm going to go ahead and see what I can do about getting an
Oauth10aMixin started.
My current goals are to have authentication and data retrieval working
for Twitter, Facebook, Myspace, and Youtube. I'll be working with
more, but I believe once I can get all 4 of these working I'll stand a
better chance of having a library capable of interfacing with just
about anything. I'm still using the Facebook Connect for Facebook.
After I get Oauth 1.0a working I'll take a stab at Oauth2 if no one
beats me to it. I should note, I will be a while, I only get a couple
hours a week to work on this stuff.
On Apr 23, 10:30 am, Elias Torres <el...@torrez.us> wrote:
Joe Bowman
http://github.com/joerussbowman/tornado
It's not working yet, currently the oauth signature is failing. I'm
done, possibly for the weekend as I got family stuff to do. If anyone
else wants to pick it up, feel free.
Joe Bowman
had to make an alternate oauth10a_signature method, because the key
wasn't getting escaped correctly in oauth_signature, however since it
works for Twitter and such I'm not going to modify the existing one.
I still don't have it working for a full oauth transaction yet, so
will hold of committing until that's done.
On Apr 23, 11:43 pm, Joe Bowman <bowman.jos...@gmail.com> wrote:
> I've committed what I have towards Oauth10aMixin and YoutubeMixin athttp://github.com/joerussbowman/tornado