Merge pull request #635 from web-auth/deprecations/icon

Deprecate 'icon' parameter from PublicKeyCredentialCreationOptionsFac…
web-auth · Jul 20, 2024 · 22f848e · 22f848e
2 parents 01f5d0b + b61e4ea
commit 22f848e
Show file tree

Hide file tree

Showing 9 changed files with 140 additions and 143 deletions.
diff --git a/.gitignore b/.gitignore
@@ -8,3 +8,4 @@ yarn-error.log
 /composer.lock
 /vendor
 /.phpunit.cache/
+/.castor.stub.php
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -545,11 +545,6 @@ parameters:
 			count: 1
 			path: src/symfony/src/Service/PublicKeyCredentialCreationOptionsFactory.php
 
-		-
-			message: "#^Cannot access offset 'icon' on mixed\\.$#"
-			count: 1
-			path: src/symfony/src/Service/PublicKeyCredentialCreationOptionsFactory.php
-
 		-
 			message: "#^Cannot access offset 'id' on mixed\\.$#"
 			count: 1
@@ -635,11 +630,6 @@ parameters:
 			count: 1
 			path: src/symfony/src/Service/PublicKeyCredentialCreationOptionsFactory.php
 
-		-
-			message: "#^Parameter \\#3 \\$icon of static method Webauthn\\\\PublicKeyCredentialRpEntity\\:\\:create\\(\\) expects string\\|null, mixed given\\.$#"
-			count: 1
-			path: src/symfony/src/Service/PublicKeyCredentialCreationOptionsFactory.php
-
 		-
 			message: "#^Parameter \\#3 \\$residentKey of static method Webauthn\\\\AuthenticatorSelectionCriteria\\:\\:create\\(\\) expects string\\|null, mixed given\\.$#"
 			count: 1

diff --git a/src/symfony/src/DependencyInjection/Configuration.php b/src/symfony/src/DependencyInjection/Configuration.php
@@ -129,111 +129,120 @@ private function addCreationProfilesConfig(ArrayNodeDefinition $rootNode): void
         ];
         $rootNode->children()
             ->arrayNode('creation_profiles')
