feat: Add clearAll() API to credentials manager#951
feat: Add clearAll() API to credentials manager#951pmathew92 wants to merge 3 commits intov4_developmentfrom
Conversation
There was a problem hiding this comment.
the test name says "AndDPoPKeyPair" but we're only verifying storage.removeAll() here. If someone accidentally removes the DPoP.clearKeyPair() call from the implementation, this test would still pass. Can we add a verify for the DPoP cleanup too?
There was a problem hiding this comment.
The name is misleading. We can't directly verify the DPoP.clearKeyPair() as Mockito can't validate the call for Kotlin static calls. Hence didn't add that check
| } | ||
|
|
||
| @Test | ||
| public fun shouldClearAllCredentialsKeyPairsAndDPoPKeyPair() { |
There was a problem hiding this comment.
the test name mentions "AndDPoPKeyPair" but we're not verifying DPoP.clearKeyPair() was called. Also missing a verify for clearBiometricSession(). Would be good to cover both so the test actually validates everything the method does.
There was a problem hiding this comment.
The name is misleading. We can't directly verify the DPoP.clearKeyPair() as Mockito can't validate the call for Kotlin static calls. Hence didn't add that check
| */ | ||
| override fun clearAll() { | ||
| storage.removeAll() | ||
| crypto.deleteAllKeys() |
There was a problem hiding this comment.
Small thought — if crypto.deleteAllKeys() throws something unexpected, then clearBiometricSession() and DPoP.clearKeyPair() get skipped. Since this is a "clean up everything" method,can we wrap each step independently so one failure doesn't prevent the others from running?
There was a problem hiding this comment.
CryptoUtils , deleteAllKeys internally calls the existing deleteRSAKeys and deleteAESKeys method which internally handles if any exception being thrown. So any failure here wouldn't affect the other APIs
3 participants
Changes
Adds a new
clearAll()method toBaseCredentialsManager,CredentialsManager, andSecureCredentialsManagerthat performs a complete cleanup of all stored credentials and cryptographic key pairs.Testing
Please describe how this can be tested by reviewers. Be specific about anything not tested and reasons why. Since this library has unit testing, tests should be added for new functionality and existing tests should complete without errors.
This change adds unit test coverage
This change adds integration test coverage
This change has been tested on the latest version of the platform/language or why not
Checklist
I have read the Auth0 general contribution guidelines
I have read the Auth0 Code of Conduct
All existing and new tests complete without errors