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

Deprecate PKLoader in favor of Symfony Serializer #515

Merged
merged 4 commits into from
Nov 15, 2023
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
441 changes: 428 additions & 13 deletions phpstan-baseline.neon

Large diffs are not rendered by default.

11 changes: 9 additions & 2 deletions src/symfony/src/Controller/AssertionControllerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,18 @@ public function __construct(
private readonly SerializerInterface $serializer,
private readonly ValidatorInterface $validator,
private readonly PublicKeyCredentialRequestOptionsFactory $publicKeyCredentialRequestOptionsFactory,
private readonly PublicKeyCredentialLoader $publicKeyCredentialLoader,
private readonly null|PublicKeyCredentialLoader $publicKeyCredentialLoader,
private readonly AuthenticatorAssertionResponseValidator $authenticatorAssertionResponseValidator,
private readonly PublicKeyCredentialUserEntityRepositoryInterface $publicKeyCredentialUserEntityRepository,
private readonly PublicKeyCredentialSourceRepository|PublicKeyCredentialSourceRepositoryInterface $publicKeyCredentialSourceRepository
) {
if ($this->publicKeyCredentialLoader !== null) {
trigger_deprecation(
'web-auth/webauthn-bundle',
'4.8.0',
'The argument "$publicKeyCredentialLoader" is deprecated since 4.5.0 and will be removed in 5.0.0. Please set null instead; the serializer will be used instead.'
);
}
$this->logger = new NullLogger();
}