-            ->treatFalseLike($defaultCreationProfiles)
-            ->treatNullLike($defaultCreationProfiles)
-            ->treatTrueLike($defaultCreationProfiles)
-            ->useAttributeAsKey('name')
-            ->arrayPrototype()
-            ->addDefaultsIfNotSet()
-            ->children()
-            ->arrayNode('rp')
-            ->isRequired()
-            ->children()
-            ->scalarNode('id')
-            ->defaultNull()
-            ->end()
-            ->scalarNode('name')
-            ->isRequired()
-            ->end()
-            ->scalarNode('icon')
-            ->defaultNull()
-            ->end()
-            ->end()
-            ->end()
-            ->integerNode('challenge_length')
-            ->min(16)
-            ->defaultValue(32)
-            ->end()
-            ->integerNode('timeout')
-            ->min(0)
-            ->defaultNull()
-            ->end()
-            ->arrayNode('authenticator_selection_criteria')
-            ->addDefaultsIfNotSet()
-            ->beforeNormalization()
-            ->ifArray()
-            ->then(function (array $v): array {
-                if (isset($v['attachment_mode'])) {
-                    $v['authenticator_attachment'] = $v['attachment_mode'];
-                    unset($v['attachment_mode']);
-                }
+                ->treatFalseLike($defaultCreationProfiles)
+                ->treatNullLike($defaultCreationProfiles)
+                ->treatTrueLike($defaultCreationProfiles)
+                ->useAttributeAsKey('name')
+                ->arrayPrototype()
+                    ->addDefaultsIfNotSet()
+                    ->children()
+                        ->arrayNode('rp')
+                            ->isRequired()
+                            ->children()
+                                ->scalarNode('id')
+                                    ->defaultNull()
+                                ->end()
+                                ->scalarNode('name')
+                                    ->isRequired()
+                                ->end()
+                                ->scalarNode('icon')
+                                    ->setDeprecated(
+                                        'web-auth/webauthn-symfony-bundle',
+                                        '5.1.0',
+                                        'The child node "%node%" at path "%path%" is deprecated and has no effect.'
+                                    )
+                                    ->defaultNull()
+                                ->end()
+                            ->end()
+                        ->end()
+                        ->integerNode('challenge_length')
+                            ->min(16)
+                            ->defaultValue(32)
+                        ->end()
+                        ->integerNode('timeout')
+                            ->min(0)
+                            ->defaultNull()
+                        ->end()
+                        ->arrayNode('authenticator_selection_criteria')
+                            ->addDefaultsIfNotSet()
+                            ->beforeNormalization()
+                            ->ifArray()
+                                ->then(function (array $v): array {
+                                    if (isset($v['attachment_mode'])) {
+                                        $v['authenticator_attachment'] = $v['attachment_mode'];
+                                        unset($v['attachment_mode']);
+                                    }
 
-                return $v;
-            })
-            ->end()
-            ->children()
-            ->scalarNode('authenticator_attachment')
-            ->defaultValue(AuthenticatorSelectionCriteria::AUTHENTICATOR_ATTACHMENT_NO_PREFERENCE)
-            ->validate()
-            ->ifNotInArray([
-                AuthenticatorSelectionCriteria::AUTHENTICATOR_ATTACHMENT_NO_PREFERENCE,
-                AuthenticatorSelectionCriteria::AUTHENTICATOR_ATTACHMENT_PLATFORM,
-                AuthenticatorSelectionCriteria::AUTHENTICATOR_ATTACHMENT_CROSS_PLATFORM,
-            ])
-            ->thenInvalid($errorTemplate)
-            ->end()
-            ->end()
-            ->booleanNode('require_resident_key')
-            ->defaultFalse()
-            ->end()
-            ->scalarNode('user_verification')
-            ->defaultValue(AuthenticatorSelectionCriteria::USER_VERIFICATION_REQUIREMENT_PREFERRED)
-            ->validate()
-            ->ifNotInArray([
-                AuthenticatorSelectionCriteria::USER_VERIFICATION_REQUIREMENT_DISCOURAGED,
-                AuthenticatorSelectionCriteria::USER_VERIFICATION_REQUIREMENT_PREFERRED,
-                AuthenticatorSelectionCriteria::USER_VERIFICATION_REQUIREMENT_REQUIRED,
-            ])
-            ->thenInvalid($errorTemplate)
-            ->end()
-            ->end()
-            ->scalarNode('resident_key')
-            ->defaultValue(AuthenticatorSelectionCriteria::RESIDENT_KEY_REQUIREMENT_PREFERRED)
-            ->validate()
-            ->ifNotInArray([
-                AuthenticatorSelectionCriteria::RESIDENT_KEY_REQUIREMENT_NO_PREFERENCE,
-                AuthenticatorSelectionCriteria::RESIDENT_KEY_REQUIREMENT_DISCOURAGED,
-                AuthenticatorSelectionCriteria::RESIDENT_KEY_REQUIREMENT_PREFERRED,
-                AuthenticatorSelectionCriteria::RESIDENT_KEY_REQUIREMENT_REQUIRED,
-            ])
-            ->thenInvalid($errorTemplate)
-            ->end()
-            ->end()
-            ->end()
-            ->end()
-            ->arrayNode('extensions')
-            ->treatFalseLike([])
-            ->treatTrueLike([])
-            ->treatNullLike([])
-            ->useAttributeAsKey('name')
-            ->scalarPrototype()
-            ->end()
-            ->end()
-            ->arrayNode('public_key_credential_parameters')
-            ->integerPrototype()
-            ->end()
-            ->requiresAtLeastOneElement()
-            ->treatNullLike([])
-            ->treatFalseLike([])
-            ->treatTrueLike([])
-            ->defaultValue([])
-            ->end()
-            ->scalarNode('attestation_conveyance')
-            ->defaultValue(PublicKeyCredentialCreationOptions::ATTESTATION_CONVEYANCE_PREFERENCE_NONE)
-            ->end()
-            ->end()
-            ->end()
-            ->end()
+                                    return $v;
+                                })
+                            ->end()
+                            ->children()
+                                ->scalarNode('authenticator_attachment')
+                                    ->defaultValue(
+                                        AuthenticatorSelectionCriteria::AUTHENTICATOR_ATTACHMENT_NO_PREFERENCE
+                                    )
+                                    ->validate()
+                                        ->ifNotInArray([
+                                            AuthenticatorSelectionCriteria::AUTHENTICATOR_ATTACHMENT_NO_PREFERENCE,
+                                            AuthenticatorSelectionCriteria::AUTHENTICATOR_ATTACHMENT_PLATFORM,
+                                            AuthenticatorSelectionCriteria::AUTHENTICATOR_ATTACHMENT_CROSS_PLATFORM,
+                                        ])
+                                        ->thenInvalid($errorTemplate)
+                                    ->end()
+                                ->end()
+                                ->booleanNode('require_resident_key')
+                                    ->defaultFalse()
+                                ->end()
+                                ->scalarNode('user_verification')
+                                    ->defaultValue(
+                                        AuthenticatorSelectionCriteria::USER_VERIFICATION_REQUIREMENT_PREFERRED
+                                    )
+                                ->validate()
+                                    ->ifNotInArray([
+                                        AuthenticatorSelectionCriteria::USER_VERIFICATION_REQUIREMENT_DISCOURAGED,
+                                        AuthenticatorSelectionCriteria::USER_VERIFICATION_REQUIREMENT_PREFERRED,
+                                        AuthenticatorSelectionCriteria::USER_VERIFICATION_REQUIREMENT_REQUIRED,
+                                    ])
+                                    ->thenInvalid($errorTemplate)
+                                ->end()
+                            ->end()
+                            ->scalarNode('resident_key')
+                                ->defaultValue(AuthenticatorSelectionCriteria::RESIDENT_KEY_REQUIREMENT_PREFERRED)
+                                ->validate()
+                                    ->ifNotInArray([
+                                        AuthenticatorSelectionCriteria::RESIDENT_KEY_REQUIREMENT_NO_PREFERENCE,
+                                        AuthenticatorSelectionCriteria::RESIDENT_KEY_REQUIREMENT_DISCOURAGED,
+                                        AuthenticatorSelectionCriteria::RESIDENT_KEY_REQUIREMENT_PREFERRED,
+                                        AuthenticatorSelectionCriteria::RESIDENT_KEY_REQUIREMENT_REQUIRED,
+                                    ])
+                                    ->thenInvalid($errorTemplate)
+                                ->end()
+                            ->end()
+                        ->end()
+                    ->end()
+                    ->arrayNode('extensions')
+                        ->treatFalseLike([])
+                        ->treatTrueLike([])
+                        ->treatNullLike([])
+                        ->useAttributeAsKey('name')
+                        ->scalarPrototype()
+->end()
+                    ->end()
+                    ->arrayNode('public_key_credential_parameters')
+                        ->integerPrototype()
+->end()
+                        ->requiresAtLeastOneElement()
+                        ->treatNullLike([])
+                        ->treatFalseLike([])
+                        ->treatTrueLike([])
+                        ->defaultValue([])
+                    ->end()
+                    ->scalarNode('attestation_conveyance')
+                        ->defaultValue(PublicKeyCredentialCreationOptions::ATTESTATION_CONVEYANCE_PREFERENCE_NONE)
+                    ->end()
+                ->end()
+            ->end()
+        ->end()
             ->end();
     }
 

