A big refactor and cleanup on oauth{1,2} and twitter login

examples/todos/.meteor/packages

@@ -12,3 +12,4 @@ accounts-weibo
 accounts-google
 accounts-facebook
 accounts-passwords
+accounts-twitter

packages/accounts-oauth-helper/oauth_server.js

@@ -10,8 +10,9 @@
   //
   // @param name {String} e.g. "google", "facebook"
   // @param version {Number} OAuth version (1 or 2)
-  // @param handleOauthRequest {Function(query)}
-  //   - query is an object with the parameters passed in the query string
+  // @param handleOauthRequest {Function(oauthBinding|query)}
+  //   - (For OAuth1 only) oauthBinding {OAuth1Binding} bound to the appropriate provider
+  //   - (For OAuth2 only) query {Object} parameters passed in query string
   //   - return value is:
   //     - {options: (options), extra: (optional extra)} (same as the
   //       arguments to Meteor.accounts.updateOrCreateUser)
@@ -65,23 +66,25 @@
     });
 
   Meteor.accounts.oauth._middleware = function (req, res, next) {
-    var serviceName = requestServiceName(req);
-    if (!serviceName) {
-      // not an oauth request. pass to next middleware.
-      next();
-      return;
-    }
-
-    var service = Meteor.accounts.oauth._services[serviceName];
-
-    // Skip everything if there's no service set by the oauth middleware
-    if (!service)
-      throw new Error("Unexpected OAuth service " + serviceName);
-
-    // Make sure we're configured
-    ensureConfigured(serviceName);
-
+    // Make sure to catch any exceptions because otherwise we'd crash
+    // the runner
     try {
+      var serviceName = oauthServiceName(req);
+      if (!serviceName) {
+        // not an oauth request. pass to next middleware.
+        next();
+        return;
+      }
+
+      var service = Meteor.accounts.oauth._services[serviceName];
+
+      // Skip everything if there's no service set by the oauth middleware
+      if (!service)
+        throw new Error("Unexpected OAuth service " + serviceName);
+
+      // Make sure we're configured
+      ensureConfigured(serviceName);
+
       if (service.version === 1)
         Meteor.accounts.oauth1._handleRequest(service, req.query, res);
       else if (service.version === 2)
@@ -100,29 +103,32 @@
         Meteor.accounts.oauth._loginResultForState[req.query.state] = err;
 
       // also log to the server console, so the developer sees it.
-      Meteor._debug("Exception in oauth" + service.version + " handler", err);
+      Meteor._debug("Exception in oauth request handler", err);
+
+      // close the popup. because nobody likes them just hanging
+      // there.  when someone sees this multiple times they might
+      // think to check server logs (we hope?)
+      closePopup(res);
     }
   };
 
-  // Handle _oauth paths, gets a bunch of stuff ready for the oauth implementation middleware
+  // Handle /_oauth/* paths and extract the service name
   //
   // @returns {String|null} e.g. "facebook", or null if this isn't an
   // oauth request
