Merge pull request #3 from karser/little-issues-fix

little issues fixes

Author: karser

Form/Recaptcha3Type.php

@@ -8,7 +8,7 @@
 use Symfony\Component\Form\FormView;
 use Symfony\Component\OptionsResolver\OptionsResolver;
 
-class Recaptcha3Type extends AbstractType
+final class Recaptcha3Type extends AbstractType
 {
     /** @var string */
     private $siteKey;
@@ -43,7 +43,6 @@ public function configureOptions(OptionsResolver $resolver): void
     {
         $resolver->setDefaults([
             'mapped' => false,
-            'enabled' => true,
             'site_key' => null,
             'action_name' => 'homepage',
         ]);

README.md

@@ -148,6 +148,80 @@ grecaptcha.ready(function() {
 </script>
 ```
 
+### How to deal with functional and e2e testing:
+
+Recaptcha won't allow you to test your app efficiently unless you disable it for the environment you are testing against.
+
+```yaml
+# app/config/config.yml (or config/packages/karser_recaptcha3.yaml if using Symfony4)
+karser_recaptcha3:
+    enabled: '%env(RECAPTCHA3_ENABLED)%'
+```
+
+```bash
+#.env.test or a stage server environment
+RECAPTCHA3_ENABLED=0
+```
+
+### How to add Cloudflare IP resolver:
+
+From the [Cloudflare docs](https://support.cloudflare.com/hc/en-us/articles/200170986-How-does-Cloudflare-handle-HTTP-Request-headers-):
+To provide the client (visitor) IP address for every request to the origin, Cloudflare adds the CF-Connecting-IP header.
+```
+"CF-Connecting-IP: A.B.C.D"
+```
+
+So you can implement custom IP resolver which attempts to read the `CF-Connecting-IP` header or fallbacks with the internal IP resolver:
+ 
+```php
+<?php declare(strict_types=1);
+
+namespace App\Service;
+
+use Karser\Recaptcha3Bundle\Services\IpResolverInterface;
+use Symfony\Component\HttpFoundation\RequestStack;
+
+class CloudflareIpResolver implements IpResolverInterface
+{
+    /** @var IpResolverInterface */
+    private $decorated;
+
+    /** @var RequestStack */
+    private $requestStack;
+
+    public function __construct(IpResolverInterface $decorated, RequestStack $requestStack)
+    {
+        $this->decorated = $decorated;
+        $this->requestStack = $requestStack;
+    }
+
+    public function resolveIp(): ?string
+    {
+        return $this->doResolveIp() ?? $this->decorated->resolveIp();
+    }
+
+    private function doResolveIp(): ?string
+    {
+        $request = $this->requestStack->getCurrentRequest();
+        if ($request === null) {
+            return null;
+        }
+        return $request->server->get('HTTP_CF_CONNECTING_IP');
+    }
+}
+```
+
+Here is the service declaration. It decorates the internal resolver:
+```yaml
+#services.yaml
+services:
+    App\Service\CloudflareIpResolver:
+        decorates: 'karser_recaptcha3.ip_resolver'
+        arguments:
+            $decorated: '@App\Service\CloudflareIpResolver.inner'
+            $requestStack: '@request_stack'
+```
+
 Testing
 -------
 

Resources/config/services.yml

@@ -14,10 +14,16 @@ services:
         arguments:
             - '@karser_recaptcha3.google.recaptcha'
             - '%karser_recaptcha3.enabled%'
-            - '@request_stack'
+            - '@karser_recaptcha3.ip_resolver'
         tags:
             - { name: validator.constraint_validator, alias: karser_recaptcha3_validator }
 
+    karser_recaptcha3.ip_resolver:
+        class:  Karser\Recaptcha3Bundle\Services\IpResolver
+        public: false
+        arguments:
+            - '@request_stack'
+
     karser_recaptcha3.google.recaptcha:
         class: 'ReCaptcha\ReCaptcha'
         arguments:

Services/IpResolver.php

@@ -0,0 +1,25 @@
+<?php declare(strict_types=1);
+
+namespace Karser\Recaptcha3Bundle\Services;
+
+use Symfony\Component\HttpFoundation\RequestStack;
+
+final class IpResolver implements IpResolverInterface
+{
+    /** @var RequestStack */
+    private $requestStack;
+
+    public function __construct(RequestStack $requestStack)
+    {
+        $this->requestStack = $requestStack;
+    }
+
+    public function resolveIp(): ?string
+    {
+        $request = $this->requestStack->getCurrentRequest();
+        if ($request === null) {
+            return null;
+        }
+        return $request->getClientIp();
+    }
+}

Services/IpResolverInterface.php

@@ -0,0 +1,8 @@
+<?php declare(strict_types=1);
+
+namespace Karser\Recaptcha3Bundle\Services;
+
+interface IpResolverInterface
+{
+    public function resolveIp(): ?string;
+}

Tests/Form/Recaptcha3TypeTest.php

@@ -0,0 +1,37 @@
+<?php declare(strict_types=1);
+
+namespace Karser\Recaptcha3Bundle\Tests\Form;
+
+use Karser\Recaptcha3Bundle\Form\Recaptcha3Type;
+use Symfony\Component\Form\PreloadedExtension;
+use Symfony\Component\Form\Test\TypeTestCase;
+
+class Recaptcha3TypeTest extends TypeTestCase
+{
+    const SITEKEY = '<sitekey>';
+
+    protected function getExtensions()
+    {
+        $type = new Recaptcha3Type(self::SITEKEY, $enabled = true);
+
+        return [
+            new PreloadedExtension([$type], []),
+        ];
+    }
+
+    public function testDefaultOptions()
+    {
+        $data = '<captcha-token>';
+
+        $form = $this->factory->create(Recaptcha3Type::class);
+        $form->setData($data);
+
+        $this->assertTrue($form->isSynchronized());
+        $this->assertEquals($data, $form->getData());
+
+        $view = $form->createView();
+        $this->assertSame(self::SITEKEY, $view->vars['site_key']);
+        $this->assertSame('homepage', $view->vars['action_name']);
+        $this->assertTrue($view->vars['enabled']);
+    }
+}

Tests/Services/IpResolverTest.php

@@ -0,0 +1,27 @@
+<?php declare(strict_types=1);
+
+namespace Karser\Recaptcha3Bundle\Tests\Services;
+
+use Karser\Recaptcha3Bundle\Services\IpResolver;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
+
+class IpResolverTest extends TestCase
+{
+    public function testEmptyRequest()
+    {
+        $stack = new RequestStack();
+        $stack->push(new Request());
+        $resolver = new IpResolver($stack);
+        self::assertNull($resolver->resolveIp());
+    }
+
+    public function testRequest()
+    {
+        $stack = new RequestStack();
+        $stack->push(new Request([], [], [], [], [], ['REMOTE_ADDR' => '0.0.0.0']));
+        $resolver = new IpResolver($stack);
+        self::assertSame('0.0.0.0', $resolver->resolveIp());
+    }
+}

Tests/Validator/Constraints/Recaptcha3ValidatorTest.php

@@ -0,0 +1,78 @@
+<?php declare(strict_types=1);
+
+namespace Karser\Recaptcha3Bundle\Tests\Validator\Constraints;
+
+use Karser\Recaptcha3Bundle\Services\IpResolverInterface;
+use Karser\Recaptcha3Bundle\Tests\fixtures\RecaptchaMock;
+use Karser\Recaptcha3Bundle\Validator\Constraints\Recaptcha3;
+use Karser\Recaptcha3Bundle\Validator\Constraints\Recaptcha3Validator;
+use PHPUnit\Framework\MockObject\MockObject;
+use Symfony\Component\Validator\Test\ConstraintValidatorTestCase;
+
+class Recaptcha3ValidatorTest extends ConstraintValidatorTestCase
+{
+    /** @var IpResolverInterface|MockObject */
+    private $resolver;
+    /** @var RecaptchaMock */
+    private $recaptcha;
+
+    public function setUp()
+    {
+        $this->resolver = $this->getMockBuilder(IpResolverInterface::class)->getMock();
+        parent::setUp();
+    }
+
+    protected function createValidator()
+    {
+        $this->recaptcha = new RecaptchaMock();
+        return new Recaptcha3Validator($this->recaptcha, $enabled = true, $this->resolver);
+    }
+
+    public function testNullIsValid()
+    {
+        $this->validator->validate(null, new Recaptcha3());
+        $this->assertNoViolation();
+    }
+
+    public function testEmptyStringIsValid()
+    {
+        $this->validator->validate('', new Recaptcha3());
+        $this->assertNoViolation();
+    }
+
+    public function testValidIfNotEnabled()
+    {
+        $validator = new Recaptcha3Validator($this->recaptcha, $enabled = false, $this->resolver);
+        $this->recaptcha->nextSuccess = false;
+
+        $validator->validate('test', new Recaptcha3());
+        $this->assertNoViolation();
+    }
+
+    /**
+     * @expectedException \Symfony\Component\Validator\Exception\UnexpectedTypeException
+     */
+    public function testExpectsStringCompatibleType()
+    {
+        $this->validator->validate(new \stdClass(), new Recaptcha3());
+    }
+
+    public function testValidCase()
+    {
+        $this->recaptcha->nextSuccess = true;
+        $this->validator->validate('test', new Recaptcha3());
+        $this->assertNoViolation();
+    }
+
+    public function testInvalidCase()
+    {
+        $testToken = 'test-token';
+        $this->recaptcha->nextSuccess = false;
+        $this->validator->validate($testToken, new Recaptcha3(['message' => 'myMessage']));
+
+        $this->buildViolation('myMessage')
+            ->setParameter('{{ value }}', '"'.$testToken.'"')
+            ->setCode(Recaptcha3::INVALID_FORMAT_ERROR)
+            ->assertRaised();
+    }
+}

Validator/Constraints/Recaptcha3.php

@@ -7,7 +7,9 @@
 /**
  * @Annotation
  */
-class Recaptcha3 extends Constraint
+final class Recaptcha3 extends Constraint
 {
+    const INVALID_FORMAT_ERROR = '7147ffdb-0af4-4f7a-bd5e-e9dcfa6d7a2d';
+
     public $message = 'Your computer or network may be sending automated queries';
 }

Validator/Constraints/Recaptcha3Validator.php

@@ -2,46 +2,54 @@
 
 namespace Karser\Recaptcha3Bundle\Validator\Constraints;
 
+use Karser\Recaptcha3Bundle\Services\IpResolverInterface;
 use ReCaptcha\ReCaptcha;
-use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\ConstraintValidator;
 use Symfony\Component\Validator\Exception\UnexpectedTypeException;
 
-class Recaptcha3Validator extends ConstraintValidator
+final class Recaptcha3Validator extends ConstraintValidator
 {
     /** @var ReCaptcha */
     private $recaptcha;
 
     /** @var bool */
     private $enabled;
 
-    /** @var RequestStack */
-    private $requestStack;
+    /** @var IpResolverInterface */
+    private $ipResolver;
 
-    public function __construct($recaptcha, bool $enabled, RequestStack $requestStack)
+    public function __construct($recaptcha, bool $enabled, IpResolverInterface $ipResolver)
     {
         $this->recaptcha = $recaptcha;
         $this->enabled = $enabled;
-        $this->requestStack = $requestStack;
+        $this->ipResolver = $ipResolver;
     }
 
     public function validate($value, Constraint $constraint): void
     {
         if (!$constraint instanceof Recaptcha3) {
             throw new UnexpectedTypeException($constraint, Recaptcha3::class);
         }
+        if (null === $value || '' === $value) {
+            return;
+        }
+        if (!is_scalar($value) && !(\is_object($value) && method_exists($value, '__toString'))) {
+            throw new UnexpectedTypeException($value, 'string');
+        }
 
         if (!$this->enabled) {
             return;
         }
 
-        $request = $this->requestStack->getCurrentRequest();
-        $ip = $request ? $request->server->get('HTTP_CF_CONNECTING_IP') ?? $request->getClientIp() : null;
+        $ip = $this->ipResolver->resolveIp();
 
         $response = $this->recaptcha->verify($value, $ip);
         if (!$response->isSuccess()) {
-            $this->context->addViolation($constraint->message);
+            $this->context->buildViolation($constraint->message)
+                ->setParameter('{{ value }}', $this->formatValue($value))
+                ->setCode(Recaptcha3::INVALID_FORMAT_ERROR)
+                ->addViolation();
         }
     }
 }