Skip to content
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

[Superset] Updating credential metadata and requesting deletion of stale credentials #1967

Open
timcappalli opened this issue Sep 19, 2023 · 20 comments
Assignees
Labels
@Risk Items that are at risk for L3 type:technical
Milestone

Comments

@timcappalli
Copy link
Member

This issue is to consolidate a bunch of issues from over the years around deletion of stale credentials, updating properties of existing credentials, etc and provide a single issue to continue discussion. There was general consensus at TPAC 2023 that something like this is needed.

Existing Issues

All of these issues have now been closed. Please continue discussion here.

#1456: Personal information updates & webauthn

Ability to update things like name and displayName without creating a new credential.

#1779: Facility for an RP to indicate a change of displayName to a discoverable credential

Similar to 1456, but specific to displayName

#1560: Cleanup when creating discoverable credentials with attestations

Essentially allow for clean up of stale credentials. For example, a credential that was deleted RP-side, or a credential that doesn't meet RP requirements.

#1696: FIDO credential decommissioning

High Level Requirements

Trends observed in these issues:

  • Desire to signal to a client/authenticator that a specific credential is no longer valid and can be deleted
  • Desire to signal to a client/authenticator that name and/or displayName has changed
@MasterKale
Copy link
Contributor

  • Desire to signal to a client/authenticator that a specific credential is no longer valid and can be deleted
  • Desire to signal to a client/authenticator that name and/or displayName has changed

Yes please. The first one especially is highly desirable because passkey self-service must happen in two places, once on the RP's side and then a follow-up action within the provider's interface, and it's very easy for the two to fall out of sync.

And I see adding the ability to update a credential's username and display name as highly desirable because it makes WebAuthn adaptable to real people problems. People change their legal names, online handles, email addresses, and other things that get used as usernames for a multitude of reasons, and right now the story for getting passkeys to respect this is limited to, "have the RP make a .create() call with the new name but same RP ID and user ID, and then hope the user uses the same authenticator as before so the old credential and its outdated metadata get overwritten."

There's too much ambiguity in this current process that users can end up with two credentials, one with their old handle and a second one with their updated handle, thus exacerbating the RP's problems. I'd love for us to spend some more time on this in upcoming meetings to try and discover a better solution.

@emlun
Copy link
Member

emlun commented Oct 4, 2023

In the interest of having something concrete to discuss, here's another attempt (somewhat inspired by my previous one, but rather different). How about a new updateCredentials client extension?

extensions: {
  updateCredentials: {
    user: {
      id: new Uint8Array([1, 2, 3, 4]),  // User handle of the user account whose credentials to update/garbage-collect
      name: "example",                   // New value for user.name
      displayName: "Example User",       // New value for user.displayName
    },
    validCredentials: [
      // List of all of credentials valid for this account
      // Credentials with these IDs have their user.name and user.displayName updated
      // Credentials with above user handle and none of these IDs are deleted
      { type: "public-key", id: new Uint8Array([96, 231, 209, 27, /* ... */ ])},
      { type: "public-key", id: new Uint8Array([165, 225, 44, 123, /* ... */ ])},
      { type: "public-key", id: new Uint8Array([112, 235, 14, 6, /* ... */ ])},
      /* ... */
    ],
  },
},

As noted in the inline comments, the idea is:

  • We address credentials to be updated by the combination of (user handle, credential ID).
  • The RP lists all credentials that should remain after cleanup.
  • Authenticators delete any credentials that have the given user.id but whose credential ID is not listed in validCredentials.
  • Authenticators update the user.name and user.displayName of each matching credential to the given values.
  • No extension output, no feedback returned to the RP.

The main idea is that this way the RP doesn't need to track state changes over time, rather it only needs to report the current state.

Thoughts?

@emlun
Copy link
Member

emlun commented Oct 4, 2023

