Allow admin to remove MFA credentials (TOTP, lookup secrets, webauthn)

[halms]

Preflight checklist

• I could not find a solution in the existing issues, docs, nor discussions.
• I agree to follow this project's Code of Conduct.
• I have read and am following this repository's Contribution Guidelines.
• This issue affects my Ory Cloud project.
• I have joined the Ory Community Slack.
• I am signed up to the Ory Security Patch Newsletter.

Describe your problem

If a user loses all their configured MFA credentials (when reuired_aal is set to highest_available for the profile) there is currently no exposed way to remove the MFA credentials.

I consider it a sensible feature to expose an endpoint on the admin API to remove/reset MFA.

There are currently a few open PRs that lay the groundwork for this:
#2423 will allow updating credentials in general
#2380 will allow selectively updating the only credentials
#2438 will allow adding totp and lookup_secrets

What I consider still missing, is a way to remove the credentials via an admin API endpoint.

Describe your ideal solution

Solution 1:
Allow the PUT /admin/identities/{id} (and the future PATCH /admin/identities/{id}) to remove credentials
Possibly by passing something like (YAML for simpler notation)

credentials:
  totp: null
  webauthn: null
  lookup_secrets: null

Solution 2:
Create a new DELETE /admin/identities/{id}/mfa or similar endpoint

Solution 3:
Create a new DELETE /admin/identities/{id}/credentials/{type} (type being totp, webauthn, lookup_secrets) or similar endpoint.

Workarounds or alternatives

Any of the three solutions would fulfill the need. My personal preference would be to offer both solution 1 and one of 2 or 3, with 3 being probably the cleaner one.

Version

v0.10.1

Additional Context

No response

[maaartyyynaa]

Hi! This feature would be also useful for us and we are starting to work on the Solution 3 implementation

[aeneasr]

I think this is a great idea and solution 3 looks very good to me

[supercairos]

I would love to work on this. Is there any draft for this issue ?
What are the pitfall when removing MFA credential from a user ?

While checking the remove method from the settings part I could see:

• webauthn:
  We need to check that the webauthn credential is not the only enabled stategy:

count, err := s.d.IdentityManager().CountActiveFirstFactorCredentials(r.Context(), i)
if err != nil {
    return err
}
if count < 2 && wasPasswordless {
    return s.handleSettingsError(w, r, ctxUpdate, p, errors.WithStack(ErrNotEnoughCredentials))
}

Once this is done, we can safely remove the credential (identified by ID) by deep-coping the Credential array and removing the said WebAuthn credential:

updated := make([]Credential, 0)
for k, cred := range cc.Credentials {
    if fmt.Sprintf("%x", cred.ID) != p.Remove {
	updated = append(updated, cc.Credentials[k])
    }
}

if len(updated) == 0 {
    i.DeleteCredentialsType(identity.CredentialsTypeWebAuthn)
    ctxUpdate.UpdateIdentity(i)
    return nil
}

cc.Credentials = updated
cred.Config, err = json.Marshal(cc)
if err != nil {
    return errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unable to encode identity credentials.").WithDebug(err.Error()))
}
i.SetCredentials(s.ID(), *cred)

ctxUpdate.UpdateIdentity(i)

https://github.com/ory/kratos/blob/master/selfservice/strategy/webauthn/settings.go#L174

• totp:
  It's only a matter of removing the Credential and saving the Identity

i.DeleteCredentialsType(identity.CredentialsTypeTOTP)
ctxUpdate.UpdateIdentity(i)

https://github.com/ory/kratos/blob/master/selfservice/strategy/totp/settings.go#L226

• lookup_code:
  It's only a matter of removing the Credential and saving the Identity

i.DeleteCredentialsType(identity.CredentialsTypeLookup)
ctxUpdate.UpdateIdentity(i)

https://github.com/ory/kratos/blob/master/selfservice/strategy/lookup/settings.go#L180

[aeneasr]

Sounds great! How would the REST API for this look like?

[supercairos]

I was thinking we could have a simple API as we discussed previously (solution 3):

• DELETE /admin/identities/{id}/credentials/{mfa_id} to delete the specific mfa mechanism (not identified by type but by ID as it's more flexible [a user can have multiple Hardware Device for exemple])

[supercairos]

After trying to implement, i've decided to go with the simple solution and implement only a deletion by type.
Meaning that if you have 3 webauthn keys setup they will all get destroyed at once.

If you want to implement it by mfa_id you would need a new API to query credentials available of a specific user that lists all the webauthn, topt and lookup that a user has setup.
Whereas today, querying the GET /admin/identities/{id} only provide you what types of credentials a user has setup (but doesn't dive into it).