Skip to content

Commit

Permalink
Bugs fixed
Browse files Browse the repository at this point in the history
  • Loading branch information
Spomky committed Jan 24, 2019
1 parent 2862793 commit d71e701
Show file tree
Hide file tree
Showing 11 changed files with 34 additions and 37 deletions.
1 change: 1 addition & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
parameters:
excludes_analyse:
- %currentWorkingDirectory%/src/*/tests/*
- %currentWorkingDirectory%/src/*/var/*
includes:
- vendor/phpstan/phpstan-strict-rules/rules.neon
- vendor/phpstan/phpstan-phpunit/extension.neon
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public function addConfiguration(NodeDefinition $node): void
->scalarNode('remember_me_parameter')->defaultNull()->end()
->scalarNode('username_parameter')->defaultValue('_username')->end()
->scalarNode('assertion_parameter')->defaultValue('_assertion')->end()
->scalarNode('assertion_session_parameter')->defaultValue('_webauthn.public_key_credential_request_options')->end()
->scalarNode('csrf_parameter')->defaultValue('_csrf_token')->end()
->scalarNode('csrf_token_id')->defaultValue('authenticate')->end()
->end()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,10 +342,10 @@ private function processWithUsername(Request $request): PreWebauthnToken
private function processWithAssertion(Request $request, PreWebauthnToken $token): WebauthnToken
{
$assertion = $request->request->get($this->options['assertion_parameter']);
$PublicKeyCredentialRequestOptions = $request->getSession()->get(WebauthnUtils::PUBLIC_KEY_CREDENTIAL_REQUEST_OPTIONS);
$PublicKeyCredentialRequestOptions = $request->getSession()->get($this->options['assertion_session_parameter']);

if (!$PublicKeyCredentialRequestOptions instanceof PublicKeyCredentialRequestOptions) {
throw new BadRequestHttpException('No public key credential request potions available for this session.');
throw new BadRequestHttpException('No public key credential request options available for this session.');
}
if (!\is_string($assertion)) {
throw new BadRequestHttpException(\Safe\sprintf('The key "%s" must be a string, "%s" given.', $this->options['assertion_parameter'], \gettype($assertion)));
Expand Down
27 changes: 2 additions & 25 deletions src/symfony-security/src/Security/WebauthnUtils.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
namespace Webauthn\SecurityBundle\Security;

use Assert\Assertion;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\Security\Core\Exception\AuthenticationException;
use Symfony\Component\Security\Core\User\UserInterface;
Expand All @@ -27,26 +26,18 @@

class WebauthnUtils
{
public const PUBLIC_KEY_CREDENTIAL_REQUEST_OPTIONS = '_webauthn.public_key_credential_request_options';

/**
* @var AuthenticationUtils
*/
private $authenticationUtils;

/**
* @var RequestStack
*/
private $requestStack;

/**
* @var PublicKeyCredentialRequestOptionsFactory
*/
private $publicKeyCredentialRequestOptionsFactory;

