From a2eda047feab6d156b708848234295eb356bd680 Mon Sep 17 00:00:00 2001 From: Avital Oliver Date: Wed, 22 Aug 2012 22:02:40 -0700 Subject: [PATCH] A big refactor and cleanup on oauth{1,2} and twitter login --- examples/todos/.meteor/packages | 1 + .../accounts-oauth-helper/oauth_server.js | 85 +++++++++++-------- .../{oauth1.js => oauth1_binding.js} | 66 ++++++++------ .../accounts-oauth1-helper/oauth1_server.js | 33 ++++--- .../accounts-oauth1-helper/oauth1_tests.js | 10 +-- packages/accounts-oauth1-helper/package.js | 2 +- .../accounts-oauth2-helper/oauth2_server.js | 39 ++++----- .../accounts-oauth2-helper/oauth2_tests.js | 3 - packages/accounts-twitter/twitter_client.js | 11 ++- packages/accounts-twitter/twitter_common.js | 22 ++--- packages/accounts-twitter/twitter_server.js | 13 ++- packages/accounts-ui/login_buttons.html | 2 +- 12 files changed, 155 insertions(+), 132 deletions(-) rename packages/accounts-oauth1-helper/{oauth1.js => oauth1_binding.js} (51%) diff --git a/examples/todos/.meteor/packages b/examples/todos/.meteor/packages index c71f4c1a54..0aff14a890 100644 --- a/examples/todos/.meteor/packages +++ b/examples/todos/.meteor/packages @@ -12,3 +12,4 @@ accounts-weibo accounts-google accounts-facebook accounts-passwords +accounts-twitter diff --git a/packages/accounts-oauth-helper/oauth_server.js b/packages/accounts-oauth-helper/oauth_server.js index 49c814dc68..7f2f3aa9aa 100644 --- a/packages/accounts-oauth-helper/oauth_server.js +++ b/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/?" 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 = - ''; - 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 = + ''; + res.end(content, 'utf-8'); + }; + })(); diff --git a/packages/accounts-oauth1-helper/oauth1.js b/packages/accounts-oauth1-helper/oauth1_binding.js similarity index 51% rename from packages/accounts-oauth1-helper/oauth1.js rename to packages/accounts-oauth1-helper/oauth1_binding.js index 324eef621e..3cfb3d47a1 100644 --- a/packages/accounts-oauth1-helper/oauth1.js +++ b/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(', '); }; diff --git a/packages/accounts-oauth1-helper/oauth1_server.js b/packages/accounts-oauth1-helper/oauth1_server.js index 84be5a4ec2..3203f08955 100644 --- a/packages/accounts-oauth1-helper/oauth1_server.js +++ b/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); diff --git a/packages/accounts-oauth1-helper/oauth1_tests.js b/packages/accounts-oauth1-helper/oauth1_tests.js index 1514d8ab1f..e7a004cc7a 100644 --- a/packages/accounts-oauth1-helper/oauth1_tests.js +++ b/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'; diff --git a/packages/accounts-oauth1-helper/package.js b/packages/accounts-oauth1-helper/package.js index de76baa5d3..df47f39784 100644 --- a/packages/accounts-oauth1-helper/package.js +++ b/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'); }); diff --git a/packages/accounts-oauth2-helper/oauth2_server.js b/packages/accounts-oauth2-helper/oauth2_server.js index 41919e92c0..718d7fd4b6 100644 --- a/packages/accounts-oauth2-helper/oauth2_server.js +++ b/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); diff --git a/packages/accounts-oauth2-helper/oauth2_tests.js b/packages/accounts-oauth2-helper/oauth2_tests.js index d13328f4ba..1544225cd7 100644 --- a/packages/accounts-oauth2-helper/oauth2_tests.js +++ b/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'; diff --git a/packages/accounts-twitter/twitter_client.js b/packages/accounts-twitter/twitter_client.js index 8ecdafd6e2..d3407bb31e 100644 --- a/packages/accounts-twitter/twitter_client.js +++ b/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); }; diff --git a/packages/accounts-twitter/twitter_common.js b/packages/accounts-twitter/twitter_common.js index 812100b696..d09fc6c7aa 100644 --- a/packages/accounts-twitter/twitter_common.js +++ b/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" }; diff --git a/packages/accounts-twitter/twitter_server.js b/packages/accounts-twitter/twitter_server.js index 2cdab4baa7..285e6fd34a 100644 --- a/packages/accounts-twitter/twitter_server.js +++ b/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 } } }, diff --git a/packages/accounts-ui/login_buttons.html b/packages/accounts-ui/login_buttons.html index 5dc344b2ae..26d60bee86 100644 --- a/packages/accounts-ui/login_buttons.html +++ b/packages/accounts-ui/login_buttons.html @@ -94,7 +94,7 @@ {{#if dropdownVisible}}
- + {{> loginButtonsServicesRow}}