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.

Sign up for GitHub

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

Jump to bottom

[HttpKernel] Add temporary URI signed #51502

Merged
merged 1 commit into from Mar 17, 2024
+221 −11
Merged

[HttpKernel] Add temporary URI signed #51502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/Symfony/Component/HttpFoundation/CHANGELOG.md
View file
Open in desktop
Expand Up @@ -4,6 +4,9 @@ CHANGELOG
7.1
---

* Add optional `$expirationParameter` argument to `UriSigner::__construct()`
fabpot marked this conversation as resolved.
Show resolved Hide resolved
* Add optional `$expiration` argument to `UriSigner::sign()`
* Rename `$parameter` argument of `UriSigner::__construct()` to `$hashParameter`
* Add `UploadedFile::getClientOriginalPath()`

7.0
Expand Down
25 changes: 25 additions & 0 deletions src/Symfony/Component/HttpFoundation/Exception/ExceptionInterface.php
View file
Open in desktop
@@ -0,0 +1,25 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\HttpFoundation\Exception;

interface ExceptionInterface extends \Throwable
{
}
28 changes: 28 additions & 0 deletions src/Symfony/Component/HttpFoundation/Exception/LogicException.php
View file
Open in desktop
@@ -0,0 +1,28 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\HttpFoundation\Exception;

/**
* Base LogicException for Http Foundation component.
*/
class LogicException extends \LogicException implements ExceptionInterface
{
}
109 changes: 108 additions & 1 deletion src/Symfony/Component/HttpFoundation/Tests/UriSignerTest.php
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\HttpFoundation\Tests;

use PHPUnit\Framework\TestCase;
use Symfony\Component\HttpFoundation\Exception\LogicException;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\UriSigner;

Expand All @@ -24,21 +25,38 @@ public function testSign()
$this->assertStringContainsString('?_hash=', $signer->sign('http://example.com/foo'));
$this->assertStringContainsString('?_hash=', $signer->sign('http://example.com/foo?foo=bar'));
$this->assertStringContainsString('&foo=', $signer->sign('http://example.com/foo?foo=bar'));

$this->assertStringContainsString('?_expiration=', $signer->sign('http://example.com/foo', 1));
$this->assertStringContainsString('&_hash=', $signer->sign('http://example.com/foo', 1));
$this->assertStringContainsString('?_expiration=', $signer->sign('http://example.com/foo?foo=bar', 1));
$this->assertStringContainsString('&_hash=', $signer->sign('http://example.com/foo?foo=bar', 1));
$this->assertStringContainsString('&foo=', $signer->sign('http://example.com/foo?foo=bar', 1));
}

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

$this->assertFalse($signer->check('http://example.com/foo'));
$this->assertFalse($signer->check('http://example.com/foo?_hash=foo'));
$this->assertFalse($signer->check('http://example.com/foo?foo=bar&_hash=foo'));
$this->assertFalse($signer->check('http://example.com/foo?foo=bar&_hash=foo&bar=foo'));

$this->assertFalse($signer->check('http://example.com/foo?_expiration=4070908800'));
$this->assertFalse($signer->check('http://example.com/foo?_expiration=4070908800?_hash=foo'));
$this->assertFalse($signer->check('http://example.com/foo?_expiration=4070908800&foo=bar&_hash=foo'));
$this->assertFalse($signer->check('http://example.com/foo?_expiration=4070908800&foo=bar&_hash=foo&bar=foo'));

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

$this->assertTrue($signer->check($signer->sign('http://example.com/foo', new \DateTimeImmutable('2099-01-01 00:00:00'))));
$this->assertTrue($signer->check($signer->sign('http://example.com/foo?foo=bar', new \DateTimeImmutable('2099-01-01 00:00:00'))));
$this->assertTrue($signer->check($signer->sign('http://example.com/foo?foo=bar&0=integer', new \DateTimeImmutable('2099-01-01 00:00:00'))));

$this->assertSame($signer->sign('http://example.com/foo?foo=bar&bar=foo'), $signer->sign('http://example.com/foo?bar=foo&foo=bar'));
$this->assertSame($signer->sign('http://example.com/foo?foo=bar&bar=foo', 1), $signer->sign('http://example.com/foo?bar=foo&foo=bar', 1));
}

public function testCheckWithDifferentArgSeparator()
Expand All @@ -51,6 +69,12 @@ public function testCheckWithDifferentArgSeparator()
$signer->sign('http://example.com/foo?foo=bar&baz=bay')
);
$this->assertTrue($signer->check($signer->sign('http://example.com/foo?foo=bar&baz=bay')));

