Problem/Motivation

There are circumstances when client IPs need to be used in an access control method. The current priority value causes ReverseProxyHeaderClientIpRestore::onRequest to run after both Symfony\Component\HttpKernel\EventListener\RouterListener::onKernelRequest and Drupal\Core\EventSubscriber\AuthenticationSubscriber::getSubscribedEvents

Steps to reproduce

  • Create an implementation of hook_ENTITY_TYPE_access() and set a breakpoint or diagnostic statement in the function
  • Set another breakpoint or diagnostic statement in if (!$reverse_proxy_header_name) in ReverseProxyHeaderClientIpRestore::onRequest

Observe that the hook_ENTITY_TYPE_access() implementation runs before this module can set the client IP.

Proposed resolution

Increase the priority of the event response so that it preceeds these other event responses. AuthenticationSubscriber runs at priority 300.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

Issue fork reverse_proxy_header-3290953

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

FatherShawn created an issue. See original summary.

FatherShawn opened merge request !2

bohart’s picture

bohart
Location Lutsk, Ukraine
commented

Thanks for the merge request and great details on both issue description and code comments!

The only thing I am confused about is: what happened if someone/sometimes needs to put something between ReverseProxyHeaderClientIpRestore::onRequest (priority 301) and AuthenticationSubscriber::onRequest (priority 300)?

So I would like to propose to have priority like 350/400.
How do we feel about that?

Thanks!

fathershawn’s picture

fathershawn
Primary language English
Location New York
commented

That sounds like a fine idea

  • bohart committed a5e93320 on 1.0.x authored by FatherShawn 
    Issue #3290953 by FatherShawn, bohart: Client IP is set too late to use...

bohart closed merge request !2

bohart’s picture

bohart
Location Lutsk, Ukraine
commented
Version: 1.0.0 » 1.0.x-dev
Status: Active » Fixed

Committed to 1.0.x dev branch.
It will be a part of the next release (1.0.1).
Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.