Expand Down Expand Up @@ -111,7 +118,7 @@ public function createResponseController(
null|AuthenticatorAssertionResponseValidator $authenticatorAssertionResponseValidator = null,
): AssertionResponseController {
return new AssertionResponseController(
$this->publicKeyCredentialLoader,
$this->publicKeyCredentialLoader ?? $this->serializer,
$authenticatorAssertionResponseValidator ?? $this->authenticatorAssertionResponseValidator,
$this->logger,
$optionStorage,
Expand Down
16 changes: 14 additions & 2 deletions src/symfony/src/Controller/AssertionResponseController.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
use Symfony\Component\Security\Core\Exception\AuthenticationException;
use Symfony\Component\Security\Http\Authentication\AuthenticationFailureHandlerInterface;
use Symfony\Component\Serializer\SerializerInterface;
use Throwable;
use Webauthn\AuthenticatorAssertionResponse;
use Webauthn\AuthenticatorAssertionResponseValidator;
Expand All @@ -18,6 +19,7 @@
use Webauthn\Bundle\Security\Handler\SuccessHandler;
use Webauthn\Bundle\Security\Storage\OptionsStorage;
use Webauthn\Exception\AuthenticatorResponseVerificationException;
use Webauthn\PublicKeyCredential;
use Webauthn\PublicKeyCredentialLoader;
use Webauthn\PublicKeyCredentialRequestOptions;

Expand All @@ -27,7 +29,7 @@ final class AssertionResponseController
* @param null|string[] $securedRelyingPartyIds
*/
public function __construct(
private readonly PublicKeyCredentialLoader $publicKeyCredentialLoader,
private readonly SerializerInterface|PublicKeyCredentialLoader $publicKeyCredentialLoader,
private readonly AuthenticatorAssertionResponseValidator $assertionResponseValidator,
private readonly LoggerInterface $logger,
private readonly OptionsStorage $optionsStorage,
Expand All @@ -36,6 +38,13 @@ public function __construct(
private readonly null|array $securedRelyingPartyIds = null,
private readonly ?PublicKeyCredentialSourceRepositoryInterface $publicKeyCredentialSourceRepository = null
) {
if ($this->publicKeyCredentialLoader instanceof PublicKeyCredentialLoader) {
trigger_deprecation(
'web-auth/webauthn-bundle',
'4.8.0',
'The argument "$publicKeyCredentialLoader" is deprecated since 4.8.0 and will be removed in 5.0.0. Please inject a Symfony Serializer instead.'
);
}
}

public function __invoke(Request $request): Response
Expand All @@ -47,7 +56,10 @@ public function __invoke(Request $request): Response
) ? $request->getContentTypeFormat() : $request->getContentType();
$format === 'json' || throw new BadRequestHttpException('Only JSON content type allowed');
$content = $request->getContent();
$publicKeyCredential = $this->publicKeyCredentialLoader->load($content);

$publicKeyCredential = $this->publicKeyCredentialLoader instanceof PublicKeyCredentialLoader ? $this->publicKeyCredentialLoader->load(
$content
) : $this->publicKeyCredentialLoader->deserialize($content, PublicKeyCredential::class, 'json');
$response = $publicKeyCredential->response;
$response instanceof AuthenticatorAssertionResponse || throw new BadRequestHttpException(
'Invalid response'
Expand Down
11 changes: 9 additions & 2 deletions src/symfony/src/Controller/AttestationControllerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,17 @@ public function __construct(
private readonly SerializerInterface $serializer,
private readonly ValidatorInterface $validator,
private readonly PublicKeyCredentialCreationOptionsFactory $publicKeyCredentialCreationOptionsFactory,
private readonly PublicKeyCredentialLoader $publicKeyCredentialLoader,
private readonly null|PublicKeyCredentialLoader $publicKeyCredentialLoader,
private readonly AuthenticatorAttestationResponseValidator $attestationResponseValidator,
private readonly PublicKeyCredentialSourceRepository|PublicKeyCredentialSourceRepositoryInterface $publicKeyCredentialSourceRepository
) {
if ($this->publicKeyCredentialLoader !== null) {
trigger_deprecation(
'web-auth/webauthn-bundle',
'4.8.0',
'The argument "$publicKeyCredentialLoader" is deprecated since 4.5.0 and will be removed in 5.0.0. Please set null instead; the serializer will be used instead.'
);
}
}

/**
Expand Down Expand Up @@ -98,7 +105,7 @@ public function createResponseController(
null|AuthenticatorAttestationResponseValidator $attestationResponseValidator = null,
): AttestationResponseController {
return new AttestationResponseController(
$this->publicKeyCredentialLoader,
$this->publicKeyCredentialLoader ?? $this->serializer,
$attestationResponseValidator ?? $this->attestationResponseValidator,
$this->publicKeyCredentialSourceRepository,
$optionStorage,
Expand Down
15 changes: 13 additions & 2 deletions src/symfony/src/Controller/AttestationResponseController.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
use Symfony\Component\Security\Core\Exception\AuthenticationException;
use Symfony\Component\Security\Http\Authentication\AuthenticationFailureHandlerInterface;
use Symfony\Component\Serializer\SerializerInterface;
use Throwable;
use Webauthn\AuthenticatorAttestationResponse;
use Webauthn\AuthenticatorAttestationResponseValidator;
Expand All @@ -19,6 +20,7 @@
use Webauthn\Bundle\Security\Handler\FailureHandler;
use Webauthn\Bundle\Security\Handler\SuccessHandler;
use Webauthn\Bundle\Security\Storage\OptionsStorage;
use Webauthn\PublicKeyCredential;
use Webauthn\PublicKeyCredentialCreationOptions;
use Webauthn\PublicKeyCredentialLoader;
use Webauthn\PublicKeyCredentialSourceRepository;
Expand All @@ -30,7 +32,7 @@ final class AttestationResponseController
* @param null|string[] $securedRelyingPartyIds
*/
public function __construct(
private readonly PublicKeyCredentialLoader $publicKeyCredentialLoader,
private readonly SerializerInterface|PublicKeyCredentialLoader $publicKeyCredentialLoader,
private readonly AuthenticatorAttestationResponseValidator $attestationResponseValidator,
private readonly PublicKeyCredentialSourceRepository|PublicKeyCredentialSourceRepositoryInterface $credentialSourceRepository,
private readonly OptionsStorage $optionStorage,
Expand All @@ -49,6 +51,13 @@ public function __construct(
)
);
}
if ($this->publicKeyCredentialLoader instanceof PublicKeyCredentialLoader) {
trigger_deprecation(
'web-auth/webauthn-bundle',
'4.8.0',
'The argument "$publicKeyCredentialLoader" is deprecated since 4.8.0 and will be removed in 5.0.0. Please inject a Symfony Serializer instead.'
);
}
}

public function __invoke(Request $request): Response
Expand All @@ -63,7 +72,9 @@ public function __invoke(Request $request): Response
) ? $request->getContentTypeFormat() : $request->getContentType();
$format === 'json' || throw new BadRequestHttpException('Only JSON content type allowed');
$content = $request->getContent();
$publicKeyCredential = $this->publicKeyCredentialLoader->load($content);
$publicKeyCredential = $this->publicKeyCredentialLoader instanceof PublicKeyCredentialLoader ? $this->publicKeyCredentialLoader->load(
$content
) : $this->publicKeyCredentialLoader->deserialize($content, PublicKeyCredential::class, 'json');
$response = $publicKeyCredential->response;
$response instanceof AuthenticatorAttestationResponse || throw new BadRequestHttpException(
'Invalid response'
Expand Down
3 changes: 1 addition & 2 deletions src/symfony/src/Resources/config/security.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
use Webauthn\Bundle\Security\Storage\CacheStorage;
use Webauthn\Bundle\Security\Storage\SessionStorage;
use Webauthn\Bundle\Security\WebauthnFirewallConfig;
use Webauthn\PublicKeyCredentialLoader;
use function Symfony\Component\DependencyInjection\Loader\Configurator\abstract_arg;
use function Symfony\Component\DependencyInjection\Loader\Configurator\service;

Expand Down Expand Up @@ -50,7 +49,7 @@
abstract_arg('Options Storage'),
service(PublicKeyCredentialSourceRepositoryInterface::class),
service(PublicKeyCredentialUserEntityRepositoryInterface::class),
service(PublicKeyCredentialLoader::class),
service(SerializerInterface::class),
abstract_arg('Authenticator Assertion Response Validator'),
abstract_arg(
'Authenticator Attestation Response Validator'
Expand Down
24 changes: 10 additions & 14 deletions src/symfony/src/Resources/config/services.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use Webauthn\CeremonyStep\CeremonyStepManager;
use Webauthn\CeremonyStep\CeremonyStepManagerFactory;
use Webauthn\Counter\ThrowExceptionIfInvalid;
use Webauthn\Denormalizer\AttestationObjectDenormalizer;
use Webauthn\Denormalizer\AttestationStatementDenormalizer;
use Webauthn\Denormalizer\AuthenticationExtensionsDenormalizer;
use Webauthn\Denormalizer\AuthenticatorAssertionResponseDenormalizer;
Expand Down Expand Up @@ -97,7 +98,12 @@
->public();
$container
->set(PublicKeyCredentialLoader::class)
->args([null, service('webauthn-serializer')])
->deprecate(
'web-auth/webauthn-symfony-bundle',
'4.8.0',
'%service_id% is deprecated since 4.8.0 and will be removed in 5.0.0',
)
->args([null, service(SerializerInterface::class)])
->public();
$container
->set(PublicKeyCredentialCreationOptionsFactory::class)
Expand Down Expand Up @@ -142,7 +148,7 @@
service(SerializerInterface::class),
service(ValidatorInterface::class),
service(PublicKeyCredentialCreationOptionsFactory::class),
service(PublicKeyCredentialLoader::class),
null,
service(AuthenticatorAttestationResponseValidator::class),
service(PublicKeyCredentialSourceRepository::class)->nullOnInvalid(),
]);
Expand All @@ -152,7 +158,7 @@
service(SerializerInterface::class),
service(ValidatorInterface::class),
service(PublicKeyCredentialRequestOptionsFactory::class),
service(PublicKeyCredentialLoader::class),
null,
service(AuthenticatorAssertionResponseValidator::class),
service(PublicKeyCredentialUserEntityRepositoryInterface::class),
service(PublicKeyCredentialSourceRepository::class)->nullOnInvalid(),
Expand All @@ -179,6 +185,7 @@
->alias('webauthn.request_factory.default', RequestFactoryInterface::class);

