firebase / firebase-tools

The Firebase Command Line Tools
MIT License
4.02k stars 934 forks source link

Allow for static sms code #4290

Open cedvdb opened 2 years ago

cedvdb commented 2 years ago

Currently the emulator returns a random String code when authenticating with phone against it. You can use this code to authenticate.

However for testing it would be much easier if the code was predictable, either via a configuration option or just a static code usage.

I've submitted a PR https://github.com/firebase/firebase-tools/pull/4255 that makes the code to always be '123456' but I do not know if that's the best option here.

yuchenshi commented 2 years ago

@cedvdb The verification code in the emulator can be retrieved via a simple REST call for non-interactive testing as documented here: https://firebase.google.com/docs/emulator-suite/connect_auth#non-interactive_testing_2

The reason behind random digits is mostly to be closer to production, especially when developers want to test 2+ concurrent sign-ins. If the two API calls return the same digits, then it is hard to test the logic to use the correct verification code for the right session. Hope that makes sense.

yuchenshi commented 2 years ago

Would it unblock you if we support something like this in production, where you can predefine some phone numbers (of your choice) to have fixed verification codes? You can set them to 123456 if you like to, but you don't have to.

cedvdb commented 2 years ago

If the endpoint works I suppose this is resolved and my PR can be closed. I've implemented the retrieval but I have other tasks first before trying it out. I'll let you know in the upcoming days how that went.

cedvdb commented 2 years ago

@cedvdb The verification code in the emulator can be retrieved via a simple REST call for non-interactive testing as documented here: https://firebase.google.com/docs/emulator-suite/connect_auth#non-interactive_testing_2

This seems to work well for the first authentication, however when running automated tests, the second authentication does not regenerate a new code. I've searched a long time but cannot figure out why. I'll skip those tests for now.

It works fine when doing it manually so it might have something to do with the android emulator used.

yuchenshi commented 2 years ago

@cedvdb Is there any chance you can get a step-by-step repro? We'll gladly look into this.

cedvdb commented 2 years ago

I made a repro

https://github.com/cedvdb/flutter_repros/tree/failing_auth

However it might require some setup:

Then you can run the project on an emulator and you will see that the second time you login (press sign in, then sign out, then sign in again), the emulator does not regenerate a sms code.


When recording the below gif I noticed the code was actually generated the second time, it is because I did it slowly, keep watching as I tries to do it fast it will fail. Sorry if the gif is a bit slow, I was surprised to see it working at first.

Recording 2022-03-28 at 16 50 23


cedvdb commented 2 years ago

@yuchenshi have you had the chance of using the repro ? I've a feeling the issue might not be with the emulator but with the underlying sdk as this seems to work with web.

In any case, I'd appreciate if something like was done in my PR was added where you can specify the sms code in advance with the emulator option. That'd make life very much easier as currently, to use test phone numbers with the real firebase, you essentially have a static code, so I think both behavior should be streamlined

yuchenshi commented 2 years ago

@cedvdb Thanks for sharing your findings about the issue not being present in web. That hints at issue being in some specific interactions between Flutter and Auth emulator, and we're waiting for my colleagues with Flutter expertise to take a look.

On the other hand though, I think your feature request is a perfectly reasonable one. As we strive to make the Auth emulator align with production behavior, we'd like to make static SMS code opt-in just like production. As such, we're open to reviewing your PR if you could modify it to allow such configuration using the same API as production.

While this may sounds like an extra step for your particular test setup, it actually helps us stay in sync with production, which unblocks many other developers from using the same setup script for both production and emulator-based integration testing.

LMK what you think! The config implementation can live somewhere here, BTW:

https://github.com/firebase/firebase-tools/blob/4ee276e0775fbb43d777ea4e1237f94f966d4a6a/src/emulator/auth/operations.ts#L2003

cedvdb commented 2 years ago

LMK what you think!

I've hit other issues with the emulator, which I'd say might not be due to the emulator but could be due to the client firebase sdk. Long story short, the testing strategy has changed to not use the emulator as there are too many roadblocks currently.

I'd have gladly help with this if that was a win for me too, but as it stands I'm quite busy so I'll prioritize my time for stuff that has impact for me.

FYI, the other issue that was hit with the emulator was when using firebase storage on the web. An error was thrown (by the sdk) that an unknown error happened and to check the http response. The http response was however successful, the emulator was dropped at that point, for the time being.

yuchenshi commented 2 years ago

@cedvdb Sorry to hear you're having troubles here. We take tackling these kind of roadblocks seriously and I'll be relaying your feedback to the right team too.

mikehardy commented 2 years ago

Of note: we're implementing multi-factor auth in react-native-firebase as well, and with phone auth it appears that we ran into this as well. If you auth the user a second time with the same phone number, apparently the emulator prints out a new code in the log but it is not available via the REST endpoint?

reference: https://github.com/invertase/react-native-firebase/pull/6593 https://github.com/invertase/react-native-firebase/pull/6593/files#diff-50df2eada48dcacc67e8af24c907da9a349aa7fb0fd2e3ed01aaf0258a2f04baR291-R294

I like the REST endpoint (mainly because it's working code in our repo, and I like working code...) just seems there may be a fault in the code generation + addition to REST endpoint set of codes pathway?

yuchenshi commented 2 years ago

@mikehardy Would you mind opening a separate issue with more details on generating a code the second time? A few steps or a repro will help a lot.