public function __construct(PublicKeyCredentialRequestOptionsFactory $publicKeyCredentialRequestOptionsFactory, RequestStack $requestStack)
{
$this->requestStack = $requestStack;
$this->authenticationUtils = new AuthenticationUtils($requestStack);
$this->publicKeyCredentialRequestOptionsFactory = $publicKeyCredentialRequestOptionsFactory;
}
Expand Down Expand Up @@ -74,18 +65,14 @@ public function generateRequest(UserInterface $user, int $challengeLength = 16,
Assertion::min($challengeLength, 1, 'Invalid challenge length');
$allowedCredentials = $this->getAllowedCredentials($user);

$request = $this->getRequest();
$publicKeyCredentialRequestOptions = new PublicKeyCredentialRequestOptions(
return new PublicKeyCredentialRequestOptions(
random_bytes($challengeLength),
$timeout,
$rpId ?? $request->getHost(),
$rpId,
$allowedCredentials,
$userVerification,
$extensions
);
$request->getSession()->set(self::PUBLIC_KEY_CREDENTIAL_REQUEST_OPTIONS, $publicKeyCredentialRequestOptions);

return $publicKeyCredentialRequestOptions;
}

/**
Expand All @@ -105,14 +92,4 @@ private function getAllowedCredentials(UserInterface $user): array

return $credentials;
}

private function getRequest(): Request
{
$request = $this->requestStack->getCurrentRequest();
if (null === $request) {
throw new \LogicException('Request should exist so it can be processed for error.');
}

return $request;
}
}
5 changes: 3 additions & 2 deletions src/symfony-security/tests/functional/SecurityController.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

namespace Webauthn\SecurityBundle\Tests\Functional;

use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\User\UserInterface;
Expand Down Expand Up @@ -56,13 +57,13 @@ public function login(): Response
return new Response($page);
}

public function assertion(): Response
public function assertion(Request $request): Response
{
/** @var UserInterface $user */
$user = $this->tokenStorage->getToken()->getUser();
$publicKeyCredentialRequestOptions = $this->webauthnUtils->generateRequest($user);
$request->getSession()->set('_webauthn.public_key_credential_request_options', $publicKeyCredentialRequestOptions);
$error = $this->webauthnUtils->getLastAuthenticationError();

$page = $this->twig->render('assertion.html.twig', [
'error' => $error,
'user' => $user,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ public function has(string $key): bool
return array_key_exists($key, $this->attStmt);
}

/**
* @return mixed
*/
public function get(string $key)
{
Assertion::true($this->has($key), \Safe\sprintf('The attestation statement has no key "%s".', $key));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ private function extractPublicKey(?string $publicKey): string
return "\x04".$publicKey[-2].$publicKey[-3];
}

private function checkCertificate(?string $publicKey): void
private function checkCertificate(string $publicKey): void
{
try {
$resource = \Safe\openssl_pkey_get_public($publicKey);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ class AuthenticationExtension implements \JsonSerializable
*/
private $value;

/**
* @param mixed $value
*/
public function __construct(string $name, $value)
{
$this->name = $name;
Expand All @@ -36,11 +39,17 @@ public function name(): string
return $this->name;
}

/**
* @return mixed
*/
public function value()
{
return $this->value;
}

/**
* @return mixed
*/
public function jsonSerialize()
{
return $this->value;
Expand Down
4 changes: 2 additions & 2 deletions src/webauthn/src/AuthenticatorAssertionResponseValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public function check(string $credentialId, AuthenticatorAssertionResponse $auth
$credentialPublicKey = $attestedCredentialData->getCredentialPublicKey();
Assertion::notNull($credentialPublicKey, 'No public key available.');

$credentialPublicKey = $this->decoder->decode(
$credentialPublicKeyStream = $this->decoder->decode(
new StringStream($credentialPublicKey)
);

Expand Down Expand Up @@ -109,7 +109,7 @@ public function check(string $credentialId, AuthenticatorAssertionResponse $auth
$getClientDataJSONHash = hash('sha256', $authenticatorAssertionResponse->getClientDataJSON()->getRawData(), true);

/* @see 7.2.16 */
$coseKey = $credentialPublicKey->getNormalizedData();
$coseKey = $credentialPublicKeyStream->getNormalizedData();
$key = "\04".$coseKey[-2].$coseKey[-3];
Assertion::eq(1, openssl_verify($authenticatorAssertionResponse->getAuthenticatorData()->getAuthData().$getClientDataJSONHash, $authenticatorAssertionResponse->getSignature(), $this->getPublicKeyAsPem($key), OPENSSL_ALGO_SHA256), 'Invalid signature.');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ public function check(AuthenticatorAttestationResponse $authenticatorAttestation

$parsedRelyingPartyId = parse_url($C->getOrigin());
Assertion::true(array_key_exists('host', $parsedRelyingPartyId) && \is_string($parsedRelyingPartyId['host']), 'Invalid origin rpId.');

Assertion::false(null !== $rpId && $parsedRelyingPartyId['host'] !== $rpId, 'rpId mismatch.');
Assertion::false($parsedRelyingPartyId['host'] !== $rpId, 'rpId mismatch.');

/* @see 7.1.6 */
if (null !== $C->getTokenBinding()) {
Expand Down Expand Up @@ -103,7 +102,10 @@ public function check(AuthenticatorAttestationResponse $authenticatorAttestation
/** @see 7.1.15 */
/** @see 7.1.16 */
/** @see 7.1.17 */
$credentialId = $attestationObject->getAuthData()->getAttestedCredentialData()->getCredentialId();
Assertion::true($attestationObject->getAuthData()->hasAttestedCredentialData(), 'There is no attested credential data.');
$attestedCredentialData = $attestationObject->getAuthData()->getAttestedCredentialData();
Assertion::notNull($attestedCredentialData, 'There is no attested credential data.');
$credentialId = $attestedCredentialData->getCredentialId();
Assertion::false($this->credentialRepository->has($credentialId), 'The credential ID already exists.');

/* @see 7.1.18 */
Expand Down
7 changes: 5 additions & 2 deletions src/webauthn/src/CollectedClientData.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class CollectedClientData
private $origin;

/**
* @var TokenBinding
* @var null|array
*/
private $tokenBinding;

Expand Down Expand Up @@ -85,7 +85,7 @@ public function getOrigin(): string

public function getTokenBinding(): ?TokenBinding
{
return $this->tokenBinding ? TokenBinding::createFormArray($this->tokenBinding) : null;
return null === $this->tokenBinding ? null : TokenBinding::createFormArray($this->tokenBinding);
}

public function getRawData(): string
Expand All @@ -106,6 +106,9 @@ public function has(string $key): bool
return array_key_exists($key, $this->data);
}

/**
* @return mixed
*/
public function get(string $key)
{
if (!$this->has($key)) {
Expand Down

0 comments on commit d71e701

Please sign in to comment.