Skip to content

Commit

Permalink
[11.x] Add support for previous apps keys in signed URL verification (#…
Browse files Browse the repository at this point in the history
…51222)

* Add support for previous app keys in signed URL verification

* Simplify array wrap and unwrap logic in UrlGenerator

* Fix style

* Remove double whitespace in comment

* formatting

---------

Co-authored-by: Taylor Otwell <[email protected]>
  • Loading branch information
Krisell and taylorotwell authored May 9, 2024
1 parent 2f8b95a commit 6659c30
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 12 deletions.
4 changes: 3 additions & 1 deletion src/Illuminate/Routing/RoutingServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ protected function registerUrlGenerator()
});

$url->setKeyResolver(function () {
return $this->app->make('config')->get('app.key');
$config = $this->app->make('config');

return [$config->get('app.key'), ...($config->get('app.previous_keys') ?? [])];
});

// If the route collection is "rebound", for example, when the routes stay
Expand Down
21 changes: 18 additions & 3 deletions src/Illuminate/Routing/UrlGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,11 @@ public function signedRoute($name, $parameters = [], $expiration = null, $absolu
$key = call_user_func($this->keyResolver);

return $this->route($name, $parameters + [
'signature' => hash_hmac('sha256', $this->route($name, $parameters, $absolute), $key),
'signature' => hash_hmac(
'sha256',
$this->route($name, $parameters, $absolute),
is_array($key) ? $key[0] : $key
),
], $absolute);
}

Expand Down Expand Up @@ -455,9 +459,20 @@ public function hasCorrectSignature(Request $request, $absolute = true, array $i

$original = rtrim($url.'?'.$queryString, '?');

$signature = hash_hmac('sha256', $original, call_user_func($this->keyResolver));
$keys = call_user_func($this->keyResolver);

return hash_equals($signature, (string) $request->query('signature', ''));
$keys = is_array($keys) ? $keys : [$keys];

foreach ($keys as $key) {
if (hash_equals(
hash_hmac('sha256', $original, $key),
(string) $request->query('signature', '')
)) {
return true;
}
}

return false;
}

/**
Expand Down
26 changes: 26 additions & 0 deletions tests/Integration/Routing/UrlSigningTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,32 @@ public function testItCanGenerateMiddlewareDefinitionViaStaticMethod()
$this->assertSame('Illuminate\Routing\Middleware\ValidateSignature:foo,bar', $signature);
}

public function testUrlsSignedByPreviousAppKeysAreValidWhenAddedAsPreviousKeys()
{
Route::get('/foo/{id}', function (Request $request, $id) {
return $request->hasValidSignature() ? 'valid' : 'invalid';
})->name('foo');

config(['app.key' => 'oldest-key']);
$oldestURL = URL::signedRoute('foo', ['id' => 1]);

config(['app.key' => 'old-key']);
$oldURL = URL::signedRoute('foo', ['id' => 1]);

config(['app.key' => 'new-key']);
$newUrl = URL::signedRoute('foo', ['id' => 1]);

tap($this->get($oldestURL), fn ($response) => $this->assertSame('invalid', $response->original));
tap($this->get($oldURL), fn ($response) => $this->assertSame('invalid', $response->original));
tap($this->get($newUrl), fn ($response) => $this->assertSame('valid', $response->original));

config(['app.previous_keys' => ['old-key', 'oldest-key']]);

tap($this->get($oldestURL), fn ($response) => $this->assertSame('valid', $response->original));
tap($this->get($oldURL), fn ($response) => $this->assertSame('valid', $response->original));
tap($this->get($newUrl), fn ($response) => $this->assertSame('valid', $response->original));
}

protected function createValidateSignatureMiddleware(array $ignore)
{
return new class($ignore) extends ValidateSignature
Expand Down
24 changes: 16 additions & 8 deletions tests/Routing/RoutingUrlGeneratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -894,32 +894,40 @@ public function testSignedUrlWithKeyResolver()
$request = Request::create('http://www.foo.com/')
);
$url->setKeyResolver(function () {
return 'secret';
return 'first-secret';
});

$route = new Route(['GET'], 'foo', ['as' => 'foo', function () {
//
}]);
$routes->add($route);

$request = Request::create($url->signedRoute('foo'));
$firstRequest = Request::create($url->signedRoute('foo'));

$this->assertTrue($url->hasValidSignature($request));
$this->assertTrue($url->hasValidSignature($firstRequest));

$request = Request::create($url->signedRoute('foo').'?tampered=true');

$this->assertFalse($url->hasValidSignature($request));

$url2 = $url->withKeyResolver(function () {
return 'other-secret';
return 'second-secret';
});

$this->assertFalse($url2->hasValidSignature($request));
$this->assertFalse($url2->hasValidSignature($firstRequest));

$request = Request::create($url2->signedRoute('foo'));
$secondRequest = Request::create($url2->signedRoute('foo'));

$this->assertTrue($url2->hasValidSignature($request));
$this->assertFalse($url->hasValidSignature($request));
$this->assertTrue($url2->hasValidSignature($secondRequest));
$this->assertFalse($url->hasValidSignature($secondRequest));

// Key resolver also supports multiple keys, for app key rotation via the config "app.previous_keys"
$url3 = $url->withKeyResolver(function () {
return ['first-secret', 'second-secret'];
});

$this->assertTrue($url3->hasValidSignature($firstRequest));
$this->assertTrue($url3->hasValidSignature($secondRequest));
}

public function testMissingNamedRouteResolution()
Expand Down

0 comments on commit 6659c30

Please sign in to comment.