Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sanitize_redirect don't work with Django's reverse_lazy #49

Merged
merged 2 commits into from
Oct 8, 2013

Conversation

volrath
Copy link
Contributor

@volrath volrath commented Oct 7, 2013

Hello,

There's an issue when working with Django and setting some of the SOCIAL_AUTH_*_URL to reverse_lazy proxies. The problem is caused in two functions:

sanitize_redirect: Does a quick check first where ask if the given redirect_to is a True-able value and if it's instance of a string type (six.string_types). The problem here is that reverse_lazy returns a Django proxy (reverse_lazy ~= lazy(reverse, str)) and this is not validated by this first check, hence returning None.

do_complete: Here we call sanitize_redirect if the settings say so, but there's no checking if sanitize_redirect returns None. So, if this is the case, a misleading Exception is raised ('NoneType' object has no attribute 'find') since url is set to None and after that, somewhere in the way, a url.find(...) is called.

I forked the project to provided solutions. For sanitize_redirect I'm using the str.decode method to force a reverse_lazy string to get evaluated. This was suggested as one of the solutions for this ticket: https://code.djangoproject.com/ticket/18776 The actual solution is better but is very django-dependent.

For do_complete I'm just adding the extra check after calling sanitize_redirect, maybe that check can be set in a lower indentation level.

Hope this helps.

This commit is intended to make str proxies (such as the one created by
django's `reverse_lazy` urlresolver) safely pass the first quick sanity
check in `sanitize_redirect`. It assumes that if redirect_to exist but
it's not a six.string_types instance, it can have a `decode` method that
will return the valid string.

Based on one of the proposed solutions for this Django ticket:
https://code.djangoproject.com/ticket/18776
omab added a commit that referenced this pull request Oct 8, 2013
`sanitize_redirect` don't work with Django's `reverse_lazy`
@omab omab merged commit fe4e053 into omab:master Oct 8, 2013
@omab
Copy link
Owner

omab commented Oct 8, 2013

Thanks!

omab added a commit that referenced this pull request Oct 8, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants