-
-
Notifications
You must be signed in to change notification settings - Fork 157
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
Consolidate Id/CredentialId? #510
Comments
This was referenced Mar 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 inStoredCredential.Id
and once wrapped in aPublicKeyCredentialDescriptor
inStoredCredential.Descriptor
: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.Naming
AttestedCredentialData
(lib-input in Attestation/Assertion authenticator responses) hasCredentialId
RegisteredPublicKeyCredential
(lib-output when cred was created / after attestation) hasId
StoredCredential
(dev storage) hasId
(redundant, see above)PublicKeyCredentialDescriptor
(used to identify a credential throughout the lib; usually named like it is the credential) hasId
VerifyAssertionResult
(lib-output when cred was verified / after assertion) hasCredentialId
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 forPublicKeyCredentialDescriptor
.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.
The text was updated successfully, but these errors were encountered: