Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decouple HTTP client and validator so we can verify responses outside Forms #7

Merged
merged 3 commits into from
May 24, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -11,7 +11,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3

- name: Setup PHP, with composer and extensions
uses: shivammathur/setup-php@v2 #https://github.com/shivammathur/setup-php
with:
@@ -31,9 +31,12 @@ jobs:

- name: Check for vulnerabilities
uses: symfonycorp/security-checker-action@v4

- name: Run Easy Coding Standard
run: vendor/bin/ecs check src
run: vendor/bin/ecs check

- name: Run phpstan
run: vendor/bin/phpstan analyse
run: vendor/bin/phpstan analyse -c phpstan.neon src

- name: Run tests
run: vendor/bin/phpunit --configuration=./tests/phpunit.xml.dist
8 changes: 7 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
@@ -28,10 +28,16 @@
"PixelOpen\\CloudflareTurnstileBundle\\": "src/"
}
},
"autoload-dev": {
"psr-4": {
"PixelOpen\\CloudflareTurnstileBundle\\": "tests/"
}
},
"require-dev": {
"phpstan/phpstan": "^1.8",
"phpstan/phpstan-symfony": "^1.2",
"symplify/easy-coding-standard": "^11.1",
"rector/rector": "^0.14.5"
"rector/rector": "^0.14.5",
"phpunit/phpunit": "^9.6"
}
}
5 changes: 4 additions & 1 deletion ecs.php
Original file line number Diff line number Diff line change
@@ -7,7 +7,10 @@
use Symplify\EasyCodingStandard\ValueObject\Set\SetList;

