Bug 1528136 - refactor OAuth2 variable namings to use normal terminology from RFC 6749. r=Fallen
authorMagnus Melin <mkmelin+mozilla@iki.fi>
Thu, 16 Apr 2020 13:51:12 +0300
changeset 37967 00b5ea7d8c05ba5053e66fad8a657c3b9606c092
parent 37966 89315675d1ce07f27e7add439b6c2ecc68fdc651
child 37968 7c79228f232df09e1f6787a62444decd311668c8
push id2595
push userclokep@gmail.com
push dateMon, 04 May 2020 19:02:04 +0000
treeherdercomm-beta@f53913797371 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersFallen
bugs1528136
Bug 1528136 - refactor OAuth2 variable namings to use normal terminology from RFC 6749. r=Fallen Use standard terminology, and actually use the passed in endpoints! Not just setting them from outside later...
calendar/providers/caldav/modules/CalDavSession.jsm
mailnews/base/src/OAuth2Module.jsm
mailnews/base/util/OAuth2.jsm
mailnews/base/util/OAuth2Providers.jsm
--- a/calendar/providers/caldav/modules/CalDavSession.jsm
+++ b/calendar/providers/caldav/modules/CalDavSession.jsm
@@ -22,33 +22,40 @@ const OAUTH_GRACE_TIME = 30 * 1000;
 class CalDavGoogleOAuth extends OAuth2 {
   /**
    * Constructs a new Google OAuth autentication provider
    *
    * @param {String} sessionId    The session id, used in the password manager
    * @param {String} name         The user-readable description of this session
    */
   constructor(sessionId, name) {
-    // eslint-disable-next-line no-undef
-    super(OAUTH_BASE_URI, OAUTH_SCOPE, OAUTH_CLIENT_ID, OAUTH_HASH);
+    /* eslint-disable no-undef */
+    super(
+      OAUTH_BASE_URI + "oauth2/auth",
+      OAUTH_BASE_URI + "oauth2/token",
+      OAUTH_SCOPE,
+      OAUTH_CLIENT_ID,
+      OAUTH_HASH
+    );
+    /*  eslint-enable no-undef */
 
     this.id = sessionId;
     this.pwMgrId = "Google CalDAV v2";
     this.requestWindowTitle = cal.l10n.getAnyString(
       "global",
       "commonDialogs",
       "EnterUserPasswordFor2",
       [name]
     );
 
     this.requestWindowFeatures = "chrome,private,centerscreen,width=430,height=600";
   }
 
   /**
-   * Returns true if the token has expired, or will expire within the grace time
+   * Returns true if the token has expired, or will expire within the grace time.
    */
   get tokenExpired() {
     let now = new Date().getTime();
     return this.tokenExpires - OAUTH_GRACE_TIME < now;
   }
 
   /**
    * Retrieves the refresh token from the password manager. The token is cached.
--- a/mailnews/base/src/OAuth2Module.jsm
+++ b/mailnews/base/src/OAuth2Module.jsm
@@ -7,31 +7,28 @@
 var EXPORTED_SYMBOLS = ["OAuth2Module"];
 
 var { OAuth2 } = ChromeUtils.import("resource:///modules/OAuth2.jsm");
 var { OAuth2Providers } = ChromeUtils.import(
   "resource:///modules/OAuth2Providers.jsm"
 );
 var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
-function OAuth2Module() {
-  this._refreshToken = "";
-}
+/**
+ * OAuth2Module is the glue layer that gives XPCOM access to an OAuth2
+ * bearer token it can use to authenticate in SASL steps.
+ * It also takes care of persising the refreshToken for later usage.
+ *
+ * @implements {msgIOAuth2Module}
+ */
+function OAuth2Module() {}
 OAuth2Module.prototype = {
   // XPCOM registration stuff
   QueryInterface: ChromeUtils.generateQI([Ci.msgIOAuth2Module]),
 
-  _loadOAuthClientDetails(aIssuer) {
-    let details = OAuth2Providers.getIssuerDetails(aIssuer);
-    if (details) {
-      [this._appKey, this._appSecret, this._authURI, this._tokenURI] = details;
-    } else {
-      throw Cr.NS_ERROR_INVALID_ARGUMENT;
-    }
-  },
   initFromSmtp(aServer) {
     return this._initPrefs(
       "mail.smtpserver." + aServer.key + ".",
       aServer.username,
       aServer.hostname
     );
   },
   initFromMail(aServer) {
@@ -58,34 +55,41 @@ OAuth2Module.prototype = {
       } else {
         return false;
       }
     }
 
     // Find the app key we need for the OAuth2 string. Eventually, this should
     // be using dynamic client registration, but there are no current
     // implementations that we can test this with.
-    this._loadOAuthClientDetails(issuer);
+    let [
+      clientId,
+      clientSecret,
+      authorizationEndpoint,
+      tokenEndpoint,
+    ] = OAuth2Providers.getIssuerDetails(issuer);
+    if (!clientId) {
+      return false;
+    }
 
     // Username is needed to generate the XOAUTH2 string.
     this._username = aUsername;
     // LoginURL is needed to save the refresh token in the password manager.
     this._loginUrl = "oauth://" + issuer;
     // We use the scope to indicate the realm.
     this._scope = scope;
 
     // Define the OAuth property and store it.
     this._oauth = new OAuth2(
-      this._authURI,
+      authorizationEndpoint,
+      tokenEndpoint,
       scope,
-      this._appKey,
-      this._appSecret
+      clientId,
+      clientSecret
     );
-    this._oauth.authURI = this._authURI;
-    this._oauth.tokenURI = this._tokenURI;
 
     // Try hinting the username...
     this._oauth.extraAuthParams = [["login_hint", aUsername]];
 
     // Set the window title to something more useful than "Unnamed"
     this._oauth.requestWindowTitle = Services.strings
       .createBundle("chrome://messenger/locale/messenger.properties")
       .formatStringFromName("oauth2WindowTitle", [aUsername, aHostname]);
--- a/mailnews/base/util/OAuth2.jsm
+++ b/mailnews/base/util/OAuth2.jsm
@@ -15,41 +15,49 @@ Cu.importGlobalProperties(["fetch"]);
 
 // Only allow one connecting window per endpoint.
 var gConnecting = {};
 
 /**
  * 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.
+ * @param {string} authorizationEndpoint - The authorization endpoint as
+ *   defined by RFC 6749 Section 3.1.
+ * @param {string} tokenEndpoint - The token endpoint as defined by
+ *   RFC 6749 Section 3.2.
+ * @param {?string} scope - 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
+ * @param {string} clientId - The client_id as specified by RFC 6749 Section
  *   2.3.1.
- * @param {string} [aAppSecret=null] - The client_secret as specified in
+ * @param {string} [clientSecret=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;
-  this.consumerSecret = aAppSecret;
-  this.scope = aScope;
+function OAuth2(
+  authorizationEndpoint,
+  tokenEndpoint,
+  scope,
+  clientId,
+  clientSecret = null
+) {
+  this.authorizationEndpoint = authorizationEndpoint;
+  this.tokenEndpoint = tokenEndpoint;
+  this.scope = scope;
+  this.clientId = clientId;
+  this.consumerSecret = clientSecret;
+
   this.extraAuthParams = [];
 
   this.log = Log4Moz.getConfiguredLogger("TBOAuth");
 }
 
 OAuth2.prototype = {
-  consumerKey: null,
+  clientId: null,
   consumerSecret: null,
-  completionURI: "http://localhost",
+  redirectionEndpoint: "http://localhost",
   requestWindowURI: "chrome://messenger/content/browserRequest.xhtml",
   requestWindowFeatures: "chrome,private,centerscreen,width=980,height=750",
   requestWindowTitle: "",
   scope: null,
 
   accessToken: null,
   refreshToken: null,
   tokenExpires: 0,
@@ -62,41 +70,41 @@ OAuth2.prototype = {
       aSuccess();
     } else if (this.refreshToken) {
       this.requestAccessToken(this.refreshToken, true);
     } else {
       if (!aWithUI) {
         aFailure('{ "error": "auth_noui" }');
         return;
       }
-      if (gConnecting[this.authURI]) {
+      if (gConnecting[this.authorizationEndpoint]) {
         aFailure("Window already open");
         return;
       }
       this.requestAuthorization();
     }
   },
 
   requestAuthorization() {
     let params = new URLSearchParams({
       response_type: "code",
-      client_id: this.consumerKey,
-      redirect_uri: this.completionURI,
+      client_id: this.clientId,
+      redirect_uri: this.redirectionEndpoint,
     });
 
     // The scope is optional.
     if (this.scope) {
       params.append("scope", this.scope);
     }
 
     for (let [name, value] of this.extraAuthParams) {
       params.append(name, value);
     }
 
-    let authEndpointURI = this.authURI + "?" + params.toString();
+    let authEndpointURI = this.authorizationEndpoint + "?" + params.toString();
     this.log.info(
       "Interacting with the resource owner to obtain an authorization grant " +
         "from the authorization endpoint: " +
         authEndpointURI
     );
 
     this._browserRequest = {
       account: this,
@@ -131,23 +139,23 @@ OAuth2.prototype = {
           ]),
 
           _cleanUp() {
             this.webProgress.removeProgressListener(this);
             this.window.close();
             delete this.window;
           },
 
-          _checkForRedirect(aURL) {
-            if (aURL.indexOf(this._parent.completionURI) != 0) {
+          _checkForRedirect(url) {
+            if (!url.startsWith(this._parent.redirectionEndpoint)) {
               return;
             }
 
             this._parent.finishAuthorizationRequest();
-            this._parent.onAuthorizationReceived(aURL);
+            this._parent.onAuthorizationReceived(url);
           },
 
           onStateChange(aWebProgress, aRequest, aStateFlags, aStatus) {
             const wpl = Ci.nsIWebProgressListener;
             if (aStateFlags & (wpl.STATE_START | wpl.STATE_IS_NETWORK)) {
               this._checkForRedirect(aRequest.name);
             }
           },
@@ -162,27 +170,27 @@ OAuth2.prototype = {
           this._listener,
           Ci.nsIWebProgress.NOTIFY_ALL
         );
         aWindow.document.title = this.account.requestWindowTitle;
       },
     };
 
     this.wrappedJSObject = this._browserRequest;
-    gConnecting[this.authURI] = true;
+    gConnecting[this.authorizationEndpoint] = true;
     Services.ww.openWindow(
       null,
       this.requestWindowURI,
       null,
       this.requestWindowFeatures,
       this
     );
   },
   finishAuthorizationRequest() {
-    gConnecting[this.authURI] = false;
+    gConnecting[this.authorizationEndpoint] = false;
     if (!("_browserRequest" in this)) {
       return;
     }
 
     this._browserRequest._active = false;
     if ("_listener" in this._browserRequest) {
       this._browserRequest._listener._cleanUp();
     }
@@ -209,65 +217,66 @@ OAuth2.prototype = {
    * @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 data = new URLSearchParams();
-    data.append("client_id", this.consumerKey);
+    data.append("client_id", this.clientId);
     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(
-        `Making a refresh request to the token endpoint: ${this.tokenURI}`
+        `Making a refresh request to the token endpoint: ${this.tokenEndpoint}`
       );
       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}`
+        `Making access token request to the token endpoint: ${this.tokenEndpoint}`
       );
       data.append("grant_type", "authorization_code");
       data.append("code", aCode);
-      data.append("redirect_uri", this.completionURI);
+      data.append("redirect_uri", this.redirectionEndpoint);
     }
 
-    fetch(this.tokenURI, {
+    fetch(this.tokenEndpoint, {
       method: "POST",
       cache: "no-cache",
       body: data,
     })
       .then(response => response.json())
       .then(result => {
+        let resultStr = JSON.stringify(result, null, 2);
         if ("error" in result) {
           // RFC 6749 section 5.2. Error Response
           this.log.info(
-            `The authorization server returned an error response: ${JSON.stringify(
-              result
-            )}`
+            `The authorization server returned an error response: ${resultStr}`
           );
           // 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();
           return;
         }
 
         // RFC 6749 section 5.1. Successful Response
-        this.log.info("The authorization server issued an access token.");
+        this.log.info(
+          `Successful response from the authorization server: ${resultStr}`
+        );
         this.accessToken = result.access_token;
         if ("refresh_token" in result) {
           this.refreshToken = result.refresh_token;
         }
         if ("expires_in" in result) {
           this.tokenExpires = new Date().getTime() + result.expires_in * 1000;
         } else {
           this.tokenExpires = Number.MAX_VALUE;
--- a/mailnews/base/util/OAuth2Providers.jsm
+++ b/mailnews/base/util/OAuth2Providers.jsm
@@ -2,17 +2,19 @@
  * 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/. */
 
 /**
  * Details of supported OAuth2 Providers.
  */
 var EXPORTED_SYMBOLS = ["OAuth2Providers"];
 
-// map of hostnames to [issuer, scope]
+/**
+ * Map of hostnames to [issuer, scope].
+ */
 var kHostnames = new Map([
   ["imap.googlemail.com", ["accounts.google.com", "https://mail.google.com/"]],
   ["smtp.googlemail.com", ["accounts.google.com", "https://mail.google.com/"]],
   ["pop.googlemail.com", ["accounts.google.com", "https://mail.google.com/"]],
   ["imap.gmail.com", ["accounts.google.com", "https://mail.google.com/"]],
   ["smtp.gmail.com", ["accounts.google.com", "https://mail.google.com/"]],
   ["pop.gmail.com", ["accounts.google.com", "https://mail.google.com/"]],
 
@@ -26,23 +28,27 @@ var kHostnames = new Map([
   ["pop.mail.yahoo.com", ["login.yahoo.com", "mail-w"]],
   ["smtp.mail.yahoo.com", ["login.yahoo.com", "mail-w"]],
 
   ["imap.aol.com", ["login.aol.com", "mail-w"]],
   ["pop.aol.com", ["login.aol.com", "mail-w"]],
   ["smtp.aol.com", ["login.aol.com", "mail-w"]],
 ]);
 
-// map of issuers to appKey, appSecret, authURI, tokenURI
-
-// For the moment, these details are hard-coded, since Google does not
-// provide dynamic client registration. Don't copy these values for your
-// own application--register it yourself. This code (and possibly even the
-// registration itself) will disappear when this is switched to dynamic
-// client registration.
+/**
+ * Map of issuers to clientId, clientSecret, authorizationEndpoint, tokenEndpoint.
+ * Issuer is a unique string for the organization that a Thunderbird account
+ * was registered at.
+ *
+ * For the moment these details are hard-coded, since dynamic client
+ * registration is not yet supported. Don't copy these values for your
+ * own application - register one for yourself! This code (and possibly even the
+ * registration itself) will disappear when this is switched to dynamic
+ * client registration.
+ */
 var kIssuers = new Map([
   [
     "accounts.google.com",
     [
       "406964657835-aq8lmia8j95dhl1a2bvharmfk3t1hgqj.apps.googleusercontent.com",
       "kSmqreRr0qwBWJgbf5Y-PjSU",
       "https://accounts.google.com/o/oauth2/auth",
       "https://www.googleapis.com/oauth2/v3/token",
@@ -82,43 +88,43 @@ var kIssuers = new Map([
       "79c1c11991d148ddd02a919000d69879942fc278",
       "https://api.login.aol.com/oauth2/request_auth",
       "https://api.login.aol.com/oauth2/get_token",
     ],
   ],
 ]);
 
 /**
- *  OAuth2Providers: Methods to lookup OAuth2 parameters for supported
- *                   email providers.
+ * OAuth2Providers: Methods to lookup OAuth2 parameters for supported OAuth2
+ * providers.
  */
 var OAuth2Providers = {
   /**
    * Map a hostname to the relevant issuer and scope.
    *
-   * @param aHostname  String representing the url for an imap or smtp
-   *                   server (example "imap.googlemail.com").
+   * @param {string} hostname - The hostname of the server. For example
+   *  "imap.googlemail.com".
    *
-   * @returns          Array with [issuer, scope] for the hostname if found,
-   *                   else undefined. issuer is a string representing the
-   *                   organization, scope is an oauth parameter describing\
-   *                   the required access level.
+   * @returns {Array} An array containing [issuer, scope] for the hostname, or
+   *   undefined if not found.
+   *   - issuer is a string representing the organization
+   *   - scope is an OAuth2 parameter describing the required access level
    */
-  getHostnameDetails(aHostname) {
-    return kHostnames.get(aHostname);
+  getHostnameDetails(hostname) {
+    return kHostnames.get(hostname);
   },
 
   /**
    * Map an issuer to OAuth2 account details.
    *
-   * @param aIssuer    The organization issuing oauth2 parameters, example
-   *                   "accounts.google.com".
+   * @param {string} issuer - The organization issuing OAuth2 parameters, e.g.
+   *   "accounts.google.com".
    *
-   * @return           Array containing [appKey, appSecret, authURI, tokenURI]
-   *                   where appKey and appDetails are strings representing the
-   *                   account registered for Thunderbird with the organization,
-   *                   authURI and tokenURI are url strings representing
-   *                   endpoints to access OAuth2 authentication.
+   * @returns {Array} An array containing [clientId, clientSecret, authorizationEndpoint, tokenEndpoint].
+   *   clientId and clientSecret are strings representing the account registered
+   *   for Thunderbird with the organization.
+   *   authorizationEndpoint and tokenEndpoint are url strings representing
+   *   endpoints to access OAuth2 authentication.
    */
-  getIssuerDetails(aIssuer) {
-    return kIssuers.get(aIssuer);
+  getIssuerDetails(issuer) {
+    return kIssuers.get(issuer);
   },
 };