feat: allow deleting the code credential via the admin API - #4592
feat: allow deleting the code credential via the admin API#4592walter-hostaway wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughCode credential deletion now supports ChangesCode credential deletion
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The admin API now permits deleting code credentials while preserving the existing first-factor protection where passwordless code login requires it. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The admin endpoint DELETE /admin/identities/{id}/credentials/{type}
handled every credential type except code, which fell through to the
default branch and returned 400 "Credentials type code cannot be
deleted." This made it impossible to detach an email/phone code
credential from an identity in place.
Handle CredentialsTypeCodeAuth in the delete switch. Because code is a
first factor only when passwordless login with code is enabled, apply
the last-first-factor guard only in that case; as a pure MFA second
factor it is freely removable like TOTP or lookup secrets.
Resolves ory#4462
7f80a83 to
8408528
Compare
The admin endpoint
DELETE /admin/identities/{id}/credentials/{type}handlesevery credential type except
code:totp,lookup_secret,webauthn,oidc,saml,password, anddeviceauthnare all removable, butcodefalls through to the
defaultbranch of the switch indeleteIdentityCredentialsand returns400 "Credentials type code cannot be deleted."Combined with the fact that removing the sourcing trait does not dropthe derived
codecredential, there is no way to detach an email/phonecodecredential from an identity in place (see #4462).
This change handles
CredentialsTypeCodeAuthin the delete switch. Becausecodeis a first factor only when passwordless code login is enabled, theexisting "last first factor" guard is applied only when
SelfServiceCodeStrategy.PasswordlessEnabledis true; as a pure MFA secondfactor
codeis freely removable, matchingtotp/lookup_secret. Routing itthrough the
password/oidcguard unconditionally would wrongly block removalin MFA-only deployments, where
CountActiveFirstFactorCredentialsreturns 1.Related issue(s)
Resolves #4462
Checklist
identity/handler_test.go,TestHandlerpassing).Further Comments
Design note: the naive fix (routing
codethrough the existing password/oidcguarded branch) is incorrect for MFA-only setups, where the guard would block
removing a purely-second-factor
code. Gating the guard onPasswordlessEnabledavoids that while still protecting passwordless-code-only identities from losing
their last first factor. Happy to adjust the guard semantics to maintainer
preference.
AI assistance: the analysis, patch, and tests were prepared with Claude Code;
the change is human-reviewed, built (
go build ./...,go vet ./identity/), andtested (
go test ./identity/ -run TestHandler).Summary by CodeRabbit
New Features
Bug Fixes