I'll start: You probably don't want to expose all this information before the user is authenticated. This also doesn't work at all when the user isn't yet identified (empty allowCredentials, username-less/passkey authentication). This makes it rather useless as an assertion extension, as that would require a second assertion ceremony. And since the RP isn't supposed to need to track state over time, presumably the RP should just always send the extension? So always two assertion ceremonies? That won't work.

So it needs to be something you can call after successful authentication. Maybe a static function on PublicKeyCredential?

Or maybe the extension outputs a callback that the RP can invoke after retrieving the right state for the user?

const credential = await navigator.credentials.get({
  publicKey: {
    // ...
    extensions: {
      updateCredentials: true,  // Request that extension outputs include the callback
    },
  },
});

// Finish authentication on backend
const userSession = await finishAuthentication(credential);

credential.getClientExtensionResults().updateCredentials(userSession.updateCredentials);

@nsatragno
Copy link
Member

I don't think we want to tie the update to an assertion, whether through an extension or a method on the returned credential. We should support an RP triggering updates & deletions without requiring a user to complete an assertion ceremony for each authenticator they own, e.g. from a settings page.

I prefer the static function approach. Especially if we can update other credential types on the same call. A possible API could be,

navigator.credentials.update([
  { type: "public-key", id: ..., displayName: ..., etc ])}  // missing attributes are ignored
]);

navigator.credentials.delete([
  { type: "public-key", id: ...])}
]);

These would not be guaranteed to do anything. The browser would execute these actions opportunistically. For example, if a user just used a security key to sign in and the RP calls delete, the browser might send a ctap credential management request to the security key. Local platform authenticators would likely always be able to perform these. The counterpoint is that if the authenticator is not available, it's probably okay to not bother the user finding & activating it to make an update.

In my mind, an RP would call the same delete function in two different contexts: their "manage passkeys" settings page, and after a failed sign in attempt with a previously deleted, now unrecognized passkey.

update could similarly happen at both of those stages. However, currently the user's name and displayName are not part of the assertion, so the RP would have to call update after every sign in which seems suboptimal.

--

There are pros and cons to making this an action users have to consent to (and therefore, that we can inform the website that an update took place) vs a silent update. I think I prefer making these updates silently, so that RPs can build whatever experience they want around it, and we don't need to bother users twice. Showing UI for an update would also not be that dissimilar to calling create to override an existing passkey. But I would love to hear what RPs think!

@Firstyear
Copy link
Contributor

There are pros and cons to making this an action users have to consent to (and therefore, that we can inform the website that an update took place) vs a silent update. I think I prefer making these updates silently, so that RPs can build whatever experience they want around it, and we don't need to bother users twice. Showing UI for an update would also not be that dissimilar to calling create to override an existing passkey. But I would love to hear what RPs think!

I think delete here is a bit of a tricky one. For example, a resident key could be stored on a yubikey and not attached to the machine when the RP triggers the delete. This would lead to the delete call failing, and leaving the rk on the device. Similar, a lot of devices in circulation don't support ctap2.1pre or ctap2.1 anyway, so this delete action may not even work. When this delete then fails what does the RP do? Do we delete the credential anyway and then the user has to cleanup in their pwmanager / key manager? Do we insist on the device being attached so we can proceed? I think it's going to be hard to structure a UI/UX here. If we contrast to passwords today, when you change/delete a password in a password manager, the user is expected to cleanup after themself in the pw manager, so I don't know if we should try and do something different here.

I think update however could be useful. My concern though is that by having update available, rp's will rely on it to be present and will start to store PII in the ID field of the rk expecting they can update it. Contrast to what it should be which is an unchanging, permanent, unique id like a uuid. I think update should only be able to update the displayName if implemented.

Where update would be useful is for users when they want to change their displayName in the credential since rk's allow the RP displayName to de-sync from the stored displayName in the credential, so having a way to update this would be useful. I'm not sure how an RP would structure this workflow and communicate that to a user though. From a UI/UX perspective a flow to get a user to see that the displayName state is stored in two places, communicating that, and what they need to do would be extremely challenging.