$container->set(ExtensionDescriptorDenormalizer::class);
$container->set(AttestationObjectDenormalizer::class);
$container->set(AttestationStatementDenormalizer::class)
->args([service(AttestationStatementSupportManager::class)])
;
Expand All @@ -196,17 +203,6 @@
->args([service(AttestationStatementSupportManager::class)])
;
$container->set(MetadataStatementSerializerFactory::class);
$container->set('webauthn-serializer')
->class(SerializerInterface::class)
->factory([service(WebauthnSerializerFactory::class), 'create'])
->public()
;
$container->set('mds-serializer')
->class(SerializerInterface::class)
->factory([service(MetadataStatementSerializerFactory::class), 'create'])
->public()
;

$container->set(DefaultFailureHandler::class);
$container->set(DefaultSuccessHandler::class);
};
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\RememberMeBadge;
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
use Symfony\Component\Serializer\SerializerInterface;
use Throwable;
use Webauthn\AuthenticatorAssertionResponse;
use Webauthn\AuthenticatorAssertionResponseValidator;
Expand All @@ -37,6 +38,7 @@
use Webauthn\Exception\AuthenticatorResponseVerificationException;
use Webauthn\Exception\InvalidDataException;
use Webauthn\MetadataService\CanLogData;
use Webauthn\PublicKeyCredential;
use Webauthn\PublicKeyCredentialCreationOptions;
use Webauthn\PublicKeyCredentialLoader;
use Webauthn\PublicKeyCredentialRequestOptions;
Expand All @@ -55,7 +57,7 @@ public function __construct(
private readonly OptionsStorage $optionsStorage,
private readonly PublicKeyCredentialSourceRepository|PublicKeyCredentialSourceRepositoryInterface $publicKeyCredentialSourceRepository,
private readonly PublicKeyCredentialUserEntityRepositoryInterface $credentialUserEntityRepository,
private readonly PublicKeyCredentialLoader $publicKeyCredentialLoader,
private readonly PublicKeyCredentialLoader|SerializerInterface $publicKeyCredentialLoader,
private readonly AuthenticatorAssertionResponseValidator $assertionResponseValidator,
private readonly AuthenticatorAttestationResponseValidator $attestationResponseValidator
) {
Expand All @@ -70,6 +72,13 @@ public function __construct(
)
);
}
if ($this->publicKeyCredentialLoader instanceof PublicKeyCredentialLoader) {
trigger_deprecation(
'web-auth/webauthn-bundle',
'4.8.0',
'The argument "$publicKeyCredentialLoader" is deprecated since 4.8.0 and will be removed in 5.0.0. Please inject a Symfony Serializer instead.'
);
}
$this->logger = new NullLogger();
}