return static function (ECSConfig $ecsConfig): void {
$ecsConfig->paths([__DIR__ . '/src']);
$ecsConfig->paths([
__DIR__ . '/src',
__DIR__ . '/tests',
]);
$ecsConfig->sets([SetList::PSR_12]);

$ecsConfig->ruleWithConfiguration(ArraySyntaxFixer::class, [
8 changes: 1 addition & 7 deletions phpstan.neon
Original file line number Diff line number Diff line change
@@ -2,12 +2,6 @@ includes:
- vendor/phpstan/phpstan-symfony/extension.neon

parameters:
paths:
- src
level: 7
excludes_analyse:
- %currentWorkingDirectory%/src/DependencyInjection/Configuration.php
- %currentWorkingDirectory%/vendor/*
- %currentWorkingDirectory%/Tests/*
ignoreErrors:
- '#^Access to an undefined property Symfony\\Component\\Validator\\Constraint\:\:\$message\.$#'
- '#^Access to an undefined property Symfony\\Component\\Validator\\Constraint\:\:\$message\.$#'
83 changes: 0 additions & 83 deletions src/Constraints/CloudflareTurnstileValidator.php

This file was deleted.

58 changes: 58 additions & 0 deletions src/Http/CloudflareTurnstileHttpClient.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php

declare(strict_types=1);

namespace PixelOpen\CloudflareTurnstileBundle\Http;

use Psr\Log\LoggerInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Contracts\HttpClient\Exception\ExceptionInterface;
use Symfony\Contracts\HttpClient\HttpClientInterface;

final class CloudflareTurnstileHttpClient
{
private const SITEVERIFY_ENDPOINT = 'https://challenges.cloudflare.com/turnstile/v0/siteverify';

private HttpClientInterface $httpClient;

private string $secret;

private LoggerInterface $logger;

public function __construct(string $secret, HttpClientInterface $httpClient, LoggerInterface $logger)
{
$this->secret = $secret;
$this->httpClient = $httpClient;
$this->logger = $logger;
}

public function verifyResponse(string $turnstileResponse): bool
{
$response = $this->httpClient->request(
Request::METHOD_POST,
self::SITEVERIFY_ENDPOINT,
[
'body' => [
'response' => $turnstileResponse,
'secret' => $this->secret,
],
]
);

try {
$content = $response->toArray();
} catch (ExceptionInterface $e) {
$this->logger->error(
\sprintf(
'Cloudflare Turnstile HTTP exception (%s) with a message: %s',
\get_class($e),
$e->getMessage(),
),
);

return false;
}

return \array_key_exists('success', $content) && $content['success'] === true;
}
}
11 changes: 8 additions & 3 deletions src/Resources/config/services.yml
Original file line number Diff line number Diff line change
@@ -6,9 +6,14 @@ services:
$key: '%pixelopen_cloudflare_turnstile.key%'
$enable: '%pixelopen_cloudflare_turnstile.enable%'
turnstile.validator:
class: PixelOpen\CloudflareTurnstileBundle\Constraints\CloudflareTurnstileValidator
class: PixelOpen\CloudflareTurnstileBundle\Validator\CloudflareTurnstileValidator
tags: ['validator.constraint_validator']
arguments:
$secret: '%pixelopen_cloudflare_turnstile.secret%'
$enable: '%pixelopen_cloudflare_turnstile.enable%'
autowire: true
$turnstileHttpClient: '@turnstile.http_client'
autowire: true
turnstile.http_client:
class: PixelOpen\CloudflareTurnstileBundle\Http\CloudflareTurnstileHttpClient
arguments:
$secret: '%pixelopen_cloudflare_turnstile.secret%'
autowire: true
2 changes: 1 addition & 1 deletion src/Type/TurnstileType.php
Original file line number Diff line number Diff line change
@@ -4,7 +4,7 @@

namespace PixelOpen\CloudflareTurnstileBundle\Type;

use PixelOpen\CloudflareTurnstileBundle\Constraints\CloudflareTurnstile;
use PixelOpen\CloudflareTurnstileBundle\Validator\CloudflareTurnstile;
use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\Extension\Core\Type\TextType;
use Symfony\Component\Form\FormInterface;
Original file line number Diff line number Diff line change
@@ -2,11 +2,11 @@

declare(strict_types=1);

namespace PixelOpen\CloudflareTurnstileBundle\Constraints;
namespace PixelOpen\CloudflareTurnstileBundle\Validator;

use Symfony\Component\Validator\Constraint;

class CloudflareTurnstile extends Constraint
final class CloudflareTurnstile extends Constraint
{
/**
* @var string
58 changes: 58 additions & 0 deletions src/Validator/CloudflareTurnstileValidator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php

declare(strict_types=1);

namespace PixelOpen\CloudflareTurnstileBundle\Validator;

use PixelOpen\CloudflareTurnstileBundle\Http\CloudflareTurnstileHttpClient;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\Validator\Constraint;
use Symfony\Component\Validator\ConstraintValidator;

final class CloudflareTurnstileValidator extends ConstraintValidator
{
private bool $enable;

private RequestStack $requestStack;

private CloudflareTurnstileHttpClient $turnstileHttpClient;

public function __construct(
bool $enable,
RequestStack $requestStack,
CloudflareTurnstileHttpClient $turnstileHttpClient
) {
$this->enable = $enable;
$this->requestStack = $requestStack;
$this->turnstileHttpClient = $turnstileHttpClient;
}

/**
* Checks if the passed value is valid.
*
* @param mixed $value The value that should be validated
* @param Constraint $constraint The constraint for the validation
*/
public function validate($value, Constraint $constraint): void
{
if ($this->enable === false) {
return;
}

$request = $this->requestStack->getCurrentRequest();
\assert($request !== null);
$turnstileResponse = (string) $request->request->get('cf-turnstile-response');

if ($turnstileResponse === '') {
$this->context->buildViolation($constraint->message)
->addviolation();

return;
}

if ($this->turnstileHttpClient->verifyResponse($turnstileResponse) === false) {
$this->context->buildViolation($constraint->message)
->addviolation();
}
}
}
97 changes: 97 additions & 0 deletions tests/Http/CloudflareTurnstileHttpClientTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
<?php

declare(strict_types=1);

namespace PixelOpen\CloudflareTurnstileBundle\Http;

use PHPUnit\Framework\TestCase;
use Psr\Log\NullLogger;
use Symfony\Contracts\HttpClient\HttpClientInterface;

final class CloudflareTurnstileHttpClientTest extends TestCase
{
private const DUMMY_SECRET = 'dummy-secret';

private const DUMMY_TURNSTILE_RESPONSE = 'dummy-response';

/**
* @dataProvider provideResponseContents
*/
public function testShouldVerifyResponse(bool $expectedVerificationResult, array $responseContent): void
{
$httpClientMock = $this->createMock(HttpClientInterface::class);
$httpClientMock
->expects(self::once())
->method('request')
->with(
'POST',
'https://challenges.cloudflare.com/turnstile/v0/siteverify',
[
'body' => [
'response' => 'dummy-response',
'secret' => 'dummy-secret',
],
],
)
->willReturn(new TestResponse($responseContent));

$testLogger = new NullLogger();
$turnstileHttpClient = new CloudflareTurnstileHttpClient(self::DUMMY_SECRET, $httpClientMock, $testLogger);

$actualVerificationResult = $turnstileHttpClient->verifyResponse(self::DUMMY_TURNSTILE_RESPONSE);

self::assertSame($expectedVerificationResult, $actualVerificationResult);
}

public static function provideResponseContents(): iterable
{
yield 'successful verification' => [
true, [
'success' => true,
]];
yield 'failed verification' => [
false, [
'success' => false,
]];
yield 'invalid response format' => [
false, [
'success' => 'true',
]];
yield 'invalid response format 2' => [
false, [
'success' => null,
]];
yield 'invalid response - missing success field' => [
false, [
'verified' => true,
]];
}

public function testShouldFailVerificationWhenHttpExceptionThrown(): void
{
$httpClientMock = $this->createMock(HttpClientInterface::class);
$httpClientMock
->expects(self::once())
->method('request')
->with(
'POST',
'https://challenges.cloudflare.com/turnstile/v0/siteverify',
[
'body' => [
'response' => 'dummy-response',
'secret' => 'dummy-secret',
],
],
)
->willReturn(new TestResponse([
'success' => true,
], 'throwException'));

$testLogger = new NullLogger();
$turnstileHttpClient = new CloudflareTurnstileHttpClient(self::DUMMY_SECRET, $httpClientMock, $testLogger);

$verificationResult = $turnstileHttpClient->verifyResponse(self::DUMMY_TURNSTILE_RESPONSE);

self::assertFalse($verificationResult);
}
}
55 changes: 55 additions & 0 deletions tests/Http/TestResponse.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<?php

declare(strict_types=1);

namespace PixelOpen\CloudflareTurnstileBundle\Http;

use Symfony\Component\HttpClient\Exception\JsonException;
use Symfony\Contracts\HttpClient\ResponseInterface;

final class TestResponse implements ResponseInterface
{
private string $content;

private array $contentAsArray;

public function __construct(array $contentAsArray = [], string $content = '')
{
$this->content = $content;
$this->contentAsArray = $contentAsArray;
}

public function getStatusCode(): int
{
return 200;
}

public function getHeaders(bool $throw = true): array
{
return [];
}

public function getContent(bool $throw = true): string
{
return $this->content;
}

public function cancel(): void
{
// do nothing
}

public function getInfo(?string $type = null): mixed
{
return [];
}

public function toArray(bool $throw = true): array
{
if ($throw && $this->content === 'throwException') {
throw new JsonException('test error');
}

return $this->contentAsArray;
}
}
18 changes: 18 additions & 0 deletions tests/phpunit.xml.dist
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?xml version="1.0" encoding="UTF-8"?>
<!-- https://phpunit.de/manual/current/en/appendixes.configuration.html -->
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.6/phpunit.xsd"
colors="true"
failOnRisky="true"
beStrictAboutChangesToGlobalState="true"
beStrictAboutOutputDuringTests="true"
failOnWarning="true"
convertDeprecationsToExceptions="true"
cacheResult="false"
>
<testsuites>
<testsuite name="unit">
<directory>./</directory>
</testsuite>
</testsuites>
</phpunit>