flavors/django-graphql-social-auth

SocialAuthMutation crashes when the auth pipeline returns a redirect

PiDelport opened this issue · 3 comments

Problem

SocialAuthMutation currently assumes that the social auth pipeline will either return a user instance or None.

However, this fails rather ungracefully when a partial pipeline step (PSA docs) returns an HTTP redirect instead of a user instance:

Traceback (most recent call last):
  File "…/site-packages/graphql/execution/executor.py", line 447, in resolve_or_error
    return executor.execute(resolve_fn, source, info, **args)
  File "…/site-packages/graphql/execution/executors/sync.py", line 16, in execute
    return fn(*args, **kwargs)
  File "…/site-packages/graphql_social_auth/decorators.py", line 28, in wrapper
    login(info.context, user)
  File "…/site-packages/django/contrib/auth/__init__.py", line 155, in login
    request.session[SESSION_KEY] = user._meta.pk.value_to_string(user)
graphql.error.located_error.GraphQLLocatedError: 'HttpResponseRedirect' object has no attribute '_meta'

The relevant code is in psa decorator (code link).

Approach

I'm not sure what the best approach is to deal with this, since there's probably not a universally useful way to handle HTTP redirects in this context, but checking for this and raising a better exception is probably the minimum that could happen, and having some supported and documented way for user code to customise and handle this in some application-specific way would probably be ideal.

I'm currently working around this with the following small local hack (modification marked with comment):

class GraphQLSocialAuthResultIsNotUser(exceptions.GraphQLSocialAuthError):
    """
    The social auth pipeline result is not a user object.
    """

    def __init__(self, result):
        super().__init__(result)
        self.result = result


def psa(f):
    @wraps(f)
    def wrapper(cls, root, info, provider, access_token, **kwargs):
        strategy = load_strategy(info.context)
        try:
            backend = load_backend(strategy, provider, redirect_uri=None)
        except MissingBackend:
            raise exceptions.GraphQLSocialAuthError(_('Provider not found'))

        user = backend.do_auth(access_token)

        if user is None:
            raise exceptions.GraphQLSocialAuthError(_('Invalid token'))

        # BEGIN local modification
        # Raise non-user result values as GraphQLSocialAuthResultIsNotUser.
        # (Type-checking code referenced from `social_core.actions.do_complete` for consistency.)
        user_model = strategy.storage.user.user_model()
        if user and not isinstance(user, user_model):
            raise GraphQLSocialAuthResultIsNotUser(user)
        # END local modification

        if not issubclass(cls, mixins.JSONWebTokenMixin):
            login(info.context, user)

        social = user.social_auth.get(provider=provider)

        return f(cls, root, info, social, **kwargs)
    return wrapper

My application then uses a SocialAuth subclass to extend mutate to catch GraphQLSocialAuthResultIsNotUser and handle the redirect by returning it as part of the mutation result.

Would something like the above approach be workable for including into the library? It lets applications catch and handle GraphQLSocialAuthResultIsNotUser in their own appropriate way, while for applications that don't the behaviour won't be different than before, except that the raised exception should be clearer and easier to understand than the current AttributeError.

@pjdelport Hi and thanks!,
I understand the problem and I think your approach is right.
I would prefer to use the generic exception, see the PR #8.

done!, pip install django-graphql-social-auth >= 0.1.2