diff --git a/src/symfony/src/Service/PublicKeyCredentialCreationOptionsFactory.php b/src/symfony/src/Service/PublicKeyCredentialCreationOptionsFactory.php
@@ -123,11 +123,7 @@ private function createAuthenticatorSelectionCriteria(array $profile): Authentic
      */
     private function createRpEntity(array $profile): PublicKeyCredentialRpEntity
     {
-        return PublicKeyCredentialRpEntity::create(
-            $profile['rp']['name'],
-            $profile['rp']['id'],
-            $profile['rp']['icon']
-        );
+        return PublicKeyCredentialRpEntity::create($profile['rp']['name'], $profile['rp']['id']);
     }
 
     /**

diff --git a/src/webauthn/src/Denormalizer/PublicKeyCredentialUserEntityDenormalizer.php b/src/webauthn/src/Denormalizer/PublicKeyCredentialUserEntityDenormalizer.php
@@ -21,12 +21,7 @@ public function denormalize(mixed $data, string $type, string $format = null, ar
         }
         $data['id'] = Base64::decode($data['id']);
 
-        return PublicKeyCredentialUserEntity::create(
-            $data['name'],
-            $data['id'],
-            $data['displayName'],
-            $data['icon'] ?? null
-        );
+        return PublicKeyCredentialUserEntity::create($data['name'], $data['id'], $data['displayName']);
     }
 
     public function supportsDenormalization(mixed $data, string $type, string $format = null, array $context = []): bool
@@ -47,17 +42,14 @@ public function getSupportedTypes(?string $format): array
     /**
      * @return array<string, mixed>
      */