We're already seeing a ton of issues in adoption of passkeys in general with confusion about how to enable them, how they work, and the ui/ux to even use them to do a login. I think throwing in more elements here could just further damage the situation.

So from the view of an RP and an RP library, my overall assessment is that while these functions have interesting properties, I likely would not implement them or use them because they risk confusing users. This is similar to my assessment of functions like isUVPAA and isPasskeyAvailable, where these functions only add extra complexity to interactions and workflows, leading to user confusion and rejection of webauth.

As a more general meta commentary, there are two aspects to standards - the standard as written and envisaged by it's authors, and the standard as actually used. I think that webauthn has a huge surface area in the standard, and really most RP's will only use a fraction of that. So far that surface area at a high level is:

  • Create new credentials (nav.cred.create)
  • Perform authentication (nav.cred.get)
  • Attest hardware security keys (attestation=direct in nav.cred.get).
  • to/from base64 for create/get requests.

Outside of those use cases, we have a lot of "fluff" in the spec that the majority of RP's won't ever deploy or use. And the small percentage that do deploy and rely on those fluffy components will run into issues. So we need to ask ourselves is update and delete just adding more fluff? Or is it going to extend and become something critical that RP's are crying out for?

@nsatragno
Copy link
Member

When this delete then fails what does the RP do? Do we delete the credential anyway and then the user has to cleanup in their pwmanager / key manager?

The credentials should always be deleted (or rendered inoperable) on the server side. The way I envision it, deleting the credential on the authenticator is a nice-to-have. If the user tries to use that credential to sign in later, the RP can then and there attempt deleting it again, where the deletion is a lot more likely to succeed.

@emlun
Copy link
Member

emlun commented Oct 5, 2023

When this delete then fails what does the RP do? Do we delete the credential anyway and then the user has to cleanup in their pwmanager / key manager?

The credentials should always be deleted (or rendered inoperable) on the server side. The way I envision it, deleting the credential on the authenticator is a nice-to-have.

This is why I proposed a state transfer approach instead of an action-oriented approach - the former eliminates the problem by letting the authenticator(s), instead of the RP, figure out what needs to change to reflect the most recent state. If the user unplugs their USB security key before clicking "delete" in the RP UI, that doesn't matter because the RP can just send the current state at the next opportunity (be it login, a credential management operation or whatever).

@Firstyear
Copy link
Contributor

I think you both have missed my point - the UI/UX around trying to coordinate a delete function as an RP will be extremely tricky to communicate to a user, and tough for an RP to coordinate and drive. Regardless of any technical ideas, I think this risks being added to the spec and not used by RP's.

@arnar
Copy link
Contributor

arnar commented Dec 5, 2023

(with @nsatragno)

We've had multiple requests for this from RPs, and some consider it blocking to implement discoverable flows as they correctly point out that account selectors will forever have stale info in them for many users. So I think this is a real issue and a solution here would be useful. (we can ask some of the RPs to chime in here if needed)

We think the state transfer approach is better for the general update - as that can be made repeatedly for cheap and thus help propagate updates to authenticators/providers that aren't always available, depending on what client the user is on. However, that only works if the user is authenticated already, so an action-type report for when a credential is rejected and no user is signed in seems useful as well.

We also agree trying to make WebAuthn a full CRUD-like API is not workable, nor desired. Rather, an opportunistic API for RPs to convey information back to authenticators and credential providers on a best-effort basis seems both useful and doable. For the lack of a better word we've been thinking about as reports RPs make to the client, knowing that sometimes they may go nowhere.

As a concrete proposal, we posted this explainer: https://github.com/w3c/webauthn/wiki/Explainer:-WebAuthn-Report-API-explainer

@jgimenez
Copy link

