Skip to content

Safari breaks with TypeError when authenticatorAttachment is null #34

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

Open
tomsommer opened this issue Jan 8, 2025 · 5 comments
Open
Assignees

Comments

@tomsommer
Copy link

tomsommer commented Jan 8, 2025

When setting AUTHENTICATOR_ATTACHMENT_NO_PREFERENCE authenticatorAttachment becomes null in the serialized output. Safari breaks with a TypeError if this happens. authenticatorAttachment should instead be removed from the output, null is not a valid value, according to Safari.

@tomsommer tomsommer changed the title Safari breaks when authenticatorAttachment is null Safari breaks with TypeError when authenticatorAttachment is null Jan 8, 2025
@Spomky Spomky self-assigned this Jan 8, 2025
@Spomky Spomky added the bug Something isn't working label Jan 8, 2025
@Spomky
Copy link
Collaborator

Spomky commented Jan 8, 2025

Hi @tomsommer,

Do you use the Symfony Bundle with custom option handlers or the library?
Normally, null values are not in the serialized options. See DefaultCreationOptionsHandler.php#L29 or DefaultRequestOptionsHandler.php#L29

@tomsommer
Copy link
Author

I don't use any bundle, I just do
(new WebauthnSerializerFactory(AttestationStatementSupportManager::create()))->create()->serialize(.... ,'json')

@Spomky
Copy link
Collaborator

Spomky commented Jan 8, 2025

OK, then can you try using the context options:

use Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer;

(new WebauthnSerializerFactory(AttestationStatementSupportManager::create()))->create()->serialize(.... ,'json', [AbstractObjectNormalizer::SKIP_NULL_VALUES => true])

@tomsommer
Copy link
Author

tomsommer commented Jan 8, 2025

This solves it. I would suggest this be the default then, since it will not produce valid output otherwise :)

Or at least note it somewhere in the docs, so other people won't hit the same issue.

@Spomky
Copy link
Collaborator

Spomky commented Jan 8, 2025

👍Perfect. Many thanks for the confirmation.
This context argument is used in the documentation, but not highlighted as it should be.
I move this issue to the doc repository and will update some pages.

@Spomky Spomky removed the bug Something isn't working label Jan 8, 2025
@Spomky Spomky transferred this issue from web-auth/webauthn-framework Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants