-
-
Notifications
You must be signed in to change notification settings - Fork 964
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
feat: add a new admin API to remove a specific 2nd factor credential #2962
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice! Only a few nit picks :)
Codecov Report
@@ Coverage Diff @@
## master #2962 +/- ##
==========================================
- Coverage 77.31% 77.30% -0.01%
==========================================
Files 313 313
Lines 19454 19462 +8
==========================================
+ Hits 15041 15046 +5
- Misses 3254 3255 +1
- Partials 1159 1161 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, this is very neat! Also great tests! There's two things that need to be improved in the tests, but I think those changes should be doable in little time :) Once these are addressed, this is good to go to master!
@aeneasr thanks for the review, I should have fixed all your raised concerns. Let me know if it looks good :) 🙏 |
e1dbe10
to
703eb16
Compare
Fix SDK generation Add some more tests cases
703eb16
to
b0b246f
Compare
Great work! |
Hellooo the type is actually |
This PR provide a way for an admin to remove a specific credential method.
For now, only 2nd factor methods are allowed (TOTP, Lookup Secrets, WebAuthn)
This endpoint can be used by support services to unlock a custom account in case they've lost they MFA device.
Following the discussion with issue #2505 i've decided to implement in the following way:
DELETE /admin/identities/{id}/credential/{type}
id: being the targeted user id
type: being one of the 2FA kratos supports (totp, lookup, webauthn)
Related issue(s)
#2505
Checklist
introduces a new feature.
contributing code guidelines.
vulnerability. If this pull request addresses a security vulnerability, I
confirm that I got the approval (please contact
security@ory.sh) from the maintainers to push
the changes.
works.
Further Comments
I've moved the credentials config models found previously in the
selfservice/strategy
pkg to theidentity
package to be able to use them from the admin endpoints and to avoid some cyclic pkg inclusion