Problem/Motivation

In #3582177: Rewrite IP allowlist using a settings.php setting we discussed adding a ban.allowlist service to allow other modules can

  • easily request the allowed IPs, so ban-related modules like Crowdsec or Perimeter could return early themselves
  • handle these allowlisted-IPs differently
  • replace the service with anything they need, for example a different source for these IPs

See #10:

> Do you think that'd make sense? It would be again another (although really lightweight) service which needs to be injected in the Middleware.

I think the overhead of one extra class/service with no dependencies is minimal. Someone could implement a more complex/dynamic allowlist-provider then if they're willing to live with the possibly slower performance.

It would make BC more complicated though, you'd then switch the passed argument with something else, would need to be optional and without type. Also makes further changes a bit more complicated. Right now only the drush ban commands seems to respect the allow list and not the UI, that seems pretty inconsistent, this might make more sense within the BanIpManger? With a setting, you can just move the code, with DI, you again need to remove and add the service in a different place. Your call if that is worth it.

We finally decided to postpone that until someone needs it. If anyone needs it, please prepare a MR accordingly (caring for BC)!

Steps to reproduce

See above

Proposed resolution

Implement a ban.allowlist service in ban and replace the direct Settings::get('ban_allowlist', []); calls by calling this service instead.

The service should only have one method returning the IPs, e.g. getAllowedIps()

Remaining tasks

User interface changes

None

API changes

See above

Data model changes

None

Comments

anybody created an issue.