jgimenez commented Dec 6, 2023

Maybe a crazy take on the other direction, but I think would make things simpler: if passkeys are designed to identify a single person (maybe using multiple devices in sync), why not just remove the name and displayName from the protocol?

If they wish a label, users can still label a passkey directly on their client, but not necessarily with a name coming from the RP, the same way right now it's possible to change it in some client implementations. For example, if someone shared a passkey with me, my client could automatically label the passkey as "Alice's passkey"; or I could create a second passkey for an RP and my client would ask me how I want to name it so I can distinguish them; but by default they would have no label.

RPs can still allow a single passkey with multiple accounts by presenting an account selector after login.

This does not address the point of credential deletion, but I think that's quite a differentiated need. Maybe this doesn't need to be provided, either. After all, if I change the lock on my apartment, there's no more lock; I am responsible for disposing of the key. I can see many ways this could go wrong: RPs that disappear without wiping their user's passkeys (for example, the company goes bankrupt or the domain name is changed) or an RP accidentally removing their user's passkeys and locking them out.

@Firstyear
Copy link
Contributor

Maybe a crazy take on the other direction, but I think would make things simpler: if passkeys are designed to identify a single person (maybe using multiple devices in sync), why not just remove the name and displayName from the protocol?

Then just don't use residentKeys/discoverableCredentials. This whole issue is avoided if you do that.

@jgimenez
Copy link

jgimenez commented Dec 6, 2023

Then just don't use residentKeys/discoverableCredentials. This whole issue is avoided if you do that.

Not really, they're still useful to present credentials for a given RP. In 99% of cases you will have 1 credential, in the other cases a label can come in handy.

@serianox
Copy link

serianox commented Dec 6, 2023

As some pointed out, it is impossible nor desirable for a RP to delete remotely stale credentials:

  • security key may not be attached,
  • probably some privacy/DOS issues?

I think the only way to delete stale credentials is a user side garbage collection - the client could list all the credentials/RP and use a well-known endpoint to check if the credential still exists or can be deleted.

@arianvp
Copy link

arianvp commented Dec 6, 2023

A https://${origin}/.well-known/webauthn-credential/ping URL would be useful. But now suddenly we'd be in the business of standardising on a RP<->Client protocol which we've refrained from thus-far completely. I'd be a big step in my opinion. And not sure if it's a favorable one.

@arianvp
Copy link

arianvp commented Dec 6, 2023

Then just don't use residentKeys/discoverableCredentials. This whole issue is avoided if you do that.

There's no reason why we couldn't have a nice autofill UI even if the RP provides an allowCredentials list no? I never understood why discoverable credentials are conflated with the nice browser UI. It sounds like the most elegant solution to me. Let the RP set allowCredentials and then the client only displays the intersection of allowCredentials and the credentials that are actually stored in the browser.

Having allowCredentials: [] maybe just is not as useful as we originally thought. The RP needs to check if the returned credential is valid; so he might as well populate allowCredentials beforehand... All the info is there for the RP anyway and it will stop the client from showing stale credentials in the autofill

@nsatragno
Copy link
Member

Let the RP set allowCredentials and then the client only displays the intersection of allowCredentials and the credentials that are actually stored in the browser.

OT, but if you want to build a username first into password + autofill UI flow today you can do it. See credentialIdFilter in https://w3c.github.io/webauthn/#sctn-discover-from-external-source, conditional UI supports passing an allow list to filter the credentials. They still have to be discoverable because they need a label to be put in autofill, and non discoverable credentials may forgo storing a name / display name.

@arnar
Copy link
Contributor

arnar commented Dec 6, 2023

Having allowCredentials: [] maybe just is not as useful as we originally thought.

The RP cannot populate the allow list unless it knows who the user is, so they'd always have to ask for a username. Granted that could be autofilled, but it'd always be two confirmations for the user, as we could not automatically submit usernames to websites wihtout user approval.

