-
Notifications
You must be signed in to change notification settings - Fork 289
feat(auth): Add auth emulator support #523
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
Conversation
* Adds Firebase Auth emulator support for FirebaseUserManager and FirebaseTenantClient classes. * Adds tests and minor fixes
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.
Thanks @ssbushi. Looks mostly good. My main observations are:
- We should avoid duplication by adding emulator support to the existing
FirebaseTokenVerfierImpl
class, as opposed to implementing a whole new class. - Avoid duplication by putting env variable lookups in a shared util.
- We need a better way to test the functionality of all APIs in emulator mode. In other languages we used techniques like test parameterization and fixtures to achieve this. We should try to find an equivalent strategy for junit4 (although it's ok to do so in a separate PR).
src/main/java/com/google/firebase/auth/FirebaseEmulatorTokenVerifier.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/firebase/auth/FirebaseUserManager.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/firebase/auth/FirebaseUserManager.java
Outdated
Show resolved
Hide resolved
src/test/java/com/google/firebase/auth/multitenancy/FirebaseTenantClientTest.java
Show resolved
Hide resolved
src/test/java/com/google/firebase/auth/FirebaseTokenVerifierImplTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/google/firebase/auth/FirebaseEmulatorTokenVerifierTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/google/firebase/auth/FirebaseTokenVerifierImplTest.java
Show resolved
Hide resolved
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.
Thanks for making the changes. Looks pretty good. Just a few more minor updates needed.
src/main/java/com/google/firebase/auth/FirebaseTokenVerifierImpl.java
Outdated
Show resolved
Hide resolved
src/test/java/com/google/firebase/auth/FirebaseTokenVerifierImplTest.java
Outdated
Show resolved
Hide resolved
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.
LGTM 👍
Discussion
Add support to communicate with the Auth Emulator #493.
Testing
API Changes