Expand Down Expand Up @@ -177,7 +186,9 @@ private function processWithAssertion(Request $request): Passport
) ? $request->getContentTypeFormat() : $request->getContentType();
$format === 'json' || throw InvalidDataException::create($format, 'Only JSON content type allowed');
$content = $request->getContent();
$publicKeyCredential = $this->publicKeyCredentialLoader->load($content);
$publicKeyCredential = $this->publicKeyCredentialLoader instanceof PublicKeyCredentialLoader ? $this->publicKeyCredentialLoader->load(
$content
) : $this->publicKeyCredentialLoader->deserialize($content, PublicKeyCredential::class, 'json');
$response = $publicKeyCredential->response;
$response instanceof AuthenticatorAssertionResponse || throw InvalidDataException::create(
$response,
Expand Down Expand Up @@ -243,7 +254,9 @@ private function processWithAttestation(Request $request): Passport
) ? $request->getContentTypeFormat() : $request->getContentType();
$format === 'json' || throw InvalidDataException::create($format, 'Only JSON content type allowed');
$content = $request->getContent();
$publicKeyCredential = $this->publicKeyCredentialLoader->load($content);
$publicKeyCredential = $this->publicKeyCredentialLoader instanceof PublicKeyCredentialLoader ? $this->publicKeyCredentialLoader->load(
$content
) : $this->publicKeyCredentialLoader->deserialize($content, PublicKeyCredential::class, 'json');
$response = $publicKeyCredential->response;
$response instanceof AuthenticatorAttestationResponse || throw InvalidDataException::create(
$response,
Expand Down
27 changes: 12 additions & 15 deletions src/webauthn/src/Denormalizer/AttestationObjectDenormalizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@
use Symfony\Component\Serializer\Normalizer\DenormalizerAwareTrait;
use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
use Webauthn\AttestationStatement\AttestationObject;
use Webauthn\AttestationStatement\AttestationStatement;
use Webauthn\AuthenticatorData;
use Webauthn\Exception\InvalidDataException;
use Webauthn\StringStream;

final class AttestationObjectDenormalizer implements DenormalizerInterface, DenormalizerAwareInterface
{
use DenormalizerAwareTrait;

private const ALREADY_CALLED = 'ATTESTATION_OBJECT_PREPROCESS_ALREADY_CALLED';

public function denormalize(mixed $data, string $type, string $format = null, array $context = [])
{
if ($this->denormalizer === null) {
Expand All @@ -38,23 +38,20 @@ public function denormalize(mixed $data, string $type, string $format = null, ar
'Invalid attestation object. Presence of extra bytes.'
);
$stream->close();
$authData = $attestationObject['authData'] ?? throw InvalidDataException::create(
$attestationObject,
'Invalid attestation object. Missing "authData" field.'
);

$data = [
'rawAttestationObject' => $data,
'attStmt' => $attestationObject,
'authData' => $attestationObject['authData'],
];
$context[self::ALREADY_CALLED] = true;

return $this->denormalizer->denormalize($data, $type, $format, $context);
return AttestationObject::create(
$data,
$this->denormalizer->denormalize($attestationObject, AttestationStatement::class, $format, $context),
$this->denormalizer->denormalize($authData, AuthenticatorData::class, $format, $context),
);
}

public function supportsDenormalization(mixed $data, string $type, string $format = null, array $context = []): bool
{
if ($context[self::ALREADY_CALLED] ?? false) {
return false;
}

return $type === AttestationObject::class;
}

Expand All @@ -64,7 +61,7 @@ public function supportsDenormalization(mixed $data, string $type, string $forma
public function getSupportedTypes(?string $format): array
{
return [
AttestationObject::class => false,
AttestationObject::class => true,
];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public function supportsDenormalization(mixed $data, string $type, string $forma
public function getSupportedTypes(?string $format): array
{
return [
AttestationStatement::class => false,
AttestationStatement::class => true,
];
}
}
Loading
Loading