Skip to content

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

Merged
merged 13 commits into from
Apr 16, 2021
Merged

Conversation

ssbushi
Copy link
Contributor

@ssbushi ssbushi commented Apr 8, 2021

Discussion

Add support to communicate with the Auth Emulator #493.

Testing

  • Unit tests.
  • Manual testing.

API Changes

  • N/A.

@ssbushi ssbushi marked this pull request as ready for review April 8, 2021 17:56
@ssbushi ssbushi requested review from hiranya911 and yuchenshi April 8, 2021 17:56
Copy link
Contributor

@hiranya911 hiranya911 left a 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:

  1. We should avoid duplication by adding emulator support to the existing FirebaseTokenVerfierImpl class, as opposed to implementing a whole new class.
  2. Avoid duplication by putting env variable lookups in a shared util.
  3. 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).

@ssbushi ssbushi requested a review from hiranya911 April 14, 2021 22:47
Copy link
Contributor

@hiranya911 hiranya911 left a 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.

@ssbushi ssbushi requested a review from hiranya911 April 15, 2021 15:32
Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@hiranya911 hiranya911 changed the title Add auth emulator support feat(auth): Add auth emulator support Apr 15, 2021
@ssbushi ssbushi merged commit 1e26ef3 into master Apr 16, 2021
@ssbushi ssbushi deleted the add-auth-emulator-support branch April 16, 2021 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants