-
Notifications
You must be signed in to change notification settings - Fork 248
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
feat(auth): Recognize FIREBASE_AUTH_EMULATOR_HOST
environment variable
#414
Conversation
* Change endpoint urls according to the environment variable
option.WithoutAuthentication(), | ||
}, | ||
} | ||
client, err := NewClient(context.Background(), conf) |
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.
I'm not sure using no-auth config is appropriate for this test.
} | ||
if umc.providerConfigEndpoint != providerConfigEndpoint { | ||
return fmt.Errorf("providerConfigEndpoint = %q; want = %q", umc.providerConfigEndpoint, providerConfigEndpoint) | ||
if baseClient.tenantMgtEndpoint != defaultIDToolkitV2Beta1Endpoint { |
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.
Added the check for tenantMgtEndpoint
.
@maky693 thank you for the contribution! General approach LGTM but assigning to @hiranya911 for Go / Admin SDK review. |
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. Only question is should we release this without support for emulating token verification?
@hiranya911 - as a user waiting for this feature, my 2c worth is that although token verification emulation is necessary for complete end-to-end testing, making it possible to talk to the emulator at all in this manner is still useful to release as-is, since it unblocks testing of a variety of workflows |
Agreed, this would be a huge win for me. |
I'd propose a middle ground where we release a version first, where everything works but token verification panics with "not implemented" for now when the env var is set. This should ensure that SDK don't call production APIs in emulator mode and also minimize any confusion caused by the SDK verifying signatures using production public keys. (This idea can also apply to the Python SDK, using thrown exceptions instead of panics.) |
@yuchenshi I like that idea! SGTM |
Yeah, I like that too. Any chance we can get this PR updated to incorporate that change? |
Thank you for review! I'll update this PR this weekend. |
@@ -100,9 +100,10 @@ type tokenVerifier struct { | |||
expiredTokenCode string | |||
keySource keySource | |||
clock internal.Clock | |||
isEmulator bool |
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.
This might be too simple. It's better to implement custom keySource
for emulator mode.
But I don't sure custom keySource
is necessary because we didn't decide how to verify tokens in the emulator mode yet.
@yuchenshi Is the point of the change to prevent calling production APIs in emulator mode? |
For Auth Emulator doesn't support multi-tenancy (at least for now) and I'm indifferent whether the SDK should panic or forward them to the Auth Emulator which rejects such requests. I'm fine as long as they won't reach production. |
Ah, I understand. I'll update not to sign tokens. |
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.
Looks reasonable to me.
@yuchenshi wdyt?
…HOST environment variable. (#531) * Support auth emulator via FIREBASE_AUTH_EMULATOR_HOST Modeled on firebase/firebase-admin-go#414 * Tests for emulator support in auth, user mgmt and token gen To minimize modification of tests, the app fixture and instrumentation have been modified to use a global dict of URLs, which are then monkey-patched based on fixture parameters. Essentially, all tests using the app fixture are run twice, once with the emulated endpoint and once without. * fallback for monkeypatch in python 3.5 * Token verification for the auth emulator * Accommodate auth emulator behaviour in tests. Where possible, tests are modified to account for the current behaviour in emulator mode (e.g., invalid or expired tokens or cookies still work). Fixtures were changed to function scope to avoid problems caused by overlap when some fixtures being in emulator mode and some in normal mode concurrently.
If
FIREBASE_AUTH_EMULATOR_HOST
is set, auth API request will be sent to the host according to the environment variable.Discussion
#409
Reflecting the comment on the issue, the behavior of signers and token verifiers are unchanged.Now the signer is replaced to
emulatedSigner
, and token verifiers panic when the environment variable is set. #414 (comment) #414 (comment)RELEASE NOTE: The Admin SDK now supports the Firebase emulator. Developers can configure the SDK to run against the emulator by setting the
FIREBASE_AUTH_EMULATOR_HOST
environment variable.