-    public function normalize(mixed $data, ?string $format = null, array $context = []): array
+    public function normalize(mixed $object, ?string $format = null, array $context = []): array
     {
-        assert($data instanceof PublicKeyCredentialUserEntity);
-        $normalized = [
-            'id' => Base64UrlSafe::encodeUnpadded($data->id),
-            'name' => $data->name,
-            'displayName' => $data->displayName,
-            'icon' => $data->icon,
+        assert($object instanceof PublicKeyCredentialUserEntity);
+        return [
+            'id' => Base64UrlSafe::encodeUnpadded($object->id),
+            'name' => $object->name,
+            'displayName' => $object->displayName,
         ];
-
-        return array_filter($normalized, static fn ($value): bool => $value !== null);
     }
 
     public function supportsNormalization(mixed $data, ?string $format = null, array $context = []): bool

diff --git a/src/webauthn/src/PublicKeyCredentialEntity.php b/src/webauthn/src/PublicKeyCredentialEntity.php
@@ -6,9 +6,21 @@
 
 abstract class PublicKeyCredentialEntity
 {
+    /**
+     * @@deprecated since 5.1.0 and will be removed in 6.0.0. This value is always null.
+     */
+    public ?string $icon = null;
+
     public function __construct(
         public readonly string $name,
-        public readonly ?string $icon
+        ?string $icon = null
     ) {
+        if ($icon !== null) {
+            trigger_deprecation(
+                'web-auth/webauthn-lib',
+                '5.1.0',
+                'The parameter "$icon" is deprecated since 5.1.0 and will be removed in 6.0.0. This value has no effect. Please set "null" instead.'
+            );
+        }
     }
 }
diff --git a/tests/library/Unit/EntityTest.php b/tests/library/Unit/EntityTest.php
@@ -18,14 +18,13 @@ final class EntityTest extends AbstractTestCase
     #[Test]
     public function anPublicKeyCredentialUserEntityCanBeCreatedAndValueAccessed(): void
     {
-        $user = PublicKeyCredentialUserEntity::create('name', 'id', 'display_name', 'icon');
+        $user = PublicKeyCredentialUserEntity::create('name', 'id', 'display_name');
 
         static::assertSame('name', $user->name);
         static::assertSame('display_name', $user->displayName);
-        static::assertSame('icon', $user->icon);
         static::assertSame('id', $user->id);
         static::assertSame(
-            '{"id":"aWQ","name":"name","displayName":"display_name","icon":"icon"}',
+            '{"id":"aWQ","name":"name","displayName":"display_name"}',
             $this->getSerializer()
                 ->serialize($user, 'json', [
                     AbstractObjectNormalizer::SKIP_NULL_VALUES => true,
@@ -36,12 +35,11 @@ public function anPublicKeyCredentialUserEntityCanBeCreatedAndValueAccessed(): v
     #[Test]
     public function anPublicKeyCredentialRpEntityCanBeCreatedAndValueAccessed(): void
     {
-        $rp = PublicKeyCredentialRpEntity::create('name', 'id', 'icon');
+        $rp = PublicKeyCredentialRpEntity::create('name', 'id');
 
         static::assertSame('name', $rp->name);
-        static::assertSame('icon', $rp->icon);
         static::assertSame('id', $rp->id);
-        static::assertSame('{"id":"id","name":"name","icon":"icon"}', $this->getSerializer()->serialize($rp, 'json', [
+        static::assertSame('{"id":"id","name":"name"}', $this->getSerializer()->serialize($rp, 'json', [
             AbstractObjectNormalizer::SKIP_NULL_VALUES => true,
         ]));
     }

diff --git a/tests/symfony/config/config.yml b/tests/symfony/config/config.yml
@@ -159,7 +159,6 @@ webauthn:
       rp:
         name: 'My other application'
         id: 'localhost'
-        icon: null
       challenge_length: 32
       authenticator_selection_criteria:
         authenticator_attachment: !php/const Webauthn\AuthenticatorSelectionCriteria::AUTHENTICATOR_ATTACHMENT_NO_PREFERENCE

diff --git a/tests/symfony/functional/PublicKeyCredentialUserEntityRepository.php b/tests/symfony/functional/PublicKeyCredentialUserEntityRepository.php
@@ -55,7 +55,7 @@ public function generateNextUserEntityId(): string
     public function saveUserEntity(PublicKeyCredentialUserEntity $userEntity): void
     {
         if (! $userEntity instanceof User) {
-            $userEntity = User::create($userEntity->name, $userEntity->id, $userEntity->displayName, $userEntity->icon);
+            $userEntity = User::create($userEntity->name, $userEntity->id, $userEntity->displayName);
         }
 
         $item = $this->cacheItemPool->getItem('user-id' . Base64UrlSafe::encodeUnpadded($userEntity->id));