-  var requestServiceName = function (req) {
+  var oauthServiceName = function (req) {
 
     // req.url will be "/_oauth/<service name>?<action>"
     var barePath = req.url.substring(0, req.url.indexOf('?'));
     var splitPath = barePath.split('/');
 
+    // Any non-oauth request will continue down the default
+    // middlewares.
+    if (splitPath[1] !== '_oauth')
+      return null;
+
     // Find service based on url
     var serviceName = splitPath[2];
-
-    // Any non-oauth request will continue down the default middlewares
-    // Same goes for service that hasn't been registered
-    if (splitPath[1] !== '_oauth') {
-      return null;
-    }
-
     return serviceName;
   };
 
@@ -130,24 +136,22 @@
   var ensureConfigured = function(serviceName) {
     var service = Meteor.accounts[serviceName];
 
-    _.each(Meteor.accounts[serviceName]._requireConfigs, function(key) {
+    _.each(service._requireConfigs, function(key) {
       if (!service[key])
-        throw new Meteor.accounts.ConfigError("Need to call Meteor.accounts." + serviceName + ".config first");
+        throw new Meteor.accounts.ConfigError(
+          "Need to call Meteor.accounts." + serviceName + ".config first");
     });
 
-    if (!service._secret)
-      throw new Meteor.accounts.ConfigError("Need to call Meteor.accounts." + serviceName + ".setSecret first");
+    if (Meteor.is_server && !service._secret)
+      throw new Meteor.accounts.ConfigError(
+        "Need to call Meteor.accounts." + serviceName + ".setSecret first");
   };
 
   Meteor.accounts.oauth._renderOauthResults = function(res, query) {
     // We support ?close and ?redirect=URL. Any other query should
     // just serve a blank page
     if ('close' in query) { // check with 'in' because we don't set a value
-      // Close the popup window
-      res.writeHead(200, {'Content-Type': 'text/html'});
-      var content =
-            '<html><head><script>window.close()</script></head></html>';
-      res.end(content, 'utf-8');
+      closePopup(res);
     } else if (query.redirect) {
       res.writeHead(302, {'Location': query.redirect});
       res.end();
@@ -157,6 +161,13 @@
     }
   };
 
+  var closePopup = function(res) {
+    res.writeHead(200, {'Content-Type': 'text/html'});
+    var content =
+          '<html><head><script>window.close()</script></head></html>';
+    res.end(content, 'utf-8');
+  };
+
 })();
 
 

packages/accounts-oauth1-helper/oauth1_binding.js

@@ -1,23 +1,38 @@
 var crypto = __meteor_bootstrap__.require("crypto");
 var querystring = __meteor_bootstrap__.require("querystring");
 
-OAuth1 = function(config) {
-  _.extend(this, config);
+// An OAuth1 wrapper around http calls which helps get tokens and
+// takes care of HTTP headers
+//
+// @param consumerKey {String} As supplied by the OAuth1 provider
+// @param consumerSecret {String} As supplied by the OAuth1 provider
+// @param urls {Object}
+//   - requestToken (String): url
+//   - authorize (String): url
+//   - accessToken (String): url
+//   - authenticate (String): url
+OAuth1Binding = function(consumerKey, consumerSecret, urls) {
+  this._consumerKey = consumerKey;
+  this._secret = consumerSecret;
+  this._urls = urls;
 };
 
-OAuth1.prototype.getRequestToken = function(callbackUrl) {
+OAuth1Binding.prototype.prepareRequestToken = function(callbackUrl) {
 
   var headers = this._buildHeader({
     oauth_callback: callbackUrl
   });
 
-  var response = this._call('post', this._urls.requestToken, headers);
+  var response = this._call('POST', this._urls.requestToken, headers);
   var tokens = querystring.parse(response.content);
 
+  // XXX should we also store oauth_token_secret here?
+  if (!tokens.oauth_callback_confirmed)
+    throw new Error("oauth_callback_confirmed false when requesting oauth1 token", tokens);
   this.requestToken = tokens.oauth_token;
 };
 
-OAuth1.prototype.getAccessToken = function(query) {
+OAuth1Binding.prototype.prepareAccessToken = function(query) {
   var headers = this._buildHeader({
     oauth_token: query.oauth_token
   });
@@ -26,30 +41,29 @@ OAuth1.prototype.getAccessToken = function(query) {
     oauth_verifier: query.oauth_verifier
   };
 
-  var response = this._call('post', this._urls.accessToken, headers, params);
+  var response = this._call('POST', this._urls.accessToken, headers, params);
   var tokens = querystring.parse(response.content);
 
   this.accessToken = tokens.oauth_token;
   this.accessTokenSecret = tokens.oauth_token_secret;
 };
 
-OAuth1.prototype.call = function(method, url) {
+OAuth1Binding.prototype.call = function(method, url) {
   var headers = this._buildHeader({
     oauth_token: this.accessToken
   });
- 
-  var response = this._call(method, url, headers);
 
+  var response = this._call(method, url, headers);
   return response.data;
 };
 
-OAuth1.prototype.get = function(url) {
-  return this.call('get', url);
+OAuth1Binding.prototype.get = function(url) {
+  return this.call('GET', url);
 };
 
-OAuth1.prototype._buildHeader = function(headers) {
+OAuth1Binding.prototype._buildHeader = function(headers) {
   return _.extend({
-    oauth_consumer_key: this._appId,
+    oauth_consumer_key: this._consumerKey,
     oauth_nonce: Meteor.uuid().replace(/\W/g, ''),
     oauth_signature_method: 'HMAC-SHA1',
     oauth_timestamp: (new Date().valueOf()/1000).toFixed().toString(),
@@ -57,7 +71,7 @@ OAuth1.prototype._buildHeader = function(headers) {
   }, headers);
 };
 
-OAuth1.prototype._getSignature = function(method, url, rawHeaders, oauthSecret) {
+OAuth1Binding.prototype._getSignature = function(method, url, rawHeaders, accessTokenSecret) {
 
   var headers = this._encodeHeader(rawHeaders);
 
@@ -72,43 +86,45 @@ OAuth1.prototype._getSignature = function(method, url, rawHeaders, oauthSecret)
   ].join('&');
 
   var signingKey = encodeURIComponent(this._secret) + '&';
-  if (oauthSecret)
-    signingKey += encodeURIComponent(oauthSecret);
+  if (accessTokenSecret)
+    signingKey += encodeURIComponent(accessTokenSecret);
 
   return crypto.createHmac('SHA1', signingKey).update(signatureBase).digest('base64');
 };
 
-OAuth1.prototype._call = function(method, url, headers, params) {
-  
+OAuth1Binding.prototype._call = function(method, url, headers, params) {
+
   // Get the signature
-  headers.oauth_signature = this._getSignature(method.toUpperCase(), url, headers, this.accessTokenSecret);
+  headers.oauth_signature = this._getSignature(method, url, headers, this.accessTokenSecret);
 
   // Make a authorization string according to oauth1 spec
   var authString = this._getAuthHeaderString(headers);
 
   // Make signed request
-  var response = Meteor.http[method.toLowerCase()](url, {
+  var response = Meteor.http.call(method, url, {
     params: params,
     headers: {
       Authorization: authString
     }
   });
 
-  if (response.error)
+  if (response.error) {
+    Meteor._debug('Error sending OAuth1 HTTP call', method, url, params, authString);
     throw response.error;
-    
+  }
+
   return response;
 };
 
-OAuth1.prototype._encodeHeader = function(header) {
+OAuth1Binding.prototype._encodeHeader = function(header) {
   return _.reduce(header, function(memo, val, key) {
     memo[encodeURIComponent(key)] = encodeURIComponent(val);
     return memo;
   }, {});
 };
 
-OAuth1.prototype._getAuthHeaderString = function(headers) {
+OAuth1Binding.prototype._getAuthHeaderString = function(headers) {
   return 'OAuth ' +  _.map(headers, function(val, key) {
-    return encodeURIComponent(key) + '="' + encodeURIComponent(val) + '"'; 
+    return encodeURIComponent(key) + '="' + encodeURIComponent(val) + '"';
   }).sort().join(', ');
 };

packages/accounts-oauth1-helper/oauth1_server.js

@@ -7,45 +7,44 @@
   // connect middleware
   Meteor.accounts.oauth1._handleRequest = function (service, query, res) {
 
-    // Make sure we prepare the login results before returning.
-    // This way the subsequent call to the `login` method will be
-    // immediate.
-
     var config = Meteor.accounts[service.serviceName];
-    var oauth = new OAuth1(config);
+    var oauthBinding = new OAuth1Binding(config._consumerKey, config._secret, config._urls);
 
-    // If we get here with a callback url we need a request token to
-    // start the logic process
-    if (query.callbackUrl) {
+    if (query.requestTokenAndRedirect) {
+      // step 1 - get and store a request token
 
       // Get a request token to start auth process
-      oauth.getRequestToken(query.callbackUrl);
+      oauthBinding.prepareRequestToken(query.requestTokenAndRedirect);
 
       // Keep track of request token so we can verify it on the next step
-      Meteor.accounts.oauth1._requestTokens[query.state] = oauth.requestToken;
+      Meteor.accounts.oauth1._requestTokens[query.state] = oauthBinding.requestToken;
 
-      var redirectUrl = config._urls.authenticate + '?oauth_token=' + oauth.requestToken;
+      // redirect to provider login, which will redirect back to "step 2" below
+      var redirectUrl = config._urls.authenticate + '?oauth_token=' + oauthBinding.requestToken;
       res.writeHead(302, {'Location': redirectUrl});
       res.end();
 
-    // If we get here without a callback url we've just
-    // returned from authentication via the oauth provider
-
     } else {
+      // step 2, redirected from provider login - complete the login
+      // process: if the user authorized permissions, get an access
+      // token and access token secret and log in as user
 
       // Get the user's request token so we can verify it and clear it
       var requestToken = Meteor.accounts.oauth1._requestTokens[query.state];
       delete Meteor.accounts.oauth1._requestTokens[query.state];
 
-      // Verify user authorized access and the oauth_token matches 
+      // Verify user authorized access and the oauth_token matches
       // the requestToken from previous step
       if (query.oauth_token && query.oauth_token === requestToken) {
 
+        // Prepare the login results before returning.  This way the
+        // subsequent call to the `login` method will be immediate.
+
         // Get the access token for signing requests
-        oauth.getAccessToken(query);
+        oauthBinding.prepareAccessToken(query);
 
         // Get or create user id
-        var oauthResult = service.handleOauthRequest(oauth);
+        var oauthResult = service.handleOauthRequest(oauthBinding);
         var userId = Meteor.accounts.updateOrCreateUser(
           oauthResult.options, oauthResult.extra);
 

packages/accounts-oauth1-helper/oauth1_tests.js

@@ -6,8 +6,8 @@ Tinytest.add("oauth1 - loginResultForState is stored", function (test) {
   var twitterfooAccessToken = Meteor.uuid();
   var twitterfooAccessTokenSecret = Meteor.uuid();
 
-  OAuth1.prototype.getRequestToken = function() {};
-  OAuth1.prototype.getAccessToken = function() {
+  OAuth1Binding.prototype.prepareRequestToken = function() {};
+  OAuth1Binding.prototype.prepareAccessToken = function() {
     this.accessToken = twitterfooAccessToken;
     this.accessTokenSecret = twitterfooAccessTokenSecret;
   };
@@ -18,8 +18,6 @@ Tinytest.add("oauth1 - loginResultForState is stored", function (test) {
   Meteor.accounts.oauth._loginResultForState = {};
   Meteor.accounts.oauth._services = {};
 
-  // XXX can we make this unnecessary? Not totally sold on _requireConfigs
-  // yet, but maybe I'm just being overly delicate.
   Meteor.accounts.twitterfoo = {};
   Meteor.accounts.twitterfoo._requireConfigs = [];
   Meteor.accounts.twitterfoo._secret = 'XXX';
@@ -42,7 +40,7 @@ Tinytest.add("oauth1 - loginResultForState is stored", function (test) {
 
   // simulate logging in using twitterfoo
   Meteor.accounts.oauth1._requestTokens['STATE'] = twitterfooAccessToken;
-  
+
   var req = {
     method: "POST",
     url: "/_oauth/twitterfoo?close",
@@ -80,8 +78,6 @@ Tinytest.add("oauth1 - error in user creation", function (test) {
   var twitterfailAccessToken = Meteor.uuid();
   var twitterfailAccessTokenSecret = Meteor.uuid();
 
-  // XXX can we make this unnecessary? Not totally sold on _requireConfigs
-  // yet, but maybe I'm just being overly delicate.
   Meteor.accounts.twitterfail = {};
   Meteor.accounts.twitterfail._requireConfigs = [];
   Meteor.accounts.twitterfail._secret = 'XXX';

packages/accounts-oauth1-helper/package.js

@@ -6,7 +6,7 @@ Package.describe({
 Package.on_use(function (api) {
   api.use('accounts-oauth-helper', 'client');
 
-  api.add_files('oauth1.js', 'server');
+  api.add_files('oauth1_binding.js', 'server');
   api.add_files('oauth1_common.js', ['client', 'server']);
   api.add_files('oauth1_server.js', 'server');
 });

packages/accounts-oauth2-helper/oauth2_server.js

@@ -3,29 +3,26 @@
 
   // connect middleware
   Meteor.accounts.oauth2._handleRequest = function (service, query, res) {
-    if (query.error) {
-      // The user didn't authorize access
-      return;
+    // check if user authorized access
+    if (!query.error) {
+      // Prepare the login results before returning.  This way the
+      // subsequent call to the `login` method will be immediate.
+
+      // Get or create user id
+      var oauthResult = service.handleOauthRequest(query);
+
+      var userId = Meteor.accounts.updateOrCreateUser(
+        oauthResult.options, oauthResult.extra);
+
+      // Generate and store a login token for reconnect
+      // XXX this could go in accounts_server.js instead
+      var loginToken = Meteor.accounts._loginTokens.insert({userId: userId});
+
+      // Store results to subsequent call to `login`
+      Meteor.accounts.oauth._loginResultForState[query.state] =
+        {token: loginToken, id: userId};
     }
 
-    // Make sure we prepare the login results before returning.
-    // This way the subsequent call to the `login` method will be
-    // immediate.
-
-    // Get or create user id
-    var oauthResult = service.handleOauthRequest(query);
-
-    var userId = Meteor.accounts.updateOrCreateUser(
-      oauthResult.options, oauthResult.extra);
-
-    // Generate and store a login token for reconnect
-    // XXX this could go in accounts_server.js instead
-    var loginToken = Meteor.accounts._loginTokens.insert({userId: userId});
-
-    // Store results to subsequent call to `login`
-    Meteor.accounts.oauth._loginResultForState[query.state] =
-      {token: loginToken, id: userId};
-
     // Either close the window, redirect, or render nothing
     // if all else fails
     Meteor.accounts.oauth._renderOauthResults(res, query);

packages/accounts-oauth2-helper/oauth2_tests.js

@@ -9,8 +9,6 @@ Tinytest.add("oauth2 - loginResultForState is stored", function (test) {
   Meteor.accounts.oauth._loginResultForState = {};
   Meteor.accounts.oauth._services = {};
 
-  // XXX can we make this unnecessary? Not totally sold on _requireConfigs
-  // yet, but maybe I'm just being overly delicate.
   Meteor.accounts.foobook = {};
   Meteor.accounts.foobook._requireConfigs = [];
   Meteor.accounts.foobook._secret = 'XXX';
@@ -54,7 +52,6 @@ Tinytest.add("oauth2 - error in user creation", function (test) {
   var email = Meteor.uuid() + "@example.com";
   var state = Meteor.uuid();
 
-  // XXX this shouldn't be necessary
   Meteor.accounts.failbook = {};
   Meteor.accounts.failbook._requireConfigs = [];
   Meteor.accounts.failbook._secret = 'XXX';

packages/accounts-twitter/twitter_client.js

@@ -1,14 +1,21 @@
 (function () {
   Meteor.loginWithTwitter = function () {
-    if (!Meteor.accounts.twitter._appId || !Meteor.accounts.twitter._appUrl)
+    if (!Meteor.accounts.twitter._appUrl)
       throw new Meteor.accounts.ConfigError("Need to call Meteor.accounts.twitter.config first");
 
     var state = Meteor.uuid();
     // We need to keep state across the next two 'steps' so we're adding
     // a state parameter to the url and the callback url that we'll be returned
     // to by oauth provider
+
+    // url back to app, enters "step 2" as described in
+    // packages/accounts-oauth1-helper/oauth1_server.js
     var callbackUrl = Meteor.accounts.twitter._appUrl + '/_oauth/twitter?close&state=' + state;
-    var url = '/_oauth/twitter/request_token?callbackUrl=' + encodeURIComponent(callbackUrl) + '&state=' + state
+
+    // url to app, enters "step 1" as described in
+    // packages/accounts-oauth1-helper/oauth1_server.js
+    var url = '/_oauth/twitter/?requestTokenAndRedirect=' + encodeURIComponent(callbackUrl)
+          + '&state=' + state;
 
     Meteor.accounts.oauth.initiateLogin(state, url);
   };

packages/accounts-twitter/twitter_common.js

@@ -1,17 +1,17 @@
 if (!Meteor.accounts.twitter) {
   Meteor.accounts.twitter = {};
-  Meteor.accounts.twitter._requireConfigs = ['_appId', '_appUrl'];
+  Meteor.accounts.twitter._requireConfigs = ['_consumerKey', '_appUrl'];
 }
 
-Meteor.accounts.twitter.config = function(appId, appUrl, options) {
-  Meteor.accounts.twitter._appId = appId;
+Meteor.accounts.twitter.config = function(consumerKey, appUrl, options) {
+  Meteor.accounts.twitter._consumerKey = consumerKey;
   Meteor.accounts.twitter._appUrl = appUrl;
-  Meteor.accounts.twitter._options = options;
-  
-  Meteor.accounts.twitter._urls = {
-    requestToken: "https://api.twitter.com/oauth/request_token",
-    authorize: "https://api.twitter.com/oauth/authorize",
-    accessToken: "https://api.twitter.com/oauth/access_token",
-    authenticate: "https://api.twitter.com/oauth/authenticate"
-  };
+  Meteor.accounts.twitter._options = options; // xcx don't need this
+};
+
+Meteor.accounts.twitter._urls = {
+  requestToken: "https://api.twitter.com/oauth/request_token",
+  authorize: "https://api.twitter.com/oauth/authorize",
+  accessToken: "https://api.twitter.com/oauth/access_token",
+  authenticate: "https://api.twitter.com/oauth/authenticate"
 };

packages/accounts-twitter/twitter_server.js

@@ -1,12 +1,11 @@
 (function () {
 
-  Meteor.accounts.twitter.setSecret = function (secret) {
-    Meteor.accounts.twitter._secret = secret;
+  Meteor.accounts.twitter.setSecret = function (consumerSecret) {
+    Meteor.accounts.twitter._secret = consumerSecret;
   };
 
-  Meteor.accounts.oauth.registerService('twitter', 1, function(oauth) {
-
-    var identity = oauth.get('https://api.twitter.com/1/account/verify_credentials.json');
+  Meteor.accounts.oauth.registerService('twitter', 1, function(oauthBinding) {
+    var identity = oauthBinding.get('https://api.twitter.com/1/account/verify_credentials.json');
 
     return {
       options: {
@@ -14,8 +13,8 @@
           twitter: {
             id: identity.id,
             screenName: identity.screen_name,
-            accessToken: oauth.accessToken,
-            accessTokenSecret: oauth.accessTokenSecret
+            accessToken: oauthBinding.accessToken,
+            accessTokenSecret: oauthBinding.accessTokenSecret
           }
         }
       },

packages/accounts-ui/login_buttons.html

@@ -94,7 +94,7 @@
     <a class="login-link-text">Sign in</a>
     {{#if dropdownVisible}}
     <div id="login-dropdown-list" class="accounts-dialog">
-      <a class="login-close-text">Close</a>
+      <a class="login-close-text">Sign in</a>
       <div class="login-close-text-clear"></div>
       {{> loginButtonsServicesRow}}
     </div>