$this->assertSame(
'http://example.com/foo?_expiration=4070908800&_hash=xfui5FoP0vbD9Cp7pI0tHnqR1Fmj2UARqkIUw7SZVfQ%3D&baz=bay&foo=bar',
$signer->sign('http://example.com/foo?foo=bar&baz=bay', new \DateTimeImmutable('2099-01-01 00:00:00'))
);
$this->assertTrue($signer->check($signer->sign('http://example.com/foo?foo=bar&baz=bay', new \DateTimeImmutable('2099-01-01 00:00:00'))));
}

public function testCheckWithRequest()
Expand All @@ -60,17 +84,27 @@ public function testCheckWithRequest()
$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'))));

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

public function testCheckWithDifferentParameter()
{
$signer = new UriSigner('foobar', 'qux');
$signer = new UriSigner('foobar', 'qux', 'abc');

$this->assertSame(
'http://example.com/foo?baz=bay&foo=bar&qux=rIOcC%2FF3DoEGo%2FvnESjSp7uU9zA9S%2F%2BOLhxgMexoPUM%3D',
$signer->sign('http://example.com/foo?foo=bar&baz=bay')
);
$this->assertTrue($signer->check($signer->sign('http://example.com/foo?foo=bar&baz=bay')));

$this->assertSame(
'http://example.com/foo?abc=4070908800&baz=bay&foo=bar&qux=hdhUhBVPpzKJdz5ZjC%2FkLvtOYdGKOvKVOczmmMIZK0A%3D',
$signer->sign('http://example.com/foo?foo=bar&baz=bay', new \DateTimeImmutable('2099-01-01 00:00:00'))
);
$this->assertTrue($signer->check($signer->sign('http://example.com/foo?foo=bar&baz=bay', new \DateTimeImmutable('2099-01-01 00:00:00'))));
}

public function testSignerWorksWithFragments()
Expand All @@ -81,6 +115,79 @@ public function testSignerWorksWithFragments()
'http://example.com/foo?_hash=EhpAUyEobiM3QTrKxoLOtQq5IsWyWedoXDPqIjzNj5o%3D&bar=foo&foo=bar#foobar',
$signer->sign('http://example.com/foo?bar=foo&foo=bar#foobar')
);

$this->assertTrue($signer->check($signer->sign('http://example.com/foo?bar=foo&foo=bar#foobar')));

$this->assertSame(
'http://example.com/foo?_expiration=4070908800&_hash=qHl626U5d7LMsVtBxPt9GNzysdSxyOQ1fHA59Y1ib0Y%3D&bar=foo&foo=bar#foobar',
$signer->sign('http://example.com/foo?bar=foo&foo=bar#foobar', new \DateTimeImmutable('2099-01-01 00:00:00'))
);

$this->assertTrue($signer->check($signer->sign('http://example.com/foo?bar=foo&foo=bar#foobar', new \DateTimeImmutable('2099-01-01 00:00:00'))));
}

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

$this->assertSame($signer->sign('http://example.com/foo?foo=bar&bar=foo', new \DateTimeImmutable('2099-01-01 00:00:00')), $signer->sign('http://example.com/foo?bar=foo&foo=bar', 4070908800));
}

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

$this->expectException(LogicException::class);

$signer->sign('http://example.com/foo?_hash=bar');
}

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

$this->expectException(LogicException::class);

$signer->sign('http://example.com/foo?_expiration=4070908800');
}

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

$this->expectException(LogicException::class);

$signer->sign('http://example.com/foo?_hash=bar', new \DateTimeImmutable('2099-01-01 00:00:00'));
}

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

$this->expectException(LogicException::class);

$signer->sign('http://example.com/foo?_expiration=4070908800', new \DateTimeImmutable('2099-01-01 00:00:00'));
}

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

$this->assertFalse($signer->check($signer->sign('http://example.com/foo', new \DateTimeImmutable('2000-01-01 00:00:00'))));
$this->assertFalse($signer->check($signer->sign('http://example.com/foo?foo=bar', new \DateTimeImmutable('2000-01-01 00:00:00'))));
$this->assertFalse($signer->check($signer->sign('http://example.com/foo?foo=bar&0=integer', new \DateTimeImmutable('2000-01-01 00:00:00'))));

$this->assertFalse($signer->check($signer->sign('http://example.com/foo', 1577836800))); // 2000-01-01
$this->assertFalse($signer->check($signer->sign('http://example.com/foo?foo=bar', 1577836800))); // 2000-01-01
$this->assertFalse($signer->check($signer->sign('http://example.com/foo?foo=bar&0=integer', 1577836800))); // 2000-01-01

$relativeUriFromNow1 = $signer->sign('http://example.com/foo', new \DateInterval('PT3S'));
$relativeUriFromNow2 = $signer->sign('http://example.com/foo?foo=bar', new \DateInterval('PT3S'));
$relativeUriFromNow3 = $signer->sign('http://example.com/foo?foo=bar&0=integer', new \DateInterval('PT3S'));
sleep(10);

