Skip to content

Commit 9b68ae9

Browse files
asbiinasbiin
authored
fix: fix timeout and cache behavior (#500)
1 parent 1a4eaca commit 9b68ae9

11 files changed

+424
-15
lines changed

‎config/webauthn.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,10 +173,11 @@
173173
|--------------------------------------------------------------------------
174174
|
175175
| Time that the caller is willing to wait for the call to complete.
176+
| See https://webauthn-doc.spomky-labs.com/symfony-bundle/configuration-references#timeout
176177
|
177178
*/
178179

179-
'timeout' => 60000,
180+
'timeout' => null,
180181

181182
/*
182183
|--------------------------------------------------------------------------

‎src/Services/JsonWrapper.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ class JsonWrapper implements JsonSerializable
1818
* @param T $data
1919
*/
2020
public function __construct(
21-
private SerializerInterface $loader,
2221
public mixed $data
2322
) {}
2423

@@ -37,7 +36,7 @@ public function jsonSerialize(): array
3736
#[\Override]
3837
public function __toString()
3938
{
40-
return $this->loader->serialize($this->data, 'json', [
39+
return app(SerializerInterface::class)->serialize($this->data, 'json', [
4140
AbstractObjectNormalizer::SKIP_NULL_VALUES => true,
4241
JsonEncode::OPTIONS => JSON_THROW_ON_ERROR,
4342
]);

‎src/Services/Webauthn/CreationOptionsFactory.php‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
use LaravelWebauthn\Facades\Webauthn;
1111
use Webauthn\AuthenticatorSelectionCriteria;
1212
use Webauthn\PublicKeyCredentialCreationOptions as PublicKeyCredentialCreationOptionsBase;
13-
use Webauthn\PublicKeyCredentialDescriptor;
1413
use Webauthn\PublicKeyCredentialParameters;
1514
use Webauthn\PublicKeyCredentialRpEntity;
1615
use Webauthn\PublicKeyCredentialUserEntity;
@@ -51,7 +50,7 @@ public function __invoke(User $user): PublicKeyCredentialCreationOptions
5150
);
5251

5352
return tap(PublicKeyCredentialCreationOptions::create($publicKey), function (PublicKeyCredentialCreationOptions $result) use ($user): void {
54-
$this->cache->put($this->cacheKey($user), (string) $result, $this->timeout);
53+
$this->cache->put($this->cacheKey($user), (string) $result, $this->timeoutCache);
5554
});
5655
}
5756

@@ -74,10 +73,7 @@ private function getUserEntity(User $user): PublicKeyCredentialUserEntity
7473
private function createCredentialParameters(): array
7574
{
7675
return collect($this->algorithmManager->list())
77-
->map(fn ($algorithm): PublicKeyCredentialParameters => new PublicKeyCredentialParameters(
78-
PublicKeyCredentialDescriptor::CREDENTIAL_TYPE_PUBLIC_KEY,
79-
$algorithm
80-
))
76+
->map(fn (int $algorithm): PublicKeyCredentialParameters => PublicKeyCredentialParameters::createPk($algorithm))
8177
->toArray();
8278
}
8379

‎src/Services/Webauthn/CredentialAssertionValidator.php‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public function __invoke(?User $user, array $data): bool
4242
$this->getResponse($publicKeyCredential),
4343
$this->pullPublicKey($user),
4444
$this->request->host(),
45-
$user->getAuthIdentifier()
45+
optional($user)->getAuthIdentifier()
4646
);
4747

4848
return true;
@@ -60,10 +60,14 @@ protected function pullPublicKey(?User $user): PublicKeyCredentialRequestOptions
6060
$value = $this->cache->pull($this->cacheKey(null));
6161
}
6262

63+
if ($value === null) {
64+
abort(404, 'No public key credential found');
65+
}
66+
6367
return $this->loader->deserialize($value, PublicKeyCredentialRequestOptions::class, 'json');
6468
} catch (\Exception $e) {
6569
app('webauthn.log')->debug('Webauthn publickKey deserialize error', ['exception' => $e]);
66-
abort(404);
70+
abort(404, $e->getMessage());
6771
}
6872
}
6973

