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
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
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).
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.
>
-M
On Apr 4, 7:49 pm, Elias Torres <el...@torrez.us> wrote:
> ...
>
> read more »