$this->assertFalse($signer->check($relativeUriFromNow1));
$this->assertFalse($signer->check($relativeUriFromNow2));
$this->assertFalse($signer->check($relativeUriFromNow3));
}
}
67 changes: 57 additions & 10 deletions src/Symfony/Component/HttpFoundation/UriSigner.php
View file
Open in desktop
Expand Up @@ -11,34 +11,47 @@

namespace Symfony\Component\HttpFoundation;

use Symfony\Component\HttpFoundation\Exception\LogicException;

/**
* @author Fabien Potencier <fabien@symfony.com>
*/
class UriSigner
{
private string $secret;
private string $parameter;
private string $hashParameter;
private string $expirationParameter;

/**
* @param string $parameter Query string parameter to use
* @param string $hashParameter Query string parameter to use
* @param string $expirationParameter Query string parameter to use for expiration
*/
public function __construct(#[\SensitiveParameter] string $secret, string $parameter = '_hash')
public function __construct(#[\SensitiveParameter] string $secret, string $hashParameter = '_hash', string $expirationParameter = '_expiration')
{
if (!$secret) {
throw new \InvalidArgumentException('A non-empty secret is required.');
}

$this->secret = $secret;
$this->parameter = $parameter;
$this->hashParameter = $hashParameter;
$this->expirationParameter = $expirationParameter;
}

/**
* Signs a URI.
*
* The given URI is signed by adding the query string parameter
* which value depends on the URI and the secret.
*
* @param \DateTimeInterface|\DateInterval|int|null $expiration The expiration for the given URI.
* If $expiration is a \DateTimeInterface, it's expected to be the exact date + time.
* If $expiration is a \DateInterval, the interval is added to "now" to get the date + time.
* If $expiration is an int, it's expected to be a timestamp in seconds of the exact date + time.
* If $expiration is null, no expiration.
*
* The expiration is added as a query string parameter.
*/
public function sign(string $uri): string
public function sign(string $uri, \DateTimeInterface|\DateInterval|int|null $expiration = null): string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed that this is a BC break. Instead, we should comment the argument on the signature and read it with func_get_arg (see on 6.4 for similar new arguments).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it a BC even if the default value does not change the old behavior of the method ?
Do you want another PR linked to a new issue to fix this ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a BC break for classes that extend this class. Yes, it'd be great to fix it, PR welcome :)

{
$url = parse_url($uri);
$params = [];
Expand All @@ -47,14 +60,27 @@ public function sign(string $uri): string
parse_str($url['query'], $params);
}

if (isset($params[$this->hashParameter])) {
throw new LogicException(sprintf('URI query parameter conflict: parameter name "%s" is reserved.', $this->hashParameter));
}

if (isset($params[$this->expirationParameter])) {
throw new LogicException(sprintf('URI query parameter conflict: parameter name "%s" is reserved.', $this->expirationParameter));
}

if (null !== $expiration) {
$params[$this->expirationParameter] = $this->getExpirationTime($expiration);
}

$uri = $this->buildUrl($url, $params);
$params[$this->parameter] = $this->computeHash($uri);
$params[$this->hashParameter] = $this->computeHash($uri);

return $this->buildUrl($url, $params);
}

/**
* Checks that a URI contains the correct hash.
* Also checks if the URI has not expired (If you used expiration during signing).
*/
public function check(string $uri): bool
{
Expand All @@ -65,14 +91,22 @@ public function check(string $uri): bool
parse_str($url['query'], $params);
}

if (empty($params[$this->parameter])) {
if (empty($params[$this->hashParameter])) {
return false;
}

$hash = $params[$this->parameter];
unset($params[$this->parameter]);
$hash = $params[$this->hashParameter];
unset($params[$this->hashParameter]);

return hash_equals($this->computeHash($this->buildUrl($url, $params)), $hash);
if (!hash_equals($this->computeHash($this->buildUrl($url, $params)), $hash)) {
return false;
}

if ($expiration = $params[$this->expirationParameter] ?? false) {
return time() < $expiration;
}

return true;
}

public function checkRequest(Request $request): bool
Expand Down Expand Up @@ -105,4 +139,17 @@ private function buildUrl(array $url, array $params = []): string

return $scheme.$user.$pass.$host.$port.$path.$query.$fragment;
}

private function getExpirationTime(\DateTimeInterface|\DateInterval|int $expiration): string
{
if ($expiration instanceof \DateTimeInterface) {
return $expiration->format('U');
}

if ($expiration instanceof \DateInterval) {
return \DateTimeImmutable::createFromFormat('U', time())->add($expiration)->format('U');
}

return (string) $expiration;
}
}