diff --git a/packages/messaging/src/controllers/controller-interface.ts b/packages/messaging/src/controllers/controller-interface.ts index 3c20d0b77c1..8c303d7df26 100644 --- a/packages/messaging/src/controllers/controller-interface.ts +++ b/packages/messaging/src/controllers/controller-interface.ts @@ -77,15 +77,11 @@ export abstract class ControllerInterface implements FirebaseMessaging { async getToken(): Promise { // Check with permissions const currentPermission = this.getNotificationPermission_(); - if (currentPermission !== 'granted') { - if (currentPermission === 'denied') { - return Promise.reject( - errorFactory.create(ERROR_CODES.NOTIFICATIONS_BLOCKED) - ); - } - + if (currentPermission === 'denied') { + throw errorFactory.create(ERROR_CODES.NOTIFICATIONS_BLOCKED); + } else if (currentPermission !== 'granted') { // We must wait for permission to be granted - return Promise.resolve(null); + return null; } const swReg = await this.getSWRegistration_(); @@ -223,7 +219,6 @@ export abstract class ControllerInterface implements FirebaseMessaging { * unsubscribes the token from FCM and then unregisters the push * subscription if it exists. It returns a promise that indicates * whether or not the unsubscribe request was processed successfully. - * @export */ async deleteToken(token: string): Promise { // Delete the token details from the database. diff --git a/packages/messaging/src/controllers/window-controller.ts b/packages/messaging/src/controllers/window-controller.ts index fa2cd6c8fbb..1e049fead69 100644 --- a/packages/messaging/src/controllers/window-controller.ts +++ b/packages/messaging/src/controllers/window-controller.ts @@ -36,6 +36,10 @@ import { } from '../models/worker-page-message'; import { ControllerInterface } from './controller-interface'; +interface ManifestContent { + gcm_sender_id: string; +} + export class WindowController extends ControllerInterface { private registrationToUse: ServiceWorkerRegistration | null = null; private publicVapidKeyToUse: Uint8Array | null = null; @@ -75,55 +79,12 @@ export class WindowController extends ControllerInterface { * @return Returns a promise that resolves to an FCM token or null if * permission isn't granted. */ - getToken(): Promise { - return this.manifestCheck_().then(() => { - return super.getToken(); - }); - } - - /** - * The method checks that a manifest is defined and has the correct GCM - * sender ID. - * @return Returns a promise that resolves if the manifest matches - * our required sender ID - */ - // Visible for testing - // TODO: make private - manifestCheck_(): Promise { - if (this.manifestCheckPromise) { - return this.manifestCheckPromise; - } - - const manifestTag = document.querySelector( - 'link[rel="manifest"]' - ); - if (!manifestTag) { - this.manifestCheckPromise = Promise.resolve(); - } else { - this.manifestCheckPromise = fetch(manifestTag.href) - .then(response => { - return response.json(); - }) - .catch(() => { - // If the download or parsing fails allow check. - // We only want to error if we KNOW that the gcm_sender_id is incorrect. - }) - .then(manifestContent => { - if (!manifestContent) { - return; - } - - if (!manifestContent['gcm_sender_id']) { - return; - } - - if (manifestContent['gcm_sender_id'] !== '103953800507') { - throw errorFactory.create(ERROR_CODES.INCORRECT_GCM_SENDER_ID); - } - }); + async getToken(): Promise { + if (!this.manifestCheckPromise) { + this.manifestCheckPromise = manifestCheck(); } - - return this.manifestCheckPromise; + await this.manifestCheckPromise; + return super.getToken(); } /** @@ -132,37 +93,18 @@ export class WindowController extends ControllerInterface { * @return Resolves if the permission was granted, otherwise rejects */ async requestPermission(): Promise { - if ( - // TODO: Remove the cast when this issue is fixed: - // https://github.com/Microsoft/TypeScript/issues/14701 - // tslint:disable-next-line no-any - ((Notification as any).permission as NotificationPermission) === 'granted' - ) { + if (this.getNotificationPermission_() === 'granted') { return; } - return new Promise((resolve, reject) => { - const managePermissionResult = (result: NotificationPermission) => { - if (result === 'granted') { - return resolve(); - } else if (result === 'denied') { - return reject(errorFactory.create(ERROR_CODES.PERMISSION_BLOCKED)); - } else { - return reject(errorFactory.create(ERROR_CODES.PERMISSION_DEFAULT)); - } - }; - - // The Notification.requestPermission API was changed to - // return a promise so now have to handle both in case - // browsers stop support callbacks for promised version - const permissionPromise = Notification.requestPermission( - managePermissionResult - ); - if (permissionPromise) { - // Prefer the promise version as it's the future API. - permissionPromise.then(managePermissionResult); - } - }); + const permissionResult = await Notification.requestPermission(); + if (permissionResult === 'granted') { + return; + } else if (permissionResult === 'denied') { + throw errorFactory.create(ERROR_CODES.PERMISSION_BLOCKED); + } else { + throw errorFactory.create(ERROR_CODES.PERMISSION_DEFAULT); + } } /** @@ -334,12 +276,12 @@ export class WindowController extends ControllerInterface { * This will return the default VAPID key or the uint8array version of the public VAPID key * provided by the developer. */ - getPublicVapidKey_(): Promise { + async getPublicVapidKey_(): Promise { if (this.publicVapidKeyToUse) { - return Promise.resolve(this.publicVapidKeyToUse); + return this.publicVapidKeyToUse; } - return Promise.resolve(DEFAULT_PUBLIC_VAPID_KEY); + return DEFAULT_PUBLIC_VAPID_KEY; } /** @@ -376,3 +318,38 @@ export class WindowController extends ControllerInterface { ); } } + +/** + * The method checks that a manifest is defined and has the correct GCM + * sender ID. + * @return Returns a promise that resolves if the manifest matches + * our required sender ID + */ +// Exported for testing +export async function manifestCheck(): Promise { + const manifestTag = document.querySelector( + 'link[rel="manifest"]' + ); + + if (!manifestTag) { + return; + } + + let manifestContent: ManifestContent; + try { + const response = await fetch(manifestTag.href); + manifestContent = await response.json(); + } catch (e) { + // If the download or parsing fails allow check. + // We only want to error if we KNOW that the gcm_sender_id is incorrect. + return; + } + + if (!manifestContent || !manifestContent.gcm_sender_id) { + return; + } + + if (manifestContent.gcm_sender_id !== '103953800507') { + throw errorFactory.create(ERROR_CODES.INCORRECT_GCM_SENDER_ID); + } +} diff --git a/packages/messaging/test/window-controller.test.ts b/packages/messaging/test/window-controller.test.ts index 3490655b435..6913d738b79 100644 --- a/packages/messaging/test/window-controller.test.ts +++ b/packages/messaging/test/window-controller.test.ts @@ -19,7 +19,10 @@ import * as sinon from 'sinon'; import { makeFakeApp } from './testing-utils/make-fake-app'; import { makeFakeSWReg } from './testing-utils/make-fake-sw-reg'; -import { WindowController } from '../src/controllers/window-controller'; +import { + manifestCheck, + WindowController +} from '../src/controllers/window-controller'; import { base64ToArrayBuffer } from '../src/helpers/base64-to-array-buffer'; import { DEFAULT_PUBLIC_VAPID_KEY } from '../src/models/fcm-details'; @@ -46,15 +49,14 @@ describe('Firebase Messaging > *WindowController', () => { return cleanup(); }); - describe('manifestCheck_()', () => { + describe('manifestCheck()', () => { it("should resolve when the tag isn't defined", () => { sandbox .stub(document, 'querySelector') .withArgs('link[rel="manifest"]') .returns(null); - const controller = new WindowController(app); - return controller.manifestCheck_(); + return manifestCheck(); }); it('should fetch the manifest if defined and resolve when no gcm_sender_id', () => { @@ -76,8 +78,7 @@ describe('Firebase Messaging > *WindowController', () => { }) ); - const controller = new WindowController(app); - return controller.manifestCheck_(); + return manifestCheck(); }); it('should fetch the manifest if defined and resolve with expected gcm_sender_id', () => { @@ -101,8 +102,7 @@ describe('Firebase Messaging > *WindowController', () => { }) ); - const controller = new WindowController(app); - return controller.manifestCheck_(); + return manifestCheck(); }); it('should fetch the manifest if defined and reject when using wrong gcm_sender_id', () => { @@ -126,8 +126,7 @@ describe('Firebase Messaging > *WindowController', () => { }) ); - const controller = new WindowController(app); - return controller.manifestCheck_().then( + return manifestCheck().then( () => { throw new Error('Expected error to be thrown.'); }, @@ -150,8 +149,7 @@ describe('Firebase Messaging > *WindowController', () => { .withArgs('https://firebase.io/messaging/example') .returns(Promise.reject(new Error('Injected Failure.'))); - const controller = new WindowController(app); - return controller.manifestCheck_(); + return manifestCheck(); }); }); @@ -206,16 +204,6 @@ describe('Firebase Messaging > *WindowController', () => { const controller = new WindowController(app); return controller.requestPermission(); }); - - it('should resolve if the requestPermission() is granted using old callback API', () => { - sandbox.stub(Notification as any, 'permission').value('default'); - sandbox.stub(Notification, 'requestPermission').callsFake(cb => { - cb('granted'); - }); - - const controller = new WindowController(app); - return controller.requestPermission(); - }); }); describe('useServiceWorker()', () => {