We are definitely finding that improved convenience is a much more appealing reason to use passkeys, for both RPs and users. The conclusion of that is that we really want to support a single-confirmation "type nothing" flow (aka usernameless).

Our goal is improved security, but convenience is the carrot.

It sounds like the most elegant solution to me. Let the RP set allowCredentials and then the client only displays the intersection of allowCredentials and the credentials that are actually stored in the browser.

It's not the greatest reason, but there are established use cases for FIDO/WebAuthn in reauth, where an RP explicitly does not want an account selector - only a UV step. That simplicity allows broader deployment of reauth, which is impactful for both friendly fraud and cookie theft.

The fact that a populated allowList is the only way for an RP to signal this is maybe subpar, but we can consider that legacy at this point I think.

use a well-known endpoint to check if the credential still exists or can be deleted.

I think at least I believe this is more complex in the end. Besides defining a protocol between authnrs/providers and RPs (which is doable), there are non-trivial privacy and transparency implications of how providers should use this when the user is not involved.

There are certainly privacy and DOS concerns with the report API, but here the user agent has full power to mitigate those.

[one passkey per person and] RPs can still allow a single passkey with multiple accounts by presenting an account selector after login.

That is a fairly deep change to ask of RPs, as it can go as deep as modifying their data model for accounts. I also don't think the goal is to make passkeys represent a person, as that brings a whole host of complex and non-technical issues over treating passkeys as account credentials only.

@kopy
Copy link

kopy commented Dec 9, 2023

Some thoughts from our perspective as a bigger-sized RP: the proposal by @nsatragno + @arnar addresses real concerns we have in planning passkeys rollout with existing users (and we also consider it blocking).

  • Changing login identification:
    • Email addresses: In the consumer field, email addresses change quite frequently. With conditional UI, these would lead to a lot of issues and support questions without any possibility for us to help. Often the old email is lost or deleted (e.g. migration to Gmail which is an ongoing trend) and would lead to a lot of uncertainty for the average consumer. This is a huge issue for any national RP that has been on the market for 10-15 years+. At the same time, they need passkeys the most.
    • Phone numbers: Change even more often.
  • Deleted passkeys: Stale passkeys are also part of our thoughts, but can be acceptably mitigated with the current standard by using an AllowList. However, it is not ideal:
    • For conditional UI without some local information (cookie/LocalStorage) as a hint to who the user might be, this means deleted keys will show up, confusing users indefinitely.
    • For authentication: We would still prefer an empty AllowList more because some browsers behave slightly differently when you have an AllowList that cannot be matched vs. an empty AllowList (but that is a minor issue).

We thought we would "chime in" as an RP. We know our focus here is on consumer-heavy situations, primarily focusing on passkeys/discoverable credentials. This is not to say all other perspectives are not important; it's just our perspective.

@arianvp
Copy link

arianvp commented Dec 13, 2023

OT, but if you want to build a username first into password + autofill UI flow today you can do it. See credentialIdFilter in https://w3c.github.io/webauthn/#sctn-discover-from-external-source, conditional UI supports passing an allow list to filter the credentials. They still have to be discoverable because they need a label to be put in autofill, and non discoverable credentials may forgo storing a name / display name.

This seems badly implemented across browsers. E.g. Safari doesn't seem to support this. It seems to work on Chrome though.

@timcappalli timcappalli assigned arnar and nsatragno and unassigned timcappalli Dec 20, 2023
@nadalin nadalin added this to the L3-WD-02 milestone Dec 20, 2023
@nadalin nadalin added the @Risk Items that are at risk for L3 label Dec 20, 2023
@timcappalli
Copy link
Member Author

These scenarios are planned to be addressed by: https://github.com/w3c/webauthn/wiki/Explainer:-WebAuthn-Report-API-explainer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@Risk Items that are at risk for L3 type:technical
Projects
None yet
Development

No branches or pull requests