Skip to content

Using async project ID discovery API in Auth and FCM #724

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 5 commits into from
Dec 13, 2019
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
84 changes: 55 additions & 29 deletions src/auth/auth-api-request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,9 @@ const MAX_LIST_TENANT_PAGE_SIZE = 1000;

/** Defines a base utility to help with resource URL construction. */
class AuthResourceUrlBuilder {

protected urlFormat: string;
private projectId: string;

/**
* The resource URL builder constructor.
Expand All @@ -97,7 +99,7 @@ class AuthResourceUrlBuilder {
* @param {string} version The endpoint API version.
* @constructor
*/
constructor(protected projectId: string | null, protected version: string = 'v1') {
constructor(protected app: FirebaseApp, protected version: string = 'v1') {
this.urlFormat = FIREBASE_AUTH_BASE_URL_FORMAT;
}

Expand All @@ -107,17 +109,41 @@ class AuthResourceUrlBuilder {
* @param {string=} api The backend API name.
* @param {object=} params The optional additional parameters to substitute in the
* URL path.
* @return {string} The corresponding resource URL.
* @return {Promise<string>} The corresponding resource URL.
*/
public getUrl(api?: string, params?: object): string {
const baseParams = {
version: this.version,
projectId: this.projectId,
api: api || '',
};
const baseUrl = utils.formatString(this.urlFormat, baseParams);
// Substitute additional api related parameters.
return utils.formatString(baseUrl, params || {});
public getUrl(api?: string, params?: object): Promise<string> {
return this.getProjectId()
.then((projectId) => {
const baseParams = {
version: this.version,
projectId,
api: api || '',
};
const baseUrl = utils.formatString(this.urlFormat, baseParams);
// Substitute additional api related parameters.
return utils.formatString(baseUrl, params || {});
});
}

private getProjectId(): Promise<string> {
if (this.projectId) {
return Promise.resolve(this.projectId);
}

return utils.findProjectId(this.app)
.then((projectId) => {
if (!validator.isNonEmptyString(projectId)) {
throw new FirebaseAuthError(
AuthClientErrorCode.INVALID_CREDENTIAL,
'Failed to determine project ID for Auth. Initialize the '
+ 'SDK with service account credentials or set project ID as an app option. '
+ 'Alternatively set the GOOGLE_CLOUD_PROJECT environment variable.',
);
}

this.projectId = projectId;
return projectId;
});
}
}

Expand All @@ -132,8 +158,8 @@ class TenantAwareAuthResourceUrlBuilder extends AuthResourceUrlBuilder {
* @param {string} tenantId The tenant ID.
* @constructor
*/
constructor(protected projectId: string | null, protected version: string, protected tenantId: string) {
super(projectId, version);
constructor(protected app: FirebaseApp, protected version: string, protected tenantId: string) {
super(app, version);
this.urlFormat = FIREBASE_AUTH_TENANT_URL_FORMAT;
}

Expand All @@ -143,10 +169,13 @@ class TenantAwareAuthResourceUrlBuilder extends AuthResourceUrlBuilder {
* @param {string=} api The backend API name.
* @param {object=} params The optional additional parameters to substitute in the
* URL path.
* @return {string} The corresponding resource URL.
* @return {Promise<string>} The corresponding resource URL.
*/
public getUrl(api?: string, params?: object) {
return utils.formatString(super.getUrl(api, params), {tenantId: this.tenantId});
public getUrl(api?: string, params?: object): Promise<string> {
return super.getUrl(api, params)
.then((url) => {
return utils.formatString(url, {tenantId: this.tenantId});
});
}
}

Expand Down Expand Up @@ -683,7 +712,7 @@ const LIST_INBOUND_SAML_CONFIGS = new ApiSettings('/inboundSamlConfigs', 'GET')
* Class that provides the mechanism to send requests to the Firebase Auth backend endpoints.
*/
export abstract class AbstractAuthRequestHandler {
protected readonly projectId: string | null;

protected readonly httpClient: AuthorizedHttpClient;
private authUrlBuilder: AuthResourceUrlBuilder;
private projectConfigUrlBuilder: AuthResourceUrlBuilder;
Expand All @@ -700,17 +729,14 @@ export abstract class AbstractAuthRequestHandler {
* @param {FirebaseApp} app The app used to fetch access tokens to sign API requests.
* @constructor
*/
constructor(app: FirebaseApp) {
constructor(protected readonly app: FirebaseApp) {
if (typeof app !== 'object' || app === null || !('options' in app)) {
throw new FirebaseAuthError(
AuthClientErrorCode.INVALID_ARGUMENT,
'First argument passed to admin.auth() must be a valid Firebase app instance.',
);
}

// TODO(rsgowman): Trace utils.getProjectId() throughout and figure out where a null return
// value will cause troubles. (Such as AuthResourceUrlBuilder::getUrl()).
this.projectId = utils.getProjectId(app);
this.httpClient = new AuthorizedHttpClient(app);
}

Expand Down Expand Up @@ -1357,15 +1383,15 @@ export abstract class AbstractAuthRequestHandler {
protected invokeRequestHandler(
urlBuilder: AuthResourceUrlBuilder, apiSettings: ApiSettings,
requestData: object, additionalResourceParams?: object): Promise<object> {
return Promise.resolve()
.then(() => {
return urlBuilder.getUrl(apiSettings.getEndpoint(), additionalResourceParams)
.then((url) => {
// Validate request.
const requestValidator = apiSettings.getRequestValidator();
requestValidator(requestData);
// Process request.
const req: HttpRequestConfig = {
method: apiSettings.getHttpMethod(),
url: urlBuilder.getUrl(apiSettings.getEndpoint(), additionalResourceParams),
url,
headers: FIREBASE_AUTH_HEADER,
data: requestData,
timeout: FIREBASE_AUTH_TIMEOUT,
Expand Down Expand Up @@ -1512,21 +1538,21 @@ export class AuthRequestHandler extends AbstractAuthRequestHandler {
*/
constructor(app: FirebaseApp) {
super(app);
this.tenantMgmtResourceBuilder = new AuthResourceUrlBuilder(utils.getProjectId(app), 'v2');
this.tenantMgmtResourceBuilder = new AuthResourceUrlBuilder(app, 'v2');
}

/**
* @return {AuthResourceUrlBuilder} A new Auth user management resource URL builder instance.
*/
protected newAuthUrlBuilder(): AuthResourceUrlBuilder {
return new AuthResourceUrlBuilder(this.projectId, 'v1');
return new AuthResourceUrlBuilder(this.app, 'v1');
}

/**
* @return {AuthResourceUrlBuilder} A new project config resource URL builder instance.
*/
protected newProjectConfigUrlBuilder(): AuthResourceUrlBuilder {
return new AuthResourceUrlBuilder(this.projectId, 'v2');
return new AuthResourceUrlBuilder(this.app, 'v2');
}

/**
Expand Down Expand Up @@ -1662,14 +1688,14 @@ export class TenantAwareAuthRequestHandler extends AbstractAuthRequestHandler {
* @return {AuthResourceUrlBuilder} A new Auth user management resource URL builder instance.
*/
protected newAuthUrlBuilder(): AuthResourceUrlBuilder {
return new TenantAwareAuthResourceUrlBuilder(this.projectId, 'v1', this.tenantId);
return new TenantAwareAuthResourceUrlBuilder(this.app, 'v1', this.tenantId);
}

/**
* @return {AuthResourceUrlBuilder} A new project config resource URL builder instance.
*/
protected newProjectConfigUrlBuilder(): AuthResourceUrlBuilder {
return new TenantAwareAuthResourceUrlBuilder(this.projectId, 'v2', this.tenantId);
return new TenantAwareAuthResourceUrlBuilder(this.app, 'v2', this.tenantId);
}

/**
Expand Down
71 changes: 42 additions & 29 deletions src/messaging/messaging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,8 @@ export class Messaging implements FirebaseServiceInterface {
public INTERNAL: MessagingInternals = new MessagingInternals();

private urlPath: string;
private appInternal: FirebaseApp;
private messagingRequestHandler: FirebaseMessagingRequestHandler;
private readonly appInternal: FirebaseApp;
private readonly messagingRequestHandler: FirebaseMessagingRequestHandler;

/**
* @param {FirebaseApp} app The app for this Messaging service.
Expand All @@ -221,18 +221,6 @@ export class Messaging implements FirebaseServiceInterface {
);
}

const projectId: string | null = utils.getProjectId(app);
if (!validator.isNonEmptyString(projectId)) {
// Assert for an explicit project ID (either via AppOptions or the cert itself).
throw new FirebaseMessagingError(
MessagingClientErrorCode.INVALID_ARGUMENT,
'Failed to determine project ID for Messaging. Initialize the '
+ 'SDK with service account credentials or set project ID as an app option. '
+ 'Alternatively set the GOOGLE_CLOUD_PROJECT environment variable.',
);
}

this.urlPath = `/v1/projects/${projectId}/messages:send`;
this.appInternal = app;
this.messagingRequestHandler = new FirebaseMessagingRequestHandler(app);
}
Expand Down Expand Up @@ -261,13 +249,13 @@ export class Messaging implements FirebaseServiceInterface {
throw new FirebaseMessagingError(
MessagingClientErrorCode.INVALID_ARGUMENT, 'dryRun must be a boolean');
}
return Promise.resolve()
.then(() => {
return this.getUrlPath()
.then((urlPath) => {
const request: {message: Message, validate_only?: boolean} = {message: copy};
if (dryRun) {
request.validate_only = true;
}
return this.messagingRequestHandler.invokeRequestHandler(FCM_SEND_HOST, this.urlPath, request);
return this.messagingRequestHandler.invokeRequestHandler(FCM_SEND_HOST, urlPath, request);
})
.then((response) => {
return (response as any).name;
Expand Down Expand Up @@ -312,18 +300,21 @@ export class Messaging implements FirebaseServiceInterface {
MessagingClientErrorCode.INVALID_ARGUMENT, 'dryRun must be a boolean');
}

const requests: SubRequest[] = copy.map((message) => {
validateMessage(message);
const request: {message: Message, validate_only?: boolean} = {message};
if (dryRun) {
request.validate_only = true;
}
return {
url: `https://${FCM_SEND_HOST}${this.urlPath}`,
body: request,
};
});
return this.messagingRequestHandler.sendBatchRequest(requests);
return this.getUrlPath()
.then((urlPath) => {
const requests: SubRequest[] = copy.map((message) => {
validateMessage(message);
const request: {message: Message, validate_only?: boolean} = {message};
if (dryRun) {
request.validate_only = true;
}
return {
url: `https://${FCM_SEND_HOST}${urlPath}`,
body: request,
};
});
return this.messagingRequestHandler.sendBatchRequest(requests);
});
}

/**
Expand Down Expand Up @@ -645,6 +636,28 @@ export class Messaging implements FirebaseServiceInterface {
);
}

private getUrlPath(): Promise<string> {
if (this.urlPath) {
return Promise.resolve(this.urlPath);
}

return utils.findProjectId(this.app)
.then((projectId) => {
if (!validator.isNonEmptyString(projectId)) {
// Assert for an explicit project ID (either via AppOptions or the cert itself).
throw new FirebaseMessagingError(
MessagingClientErrorCode.INVALID_ARGUMENT,
'Failed to determine project ID for Messaging. Initialize the '
+ 'SDK with service account credentials or set project ID as an app option. '
+ 'Alternatively set the GOOGLE_CLOUD_PROJECT environment variable.',
);
}

this.urlPath = `/v1/projects/${projectId}/messages:send`;
return this.urlPath;
});
}

/**
* Helper method which sends and handles topic subscription management requests.
*
Expand Down
12 changes: 9 additions & 3 deletions test/unit/auth/auth.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,15 @@ AUTH_CONFIGS.forEach((testConfig) => {
}).to.throw('First argument passed to admin.auth() must be a valid Firebase app instance.');
});

it('should reject given no project ID', () => {
const authWithoutProjectId = new Auth(mocks.mockCredentialApp());
authWithoutProjectId.getUser('uid')
.should.eventually.be.rejectedWith(
'Failed to determine project ID for Auth. Initialize the SDK with service '
+ 'account credentials or set project ID as an app option. Alternatively set the '
+ 'GOOGLE_CLOUD_PROJECT environment variable.');
});

it('should not throw given a valid app', () => {
expect(() => {
return new Auth(mockApp);
Expand Down Expand Up @@ -1626,8 +1635,6 @@ AUTH_CONFIGS.forEach((testConfig) => {
})
.catch((error) => {
expect(error).to.have.property('code', 'auth/invalid-page-token');
expect(validator.isNonEmptyString)
.to.have.been.calledOnce.and.calledWith(invalidToken);
});
});

Expand Down Expand Up @@ -1968,7 +1975,6 @@ AUTH_CONFIGS.forEach((testConfig) => {
})
.catch((error) => {
expect(error).to.have.property('code', 'auth/invalid-id-token');
expect(validator.isNonEmptyString).to.have.been.calledOnce.and.calledWith(invalidIdToken);
});
});

Expand Down
22 changes: 11 additions & 11 deletions test/unit/messaging/messaging.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -366,13 +366,14 @@ describe('Messaging', () => {
}).to.throw('First argument passed to admin.messaging() must be a valid Firebase app instance.');
});

it('should throw given app without project ID', () => {
expect(() => {
const appWithoutProhectId = mocks.mockCredentialApp();
return new Messaging(appWithoutProhectId);
}).to.throw('Failed to determine project ID for Messaging. Initialize the SDK with service '
+ 'account credentials or set project ID as an app option. Alternatively set the '
+ 'GOOGLE_CLOUD_PROJECT environment variable.');
it('should reject given app without project ID', () => {
const appWithoutProjectId = mocks.mockCredentialApp();
const messagingWithoutProjectId = new Messaging(appWithoutProjectId);
messagingWithoutProjectId.send({topic: 'test'})
.should.eventually.be.rejectedWith(
'Failed to determine project ID for Messaging. Initialize the SDK with service '
+ 'account credentials or set project ID as an app option. Alternatively set the '
+ 'GOOGLE_CLOUD_PROJECT environment variable.');
});

it('should not throw given a valid app', () => {
Expand Down Expand Up @@ -597,11 +598,10 @@ describe('Messaging', () => {
}).to.throw('messages list must not contain more than 500 items');
});

it('should throw when a message is invalid', () => {
it('should reject when a message is invalid', () => {
const invalidMessage: Message = {} as any;
expect(() => {
messaging.sendAll([validMessage, invalidMessage]);
}).to.throw('Exactly one of topic, token or condition is required');
messaging.sendAll([validMessage, invalidMessage])
.should.eventually.be.rejectedWith('Exactly one of topic, token or condition is required');
});

const invalidDryRun = [null, NaN, 0, 1, '', 'a', [], [1, 'a'], {}, { a: 1 }, _.noop];
Expand Down