Skip to content

Add App Check token to Auth requests #6982

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

Merged
merged 6 commits into from
Feb 1, 2023
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
6 changes: 6 additions & 0 deletions .changeset/brave-ducks-relax.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@firebase/auth': minor
'@firebase/auth-compat': minor
---

App Check <> Auth integration
6 changes: 5 additions & 1 deletion packages/auth-compat/src/auth.test.ts
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
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
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
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
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'
});
});
});
});
25 changes: 25 additions & 0 deletions packages/auth/src/core/auth/auth_impl.ts
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,30 @@ 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
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.

Why are we getting rid of the anon function here? I think we copied this from Firestore originally (not exactly sure why it's done this way)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeahh, I wasn't sure why we had the anon function either and didn't notice a difference in behavior with / without it? I think Firestore currently doesn't use an anon function:

const firestoreInstance = new Firestore(

Will leave this open if other folks have an opinion about it

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to remove the anon function if we did not observe any behavior change

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 cool

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
24 changes: 21 additions & 3 deletions packages/auth/src/core/util/handler.ts
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,18 @@ 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();
const appCheckTokenFragment = appCheckToken
? `${FIREBASE_APP_CHECK_FRAGMENT_ID}=${encodeURIComponent(appCheckToken)}`
: '';
const fragment = appCheckTokenFragment ? `#${appCheckTokenFragment}` : '';

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

function getHandlerBase({ config }: AuthInternal): string {
Expand Down
6 changes: 6 additions & 0 deletions packages/auth/src/core/util/log.ts
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
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
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