django-rest-framework icon indicating copy to clipboard operation
django-rest-framework copied to clipboard

Unit testing: calling `APIClient.force_authenticate()` with `user=None` does not work as intended

Open willbeaufoy opened this issue 2 years ago • 4 comments

Discussed in https://github.com/encode/django-rest-framework/discussions/8184

Originally posted by willbeaufoy September 24, 2021 If you call APIClient.force_authenticate() with a token param but without a user param, self.handler._force_token is set to the provided token, but then self.logout() is called, which immediately sets self.handler._force_token to None again. Surely this is not intended? The docstring and the docs say you can use either a user or a token or both, but in reality you cannot just use a token.

I discovered this while writing a unit test for an endpoint with access authorised by the OAuth client credentials method (provided by django-oauth-toolkit), where requests have a token but no user. Therefore I tried to authenticate then call my endpoint like this:

from rest_framework.test import APITestCase

class MyTests(APITestCase):
    def test_endpoint(self):
        # ... Code to create valid access_token here
        self.client.force_authenticate(token=access_token)

        response = self.client.get("/my/endpoint/")

        self.assertEqual(response.status_code, HTTPStatus.OK)

But my test fails as I get a 401 Unauthorized response.

However if I comment out the line in APIClient.logout() that sets self.handler._force_token to None, the request makes it through to my endpoint successfully and my test passes.

The PR that changed the logout() method to set self.handler._force_user and self.handler._force_token to None was done for an unrelated reason. Perhaps at the time it was overlooked that this broke the case I am trying to test above?

If you agree that this current apparently broken behaviour is a bug, then I can do a PR to fix it.

willbeaufoy avatar Oct 17 '21 14:10 willbeaufoy

Created an issue from this as I'm doing a PR to fix it.

willbeaufoy avatar Oct 17 '21 14:10 willbeaufoy

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Apr 18 '22 15:04 stale[bot]

having this issue as well -- any updates?

stcalica avatar Jun 30 '22 20:06 stcalica

having this issue as well -- any updates?

Thanks for the prompt, I just addressed the outstanding comment so hopefully it can be merged soon.

willbeaufoy avatar Jul 03 '22 11:07 willbeaufoy

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Sep 09 '22 01:09 stale[bot]

I closed my PR https://github.com/encode/django-rest-framework/pull/8212 that fixes this as it's being ignored. However it's ready to merge so if anyone gets round to looking at it, I can reopen it.

willbeaufoy avatar Sep 13 '22 15:09 willbeaufoy

Closed via #8212

tomchristie avatar Sep 15 '22 08:09 tomchristie