Skip to content

Throw an error when the Auth (exp) internal instance is used by another SDK before Auth initialization #4428

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.

Sign up for GitHub

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 2 commits into from
Feb 8, 2021
Merged

Throw an error when the Auth (exp) internal instance is used by another SDK before Auth initialization #4428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e668a1d
Make the prod error map be verbose for this one error
sam-gc Feb 5, 2021
9ee881c
Formatting
sam-gc Feb 5, 2021
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
46 changes: 45 additions & 1 deletion packages-exp/auth-exp/src/core/auth/firebase_internal.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,18 @@
* limitations under the License.
*/

import { expect } from 'chai';
import { FirebaseError } from '@firebase/util';
import { expect, use } from 'chai';
import * as sinon from 'sinon';
import * as chaiAsPromised from 'chai-as-promised';

import { testAuth, testUser } from '../../../test/helpers/mock_auth';
import { Auth } from '../../model/auth';
import { User } from '../../model/user';
import { AuthInternal } from './firebase_internal';

use(chaiAsPromised);

describe('core/auth/firebase_internal', () => {
let auth: Auth;
let authInternal: AuthInternal;
Expand All @@ -45,6 +49,16 @@ describe('core/auth/firebase_internal', () => {
await auth._updateCurrentUser(user);
expect(authInternal.getUid()).to.eq('uid');
});

it('errors if Auth is not initialized', () => {
delete ((auth as unknown) as Record<string, unknown>)[
'_initializationPromise'
];
expect(() => authInternal.getUid()).to.throw(
FirebaseError,
'auth/dependent-sdk-initialized-before-auth'
);
});
});

context('getToken', () => {
Expand All @@ -62,6 +76,16 @@ describe('core/auth/firebase_internal', () => {
accessToken: 'access-token'
});
});

it('errors if Auth is not initialized', async () => {
delete ((auth as unknown) as Record<string, unknown>)[
'_initializationPromise'
];
await expect(authInternal.getToken()).to.be.rejectedWith(
FirebaseError,
'auth/dependent-sdk-initialized-before-auth'
);
});
});

context('token listeners', () => {
Expand Down Expand Up @@ -115,6 +139,16 @@ describe('core/auth/firebase_internal', () => {

expect(tokenCount).to.eq(5);
});

it('errors if Auth is not initialized', () => {
delete ((auth as unknown) as Record<string, unknown>)[
'_initializationPromise'
];
expect(() => authInternal.addAuthTokenListener(() => {})).to.throw(
FirebaseError,
'auth/dependent-sdk-initialized-before-auth'
);
});
});

context('removeAuthTokenListener', () => {
Expand Down Expand Up @@ -168,6 +202,16 @@ describe('core/auth/firebase_internal', () => {
authInternal.addAuthTokenListener(listenerB);
expect(isProactiveRefresh).to.be.true;
});

it('errors if Auth is not initialized', () => {
delete ((auth as unknown) as Record<string, unknown>)[
'_initializationPromise'
];
expect(() => authInternal.removeAuthTokenListener(() => {})).to.throw(
FirebaseError,
'auth/dependent-sdk-initialized-before-auth'
);
});
});
});
});
13 changes: 13 additions & 0 deletions packages-exp/auth-exp/src/core/auth/firebase_internal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import { FirebaseAuthInternal } from '@firebase/auth-interop-types';

import { Auth } from '../../model/auth';
import { User } from '../../model/user';
import { _assert } from '../util/assert';
import { AuthErrorCode } from '../errors';