‎src/Services/Webauthn/CredentialAttestationValidator.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,14 @@ protected function pullPublicKey(User $user): PublicKeyCredentialCreationOptions
5151
try {
5252
$value = $this->cache->pull($this->cacheKey($user));
5353

54+
if ($value === null) {
55+
abort(404, 'No public key credential found');
56+
}
57+
5458
return $this->loader->deserialize($value, PublicKeyCredentialCreationOptions::class, 'json');
5559
} catch (\Exception $e) {
5660
app('webauthn.log')->debug('Webauthn publicKey deserialize error', ['exception' => $e]);
57-
abort(404);
61+
abort(404, $e->getMessage());
5862
}
5963
}
6064

‎src/Services/Webauthn/OptionsFactory.php‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,12 @@ abstract class OptionsFactory extends CredentialValidator
1616
/**
1717
* Timeout in milliseconds.
1818
*/
19-
protected int $timeout;
19+
protected ?int $timeout = null;
20+
21+
/**
22+
* Timeout in milliseconds.
23+
*/
24+
protected ?int $timeoutCache;
2025

2126
public function __construct(
2227
Request $request,
@@ -26,7 +31,13 @@ public function __construct(
2631
parent::__construct($request, $cache);
2732

2833
$this->challengeLength = (int) $config->get('webauthn.challenge_length', 32);
29-
$this->timeout = (int) $config->get('webauthn.timeout', 60000);
34+
$timeout = $config->get('webauthn.timeout');
35+
if ($timeout !== null) {
36+
$this->timeout = (int) $timeout;
37+
$this->timeoutCache = $this->timeout / 1000;
38+
} else {
39+
$this->timeoutCache = self::getDefaultTimeoutCache($config);
40+
}
3041
}
3142

3243
/**
@@ -38,4 +49,20 @@ protected function getChallenge(): string
3849
{
3950
return \random_bytes($this->challengeLength);
4051
}
52+
53+
/**
54+
* See https://webauthn-doc.spomky-labs.com/symfony-bundle/configuration-references#timeout
55+
*/
56+
private static function getDefaultTimeoutCache(Config $config): ?int
57+
{
58+
switch ($config->get('webauthn.user_verification', 'preferred')) {
59+
case 'discouraged':
60+
return 180;
61+
case 'preferred':
62+
case 'required':
63+
return 600;
64+
default:
65+
return null;
66+
}
67+
}
4168
}

‎src/Services/Webauthn/RequestOptionsFactory.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public function __invoke(?User $user): PublicKeyCredentialRequestOptions
4141
);
4242

4343
return tap(PublicKeyCredentialRequestOptions::create($publicKey), function (PublicKeyCredentialRequestOptions $result) use ($user): void {
44-
$this->cache->put($this->cacheKey($user), (string) $result, $this->timeout);
44+
$this->cache->put($this->cacheKey($user), (string) $result, $this->timeoutCache);
4545
});
4646
}
4747

