[Django] #35851: django.test.client.ClientMixin._login doest not set enviorn like REMOTE_ADDR can cause test failures in certain situations

[Django]

#35851: django.test.client.ClientMixin._login doest not set enviorn like
REMOTE_ADDR can cause test failures in certain situations
------------------------+---------------------------------------------
Reporter: elonzh | Type: Bug
Status: new | Component: Testing framework
Version: | Severity: Normal
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------+---------------------------------------------
Our service listens for the ''user_logged_in'' signal to log the user's IP
information, but when using ''TestClient.login/force_login'', the absence
of ''REMOTE_ADDR'' results in an error.

By reviewing the source code, I found that
''TestClient.login/force_login''(https://github.com/django/django/blob/main/django/test/client.py#L869-L882)
creates an empty HttpRequest, which behaves differently from
''django.test.client.Client.request''(https://github.com/django/django/blob/main/django/test/client.py#L401-L436).

Therefore, I believe this is an issue that needs to be addressed.
--
Ticket URL: <https://code.djangoproject.com/ticket/35851>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35851: django.test.client.ClientMixin._login doest not set enviorn like
REMOTE_ADDR can cause test failures in certain situations

-----------------------------------+--------------------------------------
Reporter: elonzh | Owner: (none)

Type: Bug | Status: new
Component: Testing framework | Version:

Severity: Normal | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-----------------------------------+--------------------------------------
Description changed by elonzh:

Old description:

> Our service listens for the ''user_logged_in'' signal to log the user's
> IP information, but when using ''TestClient.login/force_login'', the
> absence of ''REMOTE_ADDR'' results in an error.
>
> By reviewing the source code, I found that
> ''TestClient.login/force_login''(https://github.com/django/django/blob/main/django/test/client.py#L869-L882)
> creates an empty HttpRequest, which behaves differently from
> ''django.test.client.Client.request''(https://github.com/django/django/blob/main/django/test/client.py#L401-L436).
>
> Therefore, I believe this is an issue that needs to be addressed.

New description:

Our service listens for the ''user_logged_in'' signal to log the user's IP
information, but when using ''TestClient.login/force_login'', the absence
of ''REMOTE_ADDR'' results in an error.

By reviewing the source code, I found that
''TestClient.login/force_login''(https://github.com/django/django/blob/main/django/test/client.py#L869-L882)
creates an empty HttpRequest, which behaves differently from
''django.test.client.Client.request''(https://github.com/django/django/blob/main/django/test/client.py#L401-L436).

Therefore, I believe this is an issue that needs to be addressed.

----

I'd like to create a patch if this ticket is confirmed.

--
--
Ticket URL: <https://code.djangoproject.com/ticket/35851#comment:1>

[Django]

#35851: django.test.client.ClientMixin._login doest not set enviorn like
REMOTE_ADDR can cause test failures in certain situations
-----------------------------------+--------------------------------------
Reporter: elonzh | Owner: (none)

Type: New feature | Status: closed
Component: Testing framework | Version: dev
Severity: Normal | Resolution: wontfix

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------------------------

Changes (by Natalia Bidart):

* resolution: => wontfix
* status: new => closed
* type: Bug => New feature
* version: => dev

Comment:

Hello elonzh! Thank you for creating this ticket. I think I understand
your use case, but in my opinion, when testing a Django app's code, there
are two different concerns:

1. Ensure that your app's business logic is correct when a user goes thru
the login dance, and
2. Given a logged in user, ensure some other properties and qualities of
your service.

For the first case, I would think that you app has tests asserting over
the recorded IP, after having exercised a "full login process". This would
be similar to visiting the login page, entering credentials (i.e.
emulating a POST), and being redirected to a given location. So, in
practice, something like (untested):

{{{#!python
from django.contrib.auth.models import User
from django.test import TestCase
from django.urls import reverse


class LoginTestCase(TestCase):

def test_basic(self):
creds = {"username": "test", "password": "securepassword"}
user = User.objects.create_user(**creds)
ip_addr = "200.100.200.142"

response = self.client.post(
reverse("login"), data=creds, REMOTE_ADDR=ip_addr)
self.assertRedirects
response, "/accounts/profile/", fetch_redirect_response=False)
self.assertMessages(response, "You are successfully logged in.")
self.assertIPRecorded(user, expected=ip_addr)

}}}

For the second case, the test needs a logged in user as a precondition of
the test, for which the only thing that matters is the authenticated user
in the request, which is what `login` gives you. `Client.login` is not
meant to mimic a complete login process, for that you need to do something
similar to what I put in item 1.

In summary, `Client.request` emulates a "true" HTTP request, but
`Client.login` ensures you get an authenticated user in the request (which
is not the same as saying "the user did an HTTP login dance").

Given the above, I'll close this ticket accordingly, but if you disagree,
you can consider starting a new conversation on the
[https://forum.djangoproject.com/c/internals/5 Django Forum], where you'll
reach a wider audience and likely get extra feedback. More information in
[https://docs.djangoproject.com/en/stable/internals/contributing/bugs-and-
features/#requesting-features the documented guidelines for requesting
features].
--
Ticket URL: <https://code.djangoproject.com/ticket/35851#comment:2>