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.

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

[1.x] Fixes Relying Party custom ID #65

Merged
merged 1 commit into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -598,15 +598,17 @@ return [
The _Relying Party_ is just a way to uniquely identify your application in the user device:

* `name`: The name of the application. Defaults to the application name.
* `id`: An unique ID the application, like the site URL. If `null`, the device _may_ fill it internally, usually as the full domain.
* `id`: An unique ID the application, [recommended to be the site domain](https://www.w3.org/TR/webauthn-2/#rp-id). If `null`, the device _may_ fill it internally, usually as the full domain.

> Warning
>
> WebAuthn authentication only work on the top domain it was registered.

Instead of modifying the config file, you should use the environment variables to set the name and ID for WebAuthn.

```dotenv
WEBAUTHN_NAME=SecureBank
WEBAUTHN_ID=https://auth.securebank.com
WEBAUTHN_ID=auth.securebank.com
```

### Challenge configuration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
use Laragear\WebAuthn\Attestation\AuthenticatorData;
use Laragear\WebAuthn\Attestation\Validator\AttestationValidation;
use Laragear\WebAuthn\SharedPipes\CheckRelyingPartyHashSame as BaseCheckRelyingPartyHashSame;
use function parse_url;
use const PHP_URL_HOST;

/**
* 13. Verify that the rpIdHash in authData is the SHA-256 hash of the RP ID expected by the Relying Party.
Expand Down Expand Up @@ -35,6 +37,7 @@ protected function authenticatorData(AssertionValidation|AttestationValidation $
*/
protected function relyingPartyId(AssertionValidation|AttestationValidation $validation): string
{
return $this->config->get('webauthn.relying_party.id') ?? $this->config->get('app.url');
return $this->config->get('webauthn.relying_party.id')
?? parse_url($this->config->get('app.url'), PHP_URL_HOST);
}
}
4 changes: 3 additions & 1 deletion src/Attestation/Validator/Pipes/MakeWebAuthnCredential.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
use Laragear\WebAuthn\Exceptions\AttestationException;
use Laragear\WebAuthn\Exceptions\DataException;
use Ramsey\Uuid\Uuid;
use function parse_url;
use const PHP_URL_HOST;

/**
* @internal
Expand Down Expand Up @@ -41,7 +43,7 @@ public function handle(AttestationValidation $validation, Closure $next): mixed
'alias' => $validation->request->json('response.alias'),

'counter' => $validation->attestationObject->authenticatorData->counter,
'rp_id' => $this->config->get('webauthn.relying_party.id') ?? $this->config->get('app.url'),
'rp_id' => $this->config->get('webauthn.relying_party.id') ?? parse_url($this->config->get('app.url'), PHP_URL_HOST),
'origin' => $validation->clientDataJson->origin,
'transports' => $validation->request->json('response.transports'),
'aaguid' => Uuid::fromBytes($validation->attestationObject->authenticatorData->attestedCredentialData->aaguid),
Expand Down
14 changes: 14 additions & 0 deletions src/Models/WebAuthnCredential.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
use Illuminate\Database\Eloquent\Relations\MorphTo;
use Laragear\WebAuthn\Events\CredentialDisabled;
use Laragear\WebAuthn\Events\CredentialEnabled;
use function parse_url;
use const PHP_URL_HOST;

/**
* @mixin \Illuminate\Database\Eloquent\Builder
Expand Down Expand Up @@ -195,4 +197,16 @@ public function syncCounter(int $counter): void

$this->save();
}

/**
* Returns the RP ID attribute
*
* @param string $rpId
* @return string
*/
protected function getRpIdAttribute(string $rpId): string
{
// If the Relying Party is a URL, we will return the domain, otherwise, verbatim.
return ($domain = parse_url($rpId, PHP_URL_HOST)) ? $domain : $rpId;
}
}
6 changes: 1 addition & 5 deletions src/SharedPipes/CheckRelyingPartyHashSame.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
use Laragear\WebAuthn\Assertion\Validator\AssertionValidation;
use Laragear\WebAuthn\Attestation\AuthenticatorData;
use Laragear\WebAuthn\Attestation\Validator\AttestationValidation;
use function parse_url;
use const PHP_URL_HOST;

