Reset multifactor authentication#991
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 7.x #991 +/- ##
============================================
+ Coverage 89.86% 90.09% +0.22%
- Complexity 1438 1443 +5
============================================
Files 140 140
Lines 4143 4168 +25
============================================
+ Hits 3723 3755 +32
+ Misses 420 413 -7 🚀 New features to boost your workflow:
|
|
Thank you for your contribution! Unfortunately, we can't add or remove methods to/from the interfaces because this would break backward compatibility. Could you revert the changes in the Auth interface and implementation? MFA resets will have to go through |
|
Could you please also add a test for the new property to https://github.com/kreait/firebase-php/blob/7.x/tests/Integration/Request/UpdateUserTest.php ? |
|
I have added tests and removed the functions from the auth interface and implementation. In order to add tests, I also had to add a way to add multi factor authentication, otherwise there was nothing to remove, so I also added a feature to add phone second factors. According to the firebase documentation, you should also be able to add multi factor authentication on user creation, but I had trouble getting this to work, so I limited it to updating users only. I based my implementation on setting multi factor authentication on the available parameters in https://github.com/firebase/firebase-admin-node/blob/master/src/auth/auth-api-request.ts#L248 I had trouble running the integration tests locally, but the content of the testcases seemed to work when I tried the code on an actual firebase project |
|
Uuuh, that's even more than what I asked for, I just wanted the new lines to be covered to make sure that the Firebase API doesn't return errors, this is much better, thank you! I'll have a closer look once I can find some time. Thanks again! |
|
And this was when I realized that Emulator- and Integration tests are not executed with external PRs 😅 (probably because of the secrets needed). I checked out your PR locally and fixed the tests for them to pass (you couldn't know they were not working when you weren't able to run them). Users need a verified email in order to add MFA Info 🤔.o0O(💡). I'm going to create a new PR based on this one, so that the test suites indeed run, hopefully in the upcoming days (it's 1:40 am where I am right now and I have to go to sleep 😅) |
Thank you! Whoops, did not notice that a verified email is required to add MFA. In my project I create users with already verified emails. |
|
Merged with 7dd11e6 - thanks a lot, again! 🫶🏻 |
We are using multi factor authentication using TOTP factors. We need to be able to trigger a multi factor authentication reset in case the user lost their secondary factors.