Skip to content
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

Emulator Idempotency: Auth #8750

Merged
merged 7 commits into from
Feb 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/lemon-candles-vanish.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@firebase/auth': patch
'firebase': patch
---

Fixed: invoking `connectAuthEmulator` multiple times with the same parameters will no longer cause
an error. Fixes [GitHub Issue #6824](https://github.com/firebase/firebase-js-sdk/issues/6824).

35 changes: 35 additions & 0 deletions packages/auth/src/core/auth/emulator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,41 @@ describe('core/auth/emulator', () => {
);
});

it('passes with same config if a network request has already been made', async () => {
expect(() => connectAuthEmulator(auth, 'http://127.0.0.1:2020')).to.not
.throw;
await user.delete();
expect(() => connectAuthEmulator(auth, 'http://127.0.0.1:2020')).to.not
.throw;
});

it('fails with alternate config if a network request has already been made', async () => {
expect(() => connectAuthEmulator(auth, 'http://127.0.0.1:2020')).to.not
.throw;
await user.delete();
expect(() => connectAuthEmulator(auth, 'http://127.0.0.1:2021')).to.throw(
FirebaseError,
'auth/emulator-config-failed'
);
});

it('subsequent calls update the endpoint appropriately', async () => {
connectAuthEmulator(auth, 'http://127.0.0.1:2021');
expect(auth.emulatorConfig).to.eql({
protocol: 'http',
host: '127.0.0.1',
port: 2021,
options: { disableWarnings: false }
});
connectAuthEmulator(auth, 'http://127.0.0.1:2020');
expect(auth.emulatorConfig).to.eql({
protocol: 'http',
host: '127.0.0.1',
port: 2020,
options: { disableWarnings: false }
});
});

it('updates the endpoint appropriately', async () => {
connectAuthEmulator(auth, 'http://127.0.0.1:2020');
await user.delete();
Expand Down
40 changes: 31 additions & 9 deletions packages/auth/src/core/auth/emulator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { Auth } from '../../model/public_types';
import { AuthErrorCode } from '../errors';
import { _assert } from '../util/assert';
import { _castAuth } from './auth_impl';
import { deepEqual } from '@firebase/util';

/**
* Changes the {@link Auth} instance to communicate with the Firebase Auth Emulator, instead of production
Expand Down Expand Up @@ -47,12 +48,6 @@ export function connectAuthEmulator(
options?: { disableWarnings: boolean }
): void {
const authInternal = _castAuth(auth);
_assert(
authInternal._canInitEmulator,
authInternal,
AuthErrorCode.EMULATOR_CONFIG_FAILED
);

_assert(
/^https?:\/\//.test(url),
authInternal,
Expand All @@ -66,15 +61,42 @@ export function connectAuthEmulator(
const portStr = port === null ? '' : `:${port}`;

// Always replace path with "/" (even if input url had no path at all, or had a different one).
authInternal.config.emulator = { url: `${protocol}//${host}${portStr}/` };
authInternal.settings.appVerificationDisabledForTesting = true;
authInternal.emulatorConfig = Object.freeze({
const emulator = { url: `${protocol}//${host}${portStr}/` };
const emulatorConfig = Object.freeze({
host,
port,
protocol: protocol.replace(':', ''),
options: Object.freeze({ disableWarnings })
});

// There are a few scenarios to guard against if the Auth instance has already started:
if (!authInternal._canInitEmulator) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this technically works but it doesn't make the logic clear. There's two reasons to throw here, one is if they're calling this for the first time too late (after a network call has been made, that's the purpose of authInternal._canInitEmulator), and the second reason is if they're calling it for a second time with different args. This makes it look like we're only targeting the second case. It will actually work on the first case, of course, because the local vars emulator and emulatorConfig will have some properties, and won't match {}, but I guess the fact it's nested inside a deepEqual doesn't make it clear that's a case we're trying to block on.

I know it's more verbose, but instead of the || {} maybe a separate _assert before this one that just tests for the existence of authInternal.config.emulator and authInternal.emulatorConfig would make it clearer that we're covering 2 different cases?

// Applications may not initialize the emulator for the first time if Auth has already started
// to make network requests.
_assert(
authInternal.config.emulator && authInternal.emulatorConfig,
authInternal,
AuthErrorCode.EMULATOR_CONFIG_FAILED
);

// Applications may not alter the configuration of the emulator (aka pass a different config)
// once Auth has started to make network requests.
_assert(
deepEqual(emulator, authInternal.config.emulator) &&
deepEqual(emulatorConfig, authInternal.emulatorConfig),
authInternal,
AuthErrorCode.EMULATOR_CONFIG_FAILED
);

// It's valid, however, to invoke connectAuthEmulator() after Auth has started making
// connections, so long as the config matches the existing config. This results in a no-op.
return;
}

authInternal.config.emulator = emulator;
authInternal.emulatorConfig = emulatorConfig;
authInternal.settings.appVerificationDisabledForTesting = true;

if (!disableWarnings) {
emitEmulatorWarning();
}
Expand Down
Loading