Add an allowed IP list to the Ban module

I am moving this back to the feature request branch.

Tagging for test coverage.

Also issue summary appears incomplete. There are several sections of the template that should be left, and this appears to be adding new UI change, possibly API. Also works mentioning the need for this feature helps in the review.

Thanks.

@lukassykora, thanks for making an MR for this suggestion!

There is an outstanding issue for Drupal core to remove the terms 'whitelist' and 'blacklist' from the code base. There are links in the Issue Summary for this work that explains the reasons, #2993575: [meta] Remove usage of "blacklist", "whitelist", use better terms instead. Because of that the patch needs to updated.

Like #7 mentions the standard template allows us to coordinate work on an issue. Since it is no longer here I have restores the standard template and updated the remaining tasks and it to remove 'whitelist'. I also updated the title.

I added a new fork, 3392147-allowed-ips, with the patch provided in #5 plus some inclusive language changes. (Sorry if I didn't do that right—the amount that the existing MR had changed in unrelated files threw me off, so I was hesitant to try to build on top of it.) I also did a little bit of formatting cleanup, but that probably wasn't exhaustive.

One thing I'm confused by is that when I look at the allowlist, the IP addresses won't show up in the column (see attached screenshot)—something seems off with how the ID and Allowlisted IP properties are defined and saved, but I haven't yet figured out what. Additionally, it looks like the IP address is intended to be saved as the entity ID; this works for IPv4 addresses, but causes a fatal error ("Invalid character in Config object name") when allowing IPv6 addresses due to the colons (:) in the address.

The Ban Module was approved for removal in #1570102: [Policy] Deprecate Ban module.

This remains Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.

The deprecation work is in #3482198: [meta] Tasks to deprecate the Ban module and the removal work in #3488827: [meta] Tasks to remove Ban module.

Ban will be moved to a contributed project after the Drupal 12.x branch is open.

I also meant to change this to un-assigned.

Contrib request in advban module: #3544163: IP Whitelist

Still a very valid request!

The protected list was added to the https://www.drupal.org/project/advban 1.9

Moved to contrib module queue

Let's proceed here. An allowed list would be really helpful, at least if this won't get merged with the more powerful advban module.

Changed merge target to 1.1.x - please rebase

Mhm I'm not really sure MR!6 is a good approach. Won't this result in many many config entities per (whitelisted) IP?
Let's better add a settings page and a textarea IMHO. KISS! ;)

Thanks @mstrelan, can you also set it as a default branch in https://git.drupalcode.org/project/ban/-/branches?

@anybody, what the hell? I wrote a long pro / contra for this, seems to be in an unrelated issue now, great. xD

But yea I agree, currently working at the settings variation.

Alright, that's it, working splendid!

Please review!

Let me quickly add and fix up the tests.

No idea what's up with the drupal gitlab servers, but I added further tests through https://git.drupalcode.org/issue/ban-3392147/-/commit/bfe01274bfdd51cc96..., for some reason, there are not in the MR (yet?).

Please review anyway!

Thanks @grevil for this implementation and also adding tests. LGTM and as soon as we have green tests, RTBC from my side. But would be great if @mstrelan could do the final review and merge then :)

I don't think he added us as maintainers, but still wants to review everything by himself.

Just want to point out we need to be careful with BC. Adding a required constructor param to BanMiddleware could cause a BC break if people are extending this class or instantiating it directly. Ideally we would have ?ConfigFactoryInterface $config_factory = NULL and throw a deprecation if NULL is passed. Then load it via Drupal::service in that instance.

@mstrelan yea, I guess a 2.x branch could've made more sense, so we can adjust things more freely.

There is no 1.1.x release yet. We could still create the 2.x branch and move everything over there, but I'd say we keep it as is.

I created #3575604: Remove introduced breaking changes in 1.1.x, to fix the breaking changes.

I created #3578480: Worth caching allowed IP list in middleware? as follow-up because I'm unsure about possible performance implications in the middleware.