Add an 'unblock IP address' action

No, it's not possible, because Rules does not have an action to unblock an IP address. If it did, then you could just use Rules scheduler to schedule an unblock after a certain period of time. Such an action isn't hard to write though ...

Core Drupal 7 doesn't have an API for blocking/unblocking IPs - it all has to be done by making explicit DB queries. And because there's no API, any module that want to block/unblock has to do IP validation and checking independently (in core this is done ONLY when using the /admin/config/people/ip-blocking form), for example to prevent you from banning your own IP address ...

Drupal 8 *does* have an API in the form of the ban.ip_manager service but the validation/checking is still left to the admin form (which is now in the ban module).

Moving this to a feature request in D8. In addition to a new action that unblocks an IP address, the existing block IP address action should implement the same safeguard as the ban module admin form to prevent banning one's own IP address.

And as long as we're adding the new action, we should also add an "is banned" condition to test if an address is banned or not.

@TR, I'm not sure that we want to disallow a user to block their own IP address: I can envision a use case in which a rule is set up to block IP addresses for users that try to access content for which they lack access permission. I'm certainly open to discussion on this point.

Patch forthcoming for the un-ban rules action and the 'is banned' rules condition.

Patch attached.

Looks pretty good. There are a few coding standards problems (e.g. "\ No newline at end of file", and the comments in UnBanIP.php need to be updated since they were just copied from BanIP.php) - see the test results at https://www.drupal.org/pift-ci-job/1175858 for a list of things to fix.

The big thing that still needs to be done is to add some tests. Use tests/src/Unit/Integration/Action/BanIPTest.php as a template for writing the UnBanIP test, and use one of the tests in tests/src/Unit/Integration/Condition/ as a template for the condition test.

Also, for the IpIsBanned condition, the ban.ip_manager service should be injected just like in the BanIP action, instead of the explicit instantiation from the connection object that you're currently using. This requires implementing ContainerFactoryPluginInterface and coding a create() and __construct() method for the condition, just like it was done in the action.

@TR thanks so much; very helpful feedback. Updated patch attached (with apologies in advance if testing fails; my local testing environment is not functional.)

Revised patch attached.

Revised patch attached.

You should use isBanned() from the ban.ip_manager service instead of making your own database query in the condition.

Also check https://www.drupal.org/pift-ci-job/1181473 for two coding standards messages.

The reason for the database service not found error is that this is a Unit test. With Unit tests you have to mock any services you plan on using. This is done in RulesIntegrationTestBase for many common services, but database is not one of those. Any services not mocked in the test base should be mocked in your test setUp(). If you use the isBanned() method you won't need the database service, but you *will* need the ban.ip_manager service. You can see in BanIpTest how the test enables the ban module and mocks the ban.ip_manager service - that's what you'll also have do do in the test of your condition:

    // We need the ban module.
    $this->enableModule('ban');
    $this->banManager = $this->prophesize(BanIpManagerInterface::class);
    $this->container->set('ban.ip_manager', $this->banManager->reveal());

Dig a lot to resolve 'database' service. Decided to not use it. Instead, pre-set BanIpManager method to validate banned/unbanned IPs.
Implement DI in condition.

Aborted testing with PHP 5.5 & MySQL 5.5, D8.6 CI.
Ran test with PHP 7.2 & MySQL 5.5, D8.7 successfully.

Please correct the coding standards issues. See https://www.drupal.org/pift-ci-job/1189140

You can't use context_definitions in the annotation yet, because that is only allowed in Drupal 8.7.x. You have to keep using context instead. That change can't be made in Rules until 8.6.x becomes unsupported.

Your @var should use the interface type, not the class type, and the variable should be protected, not private.

@COBadger, @Vladimirrem

Are you still working on this?

@TR
Yes, will finish in a few days.

Are you going to submit that patch @Vladimirrem ?

Revised patch attached.

Here's a re-roll of #24 with some cleanup of the coding standards and some minor changes to make the code consistent. This will have one remaining coding standard complaint about a comment being followed by a blank line, but that comment looks good to me how it is - removing the complaint makes the comment less clear.

I missed one thing, so here's another patch. I don't know why the previous patch was tested against Drupal 7, but you can ignore that obvious fail ...

Committed #26.

I have opened up a separate issue for the changes to the BanIp action that I mentioned in #1. See #3070755: Rename BanIP to BanIp, consider IP validation checks