/**
* @internal
Expand Down Expand Up @@ -41,9 +39,7 @@ public function handle(AttestationValidation|AssertionValidation $validation, Cl
// This way we can get the app RP ID on attestation, and the Credential RP ID
// on assertion. The credential will have the same Relying Party ID on both
// the authenticator and the application so on assertion both should match.
$relyingParty = parse_url($this->relyingPartyId($validation), PHP_URL_HOST);

if ($this->authenticatorData($validation)->hasNotSameRPIdHash($relyingParty)) {
if ($this->authenticatorData($validation)->hasNotSameRPIdHash($this->relyingPartyId($validation))) {
static::throw($validation, 'Response has different Relying Party ID hash.');
}

Expand Down
9 changes: 5 additions & 4 deletions src/SharedPipes/CheckRelyingPartyIdContained.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,12 @@ public function handle(AttestationValidation|AssertionValidation $validation, Cl
static::throw($validation, 'Relying Party ID is invalid.');
}

$current = parse_url(
$this->config->get('webauthn.relying_party.id') ?? $this->config->get('app.url'), PHP_URL_HOST
);
// Get the current Relying Party ID for this server request. If is not set,
// fall back to extract the domain name from the application default URL.
$current = $this->config->get('webauthn.relying_party.id')
?? parse_url($this->config->get('app.url'), PHP_URL_HOST);

// Check the host is the same or is a subdomain of the current config domain.
// Check the host is the same or is a subdomain of the current domain.
if (hash_equals($current, $host) || Str::is("*.$current", $host)) {
return $next($validation);
}
Expand Down
8 changes: 4 additions & 4 deletions tests/Assertion/CreatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public function test_response_defaults_without_credentials(): void
'id' => 'test_id',
'user_id' => Uuid::NIL,
'counter' => 0,
'rp_id' => 'http://localhost',
'rp_id' => 'localhost',
'origin' => 'http://localhost:8000',
'aaguid' => Uuid::NIL,
'public_key' => 'test_key',
Expand Down Expand Up @@ -109,7 +109,7 @@ public function test_response_adds_accepted_credentials_if_there_is_credentials(
'id' => 'test_id',
'user_id' => Uuid::NIL,
'counter' => 0,
'rp_id' => 'http://localhost',
'rp_id' => 'localhost',
'origin' => 'http://localhost:8000',
'aaguid' => Uuid::NIL,
'public_key' => 'test_key',
Expand All @@ -134,7 +134,7 @@ public function test_response_doesnt_add_credentials_blacklisted(): void
'id' => 'test_id',
'user_id' => Uuid::NIL,
'counter' => 0,
'rp_id' => 'http://localhost',
'rp_id' => 'localhost',
'origin' => 'http://localhost:8000',
'aaguid' => Uuid::NIL,
'public_key' => 'test_key',
Expand Down Expand Up @@ -172,7 +172,7 @@ public function test_challenge_includes_accepted_credentials(): void
'id' => 'test_id',
'user_id' => Uuid::NIL,
'counter' => 0,
'rp_id' => 'http://localhost',
'rp_id' => 'localhost',
'origin' => 'http://localhost:8000',
'aaguid' => Uuid::NIL,
'public_key' => 'test_key',
Expand Down
2 changes: 1 addition & 1 deletion tests/Assertion/ValidationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ protected function setUp(): void
'authenticatable_id' => 1,
'user_id' => 'e8af6f703f8042aa91c30cf72289aa07',
'counter' => 0,
'rp_id' => 'http://localhost',
'rp_id' => 'localhost',
'origin' => 'http://localhost',
'aaguid' => Uuid::NIL,
'attestation_format' => 'none',
Expand Down
4 changes: 2 additions & 2 deletions tests/Attestation/ValidationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public function test_validates_attestation_and_instances_webauthn_credential():
'user_id' => $validation->credential->user_id,
'alias' => null,
'counter' => 0,
'rp_id' => 'http://localhost',
'rp_id' => 'localhost',
'origin' => 'http://localhost',
'transports' => null,
'aaguid' => Uuid::NIL,
Expand Down Expand Up @@ -603,7 +603,7 @@ public function test_credential_duplicate_check_fails_if_already_exists(): void
'authenticatable_id' => 1,
'user_id' => 'e8af6f703f8042aa91c30cf72289aa07',
'counter' => 0,
'rp_id' => 'http://localhost',
'rp_id' => 'localhost',
'origin' => 'http://localhost',
'aaguid' => Uuid::NIL,
'attestation_format' => 'none',
Expand Down
2 changes: 1 addition & 1 deletion tests/Auth/EloquentWebAuthnProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ protected function afterRefreshingDatabase(): void
'authenticatable_id' => 1,
'user_id' => 'e8af6f703f8042aa91c30cf72289aa07',
'counter' => 0,
'rp_id' => 'http://localhost',
'rp_id' => 'localhost',
'origin' => 'http://localhost',
'aaguid' => Uuid::NIL,
'attestation_format' => 'none',
Expand Down
2 changes: 1 addition & 1 deletion tests/Http/Requests/AssertedRequestTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ protected function afterRefreshingDatabase(): void
'authenticatable_id' => 1,
'user_id' => 'e8af6f703f8042aa91c30cf72289aa07',
'counter' => 0,
'rp_id' => 'http://localhost',
'rp_id' => 'localhost',
'origin' => 'http://localhost',
'aaguid' => Uuid::NIL,
'attestation_format' => 'none',
Expand Down
2 changes: 1 addition & 1 deletion tests/Http/Requests/AttestationRequestTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public function test_allows_duplicates(): void
'authenticatable_id' => 1,
'user_id' => 'e8af6f703f8042aa91c30cf72289aa07',
'counter' => 0,
'rp_id' => 'http://localhost',
'rp_id' => 'localhost',
'origin' => 'http://localhost',
'aaguid' => Uuid::NIL,
'attestation_format' => 'none',
Expand Down
12 changes: 11 additions & 1 deletion tests/Models/WebAuthnCredentialTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ protected function afterRefreshingDatabase(): void
'authenticatable_id' => 1,
'user_id' => 'e8af6f703f8042aa91c30cf72289aa07',
'counter' => 0,
'rp_id' => 'http://localhost',
'rp_id' => 'localhost',
'origin' => 'http://localhost',
'aaguid' => Uuid::NIL,
'attestation_format' => 'none',
Expand Down Expand Up @@ -153,4 +153,14 @@ public function test_shows_serializes_few_columns(): void
$json
);
}

public function test_parses_correct_rp_id_as_domain_if_stored_as_url(): void
{
WebAuthnCredential::query()->whereKey(FakeAuthenticator::CREDENTIAL_ID)
->update(['rp_id' => 'https://my.custom.url/great?something=foo']);

$credential = WebAuthnCredential::find(FakeAuthenticator::CREDENTIAL_ID);

static::assertSame('my.custom.url', $credential->rp_id);
}
}
18 changes: 9 additions & 9 deletions tests/WebAuthnAuthenticationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ protected function afterRefreshingDatabase(): void
'id' => 'test_id',
'user_id' => Uuid::NIL,
'counter' => 0,
'rp_id' => 'http://localhost',
'rp_id' => 'localhost',
'origin' => 'http://localhost:8000',
'aaguid' => Uuid::NIL,
'public_key' => 'test_key',
Expand All @@ -45,7 +45,7 @@ public function test_flushes_all_credentials(): void
'id' => 'test_id_2',
'user_id' => Uuid::NIL,
'counter' => 10,
'rp_id' => 'http://localhost',
'rp_id' => 'localhost',
'origin' => 'http://localhost:8000',
'aaguid' => Uuid::NIL,
'public_key' => 'test_key',
Expand All @@ -64,7 +64,7 @@ public function test_flushes_all_credentials_using_loaded_relation(): void
'id' => 'test_id_2',
'user_id' => Uuid::NIL,
'counter' => 10,
'rp_id' => 'http://localhost',
'rp_id' => 'localhost',
'origin' => 'http://localhost:8000',
'aaguid' => Uuid::NIL,
'public_key' => 'test_key',
Expand All @@ -89,7 +89,7 @@ public function test_flushes_all_credentials_except_given_id(): void
'id' => 'test_id_2',
'user_id' => Uuid::NIL,
'counter' => 10,
'rp_id' => 'http://localhost',
'rp_id' => 'localhost',
'origin' => 'http://localhost:8000',
'aaguid' => Uuid::NIL,
'public_key' => 'test_key',
Expand All @@ -111,7 +111,7 @@ public function test_flushes_all_credentials_using_loaded_relation_except_given_
'id' => 'test_id_2',
'user_id' => Uuid::NIL,
'counter' => 10,
'rp_id' => 'http://localhost',
'rp_id' => 'localhost',
'origin' => 'http://localhost:8000',
'aaguid' => Uuid::NIL,
'public_key' => 'test_key',
Expand Down Expand Up @@ -142,7 +142,7 @@ public function test_disables_all_credentials(): void
'id' => 'test_id_2',
'user_id' => Uuid::NIL,
'counter' => 10,
'rp_id' => 'http://localhost',
'rp_id' => 'localhost',
'origin' => 'http://localhost:8000',
'aaguid' => Uuid::NIL,
'public_key' => 'test_key',
Expand Down Expand Up @@ -171,7 +171,7 @@ public function test_disables_all_credentials_with_loaded_relation(): void
'id' => 'test_id_2',
'user_id' => Uuid::NIL,
'counter' => 10,
'rp_id' => 'http://localhost',
'rp_id' => 'localhost',
'origin' => 'http://localhost:8000',
'aaguid' => Uuid::NIL,
'public_key' => 'test_key',
Expand Down Expand Up @@ -205,7 +205,7 @@ public function test_disables_all_credentials_except_one(): void
'id' => 'test_id_2',
'user_id' => Uuid::NIL,
'counter' => 10,
'rp_id' => 'http://localhost',
'rp_id' => 'localhost',
'origin' => 'http://localhost:8000',
'aaguid' => Uuid::NIL,
'public_key' => 'test_key',
Expand Down Expand Up @@ -233,7 +233,7 @@ public function test_disables_all_credentials_with_loaded_relation_except_one():
'id' => 'test_id_2',
'user_id' => Uuid::NIL,
'counter' => 10,
'rp_id' => 'http://localhost',
'rp_id' => 'localhost',
'origin' => 'http://localhost:8000',
'aaguid' => Uuid::NIL,
'public_key' => 'test_key',
Expand Down
Loading