Fixed #20916 -- Added Client.force_login() to bypass authentication during a test login.

[jdufresne]

…on when logging in during tests.

[mjtamlyn]

I'm not completely convinced by implementing a new auth backend and patching the settings with it. I guess things break if there is no authentication backend set.

Perhaps pseudocode like this:

def force_login(self, user, backend=None):
    if backend is None:
        backend = AUTH_BACKENDS[0]
    user.backend = backend
    return self._login(user)

[MarkusH]

I agree with Marc. I think the patch is too complicated/complex for what you want to achieve. There should be an easier way to do that.

[jdufresne]

@mjtamlyn Thanks for the feedback. I like you're suggestion and approach better. I have incorporated it into the latest version.

[mjtamlyn]

Implementation looks good to me, needs docs and release notes

[timgraham]

could we call it "login_user"?

[mjtamlyn]

Not a bad plan at all. I originally suggested force_login on the ticket to make it clear that it bypasses the authentication systems completely. Everything here is pretty clearly a testing utility mind.

[MarkusH]

I'd actually prefer something like force_login for exactly that reason. login_user suggests something more generic than shortcircuting the is_active flag.

On June 16, 2015 10:10:32 PM GMT+02:00, Marc Tamlyn notifications@github.com wrote:

•        # Create a fake request to store login details.
  

•        request = HttpRequest()
  

• def force_login(self, user, backend=None):

Not a bad plan at all. I originally suggested force_login on the
ticket to make it clear that it bypasses the authentication systems
completely. Everything here is pretty clearly a testing utility mind.

---

Reply to this email directly or view it on GitHub:
https://github.com/django/django/pull/4865/files#r32564941

[timgraham]

It is more generic than shortcircuting the is_active flag, isn't it? It logs in the given user.

[MarkusH]

Agreed, but from a user's perspective I don't see the difference between login() and login_user(). I could even think that the latter would prevent staff/superusers to login.

[jdufresne]

Personally, I agree with @MarkusH. I feel the word force helps emphasize that this mechanism is bypassing authentication.

But ultimately, I don't mind either and will adopt whatever the consensus is.

[jdufresne]

I have updated the commit with more tests and documentation. All comments welcome.

If a decision is made on force_login() for login_user(), I have no problem changing the name.

Thanks for all feedback.

[mjtamlyn]

The docs currently don't give much rationale as to why you would use this instead of login(). In my opinion there are two reasons, firstly that you don't have to worry about the authentication details for that user which makes the code more straightforwards to write, but secondly that you don't have to generate verify the passwords with (potentially expensive) algorithms. Personally I always use the MD5 hasher in my test runs which helps a bit, but it would be interesting to get various benchmarks of this (use PDKFB2, use MD5, use force_login(), with and without an actual password set for the user).

If we have this we can add a section like "This makes the tests requiring a logged in user simpler to write and quicker to run. You should ensure you have some tests which check the authentication systems properly."

[jdufresne]

The docs currently don't give much rationale as to why you would use this instead of login().

Good point.

I have tried to address your concern in the latest version. Let me know if this is what you had in mind. Let me know if I should expand or reword anything.

[jdufresne]

but secondly that you don't have to generate verify the passwords with (potentially expensive) algorithms. Personally I always use the MD5 hasher in my test runs which helps a bit, but it would be interesting to get various benchmarks of this (use PDKFB2, use MD5, use force_login(), with and without an actual password set for the user).

Ran some simple benchmarks. Basically, I added a "test" that calls either login() or force_login() multiple times with different password hashers. On my computer, the results indicate that force_login() is about an order of magnitude faster than the PBKDF2 hasher, two orders of magnitude faster than the BCrypt hasher, and only marginally faster than the other hashers. Full results and code used below:

Python 2

force_login: 0.238655s
django.contrib.auth.hashers.PBKDF2PasswordHasher: 3.107719s
django.contrib.auth.hashers.PBKDF2SHA1PasswordHasher: 2.045471s
django.contrib.auth.hashers.BCryptPasswordHasher: 27.751106s
django.contrib.auth.hashers.SHA1PasswordHasher: 0.335532s
django.contrib.auth.hashers.MD5PasswordHasher: 0.334966s
django.contrib.auth.hashers.UnsaltedMD5PasswordHasher: 0.337066s
django.contrib.auth.hashers.CryptPasswordHasher: 0.336060s

