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

Update auth backend to work with django 1.11 #2

Merged
merged 1 commit into from
Apr 19, 2017

Conversation

ferranp
Copy link
Contributor

@ferranp ferranp commented Apr 18, 2017

In django 1.11 the request obbject is passed to authenticate method as kwargs, this arg should not be passed to the method that creates the user object.

In django 1.11 the request obbject is passed to authenticate method as kwargs, this arg should not be passed to the method that creates the user object.
@@ -147,7 +147,7 @@ def get_context_data(self, **context):
:rtype: dict
"""
context[self.redirect_field_name] = self.get_success_url()
return context
return super(LoginView, self).get_context_data(**context)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember correctly, I purposely didn't want the super, because it puts the form object in the context which caused an issue with AJAX requests. You only changed a few lines, so I will merge your fixes in then run my tests and see what happens. You should be able to run the tests also if you want make coverage does things in the correct order. Thanks for the fixes, I was going to find this issue in the next month or so anyway as I'll be needing this package for an SSO project I'm working on for Cisco.

@cnobile2012 cnobile2012 merged commit 3580e65 into cnobile2012:master Apr 19, 2017
@ferranp
Copy link
Contributor Author

ferranp commented Apr 19, 2017 via email

@cnobile2012
Copy link
Owner

After I mentioned that you can run the tests I realized that the tests are not easy to run for this app. If you remember from my documentation on ReadTheDocs you need to make the user that the server runs in a member of the shadow group. You then need to make a guest user on your system. Running the tests with make coverage would then work properly, however you'll be asked to enter the username and password a lot. You could mitigate this problem by creating in the base django_pam a .django_pam file. Put in the file in this order the username, password, and fake email address, each on a separate line. The tests will read this file instead of prompting on the screen for this info.

I haven't had the time yet to completely go through everything. I've been very busy with work and have little time right now. Hopefully, I'll get to it this weekend.

@ferranp
Copy link
Contributor Author

ferranp commented Apr 19, 2017

Yesterday I already have looked inside the tests and already figured the .django_pam file.
Yesteday I ran the tests with "./manage.py test", today I run the tests with "make coverage" and all tests pass with 100% coverage.

@cnobile2012
Copy link
Owner

I have just pushed version 1.2 out to pypi. I moved your fix to inside the exception since that's where it will be used. No reason to execute code that may never get run.
Thanks for the contribution.

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.

None yet

2 participants