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