pennersr / django-allauth

Integrated set of Django applications addressing authentication, registration, account management as well as 3rd party (social) account authentication.
https://allauth.org
MIT License
9.51k stars 3.03k forks source link

Dynamically extend OAuth2 scope? #381

Closed ghost closed 9 years ago

ghost commented 11 years ago

This is a feature request, or an oversight on my part.

My application relies on Facebook login, and in accordance with FB guidelines, it starts with a minimum set of permissions. When a user triggers an action that requires additional permissions, the app should request them.

As far as I can see from the readme / google / bug reports, the OAuth scope is hard-wired in the settings.

So, I came up with this:

from allauth.socialaccount.providers.facebook.provider import FacebookProvider
from allauth.socialaccount.providers.facebook.views import FacebookOAuth2Adapter
from allauth.socialaccount.providers.oauth2.views import OAuth2LoginView
from allauth.socialaccount.models import SocialApp
from django.conf import settings
from django.http import HttpResponseRedirect
from facepy import GraphAPI
from functools import wraps
from django.contrib.auth.decorators import login_required
from django.core.exceptions import PermissionDenied

def first( l, default=None):
    'Pick the first item of a sequence'
    return l[0] if l else default

def fb_account( user):
    'Facebook account adapter for a user'

    # Do not use get here because not everyone has a FB account
    return first( user.socialaccount_set.filter( provider='facebook'))

def get_graph( user=None):
    ''' Get a GraphAPI object.
        If user is not None, authenticate as user.
    '''

    if user:
            return GraphAPI( token)

class DynamicScopeProvider( FacebookProvider):
    'Facebook provider for allauth with dynamic scope'

    def __init__( self, scope):
        self.scope = scope
        super( DynamicScopeProvider, self).__init__()

    def get_scope( self):
        return set( self.scope).union( set( self.get_default_scope()))

class DynamicScopeOAuth2Adapter( FacebookOAuth2Adapter):
    'Custom adapter for allauth to enable dynamic Facebook scopes'

    def __init__( self, scope):
        self.provider = DynamicScopeProvider( scope)

    def get_provider( self):
        return self.provider

def fb_permission_required( permissions, login_url=None):
    ''' Decorator to check whether Facebook permissions are sufficient
        to execute a view. If additional permissions are needed,
        they are requested from Facebook.

        Permissions can be either a sequence of permission names
        (e.g. ['user_email', 'user_location']), or a comma-separated
        string (e.g. 'user_likes,friends_likes').
    '''

    def get_permissions( user):
        'Get current Facebook permissions for a user'
        bogus = ('installed', ) # FB returns this but will not accept it
        res = get_graph( user).get( '/me/permissions').get( 'data')
        print res
        return [ k for k, v in res[0].items() if v and k not in bogus] if res else []

    if type( permissions) in (str, unicode):
        permissions = [ p.strip() for p in permissions.split( ',') ]

    def inner( fn):
        @login_required( login_url=login_url)
        def wrapped_view( request, *args, **kwargs):
            actual = set( get_permissions( request.user) or [])
            missing = set( permissions).difference( actual)
            if not missing:
                # All permissions in place for the view
                return fn( request, *args, **kwargs)

            # Trigger OAuth login for missing permissions
            view = OAuth2LoginView()
            view.adapter = DynamicScopeOAuth2Adapter(
                missing.union( actual))
            view.request = request
            return view.dispatch( request,  *args, **kwargs)

        return wraps(fn)(wrapped_view)
    return inner

Is there a more elegant way to achieve the same result with allauth? And if not, any chance of having dynamic OAuth scopes added?

ghost commented 11 years ago

Oops, scrubbed it too much. In get_graph there is a line missing: token = fb_account(user).socialtoken_set.get()

pennersr commented 11 years ago

Would it help if on top of:

SOCIALACCOUNT_PROVIDERS =  {'facebook':  { 'SCOPE': ['email']}}                

you could do:

SOCIALACCOUNT_PROVIDERS =  {'facebook':  { 'SCOPE': 'path.to.callable'}}                

Where callable points to a method of the form def get_scope(request) ?

Or, perhaps more simpler & cleaner -- simply add a def get_provider_config(self, request, provider) to the social account adapter ?

ghost commented 11 years ago

Hi,

Either approach seems to be missing the contextual information for which additional scope is requested. Hence the use of a decorator where the required extra permissions can be declared per view. The underlying design issue seems to be that allauth assumes global scope, and regardless of whether we use a callable or adapter hook, we need to intercept view execution, and do some OAuth ping-pong for additional permissions with a 'next' parameter pointing to the view.

Also, what happens if the user declines additional permissions? The difference to the standard login flow is that the user is already logged in, and only the view will fail. This is apparently not correctly handled by the OAuth flow, as the view is called regardless of whether the user grants or declines the extra permissions.

This is handled in the current version of the decorator hack by passing a sentinel via 'next', and redirecting to a custom error URL if the permission is not granted, e.g. like so:

@fb_permission_required( settings.FB_PUBLISH_RIGHTS, on_fail=reverse_lazy( 'permfail'))
def my_view( request):
  pass # Do something useful here

Thoughts?

-D

pennersr commented 11 years ago

I'll have to give this some more thought... but I think this is the proper approach: the /accounts/facebook/login/ (and the other OAuth2 ones) should accept a GET parameter via which scope can be passed along. These views already take a "process" GET parameter indicating how to process the login. Next to "login" (process=login) and "connect" a third option, namely "redirect", seems required.

arctelix commented 9 years ago

Just starting to tackle this issue as well. Any additional/new thoughts on the matter?

pennersr commented 9 years ago

Closed in 9c14ef7619d6cdf09a78a5024f14b7c621e94398

teolemon commented 7 years ago

Hi @pennersr , do you have a link to the relevant part of the doc on this ? (Does it exist ?) I have the scope in the settings. So unsure on how to preserve it as the default scope in that path.to.defaultscope and override it with path.to.extrascope