‎tests/Unit/Services/Webauthn/CreationOptionsFactoryTest.php‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
<?php
2+
3+
namespace LaravelWebauthn\Tests\Unit\Services\Webauthn;
4+
5+
use Illuminate\Contracts\Cache\Repository as Cache;
6+
use Illuminate\Foundation\Testing\DatabaseTransactions;
7+
use LaravelWebauthn\Services\Webauthn\RequestOptionsFactory;
8+
use LaravelWebauthn\Tests\FeatureTestCase;
9+
use PHPUnit\Framework\Attributes\Test;
10+
use Webauthn\PublicKeyCredentialRequestOptions;
11+
12+
class CreationOptionsFactoryTest extends FeatureTestCase
13+
{
14+
use DatabaseTransactions;
15+
16+
#[Test]
17+
public function create_options_factory()
18+
{
19+
$user = $this->user();
20+
21+
$option = app(RequestOptionsFactory::class)($user);
22+
23+
$this->assertInstanceOf(PublicKeyCredentialRequestOptions::class, $option->data);
24+
$this->assertNotNull($option->data->challenge);
25+
$this->assertEquals(32, strlen($option->data->challenge));
26+
$this->assertNull($option->data->timeout);
27+
}
28+
29+
#[Test]
30+
public function create_options_factory_timeout()
31+
{
32+
config(['webauthn.timeout' => 300000]);
33+
34+
$user = $this->user();
35+
36+
$this->mock(Cache::class, function ($mock) {
37+
$mock->shouldReceive('put')
38+
->withArgs(function ($key, $value, $timeout) {
39+
$this->assertEquals(300, $timeout);
40+
41+
return true;
42+
})
43+
->andReturnTrue();
44+
});
45+
46+
$option = app(RequestOptionsFactory::class)($user);
47+
48+
$this->assertInstanceOf(PublicKeyCredentialRequestOptions::class, $option->data);
49+
$this->assertNotNull($option->data->challenge);
50+
$this->assertEquals(32, strlen($option->data->challenge));
51+
$this->assertEquals(300000, $option->data->timeout);
52+
}
53+
54+
#[Test]
55+
public function create_options_factory_discouraged()
56+
{
57+
config(['webauthn.user_verification' => 'discouraged']);
58+
59+
$user = $this->user();
60+
61+
$this->mock(Cache::class, function ($mock) {
62+
$mock->shouldReceive('put')
63+
->withArgs(function ($key, $value, $timeout) {
64+
$this->assertEquals(180, $timeout);
65+
66+
return true;
67+
})
68+
->andReturnTrue();
69+
});
70+
71+
$option = app(RequestOptionsFactory::class)($user);
72+
73+
$this->assertInstanceOf(PublicKeyCredentialRequestOptions::class, $option->data);
74+
$this->assertNotNull($option->data->challenge);
75+
$this->assertEquals(32, strlen($option->data->challenge));
76+
$this->assertNull($option->data->timeout);
77+
}
78+
79+
#[Test]
80+
public function create_options_factory_required()
81+
{
82+
config(['webauthn.user_verification' => 'required']);
83+
84+
$user = $this->user();
85+
86+
$this->mock(Cache::class, function ($mock) {
87+
$mock->shouldReceive('put')
88+
->withArgs(function ($key, $value, $timeout) {
89+
$this->assertEquals(600, $timeout);
90+
91+
return true;
92+
})
93+
->andReturnTrue();
94+
});
95+
96+
$option = app(RequestOptionsFactory::class)($user);
97+
98+
$this->assertInstanceOf(PublicKeyCredentialRequestOptions::class, $option->data);
99+
$this->assertNotNull($option->data->challenge);
100+
$this->assertEquals(32, strlen($option->data->challenge));
101+
$this->assertNull($option->data->timeout);
102+
}
103+
104+
#[Test]
105+
public function create_options_factory_preferred()
106+
{
107+
config(['webauthn.user_verification' => 'preferred']);
108+
109+
$user = $this->user();
110+
111+
$this->mock(Cache::class, function ($mock) {
112+
$mock->shouldReceive('put')
113+
->withArgs(function ($key, $value, $timeout) {
114+
$this->assertEquals(600, $timeout);
115+
116+
return true;
117+
})
118+
->andReturnTrue();
119+
});
120+
121+
$option = app(RequestOptionsFactory::class)($user);
122+
123+
$this->assertInstanceOf(PublicKeyCredentialRequestOptions::class, $option->data);
124+
$this->assertNotNull($option->data->challenge);
125+
$this->assertEquals(32, strlen($option->data->challenge));
126+
$this->assertNull($option->data->timeout);
127+
}
128+
}

0 commit comments

Comments
 (0)