jgorset/fandjango

Not able to Handle expired access tokens in fandjango

ansu opened this issue · 18 comments

ansu commented

if user is not login from facebook and hit my app url, everything works fine. If user is logged out and then hit my app url, user is getting error:
Error validating access token: The session is invalid because the user logged out.

Can you tell me how to redirect user if he logged out from facebook account.

@ansu I catch facepy.graph_api.FacebookError wherever I query the facebook graph with fandjango, because there are two cases that are not handled by fandjango's middleware:

  • The user changes her password which invalidates the access token
  • The user logs out of Facebook

If one of these cases happens Fandjango still thinks the access token is valid, but actually it isnt.

So I do the following in my views:

try:
   me = fandjango_user.graph.get('me')
except FacebookError:
   return authorize_application(request)

But it would probably be better to have that somewhere in fandjango's middleware, because the middleware also uses the graph.

See also: https://developers.facebook.com/blog/post/2011/05/13/how-to--handle-expired-access-tokens/

ansu commented

@Morpho: Thanks for reply. I have added your code inside middleware and it's working fine. I think someone should add below logic into middleware and should update middleware.py file.

        # Valid signed request and user has authorized the application
        if request.facebook and request.facebook.signed_request.user.has_authorized_application:
             # if user is logged out from one tab and hit app url from other
            #tab then user will be redirect to login screen.
            graph = GraphAPI(request.facebook.signed_request.oauth_token.token)
            try:
                userinfo=graph.get('me')
            except:
                return authorize_application(
                    request = request,
                    redirect_uri = get_post_authorization_redirect_url(request)
                )

@ansu Sorry, I corrected the other post. It's fandjango's User-model I use. (fandjango.models.User)
While further looking into this issue, because I also have this error every now and then, I found out that it's usually no problem to use a facebook-app when logged out of facebook, because the extended access-token is still valid. But still, sometimes the extending of the normal access-token does not work and somehow this leads to this error.

Your code looks fine, but maybe its better to catch the exception more granular, because there can be other exceptions too which have nothing to do with invalid acces-tokens.

I would love to be able to verify whether the OAuth token is valid besides catching an exception in my application code, but making a request to the Graph API in the middleware is actually quite costly in that it would introduce an overhead of 1-2 seconds for every request.

I think we should do this, but we should probably make it configurable and I even err on the side of defaulting it to off.

ansu commented

@jgorset: Thanks for your reply, Can you tell me How can I verify OAuth token is valid or not. Even when user is logged out from one tab and trying to hit app from other tab, request.facebook.signed_request.oauth_token.has_expired return false. I agree hitting graph API in middleware, will introduce an overhead of 1-2 seconds, can you tell me any other alternative to solve my problem.

Maybe I didn't make myself clear: I think we should implement token validation in the middleware like you describe, but make it opt-in so applications that don't care whether the token is valid won't suffer from the overhead of hitting the API for every request. :-)

I'll try to get around to implementing it sometime this weekend, but I'd be happy to accept a pull request, too.

Check http://stackoverflow.com/a/6168804/511484 (Edit: and https://developers.facebook.com/blog/post/2011/05/13/how-to--handle-expired-access-tokens/ ) I think it is exactly like this.

PHP Version:

 //Check for errors 
  if ($decoded_response->error) {
  // check to see if this is an oAuth error:
    if ($decoded_response->error->type== "OAuthException") {
      // Retrieving a valid access token. 
      $dialog_url= "https://www.facebook.com/dialog/oauth?"
        . "client_id=" . $app_id 
        . "&redirect_uri=" . urlencode($my_url);
      echo("<script> top.location.href='" . $dialog_url 
      . "'</script>");

I also think checking the token every request is just too much. Nobody can accept a delay of 1-2 seconds every request.
It would be far better to wrap every call to the facebook api in a try/catch block. Maybe inside the catch part can be a callable, that can be definied in the settings so everybody can decide for themself what to do when the api request fails.

Hm! It would be neat if we could catch OAuthExceptions in FacebookMiddleware and redirect the user to authorize the application, then forward them right back where they left off.

Yes, I think so too. And maybe for functions that use the graph api, a decorator would be nice that catches the exceptions and redirect, so you dont have to wrap every graph api call inside a try/catch block.

I.e.

@facebook_redirect_on_oauth_error
@facebook_authorization_required
def my_index_view(request):
   me = facepy_opengraph.get('me')

And the decorator would be something like that:

def facebook_redirect_on_oauth_error(func):
    def dec():
        try:
            func()
        except OAuthException:
            return authorize_application()
    return dec
ansu commented

@Morpho your code looks great. by using this code, we woudn't be adding any 1-2 sec. delay and easily able to catch exception. Thanks for reply

We can solve this with decorators, too, but I was actually thinking about using process_exception:

class FacebookMiddleware:
    ...

    def process_exception(self, request, exception):
        if isinstance(exception, OAuthError):
            return authorize_application(request, request.url)

@jgorset This is even better! This is perfect! I didnt even know process_exception exist. I googled something like that before but with no luck. This is by far the easiest and best solution.

ansu commented

@jgorset : thanks for reply, I have added process_exception in middleware. but when there is oautherror, it's coming inside process_exception function but not going inside if isinstance(exception, OAuthError)

You will need to install the latest version of Facepy and reference it as GraphAPI.OAuthError:

def process_exception(self, request, exception):
    if isinstance(exception, GraphAPI.OAuthError):
        ...

The tricky bit will be redirecting users to where they left off once they've reauthorized the application. It's easy enough to redirect GET requests, but we will have to facilitate for redirecting POST, PUT, PATCH and DELETE requests as well.

We might facilitate this by implementing an intermediary view for non-idempotent requests with a form that the user may submit to change the request method.

ansu commented

def process_exception(self, request, exception):
if isinstance(exception, GraphAPI.OAuthError):

I am using latest facepy app, and above code in facebook middleware, but it's not going inside if condition, even exception type is OAuthError. Can you tell me where I am wrong or Do I have to do something else also?

I'm not sure, that seems to work fine on my box — could you verify the class of the exception?

def process_exception(self, request, exception):
    raise StandardError(exception.__class__)
ansu commented
def process_exception(self, request, exception):
    print exception
    raise StandardError(exception.__class__)
         """   if isinstance(exception, GraphAPI.OAuthError):
                 return authorize_application(request, request.url)"""

print exception: output : "Caught OAuthError while rendering: Error validating access token: The session is invalid because the user logged out."
raise StandardError(exception.class) output: <class 'django.template.base.TemplateSyntaxError'>

It appears the code that generates your exception is inside a template tag. Unfortunately, Django will catch any exceptions produced during rendering and wrap them in TemplateSyntaxError, which really isn't helpful at all. I don't think there are any workarounds, but your middleware should still work as expected if the code that generates the OAuthError is outside of a template tag (e.g. in the view).