interface TokenListener {
(tok: string | null): unknown;
Expand All @@ -34,12 +36,14 @@ export class AuthInternal implements FirebaseAuthInternal {
constructor(private readonly auth: Auth) {}

getUid(): string | null {
this.assertAuthConfigured();
return this.auth.currentUser?.uid || null;
}

async getToken(
forceRefresh?: boolean
): Promise<{ accessToken: string } | null> {
this.assertAuthConfigured();
await this.auth._initializationPromise;
if (!this.auth.currentUser) {
return null;
Expand All @@ -50,6 +54,7 @@ export class AuthInternal implements FirebaseAuthInternal {
}

addAuthTokenListener(listener: TokenListener): void {
this.assertAuthConfigured();
if (this.internalListeners.has(listener)) {
return;
}
Expand All @@ -62,6 +67,7 @@ export class AuthInternal implements FirebaseAuthInternal {
}

removeAuthTokenListener(listener: TokenListener): void {
this.assertAuthConfigured();
const unsubscribe = this.internalListeners.get(listener);
if (!unsubscribe) {
return;
Expand All @@ -72,6 +78,13 @@ export class AuthInternal implements FirebaseAuthInternal {
this.updateProactiveRefresh();
}

private assertAuthConfigured(): void {
_assert(
this.auth._initializationPromise,
AuthErrorCode.DEPENDENT_SDK_INIT_BEFORE_AUTH
);
}

private updateProactiveRefresh(): void {
if (this.internalListeners.size > 0) {
this.auth._startProactiveRefresh();
Expand Down
17 changes: 16 additions & 1 deletion packages-exp/auth-exp/src/core/errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export const enum AuthErrorCode {
CREDENTIAL_ALREADY_IN_USE = 'credential-already-in-use',
CREDENTIAL_MISMATCH = 'custom-token-mismatch',
CREDENTIAL_TOO_OLD_LOGIN_AGAIN = 'requires-recent-login',
DEPENDENT_SDK_INIT_BEFORE_AUTH = 'dependent-sdk-initialized-before-auth',
DYNAMIC_LINK_NOT_ACTIVATED = 'dynamic-link-not-activated',
EMAIL_CHANGE_NEEDS_VERIFICATION = 'email-change-needs-verification',
EMAIL_EXISTS = 'email-already-in-use',
Expand Down Expand Up @@ -152,6 +153,10 @@ function _debugErrorMap(): ErrorMap<AuthErrorCode> {
[AuthErrorCode.CREDENTIAL_TOO_OLD_LOGIN_AGAIN]:
'This operation is sensitive and requires recent authentication. Log in ' +
'again before retrying this request.',
[AuthErrorCode.DEPENDENT_SDK_INIT_BEFORE_AUTH]:
'Another Firebase SDK was initialized and is trying to use Auth before Auth is ' +
'initialized. Please be sure to call `initializeAuth` or `getAuth` before ' +
'starting any other Firebase SDK.',
[AuthErrorCode.DYNAMIC_LINK_NOT_ACTIVATED]:
'Please activate Dynamic Links in the Firebase Console and agree to the terms and ' +
'conditions.',
Expand Down Expand Up @@ -351,7 +356,15 @@ export interface ErrorMapRetriever extends externs.AuthErrorMap {
}

function _prodErrorMap(): ErrorMap<AuthErrorCode> {
return {} as ErrorMap<AuthErrorCode>;
// We will include this one message in the prod error map since by the very
// nature of this error, developers will never be able to see the message
// using the debugErrorMap (which is installed during auth initialization).
return {
[AuthErrorCode.DEPENDENT_SDK_INIT_BEFORE_AUTH]:
'Another Firebase SDK was initialized and is trying to use Auth before Auth is ' +
'initialized. Please be sure to call `initializeAuth` or `getAuth` before ' +
'starting any other Firebase SDK.'
} as ErrorMap<AuthErrorCode>;
}

/**
Expand Down Expand Up @@ -386,6 +399,7 @@ type GenericAuthErrorParams = {
[key in Exclude<
AuthErrorCode,
| AuthErrorCode.ARGUMENT_ERROR
| AuthErrorCode.DEPENDENT_SDK_INIT_BEFORE_AUTH
| AuthErrorCode.INTERNAL_ERROR
| AuthErrorCode.MFA_REQUIRED
>]: {
Expand All @@ -397,6 +411,7 @@ type GenericAuthErrorParams = {

export interface AuthErrorParams extends GenericAuthErrorParams {
[AuthErrorCode.ARGUMENT_ERROR]: { appName?: AppName };
[AuthErrorCode.DEPENDENT_SDK_INIT_BEFORE_AUTH]: { appName?: AppName };
[AuthErrorCode.INTERNAL_ERROR]: { appName?: AppName };
[AuthErrorCode.MFA_REQUIRED]: {
appName: AppName;
Expand Down