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

TypeError when releasing when LOGOUT_REDIRECT_URL is None #367

Closed
LincolnPuzey opened this issue Nov 18, 2021 · 4 comments
Closed

TypeError when releasing when LOGOUT_REDIRECT_URL is None #367

LincolnPuzey opened this issue Nov 18, 2021 · 4 comments
Assignees
Labels

Comments

@LincolnPuzey
Copy link

LincolnPuzey commented Nov 18, 2021

According to the Django docs LOGOUT_REDIRECT_URL can be set to None

https://docs.djangoproject.com/en/dev/ref/settings/#logout-redirect-url

When this is the case a TypeError can be raised here in the release view

Because self.success_url == LOGOUT_REDIRECT_URL == None

Passing None to resolve_url causes a TypeError

@Mogost
Copy link
Member

Mogost commented Nov 19, 2021

None is not allowed for LOGIN_REDIRECT_URL in django.

@LincolnPuzey LincolnPuzey changed the title TypeError when releasing when LOGIN_REDIRECT_URL is None TypeError when releasing when LOGOUT_REDIRECT_URL is None Nov 19, 2021
@LincolnPuzey
Copy link
Author

My bad I got login/logout mixed up when writing the issue. I have edited to be correct. LOGOUT_REDIRECT_URL is the setting being used that can be None

@codingjoe codingjoe self-assigned this Nov 22, 2021
@codingjoe codingjoe added the bug label Nov 22, 2021
@codingjoe
Copy link
Collaborator

Hi @LincolnPuzey,

Again, welcome and nice you are digging up all those hidden little issues. I am curious, you must have a rather large and well-integrated application running on our humble little framework. That's wonderful!

Regarding the issue at hand. I looked into it, and it seems Django would render a logout screen if the LOGOUT_REDIRECT_URL setting is set to None.

Tho this behavior could be implemented, the original LogoutView isn't easily adjusted, and I don't like the idea of creating a carbon copy that we need to maintain and keep up to date with the Django implementation. After all, the smaller this library remains, the less likely is it to include security wholes.

Therefore, my solution would be to simply fall back to / if the LOGOUT_REDIRECT_URL is set to None and no explicit next value is given.

Before I go ahead and make the change, though, I'd really like to get your feedback on this.

Best Joe

@LincolnPuzey
Copy link
Author

Falling back to / seems sensible to me 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants