passwordless-lib/fido2-net-lib

Consolidate Id/CredentialId?

Regenhardt opened this issue · 1 comments

Separate discussion about point 2. from #426 as requested.

Redundancy

On Attestation, the ID of the newly created credential (RegisteredPublicKeyCredential.Id) is saved to the database redundantly, once in StoredCredential.Id and once wrapped in a PublicKeyCredentialDescriptor in StoredCredential.Descriptor:

grafik

StoredCredential.Descriptor is used in multiple places to work with the credential's ID.
StoredCredential.Id on the other hand is never read, just written to in the demos; I propose to just delete this property.

grafik

Naming

AttestedCredentialData (lib-input in Attestation/Assertion authenticator responses) has CredentialId
RegisteredPublicKeyCredential (lib-output when cred was created / after attestation) has Id
StoredCredential (dev storage) has Id (redundant, see above)
PublicKeyCredentialDescriptor (used to identify a credential throughout the lib; usually named like it is the credential) has Id
VerifyAssertionResult (lib-output when cred was verified / after assertion) has CredentialId

This one is not as clear and maybe totally correct like it is.
In things called credential, the ID is called Id, so when talking about or using it, it's usually something like "Credential ID"/credential.Id.
In things not called credential, i.e. adjacent data to an actual credential, it's called Credentialid, so talking about it would be something like "the result's credential ID" or "the attested data's credential ID".

Presuming of course the one talking about it kind of knows what they're talking about. For me, integrating the lib in the demo app was confusing, as I hadn't touched WebAuthn/FIDO2 before that.

So renaming them all to CredentialId could make it easier, but it would also deviate from the spec for PublicKeyCredentialDescriptor.
On the other hand, it would probably be way less confusing anyway if the public types have XML docs for the types and properties per #501.