Skip to content

Commit

Permalink
feature #35284 Simplify UriSigner when working with HttpFoundation's …
Browse files Browse the repository at this point in the history
…Request (Toflar)

This PR was squashed before being merged into the 5.1-dev branch (closes #35284).

Discussion
----------

Simplify UriSigner when working with HttpFoundation's Request

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        |

I'm using the `UriSigner` in my own projects from time to time and I've always wondered why I have to manually generate the URI from the `Request` instance in such a way that it is correctly validated.
Let's add a new `checkRequest(Request $request)` method to provide better DX.

Commits
-------

4887b4b Simplify UriSigner when working with HttpFoundation's Request
  • Loading branch information
fabpot committed Jan 10, 2020
2 parents f9949e3 + 4887b4b commit d099bc3
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 2 deletions.
Expand Up @@ -83,8 +83,7 @@ protected function validateRequest(Request $request)
}

// is the Request signed?
// we cannot use $request->getUri() here as we want to work with the original URI (no query string reordering)
if ($this->signer->check($request->getSchemeAndHttpHost().$request->getBaseUrl().$request->getPathInfo().(null !== ($qs = $request->server->get('QUERY_STRING')) ? '?'.$qs : ''))) {
if ($this->signer->checkRequest($request)) {
return;
}

Expand Down
10 changes: 10 additions & 0 deletions src/Symfony/Component/HttpKernel/Tests/UriSignerTest.php
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\HttpKernel\Tests;

use PHPUnit\Framework\TestCase;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\UriSigner;

class UriSignerTest extends TestCase
Expand Down Expand Up @@ -52,6 +53,15 @@ public function testCheckWithDifferentArgSeparator()
$this->assertTrue($signer->check($signer->sign('http://example.com/foo?foo=bar&baz=bay')));
}

public function testCheckWithRequest()
{
$signer = new UriSigner('foobar');

$this->assertTrue($signer->checkRequest(Request::create($signer->sign('http://example.com/foo'))));
$this->assertTrue($signer->checkRequest(Request::create($signer->sign('http://example.com/foo?foo=bar'))));
$this->assertTrue($signer->checkRequest(Request::create($signer->sign('http://example.com/foo?foo=bar&0=integer'))));
}

public function testCheckWithDifferentParameter()
{
$signer = new UriSigner('foobar', 'qux');
Expand Down
10 changes: 10 additions & 0 deletions src/Symfony/Component/HttpKernel/UriSigner.php
Expand Up @@ -11,6 +11,8 @@

namespace Symfony\Component\HttpKernel;

use Symfony\Component\HttpFoundation\Request;

/**
* Signs URIs.
*
Expand Down Expand Up @@ -78,6 +80,14 @@ public function check(string $uri)
return hash_equals($this->computeHash($this->buildUrl($url, $params)), $hash);
}

public function checkRequest(Request $request): bool
{
$qs = ($qs = $request->server->get('QUERY_STRING')) ? '?'.$qs : '';

// we cannot use $request->getUri() here as we want to work with the original URI (no query string reordering)
return $this->check($request->getSchemeAndHttpHost().$request->getBaseUrl().$request->getPathInfo().$qs);
}

private function computeHash(string $uri): string
{
return base64_encode(hash_hmac('sha256', $uri, $this->secret, true));
Expand Down

0 comments on commit d099bc3

Please sign in to comment.