From 0395a076c826ffed7f9a83f8b8f5d49730f773ba Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Mon, 30 Dec 2019 08:46:47 -0500 Subject: Bug 1176399 - Multiple requests for master password when GMail OAuth2 is enabled. --- mailnews/base/public/nsIMsgAsyncPrompter.idl | 26 +++++++++++++---- mailnews/base/src/msgAsyncPrompter.js | 33 +++++++++++++++++----- mailnews/base/src/msgOAuth2Module.js | 42 ++++++++++++++++++++++------ mailnews/imap/src/nsImapProtocol.cpp | 7 +++++ mailnews/local/src/nsPop3Protocol.cpp | 7 +++++ mailnews/news/src/nsNNTPProtocol.cpp | 7 +++++ 6 files changed, 101 insertions(+), 21 deletions(-) diff --git a/mailnews/base/public/nsIMsgAsyncPrompter.idl b/mailnews/base/public/nsIMsgAsyncPrompter.idl index 5a59c4f39..4e1f81d12 100644 --- a/mailnews/base/public/nsIMsgAsyncPrompter.idl +++ b/mailnews/base/public/nsIMsgAsyncPrompter.idl @@ -35,20 +35,36 @@ interface nsIMsgAsyncPrompter : nsISupports { in nsIMsgAsyncPromptListener aCaller); }; +[scriptable, function, uuid(acca94c9-378e-46e3-9a91-6655bf9c91a3)] +interface nsIMsgAsyncPromptCallback : nsISupports { + /** + * Called when an auth result is available. Can be passed as a function. + * + * @param aResult True if there is auth information available following the + * prompt, false otherwise. + */ + void onAuthResult(in boolean aResult); +}; + /** * This is used in combination with nsIMsgAsyncPrompter. */ [scriptable, uuid(fb5307a3-39d0-462e-92c8-c5c288a2612f)] interface nsIMsgAsyncPromptListener : nsISupports { /** - * Called when the listener should do its prompt. The listener - * should not return until the prompt is complete. - * - * @return True if there is auth information available following the prompt, - * false otherwise. + * This method has been deprecated, please use onPromptStartAsync instead. */ boolean onPromptStart(); + /** + * Called when the listener should do its prompt. This can happen + * synchronously or asynchronously, but in any case when done the callback + * method should be called. + * + * @param aCallback The callback to execute when auth prompt has completed. + */ + void onPromptStartAsync(in nsIMsgAsyncPromptCallback aCallback); + /** * Called in the case that the queued prompt was combined with another and * there is now authentication information available. diff --git a/mailnews/base/src/msgAsyncPrompter.js b/mailnews/base/src/msgAsyncPrompter.js index 58b5288e9..ae114683a 100644 --- a/mailnews/base/src/msgAsyncPrompter.js +++ b/mailnews/base/src/msgAsyncPrompter.js @@ -2,6 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +Components.utils.import("resource://gre/modules/Deprecated.jsm"); Components.utils.import("resource://gre/modules/Services.jsm"); Components.utils.import("resource://gre/modules/Task.jsm"); Components.utils.import("resource://gre/modules/XPCOMUtils.jsm"); @@ -19,28 +20,46 @@ runnablePrompter.prototype = { _asyncPrompter: null, _hashKey: null, + _promiseAuthPrompt: function(listener) { + return new Promise((resolve, reject) => { + try { + listener.onPromptStartAsync({ onAuthResult: resolve }); + } catch (e) { + if (e.result == Components.results.NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED) { + // Fall back to onPromptStart, for add-ons compat + Deprecated.warning("onPromptStart has been replaced by onPromptStartAsync", + "https://bugzilla.mozilla.org/show_bug.cgi?id=1176399"); + let ok = listener.onPromptStart(); + resolve(ok); + } else { + reject(e); + } + } + }); + }, + run: Task.async(function *() { yield Services.logins.initializationPromise; this._asyncPrompter._log.debug("Running prompt for " + this._hashKey); let prompter = this._asyncPrompter._pendingPrompts[this._hashKey]; let ok = false; try { - ok = prompter.first.onPromptStart(); - } - catch (ex) { + ok = yield this._promiseAuthPrompt(prompter.first); + } catch (ex) { Components.utils.reportError("runnablePrompter:run: " + ex + "\n"); + prompter.first.onPromptCanceled(); } delete this._asyncPrompter._pendingPrompts[this._hashKey]; for (var consumer of prompter.consumers) { try { - if (ok) + if (ok) { consumer.onPromptAuthAvailable(); - else + } else { consumer.onPromptCanceled(); - } - catch (ex) { + } + } catch (ex) { // Log the error for extension devs and others to pick up. Components.utils.reportError("runnablePrompter:run: consumer.onPrompt* reported an exception: " + ex + "\n"); } diff --git a/mailnews/base/src/msgOAuth2Module.js b/mailnews/base/src/msgOAuth2Module.js index 407ab0519..22d5dc572 100644 --- a/mailnews/base/src/msgOAuth2Module.js +++ b/mailnews/base/src/msgOAuth2Module.js @@ -126,19 +126,43 @@ OAuth2Module.prototype = { } } - // Otherwise, we need a new login, so create one and fill it in. - let login = Cc["@mozilla.org/login-manager/loginInfo;1"] - .createInstance(Ci.nsILoginInfo); - login.init(this._loginUrl, null, this._scope, this._username, token, - '', ''); - loginMgr.addLogin(login); + // Unless the token is null, we need to create and fill in a new login + if (token) { + let login = Cc["@mozilla.org/login-manager/loginInfo;1"] + .createInstance(Ci.nsILoginInfo); + login.init(this._loginUrl, null, this._scope, this._username, token, + '', ''); + loginMgr.addLogin(login); + } return token; }, connect(aWithUI, aListener) { - this._oauth.connect(() => aListener.onSuccess(this._oauth.accessToken), - x => aListener.onFailure(x), - aWithUI, false); + let oauth = this._oauth; + let promptlistener = { + onPromptStartAsync: function(callback) { + oauth.connect(() => { + this.onPromptAuthAvailable(); + callback.onAuthResult(true); + }, (err) => { + this.onPromptCanceled(); + callback.onAuthResult(false); + }, aWithUI, false); + }, + + onPromptAuthAvailable: function() { + aListener.onSuccess(oauth.accessToken); + }, + onPromptCanceled: function() { + aListener.onFailure(Components.results.NS_ERROR_ABORT); + }, + onPromptStart: function() {} + }; + + let asyncprompter = Components.classes["@mozilla.org/messenger/msgAsyncPrompter;1"] + .getService(Components.interfaces.nsIMsgAsyncPrompter); + let promptkey = this._loginUrl + "/" + this._username; + asyncprompter.queueAsyncAuthPrompt(promptkey, false, promptlistener); }, buildXOAuth2String() { diff --git a/mailnews/imap/src/nsImapProtocol.cpp b/mailnews/imap/src/nsImapProtocol.cpp index 4cfa9dab2..c8e3ceb67 100644 --- a/mailnews/imap/src/nsImapProtocol.cpp +++ b/mailnews/imap/src/nsImapProtocol.cpp @@ -8513,6 +8513,13 @@ nsresult nsImapProtocol::GetPassword(nsCString &password, return rv; } +NS_IMETHODIMP nsImapProtocol::OnPromptStartAsync(nsIMsgAsyncPromptCallback *aCallback) +{ + bool result = false; + OnPromptStart(&result); + return aCallback->OnAuthResult(result); +} + // This is called from the UI thread. NS_IMETHODIMP nsImapProtocol::OnPromptStart(bool *aResult) diff --git a/mailnews/local/src/nsPop3Protocol.cpp b/mailnews/local/src/nsPop3Protocol.cpp index 5d9d9145a..de129a494 100644 --- a/mailnews/local/src/nsPop3Protocol.cpp +++ b/mailnews/local/src/nsPop3Protocol.cpp @@ -740,6 +740,13 @@ nsresult nsPop3Protocol::StartGetAsyncPassword(Pop3StatesEnum aNextState) return rv; } +NS_IMETHODIMP nsPop3Protocol::OnPromptStartAsync(nsIMsgAsyncPromptCallback *aCallback) +{ + bool result = false; + OnPromptStart(&result); + return aCallback->OnAuthResult(result); +} + NS_IMETHODIMP nsPop3Protocol::OnPromptStart(bool *aResult) { MOZ_LOG(POP3LOGMODULE, LogLevel::Debug, (POP3LOG("OnPromptStart()"))); diff --git a/mailnews/news/src/nsNNTPProtocol.cpp b/mailnews/news/src/nsNNTPProtocol.cpp index 8ce367faa..035dff6e6 100644 --- a/mailnews/news/src/nsNNTPProtocol.cpp +++ b/mailnews/news/src/nsNNTPProtocol.cpp @@ -2472,6 +2472,13 @@ nsresult nsNNTPProtocol::PasswordResponse() return NS_ERROR_FAILURE; } +NS_IMETHODIMP nsNNTPProtocol::OnPromptStartAsync(nsIMsgAsyncPromptCallback *aCallback) +{ + bool result = false; + OnPromptStart(&result); + return aCallback->OnAuthResult(result); +} + NS_IMETHODIMP nsNNTPProtocol::OnPromptStart(bool *authAvailable) { NS_ENSURE_ARG_POINTER(authAvailable); -- cgit v1.2.3 From a4ab8fd190102e4773ec3399b5281e8eeef04eae Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Mon, 30 Dec 2019 09:24:51 -0500 Subject: Bug 1453643 - Enable proper retry on oauth2 authenication failure. This prevents mail applications from attempting to use an unauthenticated connection to mailbox(s) and avoid unexpected deletion of local mbox files and subsequent re-download of mailbox content over imap. --- mailnews/imap/src/nsImapProtocol.cpp | 59 ++++++++++++++++++++---------------- mailnews/imap/src/nsImapProtocol.h | 1 + 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/mailnews/imap/src/nsImapProtocol.cpp b/mailnews/imap/src/nsImapProtocol.cpp index c8e3ceb67..940d87cbd 100644 --- a/mailnews/imap/src/nsImapProtocol.cpp +++ b/mailnews/imap/src/nsImapProtocol.cpp @@ -5714,6 +5714,36 @@ void nsImapProtocol::ResetAuthMethods() m_failedAuthMethods = 0; } +nsresult nsImapProtocol::SendDataParseIMAPandCheckForNewMail(const char *aData, const char *aCommand) +{ + nsresult rv; + bool isResend = false; + while (true) + { + // Send authentication string (true: suppress logging the string). + rv = SendData(aData, true); + if (NS_FAILED(rv)) + break; + ParseIMAPandCheckForNewMail(aCommand); + if (!GetServerStateParser().WaitingForMoreClientInput()) + break; + + // The server is asking for the authentication string again. So we send + // the same string again although we know that it might be rejected again. + // We do that to get a firm authentication failure instead of a resend + // request. That keeps things in order before failing authentication and + // trying another method if capable. + if (isResend) + { + rv = NS_ERROR_FAILURE; + break; + } + isResend = true; + } + + return rv; +} + nsresult nsImapProtocol::AuthLogin(const char *userName, const nsCString &password, eIMAPCapabilityFlag flag) { ProgressEventFunctionUsingName("imapStatusSendingAuthLogin"); @@ -5880,29 +5910,7 @@ nsresult nsImapProtocol::AuthLogin(const char *userName, const nsCString &passwo PR_snprintf(m_dataOutputBuf, OUTPUT_BUFFER_SIZE, "%s" CRLF, base64Str); PR_Free(base64Str); - bool isResend = false; - while (true) - { - // Send authentication string (true: suppress logging the string). - rv = SendData(m_dataOutputBuf, true); - if (NS_FAILED(rv)) - break; - ParseIMAPandCheckForNewMail(currentCommand); - if (!GetServerStateParser().WaitingForMoreClientInput()) - break; - - // Server is asking for authentication string again. So we send the - // same string again although we already know that it will be - // rejected again. We do that to get a firm authentication failure - // instead of a resend request. That keeps things in order before - // failing "authenticate PLAIN" and trying another method if capable. - if (isResend) - { - rv = NS_ERROR_FAILURE; - break; - } - isResend = true; - } + rv = SendDataParseIMAPandCheckForNewMail(m_dataOutputBuf, currentCommand); } // if the last command succeeded } // if auth plain capability else if (flag & kHasAuthLoginCapability) @@ -5953,9 +5961,8 @@ nsresult nsImapProtocol::AuthLogin(const char *userName, const nsCString &passwo EscapeUserNamePasswordString(password.get(), &correctedPassword); command.Append(correctedPassword); command.Append("\"" CRLF); - rv = SendData(command.get(), true /* suppress logging */); - NS_ENSURE_SUCCESS(rv, rv); - ParseIMAPandCheckForNewMail(); + + rv = SendDataParseIMAPandCheckForNewMail(command.get(), nullptr); } #ifdef MOZ_MAILNEWS_OAUTH2 else if (flag & kHasXOAuth2Capability) diff --git a/mailnews/imap/src/nsImapProtocol.h b/mailnews/imap/src/nsImapProtocol.h index ba2594c89..8cee4f4fb 100644 --- a/mailnews/imap/src/nsImapProtocol.h +++ b/mailnews/imap/src/nsImapProtocol.h @@ -485,6 +485,7 @@ private: void Namespace(); void InsecureLogin(const char *userName, const nsCString &password); nsresult AuthLogin(const char *userName, const nsCString &password, eIMAPCapabilityFlag flag); + nsresult SendDataParseIMAPandCheckForNewMail(const char *data, const char *command); void ProcessAuthenticatedStateURL(); void ProcessAfterAuthenticated(); void ProcessSelectedStateURL(); -- cgit v1.2.3 From 3dae851d2135e2b321754a544a5a82cf155a3936 Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Mon, 30 Dec 2019 09:33:56 -0500 Subject: Bug 1597933 - clean up OAuth2 code: remove responseType which is always code. Response type token is part of the OAuth 2.0 Implicit Flow which is not used in Mail Applications, but also discouraged by the OAuth Working Group: https://developer.okta.com/blog/2019/05/01/is-the-oauth-implicit-flow-dead --- mailnews/base/util/OAuth2.jsm | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/mailnews/base/util/OAuth2.jsm b/mailnews/base/util/OAuth2.jsm index 94f850e0b..dcbfb428f 100644 --- a/mailnews/base/util/OAuth2.jsm +++ b/mailnews/base/util/OAuth2.jsm @@ -3,7 +3,8 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ /** - * Provides OAuth 2.0 authentication + * Provides OAuth 2.0 authentication. + * @see RFC 6749 */ var EXPORTED_SYMBOLS = ["OAuth2"]; @@ -41,8 +42,6 @@ OAuth2.CODE_AUTHORIZATION = "authorization_code"; OAuth2.CODE_REFRESH = "refresh_token"; OAuth2.prototype = { - - responseType: "code", consumerKey: null, consumerSecret: null, completionURI: "http://localhost", @@ -79,7 +78,7 @@ OAuth2.prototype = { requestAuthorization: function requestAuthorization() { let params = [ - ["response_type", this.responseType], + ["response_type", "code"], ["client_id", this.consumerKey], ["redirect_uri", this.completionURI], ]; @@ -173,13 +172,11 @@ OAuth2.prototype = { onAuthorizationReceived: function(aData) { this.log.info("authorization received" + aData); let results = parseURLData(aData); - if (this.responseType == "code" && results.code) { + if (results.code) { this.requestAccessToken(results.code, OAuth2.CODE_AUTHORIZATION); - } else if (this.responseType == "token") { - this.onAccessTokenReceived(JSON.stringify(results)); - } - else + } else { this.onAuthorizationFailed(null, aData); + } }, onAuthorizationFailed: function(aError, aData) { -- cgit v1.2.3 From f532cec9768595ecea79714788515190d3c16f2d Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Mon, 30 Dec 2019 09:49:29 -0500 Subject: Bug 1597933 - improve OAuth2 params parsing. --- mailnews/base/util/OAuth2.jsm | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/mailnews/base/util/OAuth2.jsm b/mailnews/base/util/OAuth2.jsm index dcbfb428f..8feee0e94 100644 --- a/mailnews/base/util/OAuth2.jsm +++ b/mailnews/base/util/OAuth2.jsm @@ -15,15 +15,6 @@ Cu.import("resource://gre/modules/Services.jsm"); Cu.import("resource://gre/modules/XPCOMUtils.jsm"); Cu.import("resource:///modules/gloda/log4moz.js"); -function parseURLData(aData) { - let result = {}; - aData.split(/[?#]/, 2)[1].split("&").forEach(function (aParam) { - let [key, value] = aParam.split("="); - result[key] = decodeURIComponent(value); - }); - return result; -} - // Only allow one connecting window per endpoint. var gConnecting = {}; @@ -169,13 +160,14 @@ OAuth2.prototype = { delete this._browserRequest; }, - onAuthorizationReceived: function(aData) { - this.log.info("authorization received" + aData); - let results = parseURLData(aData); - if (results.code) { - this.requestAccessToken(results.code, OAuth2.CODE_AUTHORIZATION); + // @see RFC 6749 section 4.1.2: Authorization Response + onAuthorizationReceived(aURL) { + this.log.info("OAuth2 authorization received: url=" + aURL); + let params = new URLSearchParams(aURL.split("?", 2)[1]); + if (params.has("code")) { + this.requestAccessToken(params.get("code"), OAuth2.CODE_AUTHORIZATION); } else { - this.onAuthorizationFailed(null, aData); + this.onAuthorizationFailed(null, aURL); } }, -- cgit v1.2.3 From fd9d39ac9756a65e3c844dafb03724e53884ce6b Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Mon, 30 Dec 2019 10:20:58 -0500 Subject: Bug 1597933 - don't pass string constants to determine OAuth refresh token or not. --- mailnews/base/util/OAuth2.jsm | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/mailnews/base/util/OAuth2.jsm b/mailnews/base/util/OAuth2.jsm index 8feee0e94..037333abc 100644 --- a/mailnews/base/util/OAuth2.jsm +++ b/mailnews/base/util/OAuth2.jsm @@ -29,9 +29,6 @@ function OAuth2(aBaseURI, aScope, aAppKey, aAppSecret) { this.log = Log4Moz.getConfiguredLogger("TBOAuth"); } -OAuth2.CODE_AUTHORIZATION = "authorization_code"; -OAuth2.CODE_REFRESH = "refresh_token"; - OAuth2.prototype = { consumerKey: null, consumerSecret: null, @@ -53,7 +50,7 @@ OAuth2.prototype = { if (!aRefresh && this.accessToken) { aSuccess(); } else if (this.refreshToken) { - this.requestAccessToken(this.refreshToken, OAuth2.CODE_REFRESH); + this.requestAccessToken(this.refreshToken, true); } else { if (!aWithUI) { aFailure('{ "error": "auth_noui" }'); @@ -165,7 +162,7 @@ OAuth2.prototype = { this.log.info("OAuth2 authorization received: url=" + aURL); let params = new URLSearchParams(aURL.split("?", 2)[1]); if (params.has("code")) { - this.requestAccessToken(params.get("code"), OAuth2.CODE_AUTHORIZATION); + this.requestAccessToken(params.get("code"), false); } else { this.onAuthorizationFailed(null, aURL); } @@ -175,18 +172,27 @@ OAuth2.prototype = { this.connectFailureCallback(aData); }, - requestAccessToken: function requestAccessToken(aCode, aType) { + /** + * Request a new access token, or refresh an existing one. + * @param {string} aCode - The token issued to the client. + * @param {boolean} aRefresh - Whether it's a refresh of a token or not. + */ + requestAccessToken(aCode, aRefresh) { + // @see RFC 6749 section 4.1.3. Access Token Request + // @see RFC 6749 section 6. Refreshing an Access Token + let params = [ ["client_id", this.consumerKey], ["client_secret", this.consumerSecret], - ["grant_type", aType], ]; - if (aType == OAuth2.CODE_AUTHORIZATION) { + if (aRefresh) { + params.push(["grant_type", "refresh_token"]); + params.push(["refresh_token", aCode]); + } else { + params.push(["grant_type", "authorization_code"]); params.push(["code", aCode]); params.push(["redirect_uri", this.completionURI]); - } else if (aType == OAuth2.CODE_REFRESH) { - params.push(["refresh_token", aCode]); } let options = { -- cgit v1.2.3 From 8fdd883f7e8b5db69b192987ff3e134e88c70cbb Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Mon, 30 Dec 2019 10:26:15 -0500 Subject: Bug 1597933 - use fetch + URLSearchParms instead of Http.jsm to request OAuth2 access token. --- mailnews/base/util/OAuth2.jsm | 64 +++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/mailnews/base/util/OAuth2.jsm b/mailnews/base/util/OAuth2.jsm index 037333abc..6b1eb84a1 100644 --- a/mailnews/base/util/OAuth2.jsm +++ b/mailnews/base/util/OAuth2.jsm @@ -10,11 +10,12 @@ var EXPORTED_SYMBOLS = ["OAuth2"]; var {classes: Cc, interfaces: Ci, results: Cr, utils: Cu} = Components; -Cu.import("resource://gre/modules/Http.jsm"); Cu.import("resource://gre/modules/Services.jsm"); Cu.import("resource://gre/modules/XPCOMUtils.jsm"); Cu.import("resource:///modules/gloda/log4moz.js"); +Cu.importGlobalProperties(["fetch"]); + // Only allow one connecting window per endpoint. var gConnecting = {}; @@ -181,49 +182,48 @@ OAuth2.prototype = { // @see RFC 6749 section 4.1.3. Access Token Request // @see RFC 6749 section 6. Refreshing an Access Token - let params = [ - ["client_id", this.consumerKey], - ["client_secret", this.consumerSecret], - ]; + let data = new URLSearchParams(); + data.append("client_id", this.consumerKey); + data.append("client_secret", this.consumerSecret); if (aRefresh) { - params.push(["grant_type", "refresh_token"]); - params.push(["refresh_token", aCode]); + data.append("grant_type", "refresh_token"); + data.append("refresh_token", aCode); } else { - params.push(["grant_type", "authorization_code"]); - params.push(["code", aCode]); - params.push(["redirect_uri", this.completionURI]); + data.append("grant_type", "authorization_code"); + data.append("code", aCode); + data.append("redirect_uri", this.completionURI); } - let options = { - postData: params, - onLoad: this.onAccessTokenReceived.bind(this), - onError: this.onAccessTokenFailed.bind(this) - } - httpRequest(this.tokenURI, options); - }, - - onAccessTokenFailed: function onAccessTokenFailed(aError, aData) { - if (aError != "offline") { - this.refreshToken = null; - } - this.connectFailureCallback(aData); - }, - - onAccessTokenReceived: function onRequestTokenReceived(aData) { - let result = JSON.parse(aData); - + this.log.info( + `Making access token request to the token endpoint: ${this.tokenURI}` + ); + fetch(this.tokenURI, { + method: "POST", + cache: "no-cache", + body: data, + }) + .then(response => response.json()) + .then(result => { + this.log.info("The authorization server issued an access token."); this.accessToken = result.access_token; if ("refresh_token" in result) { - this.refreshToken = result.refresh_token; + this.refreshToken = result.refresh_token; } if ("expires_in" in result) { - this.tokenExpires = (new Date()).getTime() + (result.expires_in * 1000); + this.tokenExpires = new Date().getTime() + result.expires_in * 1000; } else { - this.tokenExpires = Number.MAX_VALUE; + this.tokenExpires = Number.MAX_VALUE; } this.tokenType = result.token_type; - this.connectSuccessCallback(); + }) + .catch(err => { + // Getting an access token failed. + this.log.info( + `The authorization server returned an error response: ${err}` + ); + this.connectFailureCallback(err); + }); } }; -- cgit v1.2.3 From fb7de243f87fa19048e6a86c42636e809e04ba68 Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Mon, 30 Dec 2019 10:31:00 -0500 Subject: Bug 1597933 - Use URLSearchParams for setting params for OAuth2 authorization request. --- mailnews/base/util/OAuth2.jsm | 57 ++++++++++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 20 deletions(-) diff --git a/mailnews/base/util/OAuth2.jsm b/mailnews/base/util/OAuth2.jsm index 6b1eb84a1..c838660f0 100644 --- a/mailnews/base/util/OAuth2.jsm +++ b/mailnews/base/util/OAuth2.jsm @@ -66,25 +66,31 @@ OAuth2.prototype = { }, requestAuthorization: function requestAuthorization() { - let params = [ - ["response_type", "code"], - ["client_id", this.consumerKey], - ["redirect_uri", this.completionURI], - ]; - // The scope can be optional. + let params = new URLSearchParams({ + response_type: "code", + client_id: this.consumerKey, + redirect_uri: this.completionURI, + }); + + // The scope is optional. if (this.scope) { - params.push(["scope", this.scope]); + params.append("scope", this.scope); } - // Add extra parameters - params.push(...this.extraAuthParams); + for (let [name, value] of this.extraAuthParams) { + params.append(name, value); + } - // Now map the parameters to a string - params = params.map(([k,v]) => k + "=" + encodeURIComponent(v)).join("&"); + let authEndpointURI = this.authURI + "?" + params.toString(); + this.log.info( + "Interacting with the resource owner to obtain an authorization grant " + + "from the authorization endpoint: " + + authEndpointURI + ); this._browserRequest = { account: this, - url: this.authURI + "?" + params, + url: authEndpointURI, _active: true, iconURI: "", cancelled: function() { @@ -187,17 +193,20 @@ OAuth2.prototype = { data.append("client_secret", this.consumerSecret); if (aRefresh) { + this.log.info( + `Making a refresh request to the token endpoint: ${this.tokenURI}` + ); data.append("grant_type", "refresh_token"); data.append("refresh_token", aCode); } else { + this.log.info( + `Making access token request to the token endpoint: ${this.tokenURI}` + ); data.append("grant_type", "authorization_code"); data.append("code", aCode); data.append("redirect_uri", this.completionURI); } - this.log.info( - `Making access token request to the token endpoint: ${this.tokenURI}` - ); fetch(this.tokenURI, { method: "POST", cache: "no-cache", @@ -205,6 +214,18 @@ OAuth2.prototype = { }) .then(response => response.json()) .then(result => { + if ("error" in result) { + // RFC 6749 section 5.2. Error Response + this.log.info( + `The authorization server returned an error response: ${JSON.stringify( + result + )}` + ); + this.connectFailureCallback(result); + return; + } + + // RFC 6749 section 5.1. Successful Response this.log.info("The authorization server issued an access token."); this.accessToken = result.access_token; if ("refresh_token" in result) { @@ -215,14 +236,10 @@ OAuth2.prototype = { } else { this.tokenExpires = Number.MAX_VALUE; } - this.tokenType = result.token_type; this.connectSuccessCallback(); }) .catch(err => { - // Getting an access token failed. - this.log.info( - `The authorization server returned an error response: ${err}` - ); + this.log.info(`Connection to authorization server failed: ${err}`); this.connectFailureCallback(err); }); } -- cgit v1.2.3 From 12eb1554f9ff0c0d8dc49da44b6bd0081b1231a1 Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Mon, 30 Dec 2019 10:33:31 -0500 Subject: Bug 1599054 - allow callers to ommit sending OAuth2 client_secret parameter. --- mailnews/base/util/OAuth2.jsm | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/mailnews/base/util/OAuth2.jsm b/mailnews/base/util/OAuth2.jsm index c838660f0..8c9282d02 100644 --- a/mailnews/base/util/OAuth2.jsm +++ b/mailnews/base/util/OAuth2.jsm @@ -19,7 +19,21 @@ Cu.importGlobalProperties(["fetch"]); // Only allow one connecting window per endpoint. var gConnecting = {}; -function OAuth2(aBaseURI, aScope, aAppKey, aAppSecret) { +/** + * Constructor for the OAuth2 object. + * + * @constructor + * @param {string} aBaseURI - The base URI for authentication and token + * requests, oauth2/auth or oauth2/token will be added for the actual + * requests. + * @param {?string} aScope - The scope as specified by RFC 6749 Section 3.3. + * Will not be included in the requests if falsy. + * @param {string} aAppKey - The client_id as specified by RFC 6749 Section + * 2.3.1. + * @param {string} [aAppSecret=null] - The client_secret as specified in + * RFC 6749 section 2.3.1. Will not be included in the requests if null. + */ +function OAuth2(aBaseURI, aScope, aAppKey, aAppSecret = null) { this.authURI = aBaseURI + "oauth2/auth"; this.tokenURI = aBaseURI + "oauth2/token"; this.consumerKey = aAppKey; @@ -190,7 +204,12 @@ OAuth2.prototype = { let data = new URLSearchParams(); data.append("client_id", this.consumerKey); - data.append("client_secret", this.consumerSecret); + if (this.consumerSecret !== null) { + // Section 2.3.1. of RFC 6749 states that empty secrets MAY be omitted + // by the client. This OAuth implementation delegates this decission to + // the caller: If the secret is null, it will be omitted. + data.append("client_secret", this.consumerSecret); + } if (aRefresh) { this.log.info( -- cgit v1.2.3