You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The most recent part of the code (closest to the top, in that permalink) uses edx-django-utils to make various information about the client IP chain available to the application. This is currently working as desired, although it might not be the best place for it. However, the oldest part assumes there is only one true client IP, and then uses that to overwrite request.META['REMOTE_ADDR']. We'd like to delete this part, since it creates an incorrect and inconsistent view of the IP chain. This needs to be removed before the newer code can properly be extracted to a utility module somewhere—it overwrites information that the newer block needs to read. That order dependence is fragile.
In between, there's also some code that copies some values in request.META to new names, for a past Gunicorn upgrade. This will need to be carefully evaluated to determine whether it is still needed, and what to do with it. (Caution: The field and header variable names might be reversed here.) At the very least we'll want to expand the comments.
Acceptance criteria:
DEPRs filed and accepted as needed
Sweep for any remaining uses of REMOTE_ADDR or other code that thinks it has the "real client IP" and isn't already using the edx_django_utils.ip utils
No longer overwrite request.META['REMOTE_ADDR']
Move newer code (call to ip.init_client_ips, instrumentation) into a edx-django-utils middleware, and use that middleware instead
Also recommend to other teams that they use this
Do something with the request.META field copying/renaming (delete? document better?)
We should consider adding observability (maybe even a utility) for monitoring if a getter is called to know whether anyone is calling request.META['REMOTE_ADDR']. If no one is, we can remove the override that is changing its value.
XForwardedForMiddleware in edxapp contains several pieces of code relating to IP addresses.
The most recent part of the code (closest to the top, in that permalink) uses edx-django-utils to make various information about the client IP chain available to the application. This is currently working as desired, although it might not be the best place for it. However, the oldest part assumes there is only one true client IP, and then uses that to overwrite
request.META['REMOTE_ADDR']
. We'd like to delete this part, since it creates an incorrect and inconsistent view of the IP chain. This needs to be removed before the newer code can properly be extracted to a utility module somewhere—it overwrites information that the newer block needs to read. That order dependence is fragile.In between, there's also some code that copies some values in
request.META
to new names, for a past Gunicorn upgrade. This will need to be carefully evaluated to determine whether it is still needed, and what to do with it. (Caution: Thefield
andheader
variable names might be reversed here.) At the very least we'll want to expand the comments.Acceptance criteria:
REMOTE_ADDR
or other code that thinks it has the "real client IP" and isn't already using theedx_django_utils.ip
utilsrequest.META['REMOTE_ADDR']
ip.init_client_ips
, instrumentation) into a edx-django-utils middleware, and use that middleware insteadrequest.META
field copying/renaming (delete? document better?)Notes:
The text was updated successfully, but these errors were encountered: