Add Signal API - Githubissues

w3c / webauthn

Web Authentication: An API for accessing Public Key Credentials

https://w3c.github.io/webauthn/

Other

1.19k stars 172 forks source link

Add Signal API #2093

Closed nsatragno closed 2 months ago

nsatragno commented 4 months ago

This PR adds a PublicKeyCredential.signal* set of methods that relying parties can call to notify authenticators of changes on the applicability or metadata of credentials.

PublicKeyCredential.signalUnknownCredentialId This lets the relying party notify the authenticator that a request with a given credential id would be rejected.
PublicKeyCredential.signalAllAcceptedCredentialIds This lets the relying party send a snapshot of all the credential IDs it will accept for a user, allowing the authenticator to hide or remove credentials not present.
PublicKeyCredential.signalCurrentUserDetails This lets the relying party update a user's name and displayName.

Please see the explainer for more details.

Closes #1967. Closes #2038.

Preview | Diff

nsatragno commented 4 months ago

~~Just to clarify: this isn't quite ready for a review yet! I'll move it from "draft" when ready.~~

This is ready for initial review, with the understanding that we might want to tweak things as we progress with the implementation.

nsatragno commented 4 months ago

This is ready for another pass.

nsatragno commented 3 months ago

Thought: is it worth adding anything into getClientCapabilities() output, or is it enough to feature-detect on the new methods directly?

Great question actually. We put other things there that can be feature-detected in other ways. I wouldn't be opposed.

MasterKale commented 3 months ago

@nsatragno I finally caught up on the latest shape of the API. It's almost isUserVerifyingPlatformAuthenticatorAvailable again but honestly I find it hard to disagree with the reasoning for it:

@pascoej made a good point that the two primary reports, signalAllAcceptedCredentialIds() and signalUnknownCredentialId have diametrically opposite use cases (the former would be sent after a successful auth ceremony, while the latter would be sent after failed auth ceremony)
The three discrete methods also addresses @arnar's concern that the old structure technically allowed for multiple reports of the same type to be sent. The new structure definitely reduces edge cases related to this.

I would say I am generally in agreement with the latest shape of the API. I've got some comments I'll leave as Review comments in a few.

emlun commented 3 months ago

Right now we have signals for updating user.name and user.displayName, but there is also an rp.name property (display name for the RP) whose value might be stored in otherUI. Should there also be a signal for updating that?

nsatragno commented 3 months ago

rp.name isn't stored or displayed by any authenticator or credential provider that I'm aware of. I would prefer not to spec something for an attribute that is ignored in practice.

I've filed https://github.com/w3c/webauthn/issues/2121 to track whether we want to deprecate rp.name, as discussed in the w3c meeting.

@akshayku, please take a look!