Skip to content

Added Firebase App Check support to Firebase Auth. #7191

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 3 commits into from
Apr 10, 2023
Merged

Added Firebase App Check support to Firebase Auth. #7191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9c023d4
Add App Check token to Auth requests (#6982)
lisajian Feb 1, 2023
c9f7c35
Update changeset
lisajian Apr 3, 2023
2338348
Address PR comments
lisajian Apr 10, 2023
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
6 changes: 6 additions & 0 deletions .changeset/brave-ducks-relax.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@firebase/auth': minor
'@firebase/auth-compat': minor
---

[feature] Added Firebase App Check support to Firebase Auth.
6 changes: 5 additions & 1 deletion packages/auth-compat/src/auth.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ import sinonChai from 'sinon-chai';
import { Auth } from './auth';
import { CompatPopupRedirectResolver } from './popup_redirect';
import * as platform from './platform';
import { FAKE_HEARTBEAT_CONTROLLER_PROVIDER } from '../test/helpers/helpers';
import {
FAKE_APP_CHECK_CONTROLLER_PROVIDER,
FAKE_HEARTBEAT_CONTROLLER_PROVIDER
} from '../test/helpers/helpers';

use(sinonChai);

Expand All @@ -45,6 +48,7 @@ describe('auth compat', () => {
underlyingAuth = new exp.AuthImpl(
app,
FAKE_HEARTBEAT_CONTROLLER_PROVIDER,
FAKE_APP_CHECK_CONTROLLER_PROVIDER,
{
apiKey: 'api-key'
} as exp.ConfigInternal
Expand Down
16 changes: 12 additions & 4 deletions packages/auth-compat/src/popup_redirect.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ import * as exp from '@firebase/auth/internal';
import * as platform from './platform';
import { CompatPopupRedirectResolver } from './popup_redirect';
import { FirebaseApp } from '@firebase/app-compat';
import { FAKE_HEARTBEAT_CONTROLLER_PROVIDER } from '../test/helpers/helpers';
import {
FAKE_APP_CHECK_CONTROLLER_PROVIDER,
FAKE_HEARTBEAT_CONTROLLER_PROVIDER
} from '../test/helpers/helpers';

use(sinonChai);

Expand All @@ -42,9 +45,14 @@ describe('popup_redirect/CompatPopupRedirectResolver', () => {
beforeEach(() => {
compatResolver = new CompatPopupRedirectResolver();
const app = { options: { apiKey: 'api-key' } } as FirebaseApp;
auth = new exp.AuthImpl(app, FAKE_HEARTBEAT_CONTROLLER_PROVIDER, {
apiKey: 'api-key'
} as exp.ConfigInternal);
auth = new exp.AuthImpl(
app,
FAKE_HEARTBEAT_CONTROLLER_PROVIDER,
FAKE_APP_CHECK_CONTROLLER_PROVIDER,
{
apiKey: 'api-key'
} as exp.ConfigInternal
);
});

afterEach(() => {
Expand Down
7 changes: 7 additions & 0 deletions packages/auth-compat/test/helpers/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ export const FAKE_HEARTBEAT_CONTROLLER_PROVIDER = {
}
} as unknown as Provider<'heartbeat'>;

// App Check is fully tested in core auth impl
export const FAKE_APP_CHECK_CONTROLLER_PROVIDER = {
getImmediate(): undefined {
return undefined;
}
} as unknown as Provider<'app-check-internal'>;

export function initializeTestInstance(): void {
firebase.initializeApp(getAppConfig());
const stub = stubConsoleToSilenceEmulatorWarnings();
Expand Down
3 changes: 2 additions & 1 deletion packages/auth/src/api/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ export const enum HttpHeader {
X_FIREBASE_LOCALE = 'X-Firebase-Locale',
X_CLIENT_VERSION = 'X-Client-Version',
X_FIREBASE_GMPID = 'X-Firebase-gmpid',
X_FIREBASE_CLIENT = 'X-Firebase-Client'
X_FIREBASE_CLIENT = 'X-Firebase-Client',
X_FIREBASE_APP_CHECK = 'X-Firebase-AppCheck'
}

export const enum Endpoint {
Expand Down
32 changes: 32 additions & 0 deletions packages/auth/src/core/auth/auth_impl.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import { FirebaseApp } from '@firebase/app';
import { FirebaseError } from '@firebase/util';

import {
FAKE_APP_CHECK_CONTROLLER,
FAKE_APP_CHECK_CONTROLLER_PROVIDER,
FAKE_HEARTBEAT_CONTROLLER,
FAKE_HEARTBEAT_CONTROLLER_PROVIDER,
testAuth,
Expand Down Expand Up @@ -62,6 +64,7 @@ describe('core/auth/auth_impl', () => {
const authImpl = new AuthImpl(
FAKE_APP,
FAKE_HEARTBEAT_CONTROLLER_PROVIDER,
FAKE_APP_CHECK_CONTROLLER_PROVIDER,
{
apiKey: FAKE_APP.options.apiKey!,
apiHost: DefaultConfig.API_HOST,
Expand Down Expand Up @@ -582,6 +585,7 @@ describe('core/auth/auth_impl', () => {
const authImpl = new AuthImpl(
FAKE_APP,
FAKE_HEARTBEAT_CONTROLLER_PROVIDER,
FAKE_APP_CHECK_CONTROLLER_PROVIDER,
{
apiKey: FAKE_APP.options.apiKey!,
apiHost: DefaultConfig.API_HOST,
Expand Down Expand Up @@ -656,5 +660,33 @@ describe('core/auth/auth_impl', () => {
'X-Client-Version': 'v'
});
});

it('adds the App Check token if available', async () => {
sinon
.stub(FAKE_APP_CHECK_CONTROLLER, 'getToken')
.returns(Promise.resolve({ token: 'fake-token' }));
expect(await auth._getAdditionalHeaders()).to.eql({
'X-Client-Version': 'v',
'X-Firebase-AppCheck': 'fake-token'
});
});

it('does not add the App Check token if none returned', async () => {
sinon
.stub(FAKE_APP_CHECK_CONTROLLER, 'getToken')
.returns(Promise.resolve({ token: '' }));
expect(await auth._getAdditionalHeaders()).to.eql({
'X-Client-Version': 'v'
});
});

it('does not add the App Check token if controller unavailable', async () => {
sinon
.stub(FAKE_APP_CHECK_CONTROLLER, 'getToken')
.returns(undefined as any);
expect(await auth._getAdditionalHeaders()).to.eql({
'X-Client-Version': 'v'
});
});
});
});
26 changes: 26 additions & 0 deletions packages/auth/src/core/auth/auth_impl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import { _FirebaseService, FirebaseApp } from '@firebase/app';
import { Provider } from '@firebase/component';
import { AppCheckInternalComponentName } from '@firebase/app-check-interop-types';
import {
Auth,
AuthErrorMap,
Expand Down Expand Up @@ -62,6 +63,7 @@ import { _getUserLanguage } from '../util/navigator';
import { _getClientVersion } from '../util/version';
import { HttpHeader } from '../../api';
import { AuthMiddlewareQueue } from './middleware';
import { _logWarn } from '../util/log';

interface AsyncAction {
(): Promise<void>;
Expand Down Expand Up @@ -108,6 +110,7 @@ export class AuthImpl implements AuthInternal, _FirebaseService {
constructor(
public readonly app: FirebaseApp,
private readonly heartbeatServiceProvider: Provider<'heartbeat'>,
private readonly appCheckServiceProvider: Provider<AppCheckInternalComponentName>,
public readonly config: ConfigInternal
) {
this.name = app.name;
Expand Down Expand Up @@ -645,8 +648,31 @@ export class AuthImpl implements AuthInternal, _FirebaseService {
if (heartbeatsHeader) {
headers[HttpHeader.X_FIREBASE_CLIENT] = heartbeatsHeader;
}

// If the App Check service exists, add the App Check token in the headers
const appCheckToken = await this._getAppCheckToken();
if (appCheckToken) {
headers[HttpHeader.X_FIREBASE_APP_CHECK] = appCheckToken;
}

return headers;
}

async _getAppCheckToken(): Promise<string | undefined> {
const appCheckTokenResult = await this.appCheckServiceProvider
.getImmediate({ optional: true })
?.getToken();
if (appCheckTokenResult?.error) {
// Context: appCheck.getToken() will never throw even if an error happened.
// In the error case, a dummy token will be returned along with an error field describing
// the error. In general, we shouldn't care about the error condition and just use
// the token (actual or dummy) to send requests.
_logWarn(
`Error while retrieving App Check token: ${appCheckTokenResult.error}`
);
}
return appCheckTokenResult?.token;
}
}

/**
Expand Down
56 changes: 29 additions & 27 deletions packages/auth/src/core/auth/register.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,36 +63,38 @@ export function registerAuth(clientPlatform: ClientPlatform): void {
const app = container.getProvider('app').getImmediate()!;
const heartbeatServiceProvider =
container.getProvider<'heartbeat'>('heartbeat');
const appCheckServiceProvider =
container.getProvider<'app-check-internal'>('app-check-internal');
const { apiKey, authDomain } = app.options;
return ((app, heartbeatServiceProvider) => {
_assert(
apiKey && !apiKey.includes(':'),
AuthErrorCode.INVALID_API_KEY,
{ appName: app.name }
);
// Auth domain is optional if IdP sign in isn't being used
_assert(!authDomain?.includes(':'), AuthErrorCode.ARGUMENT_ERROR, {
appName: app.name
});
const config: ConfigInternal = {
apiKey,
authDomain,
clientPlatform,
apiHost: DefaultConfig.API_HOST,
tokenApiHost: DefaultConfig.TOKEN_API_HOST,
apiScheme: DefaultConfig.API_SCHEME,
sdkClientVersion: _getClientVersion(clientPlatform)
};

const authInstance = new AuthImpl(
app,
heartbeatServiceProvider,
config
);
_initializeAuthInstance(authInstance, deps);
_assert(
Copy link
Contributor

Choose a reason for hiding this comment

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

We were previously wrapping it as an anonymous function and invoking it right away, now we are just invoking it without that wrapper, correct?

Adding @sam-gc to look at this section, since we had some past discussion about whether initializeAuthInstance needs to be invoked here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, just invoking without that wrapper - no changes have been made since https://github.com/firebase/firebase-js-sdk/pull/6982/files#r1089404274. _initializeAuthInstance() is called on line 95

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think this is fine to do. It was unnecessarily complicated creating a throwaway anonymous function that's immediately called.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool thanks

apiKey && !apiKey.includes(':'),
AuthErrorCode.INVALID_API_KEY,
{ appName: app.name }
);
// Auth domain is optional if IdP sign in isn't being used
_assert(!authDomain?.includes(':'), AuthErrorCode.ARGUMENT_ERROR, {
appName: app.name
});
const config: ConfigInternal = {
apiKey,
authDomain,
clientPlatform,
apiHost: DefaultConfig.API_HOST,
tokenApiHost: DefaultConfig.TOKEN_API_HOST,
apiScheme: DefaultConfig.API_SCHEME,
sdkClientVersion: _getClientVersion(clientPlatform)
};

const authInstance = new AuthImpl(
app,
heartbeatServiceProvider,
appCheckServiceProvider,
config
);
_initializeAuthInstance(authInstance, deps);

return authInstance;
})(app, heartbeatServiceProvider);
return authInstance;
},
ComponentType.PUBLIC
)
Expand Down
23 changes: 20 additions & 3 deletions packages/auth/src/core/util/handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ const WIDGET_PATH = '__/auth/handler';
*/
const EMULATOR_WIDGET_PATH = 'emulator/auth/handler';

/**
* Fragment name for the App Check token that gets passed to the widget
*
* @internal
*/
const FIREBASE_APP_CHECK_FRAGMENT_ID = encodeURIComponent('fac');

// eslint-disable-next-line @typescript-eslint/consistent-type-definitions
type WidgetParams = {
apiKey: ApiKey;
Expand All @@ -54,14 +61,14 @@ type WidgetParams = {
tid?: string;
} & { [key: string]: string | undefined };

export function _getRedirectUrl(
export async function _getRedirectUrl(
auth: AuthInternal,
provider: AuthProvider,
authType: AuthEventType,
redirectUrl?: string,
eventId?: string,
additionalParams?: Record<string, string>
): string {
): Promise<string> {
_assert(auth.config.authDomain, auth, AuthErrorCode.MISSING_AUTH_DOMAIN);
_assert(auth.config.apiKey, auth, AuthErrorCode.INVALID_API_KEY);

Expand Down Expand Up @@ -107,7 +114,17 @@ export function _getRedirectUrl(
delete paramsDict[key];
}
}
return `${getHandlerBase(auth)}?${querystring(paramsDict).slice(1)}`;

// Sets the App Check token to pass to the widget
const appCheckToken = await auth._getAppCheckToken();
Copy link

Choose a reason for hiding this comment

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

What will happen if _getAppCheckToken() failed? Should we return a url without an app check token, or throw an exception?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

_getAppCheckToken() will generally return a token (either a real one or a dummy token, for more info see the comment in auth_impl.ts) and we will always pass that real / dummy token to the widget. If there's an error, we simply log it client side, and when the backend receives a dummy token, it would prevent the request from succeeding if App Check is enforced.

const appCheckTokenFragment = appCheckToken
? `#${FIREBASE_APP_CHECK_FRAGMENT_ID}=${encodeURIComponent(appCheckToken)}`
: '';

// Start at index 1 to skip the leading '&' in the query string
return `${getHandlerBase(auth)}?${querystring(paramsDict).slice(
1
)}${appCheckTokenFragment}`;
}

function getHandlerBase({ config }: AuthInternal): string {
Expand Down
6 changes: 6 additions & 0 deletions packages/auth/src/core/util/log.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ export function _logDebug(msg: string, ...args: string[]): void {
}
}

export function _logWarn(msg: string, ...args: string[]): void {
if (logClient.logLevel <= LogLevel.WARN) {
logClient.warn(`Auth (${SDK_VERSION}): ${msg}`, ...args);
}
}

export function _logError(msg: string, ...args: string[]): void {
if (logClient.logLevel <= LogLevel.ERROR) {
logClient.error(`Auth (${SDK_VERSION}): ${msg}`, ...args);
Expand Down
1 change: 1 addition & 0 deletions packages/auth/src/model/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ export interface AuthInternal extends Auth {
_logFramework(framework: string): void;
_getFrameworks(): readonly string[];
_getAdditionalHeaders(): Promise<Record<string, string>>;
_getAppCheckToken(): Promise<string | undefined>;

readonly name: AppName;
readonly config: ConfigInternal;
Expand Down
26 changes: 17 additions & 9 deletions packages/auth/src/platform_browser/auth.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
import { OperationType } from '../model/enums';

import {
FAKE_APP_CHECK_CONTROLLER_PROVIDER,
FAKE_HEARTBEAT_CONTROLLER_PROVIDER,
testAuth,
testUser
Expand Down Expand Up @@ -73,6 +74,7 @@ describe('core/auth/auth_impl', () => {
const authImpl = new AuthImpl(
FAKE_APP,
FAKE_HEARTBEAT_CONTROLLER_PROVIDER,
FAKE_APP_CHECK_CONTROLLER_PROVIDER,
{
apiKey: FAKE_APP.options.apiKey!,
apiHost: DefaultConfig.API_HOST,
Expand Down Expand Up @@ -141,15 +143,20 @@ describe('core/auth/initializeAuth', () => {
authDomain = FAKE_APP.options.authDomain,
blockMiddleware = false
): Promise<Auth> {
const auth = new AuthImpl(FAKE_APP, FAKE_HEARTBEAT_CONTROLLER_PROVIDER, {
apiKey: FAKE_APP.options.apiKey!,
apiHost: DefaultConfig.API_HOST,
apiScheme: DefaultConfig.API_SCHEME,
tokenApiHost: DefaultConfig.TOKEN_API_HOST,
authDomain,
clientPlatform: ClientPlatform.BROWSER,
sdkClientVersion: _getClientVersion(ClientPlatform.BROWSER)
});
const auth = new AuthImpl(
FAKE_APP,
FAKE_HEARTBEAT_CONTROLLER_PROVIDER,
FAKE_APP_CHECK_CONTROLLER_PROVIDER,
{
apiKey: FAKE_APP.options.apiKey!,
apiHost: DefaultConfig.API_HOST,
apiScheme: DefaultConfig.API_SCHEME,
tokenApiHost: DefaultConfig.TOKEN_API_HOST,
authDomain,
clientPlatform: ClientPlatform.BROWSER,
sdkClientVersion: _getClientVersion(ClientPlatform.BROWSER)
}
);

_initializeAuthInstance(auth, {
persistence,
Expand Down Expand Up @@ -378,6 +385,7 @@ describe('core/auth/initializeAuth', () => {
const auth = new AuthImpl(
FAKE_APP,
FAKE_HEARTBEAT_CONTROLLER_PROVIDER,
FAKE_APP_CHECK_CONTROLLER_PROVIDER,
{
apiKey: FAKE_APP.options.apiKey!,
apiHost: DefaultConfig.API_HOST,
Expand Down
Loading