From 990daa62773330aa06cbb083aff0bbea1f8f953f Mon Sep 17 00:00:00 2001 From: Geoff Lankow Date: Wed, 14 Feb 2024 12:06:09 +1300 Subject: [PATCH] =?UTF-8?q?Bug=201880211=20-=20Convert=20the=20OAuth2=20co?= =?UTF-8?q?de=20to=20use=20Promises=20instead=20of=20callbacks.=20r=3Dleft?= =?UTF-8?q?mostcat,mkmelin=20This=20is=20a=20prerequisite=20for=20the=20ne?= =?UTF-8?q?xt=20patch=20(and=20a=20tidy=20up=20of=20some=20quite=20ugly=20?= =?UTF-8?q?code,=20frankly)=20=E2=80=93=20using=20Promises=20instead=20of?= =?UTF-8?q?=20callbacks,=20it's=20much=20easier=20to=20have=20two=20things?= =?UTF-8?q?=20waiting=20on=20the=20same=20result.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Differential Revision: https://phabricator.services.mozilla.com/D201795 --HG-- extra : rebase_source : 2f0eaac2bba5302588bfa540143b53e5ba9fd0c1 extra : amend_source : e6e96f0ea3f1eddf664ff3865ab1bafdbc9cffab --- .../providers/caldav/CalDavCalendar.sys.mjs | 14 ++---- .../caldav/modules/CalDavSession.jsm | 18 +++---- calendar/test/unit/xpcshell.ini | 1 + mailnews/addrbook/modules/CardDAVUtils.jsm | 6 +-- mailnews/base/src/OAuth2.jsm | 50 +++++++++++-------- mailnews/base/src/OAuth2Module.sys.mjs | 14 ++---- 6 files changed, 47 insertions(+), 56 deletions(-) diff --git a/calendar/providers/caldav/CalDavCalendar.sys.mjs b/calendar/providers/caldav/CalDavCalendar.sys.mjs index 63a2d08ce5..f7ef29f07a 100644 --- a/calendar/providers/caldav/CalDavCalendar.sys.mjs +++ b/calendar/providers/caldav/CalDavCalendar.sys.mjs @@ -1424,21 +1424,15 @@ CalDavCalendar.prototype = { this.onPromptAuthAvailable(callback); }, onPromptAuthAvailable(callback) { - self.oauth.connect( + self.oauth.connect(true, aRefresh).then( () => { authSuccessCb(); - if (callback) { - callback.onAuthResult(true); - } + callback?.onAuthResult(true); }, () => { authFailureCb(); - if (callback) { - callback.onAuthResult(false); - } - }, - true, - aRefresh + callback?.onAuthResult(false); + } ); }, onPromptCanceled: authFailureCb, diff --git a/calendar/providers/caldav/modules/CalDavSession.jsm b/calendar/providers/caldav/modules/CalDavSession.jsm index ac5a301940..af25546285 100644 --- a/calendar/providers/caldav/modules/CalDavSession.jsm +++ b/calendar/providers/caldav/modules/CalDavSession.jsm @@ -114,21 +114,15 @@ class CalDavOAuth extends OAuth2 { }, onPromptAuthAvailable(callback) { - self.connect( + self.connect(aWithUI, aRefresh).then( () => { - if (callback) { - callback.onAuthResult(true); - } + callback?.onAuthResult(true); resolve(); }, - () => { - if (callback) { - callback.onAuthResult(false); - } - reject(); - }, - aWithUI, - aRefresh + error => { + callback?.onAuthResult(false); + reject(Cr.NS_ERROR_ABORT); + } ); }, onPromptCanceled: reject, diff --git a/calendar/test/unit/xpcshell.ini b/calendar/test/unit/xpcshell.ini index 2d4639d0b6..6e26271a3e 100644 --- a/calendar/test/unit/xpcshell.ini +++ b/calendar/test/unit/xpcshell.ini @@ -29,6 +29,7 @@ tags = calendar [test_bug668222.js] [test_bug759324.js] [test_caldav_requests.js] +tags = oauth [test_CalendarFileImporter.js] [test_calIteratorUtils.js] [test_calmgr.js] diff --git a/mailnews/addrbook/modules/CardDAVUtils.jsm b/mailnews/addrbook/modules/CardDAVUtils.jsm index 3212b6628d..7961742788 100644 --- a/mailnews/addrbook/modules/CardDAVUtils.jsm +++ b/mailnews/addrbook/modules/CardDAVUtils.jsm @@ -378,15 +378,13 @@ var CardDAVUtils = { requestParams.oAuth = { QueryInterface: ChromeUtils.generateQI(["msgIOAuth2Module"]), connect(withUI, listener) { - oAuth.connect( + oAuth.connect(withUI, false).then( () => listener.onSuccess( // String format based on what OAuth2Module has. btoa(`\x01auth=Bearer ${oAuth.accessToken}`) ), - () => listener.onFailure(Cr.NS_ERROR_ABORT), - withUI, - false + () => listener.onFailure(Cr.NS_ERROR_ABORT) ); }, }; diff --git a/mailnews/base/src/OAuth2.jsm b/mailnews/base/src/OAuth2.jsm index b955a256fc..f361a452ff 100644 --- a/mailnews/base/src/OAuth2.jsm +++ b/mailnews/base/src/OAuth2.jsm @@ -69,25 +69,37 @@ OAuth2.prototype = { refreshToken: null, tokenExpires: 0, - connect(aSuccess, aFailure, aWithUI, aRefresh) { - this.connectSuccessCallback = aSuccess; - this.connectFailureCallback = aFailure; - + /** + * Obtain an access token for this endpoint. If an access token has already + * been obtained, it will be reused unless `aRefresh` is true. + * + * @param {boolean} aWithUI - If UI can be shown to the user for logging in. + * @param {boolean} aRefresh - If any existing access token should be + * ignored and a new one obtained. + * @returns {Promise} - Resolves when authorisation is complete and an + * access token is available. + */ + connect(aWithUI, aRefresh) { if (this.accessToken && !this.tokenExpired && !aRefresh) { - aSuccess(); - } else if (this.refreshToken) { + return this._promise; + } + + const { promise, resolve, reject } = Promise.withResolvers(); + this._promise = promise; + this._resolve = resolve; + this._reject = reject; + + if (this.refreshToken) { this.requestAccessToken(this.refreshToken, true); + } else if (!aWithUI) { + this._reject('{ "error": "auth_noui" }'); + } else if (gConnecting[this.authorizationEndpoint]) { + this._reject("Window already open"); } else { - if (!aWithUI) { - aFailure('{ "error": "auth_noui" }'); - return; - } - if (gConnecting[this.authorizationEndpoint]) { - aFailure("Window already open"); - return; - } this.requestAuthorization(); } + + return this._promise; }, /** @@ -272,7 +284,7 @@ OAuth2.prototype = { }, onAuthorizationFailed(aError, aData) { - this.connectFailureCallback(aData); + this._reject(aData); }, /** @@ -334,11 +346,9 @@ OAuth2.prototype = { // Typically in production this would be {"error": "invalid_grant"}. // That is, the token expired or was revoked (user changed password?). - // Reset the tokens we have and call success so that the auth flow - // will be re-triggered. this.accessToken = null; this.refreshToken = null; - this.connectSuccessCallback(); + this._reject(err); return; } @@ -355,11 +365,11 @@ OAuth2.prototype = { } else { this.tokenExpires = Number.MAX_VALUE; } - this.connectSuccessCallback(); + this._resolve(); }) .catch(err => { this.log.info(`Connection to authorization server failed: ${err}`); - this.connectFailureCallback(err); + this._reject(err); }); }, }; diff --git a/mailnews/base/src/OAuth2Module.sys.mjs b/mailnews/base/src/OAuth2Module.sys.mjs index 45ae525894..6394664b48 100644 --- a/mailnews/base/src/OAuth2Module.sys.mjs +++ b/mailnews/base/src/OAuth2Module.sys.mjs @@ -170,25 +170,19 @@ OAuth2Module.prototype = { }, onPromptAuthAvailable: callback => { - oauth.connect( + oauth.connect(aWithUI, false).then( () => { aListener.onSuccess( btoa( `user=${this._username}\x01auth=Bearer ${oauth.accessToken}\x01\x01` ) ); - if (callback) { - callback.onAuthResult(true); - } + callback?.onAuthResult(true); }, () => { aListener.onFailure(Cr.NS_ERROR_ABORT); - if (callback) { - callback.onAuthResult(false); - } - }, - aWithUI, - false + callback?.onAuthResult(false); + } ); }, onPromptCanceled() {