From f653f3332d89190554fa838a2856511b109fbb3e Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Mon, 12 Mar 2018 15:28:24 -0500 Subject: [PATCH 01/11] implement signed route URLs --- .../Routing/RoutingServiceProvider.php | 7 +++ src/Illuminate/Routing/UrlGenerator.php | 52 +++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/src/Illuminate/Routing/RoutingServiceProvider.php b/src/Illuminate/Routing/RoutingServiceProvider.php index 7dc496b68004..74b4a1fbe58f 100755 --- a/src/Illuminate/Routing/RoutingServiceProvider.php +++ b/src/Illuminate/Routing/RoutingServiceProvider.php @@ -62,10 +62,17 @@ protected function registerUrlGenerator() ) ); + // Next we will set a few service resolvers on the URL generator so it can + // get the information it needs to function. This just provides some of + // the convenience features to this URL generator like "signed" URLs. $url->setSessionResolver(function () { return $this->app['session']; }); + $url->setKeyResolver(function () { + return $this->app->make('config')->get('app.key'); + }); + // If the route collection is "rebound", for example, when the routes stay // cached for the application, we will need to rebind the routes on the // URL generator instance so it has the latest version of the routes. diff --git a/src/Illuminate/Routing/UrlGenerator.php b/src/Illuminate/Routing/UrlGenerator.php index bf1839b3f5f6..9f284b7e77e5 100755 --- a/src/Illuminate/Routing/UrlGenerator.php +++ b/src/Illuminate/Routing/UrlGenerator.php @@ -3,6 +3,7 @@ namespace Illuminate\Routing; use Closure; +use DateTimeInterface; use Illuminate\Support\Arr; use Illuminate\Support\Str; use Illuminate\Http\Request; @@ -71,6 +72,13 @@ class UrlGenerator implements UrlGeneratorContract */ protected $sessionResolver; + /** + * The encryption key resolver callable. + * + * @var callable + */ + protected $keyResolver; + /** * The callback to use to format hosts. * @@ -286,6 +294,37 @@ public function formatScheme($secure) return $this->cachedSchema; } + /** + * Create a signed route URL for a named route. + * + * @param string $name + * @param array $parameters + * @return string + */ + public function signedRoute($name, $parameters = []) + { + $key = call_user_func($this->keyResolver); + + return $this->route($name, $parameters + [ + 'signature' => hash_hmac('sha256', $this->route($name, $parameters), $key), + ]); + } + + /** + * Create a temporary signed route URL for a named route. + * + * @param string $name + * @param \DateTimeInterface $expiration + * @param array $parameters + * @return string + */ + public function temporarySignedRoute($name, DateTimeInterface $expiration, $parameters = []) + { + return $this->signedRoute( + $name, $parameters + ['expires' => $expiration->getTimestamp()] + ); + } + /** * Get the URL to a named route. * @@ -614,6 +653,19 @@ public function setSessionResolver(callable $sessionResolver) return $this; } + /** + * Set the encryption key resolver. + * + * @param callable $keyResolver + * @return $this + */ + public function setKeyResolver(callable $keyResolver) + { + $this->keyResolver = $keyResolver; + + return $this; + } + /** * Set the root controller namespace. * From a77434152371e9487b4822d63ddedac48fd4b309 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Mon, 12 Mar 2018 15:28:30 -0500 Subject: [PATCH 02/11] add files --- .../Routing/Middleware/ValidateSignature.php | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 src/Illuminate/Routing/Middleware/ValidateSignature.php diff --git a/src/Illuminate/Routing/Middleware/ValidateSignature.php b/src/Illuminate/Routing/Middleware/ValidateSignature.php new file mode 100644 index 000000000000..ec9083abe9ee --- /dev/null +++ b/src/Illuminate/Routing/Middleware/ValidateSignature.php @@ -0,0 +1,61 @@ +config = $config; + } + + /** + * Handle an incoming request. + * + * @param \Illuminate\Http\Request $request + * @param \Closure $next + * @return \Illuminate\Http\Response + */ + public function handle($request, Closure $next) + { + $original = $request->url().'?'.http_build_query(Arr::except($request->query(), 'signature')); + + $hash = hash_hmac('sha256', $original, $this->config->get('app.key')); + + return $request->get('signature') === $hash && ! $this->expired($request) + ? $next($request) + : new Response('Invalid signature.', 401); + } + + /** + * Determine if the signed URL has expired. + * + * @param \Illuminate\Http\Request + * @return bool + */ + protected function expired($request) + { + $expires = Arr::get($request->query(), 'expires'); + + return $expires && Carbon::now()->getTimestamp() > $expires; + } +} From fc43c639792e7052106a3941fe5e594fe870eb8c Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Mon, 12 Mar 2018 15:55:42 -0500 Subject: [PATCH 03/11] adjust how signing works --- .../Providers/FoundationServiceProvider.php | 28 ++++++++++- .../Routing/Middleware/ValidateSignature.php | 46 ++----------------- 2 files changed, 31 insertions(+), 43 deletions(-) diff --git a/src/Illuminate/Foundation/Providers/FoundationServiceProvider.php b/src/Illuminate/Foundation/Providers/FoundationServiceProvider.php index fe384e757d0f..d49cbcd5f911 100644 --- a/src/Illuminate/Foundation/Providers/FoundationServiceProvider.php +++ b/src/Illuminate/Foundation/Providers/FoundationServiceProvider.php @@ -2,8 +2,10 @@ namespace Illuminate\Foundation\Providers; +use Illuminate\Support\Arr; use Illuminate\Support\Str; use Illuminate\Http\Request; +use Illuminate\Support\Carbon; use Illuminate\Support\AggregateServiceProvider; class FoundationServiceProvider extends AggregateServiceProvider @@ -26,7 +28,8 @@ public function register() { parent::register(); - $this->registerRequestValidate(); + $this->registerRequestValidation(); + $this->registerRequestSignatureValidation(); } /** @@ -34,7 +37,7 @@ public function register() * * @return void */ - public function registerRequestValidate() + public function registerRequestValidation() { Request::macro('validate', function (array $rules, ...$params) { validator()->validate($this->all(), $rules, ...$params); @@ -44,4 +47,25 @@ public function registerRequestValidate() })->unique()->toArray()); }); } + + /** + * Register the "hasValidSignature" macro on the request. + * + * @return void + */ + public function registerRequestSignatureValidation() + { + $app = $this->app; + + Request::macro('hasValidSignature', function () use ($app) { + $original = $this->url().'?'.http_build_query( + Arr::except($this->query(), 'signature') + ); + + $expires = Arr::get($this->query(), 'expires'); + + return $this->query('signature') === hash_hmac('sha256', $original, $app['config']['app.key']) && + ! ($expires && Carbon::now()->getTimestamp() > $expires); + }); + } } diff --git a/src/Illuminate/Routing/Middleware/ValidateSignature.php b/src/Illuminate/Routing/Middleware/ValidateSignature.php index ec9083abe9ee..5b8ea6f75e43 100644 --- a/src/Illuminate/Routing/Middleware/ValidateSignature.php +++ b/src/Illuminate/Routing/Middleware/ValidateSignature.php @@ -3,31 +3,10 @@ namespace Illuminate\Routing\Middleware; use Closure; -use Illuminate\Support\Arr; -use Illuminate\Http\Response; -use Illuminate\Support\Carbon; -use Illuminate\Contracts\Config\Repository as Config; +use Illuminate\Routing\Exceptions\InvalidSignatureException; class ValidateSignature { - /** - * The configuration repository implementation. - * - * @var \Illuminate\Contracts\Config\Repository - */ - protected $config; - - /** - * Create a new middleware instance. - * - * @param \Illuminate\Contracts\Config\Repository $config - * @return void - */ - public function __construct(Config $config) - { - $this->config = $config; - } - /** * Handle an incoming request. * @@ -37,25 +16,10 @@ public function __construct(Config $config) */ public function handle($request, Closure $next) { - $original = $request->url().'?'.http_build_query(Arr::except($request->query(), 'signature')); - - $hash = hash_hmac('sha256', $original, $this->config->get('app.key')); - - return $request->get('signature') === $hash && ! $this->expired($request) - ? $next($request) - : new Response('Invalid signature.', 401); - } - - /** - * Determine if the signed URL has expired. - * - * @param \Illuminate\Http\Request - * @return bool - */ - protected function expired($request) - { - $expires = Arr::get($request->query(), 'expires'); + if ($request->hasValidSignature($request)) { + return $next($request); + } - return $expires && Carbon::now()->getTimestamp() > $expires; + throw new InvalidSignatureException; } } From 13b4363e1c8751925de66a5accbae3cfbaafa717 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Mon, 12 Mar 2018 15:55:48 -0500 Subject: [PATCH 04/11] add files --- .../Exceptions/InvalidSignatureException.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 src/Illuminate/Routing/Exceptions/InvalidSignatureException.php diff --git a/src/Illuminate/Routing/Exceptions/InvalidSignatureException.php b/src/Illuminate/Routing/Exceptions/InvalidSignatureException.php new file mode 100644 index 000000000000..b79e433c6903 --- /dev/null +++ b/src/Illuminate/Routing/Exceptions/InvalidSignatureException.php @@ -0,0 +1,19 @@ + Date: Mon, 12 Mar 2018 15:56:15 -0500 Subject: [PATCH 05/11] add doc block --- src/Illuminate/Routing/Middleware/ValidateSignature.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Illuminate/Routing/Middleware/ValidateSignature.php b/src/Illuminate/Routing/Middleware/ValidateSignature.php index 5b8ea6f75e43..e0fa881059b3 100644 --- a/src/Illuminate/Routing/Middleware/ValidateSignature.php +++ b/src/Illuminate/Routing/Middleware/ValidateSignature.php @@ -13,6 +13,8 @@ class ValidateSignature * @param \Illuminate\Http\Request $request * @param \Closure $next * @return \Illuminate\Http\Response + * + * @throws \Illuminate\Routing\Exceptions\InvalidSignatureException */ public function handle($request, Closure $next) { From 17c750e4b7d627d649b31652d3ad8ef88ec2510b Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Mon, 12 Mar 2018 20:21:04 -0500 Subject: [PATCH 06/11] adjust approach to signing --- .../Providers/FoundationServiceProvider.php | 14 ++------ .../Routing/ValidateRequestSignature.php | 0 src/Illuminate/Routing/UrlGenerator.php | 32 ++++++++++++++++--- 3 files changed, 31 insertions(+), 15 deletions(-) create mode 100644 src/Illuminate/Foundation/Routing/ValidateRequestSignature.php diff --git a/src/Illuminate/Foundation/Providers/FoundationServiceProvider.php b/src/Illuminate/Foundation/Providers/FoundationServiceProvider.php index d49cbcd5f911..2f466365412e 100644 --- a/src/Illuminate/Foundation/Providers/FoundationServiceProvider.php +++ b/src/Illuminate/Foundation/Providers/FoundationServiceProvider.php @@ -6,6 +6,7 @@ use Illuminate\Support\Str; use Illuminate\Http\Request; use Illuminate\Support\Carbon; +use Illuminate\Support\Facades\URL; use Illuminate\Support\AggregateServiceProvider; class FoundationServiceProvider extends AggregateServiceProvider @@ -55,17 +56,8 @@ public function registerRequestValidation() */ public function registerRequestSignatureValidation() { - $app = $this->app; - - Request::macro('hasValidSignature', function () use ($app) { - $original = $this->url().'?'.http_build_query( - Arr::except($this->query(), 'signature') - ); - - $expires = Arr::get($this->query(), 'expires'); - - return $this->query('signature') === hash_hmac('sha256', $original, $app['config']['app.key']) && - ! ($expires && Carbon::now()->getTimestamp() > $expires); + Request::macro('hasValidSignature', function () { + return URL::hasValidSignature($this); }); } } diff --git a/src/Illuminate/Foundation/Routing/ValidateRequestSignature.php b/src/Illuminate/Foundation/Routing/ValidateRequestSignature.php new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/src/Illuminate/Routing/UrlGenerator.php b/src/Illuminate/Routing/UrlGenerator.php index 9f284b7e77e5..2980d11459b0 100755 --- a/src/Illuminate/Routing/UrlGenerator.php +++ b/src/Illuminate/Routing/UrlGenerator.php @@ -8,6 +8,7 @@ use Illuminate\Support\Str; use Illuminate\Http\Request; use InvalidArgumentException; +use Illuminate\Support\Carbon; use Illuminate\Support\Traits\Macroable; use Illuminate\Contracts\Routing\UrlRoutable; use Illuminate\Contracts\Routing\UrlGenerator as UrlGeneratorContract; @@ -299,10 +300,15 @@ public function formatScheme($secure) * * @param string $name * @param array $parameters + * @param \DateTimeInterface $expiration * @return string */ - public function signedRoute($name, $parameters = []) + public function signed($name, $parameters = [], DateTimeInterface $expiration = null) { + if ($expiration) { + $parameters = $parameters + ['expires' => $expiration->getTimestamp()]; + } + $key = call_user_func($this->keyResolver); return $this->route($name, $parameters + [ @@ -318,11 +324,29 @@ public function signedRoute($name, $parameters = []) * @param array $parameters * @return string */ - public function temporarySignedRoute($name, DateTimeInterface $expiration, $parameters = []) + public function temporarySigned($name, DateTimeInterface $expiration, $parameters = []) + { + return $this->signed($name, $parameters, $expiration); + } + + /** + * Determine if the given request has a valid signature. + * + * @param \Illuminate\Http\Request $request + * @return bool + */ + public function hasValidSignature(Request $request) { - return $this->signedRoute( - $name, $parameters + ['expires' => $expiration->getTimestamp()] + $original = $request->url().'?'.http_build_query( + Arr::except($request->query(), 'signature') ); + + $expires = Arr::get($request->query(), 'expires'); + + $signature = hash_hmac('sha256', $original, call_user_func($this->keyResolver)); + + return $request->query('signature') === $signature && + ! ($expires && Carbon::now()->getTimestamp() > $expires); } /** From dc58ee1670662643d06a1fe20b7183a4dfce2ebd Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Tue, 13 Mar 2018 09:31:05 -0500 Subject: [PATCH 07/11] add tests --- src/Illuminate/Routing/UrlGenerator.php | 10 +-- tests/Integration/Routing/UrlSigningTest.php | 65 ++++++++++++++++++++ 2 files changed, 70 insertions(+), 5 deletions(-) create mode 100644 tests/Integration/Routing/UrlSigningTest.php diff --git a/src/Illuminate/Routing/UrlGenerator.php b/src/Illuminate/Routing/UrlGenerator.php index 2980d11459b0..f3add3a488f0 100755 --- a/src/Illuminate/Routing/UrlGenerator.php +++ b/src/Illuminate/Routing/UrlGenerator.php @@ -303,7 +303,7 @@ public function formatScheme($secure) * @param \DateTimeInterface $expiration * @return string */ - public function signed($name, $parameters = [], DateTimeInterface $expiration = null) + public function signedRoute($name, $parameters = [], DateTimeInterface $expiration = null) { if ($expiration) { $parameters = $parameters + ['expires' => $expiration->getTimestamp()]; @@ -324,9 +324,9 @@ public function signed($name, $parameters = [], DateTimeInterface $expiration = * @param array $parameters * @return string */ - public function temporarySigned($name, DateTimeInterface $expiration, $parameters = []) + public function temporarySignedRoute($name, DateTimeInterface $expiration, $parameters = []) { - return $this->signed($name, $parameters, $expiration); + return $this->signedRoute($name, $parameters, $expiration); } /** @@ -337,9 +337,9 @@ public function temporarySigned($name, DateTimeInterface $expiration, $parameter */ public function hasValidSignature(Request $request) { - $original = $request->url().'?'.http_build_query( + $original = rtrim($request->url().'?'.http_build_query( Arr::except($request->query(), 'signature') - ); + ), '?'); $expires = Arr::get($request->query(), 'expires'); diff --git a/tests/Integration/Routing/UrlSigningTest.php b/tests/Integration/Routing/UrlSigningTest.php new file mode 100644 index 000000000000..2ac0edd3f5e6 --- /dev/null +++ b/tests/Integration/Routing/UrlSigningTest.php @@ -0,0 +1,65 @@ +hasValidSignature() ? 'valid' : 'invalid'; + })->name('foo'); + + $this->assertTrue(is_string($url = URL::signedRoute('foo', ['id' => 1]))); + $this->assertEquals('valid', $this->get($url)->original); + } + + public function test_temporary_signed_urls() + { + Route::get('/foo/{id}', function (Request $request, $id) { + return $request->hasValidSignature() ? 'valid' : 'invalid'; + })->name('foo'); + + Carbon::setTestNow(Carbon::create(2018, 1, 1)); + $this->assertTrue(is_string($url = URL::temporarySignedRoute('foo', now()->addMinutes(5), ['id' => 1]))); + $this->assertEquals('valid', $this->get($url)->original); + + Carbon::setTestNow(Carbon::create(2018, 1, 1)->addMinutes(10)); + $this->assertEquals('invalid', $this->get($url)->original); + } + + public function test_signed_middleware() + { + Route::get('/foo/{id}', function (Request $request, $id) { + return $request->hasValidSignature() ? 'valid' : 'invalid'; + })->name('foo')->middleware(ValidateSignature::class); + + Carbon::setTestNow(Carbon::create(2018, 1, 1)); + $this->assertTrue(is_string($url = URL::temporarySignedRoute('foo', now()->addMinutes(5), ['id' => 1]))); + $this->assertEquals('valid', $this->get($url)->original); + } + + public function test_signed_middleware_with_invalid_url() + { + Route::get('/foo/{id}', function (Request $request, $id) { + return $request->hasValidSignature() ? 'valid' : 'invalid'; + })->name('foo')->middleware(ValidateSignature::class); + + Carbon::setTestNow(Carbon::create(2018, 1, 1)); + $this->assertTrue(is_string($url = URL::temporarySignedRoute('foo', now()->addMinutes(5), ['id' => 1]))); + Carbon::setTestNow(Carbon::create(2018, 1, 1)->addMinutes(10)); + + $response = $this->get($url); + $response->assertStatus(401); + } +} From 8d42d1836d11792318bad389fe76eaf895a50f32 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Tue, 13 Mar 2018 09:42:19 -0500 Subject: [PATCH 08/11] Apply fixes from StyleCI (#23518) --- .../Foundation/Providers/FoundationServiceProvider.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Illuminate/Foundation/Providers/FoundationServiceProvider.php b/src/Illuminate/Foundation/Providers/FoundationServiceProvider.php index 2f466365412e..31bd0d5ecdcb 100644 --- a/src/Illuminate/Foundation/Providers/FoundationServiceProvider.php +++ b/src/Illuminate/Foundation/Providers/FoundationServiceProvider.php @@ -2,10 +2,8 @@ namespace Illuminate\Foundation\Providers; -use Illuminate\Support\Arr; use Illuminate\Support\Str; use Illuminate\Http\Request; -use Illuminate\Support\Carbon; use Illuminate\Support\Facades\URL; use Illuminate\Support\AggregateServiceProvider; From 0584d60812246bc2743598afd99b65565724bef0 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Tue, 13 Mar 2018 09:44:18 -0500 Subject: [PATCH 09/11] remove old file --- src/Illuminate/Foundation/Routing/ValidateRequestSignature.php | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 src/Illuminate/Foundation/Routing/ValidateRequestSignature.php diff --git a/src/Illuminate/Foundation/Routing/ValidateRequestSignature.php b/src/Illuminate/Foundation/Routing/ValidateRequestSignature.php deleted file mode 100644 index e69de29bb2d1..000000000000 From 6b028a034d6735c7cfae39b4d9dbb740f69a18de Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Tue, 13 Mar 2018 09:47:10 -0500 Subject: [PATCH 10/11] use interacts with time trait --- src/Illuminate/Routing/UrlGenerator.php | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Illuminate/Routing/UrlGenerator.php b/src/Illuminate/Routing/UrlGenerator.php index f3add3a488f0..eaabc2c63aae 100755 --- a/src/Illuminate/Routing/UrlGenerator.php +++ b/src/Illuminate/Routing/UrlGenerator.php @@ -10,12 +10,13 @@ use InvalidArgumentException; use Illuminate\Support\Carbon; use Illuminate\Support\Traits\Macroable; +use Illuminate\Support\InteractsWithTime; use Illuminate\Contracts\Routing\UrlRoutable; use Illuminate\Contracts\Routing\UrlGenerator as UrlGeneratorContract; class UrlGenerator implements UrlGeneratorContract { - use Macroable; + use InteractsWithTime, Macroable; /** * The route collection. @@ -300,13 +301,13 @@ public function formatScheme($secure) * * @param string $name * @param array $parameters - * @param \DateTimeInterface $expiration + * @param \DateTimeInterface|int $expiration * @return string */ - public function signedRoute($name, $parameters = [], DateTimeInterface $expiration = null) + public function signedRoute($name, $parameters = [], $expiration = null) { if ($expiration) { - $parameters = $parameters + ['expires' => $expiration->getTimestamp()]; + $parameters = $parameters + ['expires' => $this->availableAt($expiration)]; } $key = call_user_func($this->keyResolver); @@ -320,11 +321,11 @@ public function signedRoute($name, $parameters = [], DateTimeInterface $expirati * Create a temporary signed route URL for a named route. * * @param string $name - * @param \DateTimeInterface $expiration + * @param \DateTimeInterface|int $expiration * @param array $parameters * @return string */ - public function temporarySignedRoute($name, DateTimeInterface $expiration, $parameters = []) + public function temporarySignedRoute($name, $expiration, $parameters = []) { return $this->signedRoute($name, $parameters, $expiration); } From c78e5aab0b82bbe49e95752b87e609aef64f398d Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Tue, 13 Mar 2018 09:47:29 -0500 Subject: [PATCH 11/11] Apply fixes from StyleCI (#23520) --- src/Illuminate/Routing/UrlGenerator.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Illuminate/Routing/UrlGenerator.php b/src/Illuminate/Routing/UrlGenerator.php index eaabc2c63aae..26a964b95e87 100755 --- a/src/Illuminate/Routing/UrlGenerator.php +++ b/src/Illuminate/Routing/UrlGenerator.php @@ -3,7 +3,6 @@ namespace Illuminate\Routing; use Closure; -use DateTimeInterface; use Illuminate\Support\Arr; use Illuminate\Support\Str; use Illuminate\Http\Request;