Python 3

force_login: 0.276633s
django.contrib.auth.hashers.PBKDF2PasswordHasher: 3.277875s
django.contrib.auth.hashers.PBKDF2SHA1PasswordHasher: 2.034182s
django.contrib.auth.hashers.BCryptPasswordHasher: 27.827637s
django.contrib.auth.hashers.SHA1PasswordHasher: 0.385975s
django.contrib.auth.hashers.MD5PasswordHasher: 0.387751s
django.contrib.auth.hashers.UnsaltedMD5PasswordHasher: 0.383666s
django.contrib.auth.hashers.CryptPasswordHasher: 0.387391s

Test code executed to produce results (quick and dirty):

    def test_perf(self):
        import time
        hashers = [
            'django.contrib.auth.hashers.PBKDF2PasswordHasher',
            'django.contrib.auth.hashers.PBKDF2SHA1PasswordHasher',
            'django.contrib.auth.hashers.BCryptPasswordHasher',
            'django.contrib.auth.hashers.SHA1PasswordHasher',
            'django.contrib.auth.hashers.MD5PasswordHasher',
            'django.contrib.auth.hashers.UnsaltedMD5PasswordHasher',
            'django.contrib.auth.hashers.CryptPasswordHasher',
        ]
        n = 100
        print("\n\n")

        User.objects.all().delete()
        u = User(
            last_login=datetime.datetime(2006, 12, 17, 7, 3, 31), is_superuser=False, username='testclient',
            first_name='Test', last_name='Client', email='testclient@example.com', is_staff=False, is_active=True,
            date_joined=datetime.datetime(2006, 12, 17, 7, 3, 31)
        )
        u.set_password('password')
        u.save()

        t0 = time.time()
        for i in range(n):
            self.client.force_login(u)
        dt = time.time() - t0
        print("force_login: %fs" % dt)

        for hasher in hashers:
            with self.settings(PASSWORD_HASHERS=[hasher]):
                User.objects.all().delete()
                u = User(
                    last_login=datetime.datetime(2006, 12, 17, 7, 3, 31), is_superuser=False, username='testclient',
                    first_name='Test', last_name='Client', email='testclient@example.com', is_staff=False, is_active=True,
                    date_joined=datetime.datetime(2006, 12, 17, 7, 3, 31)
                )
                u.set_password('password')
                u.save()

                t0 = time.time()
                for i in range(n):
                    self.client.login(username='testclient', password='password')
                dt = time.time() - t0
                print("%s: %fs" % (hasher, dt))

        print()

Edit @MarkusH: Turned on syntax highlighting for the test function.

[mjtamlyn]

Looks good to me, and thanks for the benchmarks. There's a bit of wording style which will need amending in the docs I think but I'm rubbish at identifying that so I'll summon @timgraham instead ;)

[timgraham]

I'm not sure what tests this paragraph is implying need to be written. Using Client.login() in a few tests?

[mjtamlyn]

I guess in particular it's that if you're not using Django's built in auth system then you should test your code? I wouldn't mind if this note went away though.

[timgraham]

Minor edits: http://dpaste.com/3HABAVP
I prefer login_user() to force_login() and wrote to django-developers just to make sure I'm in the minority. Otherwise LGTM.

[jdufresne]

Minor edits: http://dpaste.com/3HABAVP

Thanks Tim. I have applied this patch to the latest version of the PR.

[timgraham]

Could I get a review of this additional paragraph to the docs? If it looks good, I'll add it and merge this.

The user will have its ``backend`` attribute set to the value of the
``backend`` argument (which should be a string of the dotted Python
path of the backend), or to ``settings.AUTHENTICATION_BACKENDS[0]`` if
a value isn't provided. The :func:`~django.contrib.auth.authenticate`
function called by :meth:`login` normally annotates the user like this.

[mjtamlyn]

The user will have its ``backend`` attribute set to the value of the ``backend``
 argument (which should be a dotted Python path string) ...

Removes a few too many backends from that sentence.

[timgraham]

merged in b44dee1, thanks!