Skip to content
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

Warning: Undefined array key "mfaEnrollmentId" #783

Closed
guillaumesmo opened this issue Apr 2, 2023 · 7 comments
Closed

Warning: Undefined array key "mfaEnrollmentId" #783

guillaumesmo opened this issue Apr 2, 2023 · 7 comments
Assignees

Comments

@guillaumesmo
Copy link

Describe the bug

When 2FA is enabled in Firebase Auth, and a user has enrolled a 2FA method, Auth::listUsers and Auth::getUser throw the following exception

ErrorException: "Warning: Undefined array key "mfaEnrollmentId"" at .../vendor/kreait/firebase-php/src/Firebase/Auth/MfaInfo.php line 42

this seems to be caused by the API returning an array of MfaInfo, while the php lib expects only one

I was able to fix with the following changes:
7.x...guillaumesmo:firebase-php:7.x

Installed packages

"kreait/firebase-php": "^7.0",


### PHP version and extensions

```shell
php                    8.2.3

Steps to reproduce the issue.

$auth->getUser($userId);

Error message/Stack trace

ErrorException: "Warning: Undefined array key "mfaEnrollmentId"" at .../vendor/kreait/firebase-php/src/Firebase/Auth/MfaInfo.php line 42

Additional information

No response

@jeromegamez
Copy link
Member

Thanks for reporting this - I had implemented this to resolve #749, but I wasn't able to test this properly (and @jesperbjerke, who requested it, might not have either 😅).

As far as I can see (from my phone) I can/would not change the existing property as it would be a breaking change, and introduce a new property instead (perhaps mfaEnrollments).

Can I rely on you properly testing this for me once I create the according PR? 🙏🏻

@guillaumesmo
Copy link
Author

Firebase supports only phone MFA, so I don't think it's needed to add another property for now as it will only contain the single item which is already there. Instead the property can be passed the only item if present, or null

And yes, I'm able to test this for you

@jeromegamez
Copy link
Member

Could you give #784 a try?

@guillaumesmo
Copy link
Author

Hello,

tested:

kreait/firebase-php                   dev-mfa-info 21b918b

works as expected!

thank you very much

@jeromegamez
Copy link
Member

Thank you 🙏🏻! I will then hopefully merge and release it later this evening, tomorrow at latest!

@jeromegamez
Copy link
Member

Fixed with 8d82198

@guillaumesmo
Copy link
Author

Thank you for the fix. Awesome package!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants