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

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

Closed
willbeaufoy opened this issue Oct 17, 2021 Discussed in #8184 · 7 comments

Comments

@willbeaufoy
Copy link
Contributor

Discussed in #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
Copy link
Contributor Author

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

@stale
Copy link

stale bot commented Apr 18, 2022

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 stale bot added the stale label Apr 18, 2022
@stcalica
Copy link

having this issue as well -- any updates?

@stale stale bot removed the stale label Jun 30, 2022
@willbeaufoy
Copy link
Contributor Author

having this issue as well -- any updates?

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

@stale
Copy link

stale bot commented Sep 9, 2022

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 stale bot added the stale label Sep 9, 2022
@willbeaufoy
Copy link
Contributor Author

I closed my PR #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.

@stale stale bot removed the stale label Sep 13, 2022
@tomchristie
Copy link
Member

Closed via #8212

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

No branches or pull requests

3 participants