omab / django-social-auth

Django social authentication made simple
https://groups.google.com/group/django-social-auth
BSD 3-Clause "New" or "Revised" License
2.65k stars 756 forks source link

Bug with catching errors? #660

Closed houmie closed 11 years ago

houmie commented 11 years ago

After hours of debugging and inconsistent behaviour I figured out the problem.

My super user had the same email as the facebook account I was trying to login with. Hence the new user could not be generated with the same email address. The result was an exception here:

def complete_process(request, backend, *args, **kwargs):
    """Authentication complete process"""
    # pop redirect value before the session is trashed on login()
    redirect_value = request.session.get(REDIRECT_FIELD_NAME, '') or \
                     request.REQUEST.get(REDIRECT_FIELD_NAME, '')
    # Django 1.5 allow us to define custom User Model, so integrity errors
    # can be raised.
    #try:
    user = auth_complete(request, backend, *args, **kwargs)
    #except IntegrityError:
    #    url = setting('SIGNUP_ERROR_URL', setting('LOGIN_ERROR_URL'))
    #    return HttpResponseRedirect(url)

Only after commenting out the try except, I was able to see the problem of having there same email address twice. Despite having defined the following below, I couldn't see the thrown exception in my template, but maybe I am missing something:

Setting.py:

SIGNUP_ERROR_URL = '/signup-error/'

Url:

url(r'^signup-error/$', TemplateView.as_view(template_name="error.html"), name='signup-error'),

error.html

There was an error
{{error}}
{{message}}
eshellman commented 11 years ago

try setting DEBUG = False. middleware won't handle the exceptions if DEBUG = True. and see issue 662 with suggested fix.

gfronza commented 11 years ago

In practice, how I deal with this problem? Do I need to fork the project and catch the exception myself? Thanks

omab commented 11 years ago

@gfronza, use a middleware to process it, you can subclass the middleware defined on this app (https://github.com/omab/django-social-auth/blob/master/social_auth/middleware.py)

gfronza commented 11 years ago

@omab Hmm, since IntegrityError is not a subclass of SocialAuthBaseException, do you think it's healthy to catch it there. Any IntegrityError thrown in my application wil be considered as a login/signup error. I think you could give us a base exception for that. Thanks!

omab commented 11 years ago

Even if IntegrityError is not a subclass of SocialAuthBaseException, you could check for request.social_auth_backend to identify if the current issue was raised by some DSA view. It's not the cleanest solution, but I prefer that than hide the real exception with another.

gfronza commented 11 years ago

Good idea @omab. But yesterday I came up with another solution. I've wrapped the DSA url include with an exception handler function. Something like...

urlpatterns = required(
    social_auth_exception_handler,
    patterns('', (r'', include('social_auth.urls')),)
)