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

Fix combining LoginRequired with other access mixins #208

Merged
merged 3 commits into from
May 31, 2016
Merged

Fix combining LoginRequired with other access mixins #208

merged 3 commits into from
May 31, 2016

Conversation

mikebryant
Copy link
Contributor

Make it possible to combine LoginRequiredMixin with other AccessMixins. This allows views which will redirect to login if unauthenticated, but raise an error if the user is unauthenticated but isn't allowed.
This stops the redirect loop in #161.

I'm using the approach in #200, which avoids api changes.

The tests pass (at least the versions I have available to test):

  py27-django15: commands succeeded
  py27-django16: commands succeeded
  py27-django17: commands succeeded
  py27-django18: commands succeeded
ERROR:   py32-django15: InterpreterNotFound: python3.2
ERROR:   py32-django16: InterpreterNotFound: python3.2
ERROR:   py32-django17: InterpreterNotFound: python3.2
ERROR:   py32-django18: InterpreterNotFound: python3.2
ERROR:   py33-django15: InterpreterNotFound: python3.3
ERROR:   py33-django16: InterpreterNotFound: python3.3
ERROR:   py33-django17: InterpreterNotFound: python3.3
ERROR:   py33-django18: InterpreterNotFound: python3.3
  py34-django15: commands succeeded
  py34-django16: commands succeeded
  py34-django17: commands succeeded
  py34-django18: commands succeeded

I've verified the tests fail, when run against the previous version of _access.py

The changes comply with pep8

Fixes #161.
Fixes #181.
Closes #196.
Closes #200.

cornmander and others added 3 commits November 1, 2015 18:29
users to the redirect url.

Tests are passing while they shouldn't be.
Ensure the tests capture the case that was failing. (In particular `test_authenticated_raises_exception`)
Also regression tests for previous behaviour.
@mikebryant mikebryant changed the title WIP - Fix combining LoginRequired with other access mixins Fix combining LoginRequired with other access mixins Mar 21, 2016
@chrisjones-brack3t
Copy link
Member

I'll look over the PR more this evening but a quick scan of it looks good.

@wildd
Copy link

wildd commented May 10, 2016

Is this going to be merged in the main branch soon? I have the same problems with many login redirects when user is already logged in, for example while using "GroupRequiredMixin". Thanks!

@kennethlove kennethlove merged commit 38ab5bc into brack3t:master May 31, 2016
@mikebryant mikebryant deleted the wip-200 branch August 3, 2016 09:08
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.

5 participants