web-auth / webauthn-framework

FIDO-U2F / FIDO2 / Webauthn Framework
MIT License
425 stars 54 forks source link

Move state validation back into the value objects #581

Closed TimWolla closed 5 months ago

TimWolla commented 8 months ago

Description

This request is somewhat similar to #464: I've noticed that the PublicKeyCredentialDenormalizer verifies that the id and rawId match, but the PublicKeyCredential does not, possibly allowing the developer to create a PublicKeyCredential that is internally inconsistent.

In fact, if both IDs need to match up, why is it necessary to explicitly pass both of them. Should PublicKeyCredential perhaps just have the id as a parameter and calculate the rawId by itself?

Example

No response

Spomky commented 7 months ago

Hi @TimWolla,

Indeed id and rawId are redondant. One of the values can be removed. I will deprecate it and remove in 5.0.0.

Spomky commented 7 months ago

Hi @TimWolla,

This will be deprecated by #589. Not an easy task and it needs to be done in 2 steps:

  1. Deprecation of $id in favor of $rawId. $rawId is used almost everywhere so I prefer to deprecate the base 64 encoded value $id. Developers are now asked to use the binary version.
  2. With 5.0.0 only rawId will exist. However I prefer the property name $id. In 5.x the property $rawId will be deprecated in favor of $id, which will have the same value.

It is strange at first sight, but I don't know how to swith from a base64 encoded to a binary value for $id without causing BC breaks.

WDYT?

Spomky commented 5 months ago

Thie first step is done. #590 is foreseen in 5.1.x Closing as followed up using the other issue

github-actions[bot] commented 3 months ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.