Foursquare Oauth2 API

[Didip Kerabat]

https://github.com/didip/tornado_api/blob/master/foursquare.py

It's not much, hope it can be useful for you guys.

- Didip -

[Joe Bowman]

Is there a reason you didn't build it off of the Oauth2Mixin in auth.py? Is Foursquare's implementation not standard or is the tornado Mixin out of date? I know I haven't looked at it in quite a while.

[Didip Kerabat]

At first, I looked at tornado/auth.py thinking that this Foursquare API will be a simple reverse-engineering job.

And then I was surprised that both OAuth2Mixin and FacebookGraphMixin code are a bit... messy. Below are my thoughts:

1) In my opinion OAuth2Mixin._oauth_request_token_url() is unnecessary. Different OAuth provider demands different arguments for _OAUTH_ACCESS_TOKEN_URL. Each Oauth client should build the necessary arguments inside get_authenticated_user()

2) OAuth2Mixin.authorize_redirect() expects _OAUTH_AUTHORIZE_URL. Which is sensible, but Foursquare always throws error on that URL. So I had to use _OAUTH_AUTHENTICATE_URL instead.

3) Foursquare.authorize_redirect() also demands yet another custom argument: { "response_type": "code" }

Overall, It was a messy affair trying to extend Oauth2Mixin. Furthermore, FacebookGraphMixin code can be simplified greatly by not extending OAuth2Mixin. Take a look: https://github.com/didip/tornado_api/blob/master/facebook.py

Perhaps tornado/auth.py needs refactoring?

- Didip -

[Joe Bowman]

possibly, when I put that together Facebook was the only one doing oauth2 so I believe I wrote it mainly guessing at how others might implement it. 

[Joe Bowman]

and actually... I think auth.py needs a bigger refactoring now that more apps are looking at things like sockjs, socket.io and such where request handlers aren't being used for communication. I ran into just this issue last night working on chatfor.us where I'll have to figure out the best solution for sending twitter requests without using twittermixin.

[Didip Kerabat]

While we are on topic about refactoring, these are some of my thoughts about tornado/auth.py in general, let's see if the community have opinions about any of them:

1) Isn't it better if web service APIs are pulled out of tornado and into their own repo/egg?
My reasoning is that companies change their API quite frequently and sometimes on a whim (A certain social-network-with-blue-background-color does this). Tying tornado release to service API updates seems silly.
Also, it seems like there's a new social network company every month. We need to be able to add new services quickly without affecting tornado release.
For backward compatibility, tornado egg can include the tornado-api egg

2) The newly refactored auth module should be explicit about the type of API and what version (I know that the following looks like java but please bear with my contrived example here):

auth.oauth2.v2.Foursquare

3) This one is a bit more subjective, auth.py and database.py felt like blemish to an otherwise stable mini framework.

- Didip -

[Joe Bowman]

separating auth.py to it's own repo has been tossed around a few times but over all the decision has always been to keep it.

I have a need to make twitter requests outside of requests for something I am doing with sockjs so I'm working on an idea for refactoring how auth can be done. I'll put up an example in a gist when I'm done.

[Joe Bowman]

This gist covers the basic direction I'm thinking it might be worth going in

https://gist.github.com/1586593

this is functional, and is based somewhat off of suggested here back in 2010 - https://groups.google.com/forum/#!searchin/python-tornado/auth$20mixin$20request/python-tornado/JkZhoLntDu4/rxlMBTYfTA4J

The idea is an OAuthMixin could be written, extending OAuth, and then a TwitterMixin could be written extending OAuthMixin and Twitter to provide the flexibility for mixins to request handlers as well as be able to use the base class when you need to outside of a request.

I'm using the above on www.chatfor.us as of tonight for sending tweets from within tornado-sockjs connections.

[Didip Kerabat]

I've finally escaped work to read you gist. I see where you are going now about using the auth module outside RequestHandler.

I've a couple of notes here that are mostly stylistic inputs:

* Shouldn't _oauth_signature, _oauth10a_signature, _oauth_escape, and _oauth_parse_response be absorbed inside Oauth class?

   It seems unlikely for these functions to be used outside Oauth context.

* Now that there's a proper Class that wraps each API (Twitter in this case),

   maybe Twitter().twitter_request() can be split into Twitter().get_request() and .post_request()

   I think by splitting them, it will increase readability.

* The constructor probably can receive optional access_token argument.

   It's fairly common for applications to store user's access_token in the database for later use.

   The constructor can also create instance of httpclient and reuse it until the end of API call life-cycle.

* Didn't see it in the gist, I am assuming Twitter class have get_authenticated_user() to acquire the access_token?

