fix(auth): capture FIREBASE_AUTH_EMULATOR_HOST at init time#3080
fix(auth): capture FIREBASE_AUTH_EMULATOR_HOST at init time#3080daytonmux wants to merge 2 commits intofirebase:mainfrom
Conversation
- Updated constructors in `AuthResourceUrlBuilder`, `TenantAwareAuthResourceUrlBuilder`, `AuthHttpClient`, and `AbstractAuthRequestHandler` to accept an optional emulator host parameter. - Modified token generation and verification methods in `BaseAuth` to utilize the emulator mode based on the new parameter. - Added tests to ensure correct behavior of emulator settings, including persistence after environment variable changes. - Improved handling of emulator state in `TenantManager` and `TenantAwareAuth` classes.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello @daytonmux, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refactors the handling of the FIREBASE_AUTH_EMULATOR_HOST environment variable to capture its value at initialization time for each Auth instance, making the emulator configuration instance-specific rather than global. The changes are consistently applied across auth-api-request.ts, base-auth.ts, and tenant-manager.ts to ensure the emulator state is correctly managed and propagated, including for lazily-created tenant-aware auth instances. The new tests in auth-api-request.spec.ts and auth.spec.ts effectively validate this new behavior. The implementation appears correct and complete.
Summary
Fixes #2948.
FIREBASE_AUTH_EMULATOR_HOSTwas read fromprocess.envon every API call, which meant it affected all initialized apps globally. This made it impossible to have one app pointing to the emulator and another to the cloud project in the same process — unlike Firestore, Database, and Storage which already capture their emulator env vars at init time.This change captures the value of
FIREBASE_AUTH_EMULATOR_HOSTonce when theAuthservice is initialized and stores it per-instance, so each app remembers its own emulator configuration independently.What changed
auth-api-request.ts:AuthResourceUrlBuilder,TenantAwareAuthResourceUrlBuilder, andAuthHttpClientnow receive the emulator host/flag as constructor parameters instead of reading fromprocess.env.AbstractAuthRequestHandlercapturesemulatorHost()once at construction and threads it through to all URL builders and the HTTP client.TenantAwareAuthRequestHandleraccepts an optional emulator host override so lazily-created tenant handlers inherit the frozen value.base-auth.ts:BaseAuthderives itsemulatorModefrom the request handler captured value instead of callinguseEmulator()on everyverifyIdToken/verifySessionCookie/_verifyAuthBlockingTokencall.createFirebaseTokenGeneratoraccepts an optionalisEmulatoroverride for the same reason.tenant-manager.ts:TenantManagercaptures the emulator host at construction and passes it toTenantAwareAuthinstances created lazily byauthForTenant(), so tenant auth inherits the original emulator state.Test plan
npm test)auth-api-request.spec.ts:auth.spec.ts:verifyIdToken()retains emulator behavior after env var deletionverifySessionCookie()retains emulator behavior after env var deletionauthForTenant()does not use emulator when env var set afterAuthinitauthForTenant()uses emulator when env var set beforeAuthinit (and deleted after)