The usage of PublicKeyCredentialDescriptor in AllowCredentials vs excludeCredentials

[nbenjamin]

Hi Team,

Currently getCredentialIdsForUsername(String username) from CredentialRepository returns Set<PublicKeyCredentialDescriptor> and its used mainly for adding excludeCredentials . AllowCredentials in PublicKeyCredentialRequestOptions takes List of PublicKeyCredentialDescriptor. So rather than returning Set<PublicKeyCredentialDescriptor> we should return List<PublicKeyCredentialDescriptor> getCredentialIdsForUsername(String username); to provide a better reuse of the method. Is there any specific reason to return Set<PublicKeyCredentialDescriptor>?

[emlun]

Hi!

There is indeed a slight mismatch here. PublicKeyCredentialRequestOptions takes a List value to reflect the allowCredentials definition in the spec, while CredentialRepository returns a Set to communicate to the implementer that there can be no duplicates and order is unimportant. The RelyingParty.startAssertion method converts the collection type automatically, so downstream users should rarely be affected by the type mismatch.

However, I was reminded now that the allowCredentials definition does say that the list is ordered in descending order of preference. So yes, the CredentialRepository interface cannot express that feature of the spec.

The order of the list has little or no effect in practice, though. Browsers generally offer the user all available authenticators and goes with the first one the user picks. So I'd call this a minor issue that's unlikely to have any practical impact. It's certainly not worth breaking the CredentialRepository interface for it, but we could consider it if we make other backwards-incompatible changes like those discussed in #274 (comment) .

Does that answer your questions?

[nbenjamin]

Thank you @emlun for your quick response.
Any plans for releasing this #274 changes

[emlun]

We plan to release it at some point, but there's no concrete time plan at the moment. Perhaps later this year.

[emlun]

The features discussed in #274 are now released in experimental release 2.6.0-alpha4, but the new CredentialRepositoryV2 interface still returns a Set of credentials to be included in allowCredentials. We decided to stick with the Set because the CredentialRepositoryV2 implementations doesn't get any other context about the request beyond just a user ID, so there isn't really any way for it to know if or how to prioritize the credentials. But it's still possible for applications to edit the PublicKeyCredentialRequestOptions (using the .toBuilder() method) before sending it to the client, if they need to.