* Oauth2 is much simpler in comparison to Oauth1a, Are you also thinking of having an abstract class of Oauth2? And similarly with OpenId?

Good stuff.

- Didip - 

[Joe Bowman]

Right now it's pretty much a hack as I'm focused on building the product, but I am tempted to take a stab at an auth2 which takes this approach for oauth, oauth2, openid, and the various mixins. I can probably work on that when my app stabilizes and I start trying to do serious beta testing (famous last words).

1) Yea I agree, in fact was thinking about that very fact this morning when I was considering the full scope of an auth2

2) I don't know I agree. In the end they are all twitter requests, just some api end points request POST and accept post_args. The rest of the logic is pretty much the same so separating the 2 just seems like more code to maintain.

3) I was thinking leave the access_token to be supplied on request, that way you can generate one Twitter object to support multiple client requests for different users. Not sure exactly why you'd want to do that, but access token just seems to be request oriented not necessary at object creation. I do agree on creating a reusable httpclient, will keep that in mind.

4) Not sure.. I was thinking authenticate and authorize would be in the final TwitterMixin, because the way oauth works the user has to be redirected to app so keeping that dependent on the request handler just makes sense. Unless I'm missing something?

Think I covered your last question at the start of this post. 

I could see if I can make the time tonight to go ahead and turn the gist into a project on github that people can follow (or contribute) when I can get to it.

[Ben Darnell]

On Wed, Jan 11, 2012 at 6:24 AM, Joe Bowman <bowman...@gmail.com> wrote:
> Right now it's pretty much a hack as I'm focused on building the product,
> but I am tempted to take a stab at an auth2 which takes this approach for
> oauth, oauth2, openid, and the various mixins. I can probably work on that
> when my app stabilizes and I start trying to do serious beta testing (famous
> last words).
>
> 1) Yea I agree, in fact was thinking about that very fact this morning when
> I was considering the full scope of an auth2

Much of twitter_request could also be pulled down to an oauth_request
method in the base class. It would be great if the base OAuth classes
were usable as-is, e.g.
twitter_client = OAuthClient(consumer_key, consumer_secret)
resp = yield gen.Task(twitter_client.oauth_fetch,
'http://api.twitter.com/blah.json', ...)
resp.rethrow()
results = json_decode(resp.body)

Service-specific subclasses or wrappers could provide convenience
methods to help construct urls and parse responses, but wouldn't be
strictly necessary.

>
> 2) I don't know I agree. In the end they are all twitter requests, just some
> api end points request POST and accept post_args. The rest of the logic is
> pretty much the same so separating the 2 just seems like more code to
> maintain.

I agree. (Does PUT get its own method, or does it share with POST
because they both take a body? What about DELETE?...) In general I'd
suggest basing the request interface on AsyncHTTPClient wherever it's
relevant (with modifications e.g. to take arguments as a dict instead
of as an encoded body string)

>
> 3) I was thinking leave the access_token to be supplied on request, that way
> you can generate one Twitter object to support multiple client requests for
> different users. Not sure exactly why you'd want to do that, but access
> token just seems to be request oriented not necessary at object creation. I
> do agree on creating a reusable httpclient, will keep that in mind.

I tend to agree, although if each of these oauth clients only has one
entry point and no state beyond saving its constructor arguments,
maybe it's useful to think of the design in terms of partial function
application rather than OO and subclassing. That would make it easier
for applications to decide which parameters they want to globally bind
and which should be per-request (although in practice since
http_client, consumer_key and consumer_secret would nearly always be
globally fixed, the class-based design is appealingly familiar).

>
> 4) Not sure.. I was thinking authenticate and authorize would be in the
> final TwitterMixin, because the way oauth works the user has to be
> redirected to app so keeping that dependent on the request handler just
> makes sense. Unless I'm missing something?

One thing I've thought about here is that instead of a TwitterMixin,
maybe there should be a BaseTwitterLoginHandler which just has an
abstract save_user() method for the application to override. This
would be nicer in the common case than the cut-n-paste blob the
current TwitterMixin uses, although maybe people need more flexibility
to integrate things into their apps.

-Ben

[Joe Bowman]

Thanks for the feedback, now I'm glad I didn't get any time to code over the long weekend. I'll take this into account and see what I can come up with when I get some time to dive into it.

[Joe Bowman]

Hey Ben,

Would you prefer I add an auth2.py to a fork of Tornado for a pull request or develop it as it's own github project which you can grab later?

[Ben Darnell]

I'd say start it as a separate repository. It's still an open
question as to whether we want this to stay a separate package (so
that it can have its own release cycle, etc) or be merged into Tornado
proper, and it's easier to start separate and merge than to start
merged and split.

-Ben