Add Signal API

[nsatragno]

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

[MasterKale]

Do we need anything here, in the name of avoiding abuse of this call (either by RP or malicious JS), to have the client remember the credential ID in the last resolved .get() response and only allow the RP to specify that ID in a subsequent unknownCredentialId report? The idea being, if an RP receives a response with an id of "credA", they shouldn't ever set anything but "credA" in their unknown credential Report.

Thinking one step further, I'd say if that idea has merit then what's the point in the RP specifying a credential ID? The RP could more simply signal "I didn't recognize that credential" and the client uses its own knowledge of the last used credential's ID to deprecate that credential. No value validation needs to go on, the RP passes a simpler value for this specific report... 🤔

[nsatragno]

In terms of abuse, malicious JS can do way more nasty things, such as steal assertions and intercept credential creation requests that we can't meaningfully defend against so I would consider it outside the threat model. I lack the imagination to see what a malicious RP could do with this endpoint as specified, do you have an example?

The RP could more simply signal "I didn't recognize that credential" and the client uses its own knowledge of the last used credential's ID to deprecate that credential. No value validation needs to go on, the RP passes a simpler value for this specific report... 🤔

I think this is conceptually fine, but significantly more complicated in terms of implementation for no discernible benefit. We'd have to specify how long we remember the credential id for, whether we do it across navigations, and what the remembered credential id is keyed to (the frame? the global browser object?). I would prefer not to go this route unless we find a good reason to avoid RPs from specifying arbitrary credential ids.

[emlun]

We'd have to specify how long we remember the credential id for, whether we do it across navigations, and what the remembered credential id is keyed to

Maybe it could be tied by exposing the signal as a callback method exposed on the PublicKeyCredential instance, but I agree this seems overcomplicated for little benefit.

[MasterKale]

Also thank you very much for applying Base64URLString as abundantly as you have, this will save RPs so much time and effort 🙏

[nsatragno]

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.

[MasterKale]

It could be argued that this report identifies itself as an exhaustive list of recognized credential IDs. If we accept that, could we shorten allAcceptedCredentialIds to credentialIds? Make it look more like UnknownCredentialIdReport.credentialId for brevity 🤔

Suggested change

	dictionary CurrentCredentialsReport {
	required USVString rpId;
	required Base64URLString userId;
	CurrentCredentialUserDetails user;
	sequence<Base64URLString> allAcceptedCredentialIds;
	};
	dictionary CurrentCredentialsReport {
	required USVString rpId;
	required Base64URLString userId;
	CurrentCredentialUserDetails user;
	sequence<Base64URLString> credentialIds;
	};

[arnar]

Are you suggesting that "Current" in "CurrentCredentialsReport" is enough of an indication that the RP must put all credential ids in here, and that it's superfluous to reinforce that with "allAccepted"?

My initial reaction is to keep it as is, because the consequences can be drastic: A provider may, and likely will delete any credential it has that is not included on the list. I.e. the meaning of the field here is very different from the credentialId field in the UnknownCredentialIdReport.

[MasterKale]

Hmm, we could go a slightly different route here and rename CurrentCredentialsReport to AllAcceptedCredentialsReport? Paired with a rename of allAcceptedCredentialIds to credentialIds it might be more explicit 🤔

To keep the bikeshedding going, there might also be a possibility of leaving it called CurrentCredentialsReport but renaming allAcceptedCredentialIds to allCurrentCredentialIds.

Having said this I think what I'm trying to do is unify the wording here. I don't particularly care if we go with "current" or "accepted", but I do feel as though there should be one word used for consistency.

[kopy]

From RP perspective: Nothing in the name suggests the implicit "deletion" of non-listed Credentials - while this is the most critical part of this operation. It could also be a separate parameter (credentialIds and deleteOtherCredentials) or maybe something hinting in that direction onlyAcceptedCredentialIds. It feels awkward to me because, on the one hand, this is a selector in the style of allowCredentials like a WHERE clause for the updated fields, but at the same time it is also an implicit DELETE.

[nsatragno]

It could also be a separate parameter (credentialIds and deleteOtherCredentials)

It should not be an explicit list of credentials to be deleted because otherwise the relying party has to keep that list ~forever.

Hmm, we could go a slightly different route here and rename CurrentCredentialsReport to AllAcceptedCredentialsReport?

How about AllAcceptedCredentialsReport and keep allAcceptedCredentialIds? That way we get consistency and an explicit string. I'll also add a warning that this will remove or hide all credentials not in the list.

[nsatragno]

I added a note warning of deletion. Splitting the username / displayname updates should help avoid the deletion footgun when a developer is only interested in updating that.

The way to invoke the API now is:

PublicKeyCredential.signalAllAcceptedCredentialIds({
    rpId: "example.com",
    userId: "aabbcc",  // user handle, base64url.
    allAcceptedCredentialIds: [
        "bb",
    ]
});

I cannot think of something more obvious short of credentialsNotPresentInTheFollowingListShouldBeDeletedIKnowWhatImDoing.

[kopy]

I cannot think of something more obvious short of credentialsNotPresentInTheFollowingListShouldBeDeletedIKnowWhatImDoing.

No good idea myself. There are only good ones when inverted .... RemoveUnlistedCredentialIds DeleteNonListedCredentialIds

[MasterKale]

How about AllAcceptedCredentialsReport and keep allAcceptedCredentialIds? That way we get consistency and an explicit string. I'll also add a warning that this will remove or hide all credentials not in the list.

I think this is probably the way to go. An issue with potential names like RemoveUnlistedCredentialIds and DeleteNonListedCredentialIds could be that different providers do different things. As suggested earlier, maybe a provider wants to tag a not in the list passkey vs delete it outright. "Accepted" vs "Remove" or "Delete" gives providers greater flexibility to handle such a signal without making it confusing to explain to RP's the impact of making such a call.

[arnar]

My only high level comment is that this structure allows combination of the two defined report types, which there is probably never a reason to do, but not a combination say two CurrentCredentialsReports with different RP-Ids, which there may be a reason to do (e.g. 'www.example.com' and 'example.com').

I don't think that's a major problem though.

[emlun]

Looks pretty good to me!

[nsatragno]

This is ready for another pass.

[Firehed]

RP perspective: This looks great overall and should solve some real usability needs in some areas that are a little clumsy today. Looking forward to adding support here!

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

[emlun]

Thanks @nsatragno!

[emlun]

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?

[timcappalli]

mostly editorial suggestions

[nsatragno]

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 #2121 to track whether we want to deprecate rp.name, as discussed in the w3c meeting.

@akshayku, please take a look!

[arnar]

Looks great to me!