Implement flood control

1. +++ b/src/Plugin/Commerce/PaymentGateway/FinDockGateway.php
   @@ -92,7 +92,9 @@ class FinDockGateway extends OffsitePaymentGatewayBase implements HasPaymentInst
   +      'ip_limit' => 0,
   +      'ip_window' => 0,
   +      ] + parent::defaultConfiguration();
      }
   

   indendation is off here.

2. +++ b/src/PluginForm/OffsiteRedirect/FinDockForm.php
   @@ -78,6 +97,16 @@ class FinDockForm extends PaymentOffsiteForm implements ContainerInjectionInterf
    
   +    if (isset($configuration['ip_limit'], $configuration['ip_window'])) {
   +      $allowed = $this->flood->isAllowed('commerce_findock.gateway_payment', $configuration['ip_limit'], $configuration['ip_window']);
   +      $this->flood->register('commerce_findock.gateway_payment', $configuration['ip_window']);
   +
   +      // Don't allow login if the limit for this IP has been reached.
   +      if (!$allowed) {
   +        throw new PaymentGatewayException($this->t('Flood control triggered.'));
   +      }
   +    }
   

   we want a double protection. on the ip and on the order id. So a bad actor needs to distribute the requests to multiple source ips *and* multiple orders.

   I'd rename the ip_limit to just limit and use the same setting for both, explaining that in the comments.

   Also, isset(0) => TRUE, you want to use !empty() or just directly if ($configuration['limit'], the default configuration should ensure that it's always set, even on existing configuration.

1. +++ b/src/Plugin/Commerce/PaymentGateway/FinDockGateway.php
   @@ -92,7 +92,9 @@ class FinDockGateway extends OffsitePaymentGatewayBase implements HasPaymentInst
   +      'ip_limit' => 0,
   +      'ip_window' => 0,
   +      ] + parent::defaultConfiguration();
      }
   

   indendation is off here.

2. +++ b/src/PluginForm/OffsiteRedirect/FinDockForm.php
   @@ -78,6 +97,16 @@ class FinDockForm extends PaymentOffsiteForm implements ContainerInjectionInterf
    
   +    if (isset($configuration['ip_limit'], $configuration['ip_window'])) {
   +      $allowed = $this->flood->isAllowed('commerce_findock.gateway_payment', $configuration['ip_limit'], $configuration['ip_window']);
   +      $this->flood->register('commerce_findock.gateway_payment', $configuration['ip_window']);
   +
   +      // Don't allow login if the limit for this IP has been reached.
   +      if (!$allowed) {
   +        throw new PaymentGatewayException($this->t('Flood control triggered.'));
   +      }
   +    }
   

   we want a double protection. on the ip and on the order id. So a bad actor needs to distribute the requests to multiple source ips *and* multiple orders.

   I'd rename the ip_limit to just limit and use the same setting for both, explaining that in the comments.

   Also, isset(0) => TRUE, you want to use !empty() or just directly if ($configuration['limit'], the default configuration should ensure that it's always set, even on existing configuration.

BTW can we be sure that we are not causing problems with the IP limits, in situation like: test environments, local dev...
(Worst would be behind a proxy without proper IP forwarding, so you always see the proxy IP like sometimes also happens on login after relaunches and incomplete prod configuration...)

It seems proper IP handlign is provided by the flood system itself?

+++ b/src/PluginForm/OffsiteRedirect/FinDockForm.php
@@ -97,12 +97,15 @@ class FinDockForm extends PaymentOffsiteForm implements ContainerInjectionInterf
+      $order = $payment->getOrder();
+      $allowed_by_ip = $this->flood->isAllowed('commerce_findock.gateway_payment', $configuration['limit'], $configuration['window']);
+      $allowed_by_oid = $this->flood->isAllowed('commerce_findock.gateway_payment', $configuration['limit'], $configuration['window'], 'commerce-findock-order-' . $order->id());
+      $this->flood->register('commerce_findock.gateway_payment', $configuration['window']);
+      $this->flood->register('commerce_findock.gateway_payment', $configuration['window'], 'commerce-findock-order-' . $order->id());
 
       // Don't allow login if the limit for this IP has been reached.
-      if (!$allowed) {
+      if (!$allowed_by_ip || !$allowed_by_oid) {
         throw new PaymentGatewayException($this->t('Flood control triggered.'));

this looks like what I expected yes. But lets do two separate if conditions and only register a flood event if it's allowed, like before, for both of them separately.

Since the identifier is namespaced with the name, I'd say just "order-ID" is enough.

And lets make the messages a little bit more clear, something like this, based on the user login flood messages:
- IP: Too many failed payment attempts from your IP address. This IP address is temporarily blocked. Try again later.
- Order: Too many failed payment attempts for this order. Payments are temporarily blocked. Try again later.

@miro: Sure, this can cause problems if you have a lot of people trying to do checkout from a single IP. Thinking about that, lets add two separate limit settings, limit_ip and limit_order. Sorry about the back and forth. I'd say same timeframe is OK? Then it's up to you if you just want to enforce a per-order limit or a higher per-IP limit. But the IP one is also the one that can easily be misused by a botnet, so you have to weigh that off.

The log messages should use commerce_findock, not commerce_payment.

Not what I meant, but I also realized that we can't actually do that as the log message is based on the exception and we can't control the channel.

Committed.