From 01aec7a4720ec359a313675709b3a10bcd2794be Mon Sep 17 00:00:00 2001 From: Tim Haines Date: Wed, 12 Mar 2014 11:16:07 -0700 Subject: [PATCH 001/219] Move oauth login results from ram to mongo, giving all servers access. --- packages/accounts-oauth/oauth_server.js | 6 +- packages/oauth/oauth_server.js | 27 ++------- packages/oauth/oauth_tests.js | 13 ++++ packages/oauth/package.js | 12 ++++ packages/oauth/transient_results.js | 80 +++++++++++++++++++++++++ packages/oauth1/oauth1_server.js | 14 ++--- packages/oauth1/oauth1_tests.js | 27 ++++----- packages/oauth2/oauth2_server.js | 17 +++--- packages/oauth2/oauth2_tests.js | 18 +++--- 9 files changed, 153 insertions(+), 61 deletions(-) create mode 100644 packages/oauth/oauth_tests.js create mode 100644 packages/oauth/transient_results.js diff --git a/packages/accounts-oauth/oauth_server.js b/packages/accounts-oauth/oauth_server.js index ca6dfa4aff9..3fb87c5f470 100644 --- a/packages/accounts-oauth/oauth_server.js +++ b/packages/accounts-oauth/oauth_server.js @@ -6,7 +6,9 @@ Accounts.registerLoginHandler(function (options) { check(options.oauth, {credentialToken: String}); - if (!Oauth.hasCredential(options.oauth.credentialToken)) { + var result = Oauth.retrieveCredential(options.oauth.credentialToken); + + if (!result) { // OAuth credentialToken is not recognized, which could be either because the popup // was closed by the user before completion, or some sort of error where // the oauth provider didn't talk to our server correctly and closed the @@ -19,7 +21,7 @@ Accounts.registerLoginHandler(function (options) { throw new Meteor.Error(Accounts.LoginCancelledError.numericError, 'No matching login attempt found'); } - var result = Oauth.retrieveCredential(options.oauth.credentialToken); + if (result instanceof Error) // We tried to login, but there was a fatal error. Report it back // to the user. diff --git a/packages/oauth/oauth_server.js b/packages/oauth/oauth_server.js index 736748e8ee8..0d18140b103 100644 --- a/packages/oauth/oauth_server.js +++ b/packages/oauth/oauth_server.js @@ -49,27 +49,10 @@ OauthTest.unregisterService = function (name) { }; -// When we get an incoming OAuth http request we complete the oauth -// handshake, account and token setup before responding. The -// results are stored in this map which is then read when the login -// method is called. Maps credentialToken --> return value of `login` -// -// NB: the oauth1 and oauth2 packages manipulate this directly. might -// be nice for them to have a setter instead -// -// XXX we should periodically clear old entries -// -Oauth._loginResultForCredentialToken = {}; - -Oauth.hasCredential = function(credentialToken) { - return _.has(Oauth._loginResultForCredentialToken, credentialToken); -} - Oauth.retrieveCredential = function(credentialToken) { - var result = Oauth._loginResultForCredentialToken[credentialToken]; - delete Oauth._loginResultForCredentialToken[credentialToken]; - return result; -} + return Oauth._retrieveTransientResult(credentialToken); +}; + // Listen to incoming OAuth http requests WebApp.connectHandlers.use(function(req, res, next) { @@ -106,14 +89,14 @@ middleware = function (req, res, next) { handler(service, req.query, res); } catch (err) { // if we got thrown an error, save it off, it will get passed to - // the approporiate login call (if any) and reported there. + // the appropriate login call (if any) and reported there. // // The other option would be to display it in the popup tab that // is still open at this point, ignoring the 'close' or 'redirect' // we were passed. But then the developer wouldn't be able to // style the error or react to it in any way. if (req.query.state && err instanceof Error) - Oauth._loginResultForCredentialToken[req.query.state] = err; + Oauth._storeTransientResult(req.query.state, err); // XXX the following is actually wrong. if someone wants to // redirect rather than close once we are done with the OAuth diff --git a/packages/oauth/oauth_tests.js b/packages/oauth/oauth_tests.js new file mode 100644 index 00000000000..a66e533701a --- /dev/null +++ b/packages/oauth/oauth_tests.js @@ -0,0 +1,13 @@ +Tinytest.add("oauth - transientResult handles errors", function (test) { + var credentialToken = Random.id(); + + var testError = new Error("This is a test error"); + testError.stack = 'test stack'; + Oauth._storeTransientResult(credentialToken, testError); + + // Test that the result for the token is the expected error + var result = Oauth._retrieveTransientResult(credentialToken); + test.instanceOf(result, Error); + test.equal(result.message, testError.message); + test.equal(result.stack, testError.stack); +}); diff --git a/packages/oauth/package.js b/packages/oauth/package.js index 4c2fdd115d2..c97c370374e 100644 --- a/packages/oauth/package.js +++ b/packages/oauth/package.js @@ -6,6 +6,8 @@ Package.describe({ Package.on_use(function (api) { api.use('routepolicy', 'server'); api.use('webapp', 'server'); + api.use('mongo-livedata', 'server'); + api.use(['underscore', 'service-configuration', 'logging'], 'server'); api.export('Oauth'); @@ -13,4 +15,14 @@ Package.on_use(function (api) { api.add_files('oauth_client.js', 'client'); api.add_files('oauth_server.js', 'server'); + api.add_files('transient_results.js', 'server'); +}); + + +Package.on_test(function (api) { + api.use('tinytest'); + api.use('random'); + api.use('service-configuration', 'server'); + api.use('oauth', 'server'); + api.add_files("oauth_tests.js", 'server'); }); diff --git a/packages/oauth/transient_results.js b/packages/oauth/transient_results.js new file mode 100644 index 00000000000..466d07580a7 --- /dev/null +++ b/packages/oauth/transient_results.js @@ -0,0 +1,80 @@ +if (typeof Oauth === 'undefined') { + Oauth = {}; +} + +// +// When an oauth request is made, Meteor receives oauth credentials +// in one browser tab, and temporarily persists them while that +// tab is closed, then retrieves them in the browser tab that +// initiated the credential request. +// +// _transientResults is the storage mechanism used to share the +// result between the 2 tabs +// + +// XXX we should periodically clear old entries + + +// Collection containing transient result of oauth credential requests +// Has token, result, at createdAt fields. +Oauth._transientResults = new Meteor.Collection( + "meteor_oauth_transientResults", { + _preventAutopublish: true + }); + +Oauth._transientResults._ensureIndex('token', {unique: 1}); + + +// Stores the token and result in the _transientResults collection +// XXX After oauth token encryption is added to Meteor, apply it here too +// +// @param credentialToken {string} +// @param result {string} The result of the credential request +// +Oauth._storeTransientResult = function (credentialToken, result) { + if (result instanceof Error) + result = _storableError(result); + + Oauth._transientResults.insert({ + token: credentialToken, + result: result, + createdAt: new Date() + }); +}; + + +// Retrieves and removes a result from the _transientResults collection +// XXX After oauth token encryption is added to Meteor, apply it here too +// +// @param credentialToken {string} +// +Oauth._retrieveTransientResult = function (credentialToken) { + var transientResult = Oauth._transientResults.findOne({ token:credentialToken }); + if (transientResult) { + Oauth._transientResults.remove({ _id: transientResult._id }); + if (transientResult.result.error) + return _recreateError(transientResult.result.error); + else + return transientResult.result; + } else { + return undefined; + } +}; + + + +var _storableError = function(error) { + var plainObject = {}; + Object.getOwnPropertyNames(error).forEach(function(key) { + plainObject[key] = error[key]; + }); + return { error: plainObject }; +}; + +var _recreateError = function(errorDoc) { + var error = new Error(); + Object.getOwnPropertyNames(errorDoc).forEach(function(key) { + error[key] = errorDoc[key]; + }); + return error; +}; diff --git a/packages/oauth1/oauth1_server.js b/packages/oauth1/oauth1_server.js index 6a61fbd6c71..5a257316fac 100644 --- a/packages/oauth1/oauth1_server.js +++ b/packages/oauth1/oauth1_server.js @@ -22,7 +22,7 @@ Oauth._requestHandlers['1'] = function (service, query, res) { // Keep track of request token so we can verify it on the next step requestTokens[query.state] = { - requestToken: oauthBinding.requestToken, + requestToken: oauthBinding.requestToken, requestTokenSecret: oauthBinding.requestTokenSecret }; @@ -38,9 +38,8 @@ Oauth._requestHandlers['1'] = function (service, query, res) { res.writeHead(302, {'Location': redirectUrl}); res.end(); } 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 + // step 2, redirected from provider login - store the result + // and close the window to allow the login handler to proceed // Get the user's request token so we can verify it and clear it var requestToken = requestTokens[query.state].requestToken; @@ -60,12 +59,13 @@ Oauth._requestHandlers['1'] = function (service, query, res) { // Run service-specific handler. var oauthResult = service.handleOauthRequest(oauthBinding); - // Add the login result to the result map - Oauth._loginResultForCredentialToken[query.state] = { + // Store the login result so it can be retrieved in another + // browser tab by the result handler + Oauth._storeTransientResult(query.state, { serviceName: service.serviceName, serviceData: oauthResult.serviceData, options: oauthResult.options - }; + }); } // Either close the window, redirect, or render nothing diff --git a/packages/oauth1/oauth1_tests.js b/packages/oauth1/oauth1_tests.js index 272e0cb123e..1b24bb7b34e 100644 --- a/packages/oauth1/oauth1_tests.js +++ b/packages/oauth1/oauth1_tests.js @@ -1,4 +1,4 @@ -Tinytest.add("oauth1 - loginResultForCredentialToken is stored", function (test) { +Tinytest.add("oauth1 - transientResult is stored", function (test) { var http = Npm.require('http'); var twitterfooId = Random.id(); var twitterfooName = 'nickname' + Random.id(); @@ -54,19 +54,18 @@ Tinytest.add("oauth1 - loginResultForCredentialToken is stored", function (test) }; OauthTest.middleware(req, new http.ServerResponse(req)); - // Test that right data is placed on the loginResult map - test.equal( - Oauth._loginResultForCredentialToken[credentialToken].serviceName, serviceName); - test.equal( - Oauth._loginResultForCredentialToken[credentialToken].serviceData.id, twitterfooId); - test.equal( - Oauth._loginResultForCredentialToken[credentialToken].serviceData.screenName, twitterfooName); - test.equal( - Oauth._loginResultForCredentialToken[credentialToken].serviceData.accessToken, twitterfooAccessToken); - test.equal( - Oauth._loginResultForCredentialToken[credentialToken].serviceData.accessTokenSecret, twitterfooAccessTokenSecret); - test.equal( - Oauth._loginResultForCredentialToken[credentialToken].options.option1, twitterOption1); + // Test that the result for the token is available + var result = Oauth._retrieveTransientResult(credentialToken); + test.equal(result.serviceName, serviceName); + test.equal(result.serviceData.id, twitterfooId); + test.equal(result.serviceData.screenName, twitterfooName); + test.equal(result.serviceData.accessToken, twitterfooAccessToken); + test.equal(result.serviceData.accessTokenSecret, twitterfooAccessTokenSecret); + test.equal(result.options.option1, twitterOption1); + + // Test that login results are transient + result = Oauth._retrieveTransientResult(credentialToken); + test.isUndefined(result); } finally { OauthTest.unregisterService(serviceName); diff --git a/packages/oauth2/oauth2_server.js b/packages/oauth2/oauth2_server.js index f788727799a..30839ac5fc5 100644 --- a/packages/oauth2/oauth2_server.js +++ b/packages/oauth2/oauth2_server.js @@ -2,18 +2,19 @@ Oauth._requestHandlers['2'] = function (service, query, res) { // 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. + // Prepare the login results before returning. // Run service-specific handler. var oauthResult = service.handleOauthRequest(query); - // Add the login result to the result map - Oauth._loginResultForCredentialToken[query.state] = { - serviceName: service.serviceName, - serviceData: oauthResult.serviceData, - options: oauthResult.options - }; + // Store the login result so it can be retrieved in another + // browser tab by the result handler + Oauth._storeTransientResult(query.state, { + serviceName: service.serviceName, + serviceData: oauthResult.serviceData, + options: oauthResult.options + }); + } // Either close the window, redirect, or render nothing diff --git a/packages/oauth2/oauth2_tests.js b/packages/oauth2/oauth2_tests.js index 00cd88e7d35..d2018119483 100644 --- a/packages/oauth2/oauth2_tests.js +++ b/packages/oauth2/oauth2_tests.js @@ -1,4 +1,4 @@ -Tinytest.add("oauth2 - loginResultForCredentialToken is stored", function (test) { +Tinytest.add("oauth2 - transientResult is stored", function (test) { var http = Npm.require('http'); var foobookId = Random.id(); var foobookOption1 = Random.id(); @@ -22,13 +22,15 @@ Tinytest.add("oauth2 - loginResultForCredentialToken is stored", function (test) query: {state: credentialToken}}; OauthTest.middleware(req, new http.ServerResponse(req)); - // Test that the login result for that user is prepared - test.equal( - Oauth._loginResultForCredentialToken[credentialToken].serviceName, serviceName); - test.equal( - Oauth._loginResultForCredentialToken[credentialToken].serviceData.id, foobookId); - test.equal( - Oauth._loginResultForCredentialToken[credentialToken].options.option1, foobookOption1); + // Test that the result for the token is available + var result = Oauth._retrieveTransientResult(credentialToken); + test.equal(result.serviceName, serviceName); + test.equal(result.serviceData.id, foobookId); + test.equal(result.options.option1, foobookOption1); + + // Test that login results are transient + result = Oauth._retrieveTransientResult(credentialToken); + test.isUndefined(result); } finally { OauthTest.unregisterService(serviceName); From 4786cc0d195cc24cc44459164724451b0b5db3cb Mon Sep 17 00:00:00 2001 From: Tim Haines Date: Wed, 12 Mar 2014 13:26:45 -0700 Subject: [PATCH 002/219] Remove orphaned transient oauth results from mongo once a minute. --- packages/oauth/transient_results.js | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/oauth/transient_results.js b/packages/oauth/transient_results.js index 466d07580a7..54fee1dbd22 100644 --- a/packages/oauth/transient_results.js +++ b/packages/oauth/transient_results.js @@ -12,8 +12,6 @@ if (typeof Oauth === 'undefined') { // result between the 2 tabs // -// XXX we should periodically clear old entries - // Collection containing transient result of oauth credential requests // Has token, result, at createdAt fields. @@ -25,6 +23,17 @@ Oauth._transientResults = new Meteor.Collection( Oauth._transientResults._ensureIndex('token', {unique: 1}); + +// Periodically clear old entries that were never retrieved +var _cleanStaleResults = function() { + // Remove results older than 1 minute + var timeCutoff = new Date(); + timeCutoff.setMinutes(timeCutoff.getMinutes() - 1); + Oauth._transientResults.remove({createdAt:{$lt:timeCutoff}}); +}; +var _cleanupHandle = Meteor.setInterval(_cleanStaleResults, 60 * 1000); + + // Stores the token and result in the _transientResults collection // XXX After oauth token encryption is added to Meteor, apply it here too // @@ -62,7 +71,7 @@ Oauth._retrieveTransientResult = function (credentialToken) { }; - +// Convert an Error into an object that can be stored in mongo var _storableError = function(error) { var plainObject = {}; Object.getOwnPropertyNames(error).forEach(function(key) { @@ -71,6 +80,7 @@ var _storableError = function(error) { return { error: plainObject }; }; +// Create an error from the error format stored in mongo var _recreateError = function(errorDoc) { var error = new Error(); Object.getOwnPropertyNames(errorDoc).forEach(function(key) { From 7839c26cef0d95b792e679bb39d53b2710defeef Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Mon, 17 Mar 2014 18:06:47 -0700 Subject: [PATCH 003/219] Remove all other tokens when a connection calls `changePassword` --- packages/accounts-password/password_server.js | 14 ++++- packages/accounts-password/password_tests.js | 59 +++++++++++++++++++ 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/packages/accounts-password/password_server.js b/packages/accounts-password/password_server.js index c011f81d276..a65305733eb 100644 --- a/packages/accounts-password/password_server.js +++ b/packages/accounts-password/password_server.js @@ -198,10 +198,18 @@ Meteor.methods({changePassword: function (options) { if (!verifier) throw new Meteor.Error(400, "Invalid verifier"); - // XXX this should invalidate all login tokens other than the current one - // (or it should assign a new login token, replacing existing ones) + // It would be better if this removed ALL existing tokens and replaced + // the token for the current connection with a new one, but that would + // be tricky, so we'll settle for just replacing all tokens other than + // the one for the current connection. + var currentToken = Accounts._getLoginToken(this.connection.id); Meteor.users.update({_id: this.userId}, - {$set: {'services.password.srp': verifier}}); + { + $set: { 'services.password.srp': verifier }, + $pull: { + 'services.resume.loginTokens': { hashedToken: { $ne: currentToken } } + } + }); var ret = {passwordChanged: true}; if (serialized) diff --git a/packages/accounts-password/password_tests.js b/packages/accounts-password/password_tests.js index 0d7e7139fc1..bf772c5a54a 100644 --- a/packages/accounts-password/password_tests.js +++ b/packages/accounts-password/password_tests.js @@ -191,6 +191,65 @@ if (Meteor.isClient) (function () { logoutStep ]); + testAsyncMulti("passwords - changing password logs out other clients", [ + function (test, expect) { + this.username = Random.id(); + this.email = Random.id() + '-intercept@example.com'; + this.password = 'password'; + this.password2 = 'password2'; + Accounts.createUser( + { username: this.username, email: this.email, password: this.password }, + loggedInAs(this.username, test, expect)); + }, + // Log in a second connection as this user. + function (test, expect) { + var self = this; + + // copied from livedata/client_convenience.js + var ddpUrl = '/'; + if (typeof __meteor_runtime_config__ !== "undefined") { + if (__meteor_runtime_config__.DDP_DEFAULT_CONNECTION_URL) + ddpUrl = __meteor_runtime_config__.DDP_DEFAULT_CONNECTION_URL; + } + // XXX can we get the url from the existing connection somehow + // instead? + + self.secondConn = DDP.connect(ddpUrl); + self.secondConn.call('login', + { user: { username: self.username }, password: self.password }, + expect(function (err, result) { + test.isFalse(err); + self.secondConn.setUserId(result.id); + test.isTrue(self.secondConn.userId()); + + self.secondConn.onReconnect = function () { + self.secondConn.apply( + 'login', + [{ resume: result.token }], + { wait: true }, + function (err, result) { + self.secondConn.setUserId(result && result.id || null); + } + ); + }; + })); + }, + function (test, expect) { + var self = this; + Accounts.changePassword(self.password, self.password2, expect(function (err) { + test.isFalse(err); + })); + }, + // Now that we've changed the password, wait until the second + // connection gets logged out. + function (test, expect) { + var self = this; + pollUntil(expect, function () { + return self.secondConn.userId() === null; + }, 10 * 1000, 100); + } + ]); + testAsyncMulti("passwords - new user hooks", [ function (test, expect) { From 67d42747edef751525f3aaa34b347e598a016e74 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Tue, 18 Mar 2014 09:47:07 -0700 Subject: [PATCH 004/219] Whitespace --- packages/accounts-password/password_server.js | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/packages/accounts-password/password_server.js b/packages/accounts-password/password_server.js index a65305733eb..8f4b8dd8629 100644 --- a/packages/accounts-password/password_server.js +++ b/packages/accounts-password/password_server.js @@ -203,13 +203,15 @@ Meteor.methods({changePassword: function (options) { // be tricky, so we'll settle for just replacing all tokens other than // the one for the current connection. var currentToken = Accounts._getLoginToken(this.connection.id); - Meteor.users.update({_id: this.userId}, - { - $set: { 'services.password.srp': verifier }, - $pull: { - 'services.resume.loginTokens': { hashedToken: { $ne: currentToken } } - } - }); + Meteor.users.update( + { _id: this.userId }, + { + $set: { 'services.password.srp': verifier }, + $pull: { + 'services.resume.loginTokens': { hashedToken: { $ne: currentToken } } + } + } + ); var ret = {passwordChanged: true}; if (serialized) @@ -321,14 +323,14 @@ Accounts.sendEnrollmentEmail = function (userId, email) { }}); var enrollAccountUrl = Accounts.urls.enrollAccount(token); - + var options = { to: email, from: Accounts.emailTemplates.from, subject: Accounts.emailTemplates.enrollAccount.subject(user), text: Accounts.emailTemplates.enrollAccount.text(user, enrollAccountUrl) }; - + if (typeof Accounts.emailTemplates.enrollAccount.html === 'function') options.html = Accounts.emailTemplates.enrollAccount.html(user, enrollAccountUrl); @@ -441,14 +443,14 @@ Accounts.sendVerificationEmail = function (userId, address) { {$push: {'services.email.verificationTokens': tokenRecord}}); var verifyEmailUrl = Accounts.urls.verifyEmail(tokenRecord.token); - + var options = { to: address, from: Accounts.emailTemplates.from, subject: Accounts.emailTemplates.verifyEmail.subject(user), text: Accounts.emailTemplates.verifyEmail.text(user, verifyEmailUrl) }; - + if (typeof Accounts.emailTemplates.verifyEmail.html === 'function') options.html = Accounts.emailTemplates.verifyEmail.html(user, verifyEmailUrl); From 7ea715173c226bf6f9089ccb41dff9f608ef5efa Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Tue, 18 Mar 2014 09:47:25 -0700 Subject: [PATCH 005/219] Use Meteor.absoluteUrl() in tests instead of digging out DDP url --- packages/accounts-password/password_tests.js | 22 ++------------------ 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/packages/accounts-password/password_tests.js b/packages/accounts-password/password_tests.js index bf772c5a54a..2daaac48673 100644 --- a/packages/accounts-password/password_tests.js +++ b/packages/accounts-password/password_tests.js @@ -205,16 +205,7 @@ if (Meteor.isClient) (function () { function (test, expect) { var self = this; - // copied from livedata/client_convenience.js - var ddpUrl = '/'; - if (typeof __meteor_runtime_config__ !== "undefined") { - if (__meteor_runtime_config__.DDP_DEFAULT_CONNECTION_URL) - ddpUrl = __meteor_runtime_config__.DDP_DEFAULT_CONNECTION_URL; - } - // XXX can we get the url from the existing connection somehow - // instead? - - self.secondConn = DDP.connect(ddpUrl); + self.secondConn = DDP.connect(Meteor.absoluteUrl()); self.secondConn.call('login', { user: { username: self.username }, password: self.password }, expect(function (err, result) { @@ -480,20 +471,11 @@ if (Meteor.isClient) (function () { function (test, expect) { var self = this; - // copied from livedata/client_convenience.js - self.ddpUrl = '/'; - if (typeof __meteor_runtime_config__ !== "undefined") { - if (__meteor_runtime_config__.DDP_DEFAULT_CONNECTION_URL) - self.ddpUrl = __meteor_runtime_config__.DDP_DEFAULT_CONNECTION_URL; - } - // XXX can we get the url from the existing connection somehow - // instead? - // Test that Meteor.logoutOtherClients logs out a second authenticated // connection while leaving Accounts.connection logged in. var token; var userId; - self.secondConn = DDP.connect(self.ddpUrl); + self.secondConn = DDP.connect(Meteor.absoluteUrl()); var expectLoginError = expect(function (err) { test.isTrue(err); From 1fade7fe6245f253ce5ddc5c285dc188feca914d Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Tue, 18 Mar 2014 09:51:26 -0700 Subject: [PATCH 006/219] Add History entry for removing tokens on password change --- History.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/History.md b/History.md index 457e2cdfafe..2174d000d7f 100644 --- a/History.md +++ b/History.md @@ -1,5 +1,9 @@ ## v.NEXT +* When a user changes their password, all login tokens except the one + being used to change the password now get deleted, which results in + other clients being logged out. + * Support oplog tailing on queries with the `limit` option. All queries except those containing `$near` or `$where` selectors or the `skip` option can now be used with the oplog driver. From 406d8d6b81f7b2ef723a872f2c8173e16ae383e0 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 18 Mar 2014 13:49:00 -0700 Subject: [PATCH 007/219] a few comments about login hooks --- packages/accounts-base/accounts_server.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/accounts-base/accounts_server.js b/packages/accounts-base/accounts_server.js index e421cc0e9ad..6f699126465 100644 --- a/packages/accounts-base/accounts_server.js +++ b/packages/accounts-base/accounts_server.js @@ -227,6 +227,9 @@ var attemptLogin = function (methodInvocation, methodName, methodArgs, result) { if (!result) throw new Error("result is required"); + // XXX A programming error in a login handler can lead to this occuring, and + // then we don't call onLogin or onLoginFailure callbacks. Should + // tryLoginMethod catch this case and turn it into an error? if (!result.userId && !result.error) throw new Error("A login method must specify a userId or an error"); @@ -245,6 +248,9 @@ var attemptLogin = function (methodInvocation, methodName, methodArgs, result) { if (user) attempt.user = user; + // validateLogin may mutate `attempt` by adding an error and changing allowed + // to false, but that's the only change it can make (and the user's callbacks + // only get a clone of `attempt`). validateLogin(methodInvocation.connection, attempt); if (attempt.allowed) { From 88310fcf9a0b1dea417845d9482916dde3264898 Mon Sep 17 00:00:00 2001 From: Nick Martin Date: Tue, 18 Mar 2014 15:18:41 -0700 Subject: [PATCH 008/219] Reword history. Make the comment more direct. --- History.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/History.md b/History.md index 8b6b059b32a..ac7f7959010 100644 --- a/History.md +++ b/History.md @@ -1,8 +1,6 @@ ## v.NEXT -* When a user changes their password, all login tokens except the one - being used to change the password now get deleted, which results in - other clients being logged out. +* Log out a users other sessions when they change their password. ## v0.7.2 From e42176101a429059cca5b688ad01ff463101f550 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Tue, 18 Mar 2014 15:25:17 -0700 Subject: [PATCH 009/219] Add an apostrophe to the History entry --- History.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/History.md b/History.md index ac7f7959010..a067d9aec40 100644 --- a/History.md +++ b/History.md @@ -1,6 +1,6 @@ ## v.NEXT -* Log out a users other sessions when they change their password. +* Log out a user's other sessions when they change their password. ## v0.7.2 From 138b42057ee1ec7cf5af74d0a2649d067d587fad Mon Sep 17 00:00:00 2001 From: Slava Kim Date: Wed, 19 Mar 2014 11:57:35 -0700 Subject: [PATCH 010/219] checks for Collection.find's options --- packages/mongo-livedata/collection.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/mongo-livedata/collection.js b/packages/mongo-livedata/collection.js index e05ca18a007..08b3b27a1f1 100644 --- a/packages/mongo-livedata/collection.js +++ b/packages/mongo-livedata/collection.js @@ -207,6 +207,13 @@ _.extend(Meteor.Collection.prototype, { if (args.length < 2) { return { transform: self._transform }; } else { + check(args[1], Match.ObjectIncluding({ + fields: Match.Optional(Object), + sort: Match.Optional(Match.OneOf(Object, Array)), + limit: Match.Optional(Number), + skip: Match.Optional(Number) + })); + return _.extend({ transform: self._transform }, args[1]); From 3f3608170bbbac25836eda89d3f2e53ecd6f0da2 Mon Sep 17 00:00:00 2001 From: David Gonzalez Date: Wed, 12 Mar 2014 11:17:26 -0700 Subject: [PATCH 011/219] Updated Stylus to use v0.42.3 --- packages/stylus/.npm/plugin/compileStylus/npm-shrinkwrap.json | 4 ++-- packages/stylus/package.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/stylus/.npm/plugin/compileStylus/npm-shrinkwrap.json b/packages/stylus/.npm/plugin/compileStylus/npm-shrinkwrap.json index d55ecb4c427..0215d0284c6 100644 --- a/packages/stylus/.npm/plugin/compileStylus/npm-shrinkwrap.json +++ b/packages/stylus/.npm/plugin/compileStylus/npm-shrinkwrap.json @@ -1,7 +1,7 @@ { "dependencies": { "stylus": { - "version": "0.42.2", + "version": "0.42.3", "dependencies": { "css-parse": { "version": "1.7.0" @@ -16,7 +16,7 @@ "version": "0.5.8" }, "glob": { - "version": "3.2.8", + "version": "3.2.9", "dependencies": { "minimatch": { "version": "0.2.14", diff --git a/packages/stylus/package.js b/packages/stylus/package.js index 85f04f65526..c9b5bd830dc 100644 --- a/packages/stylus/package.js +++ b/packages/stylus/package.js @@ -8,7 +8,7 @@ Package._transitional_registerBuildPlugin({ sources: [ 'plugin/compile-stylus.js' ], - npmDependencies: { stylus: "0.42.2", nib: "1.0.2" } + npmDependencies: { stylus: "0.42.3", nib: "1.0.2" } }); Package.on_test(function (api) { From 30c23ec8b6a4120a8bf4847f2af51e7b3ffc25b4 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Wed, 19 Mar 2014 17:05:10 -0700 Subject: [PATCH 012/219] Move OplogObserveDrive to MongoInternals Nothing was using it on MongoTest. @cmather would appreciate access to it. We don't guarantee anything about the MongoInternals interface, but why not. Fixes #1918. --- packages/mongo-livedata/oplog_observe_driver.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mongo-livedata/oplog_observe_driver.js b/packages/mongo-livedata/oplog_observe_driver.js index 425d7f8a108..c153c96a51f 100644 --- a/packages/mongo-livedata/oplog_observe_driver.js +++ b/packages/mongo-livedata/oplog_observe_driver.js @@ -846,4 +846,4 @@ var modifierCanBeDirectlyApplied = function (modifier) { }); }; -MongoTest.OplogObserveDriver = OplogObserveDriver; +MongoInternals.OplogObserveDriver = OplogObserveDriver; From 9961cef505ea8d0c1a1d44cd861ff72ef370e286 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Wed, 19 Mar 2014 17:09:33 -0700 Subject: [PATCH 013/219] Fix C.find(selector, undefined) Fixes test failure. --- packages/mongo-livedata/collection.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/mongo-livedata/collection.js b/packages/mongo-livedata/collection.js index 08b3b27a1f1..4eac2acd322 100644 --- a/packages/mongo-livedata/collection.js +++ b/packages/mongo-livedata/collection.js @@ -207,12 +207,12 @@ _.extend(Meteor.Collection.prototype, { if (args.length < 2) { return { transform: self._transform }; } else { - check(args[1], Match.ObjectIncluding({ + check(args[1], Match.Optional(Match.ObjectIncluding({ fields: Match.Optional(Object), sort: Match.Optional(Match.OneOf(Object, Array)), limit: Match.Optional(Number), skip: Match.Optional(Number) - })); + }))); return _.extend({ transform: self._transform From 63974c9be02eb707dcb2b46436bdd5066f994516 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Wed, 19 Mar 2014 17:25:22 -0700 Subject: [PATCH 014/219] Propagate Accounts.LoginCancelledError message Also revert an 0.7.2 change to that message's text Fixes #1920. In general we want to overhaul how error handling works to make this stuff simpler. https://trello.com/c/kMkw800Z/53-official-ddp-specification --- packages/accounts-oauth/oauth_client.js | 2 +- packages/accounts-oauth/oauth_server.js | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/accounts-oauth/oauth_client.js b/packages/accounts-oauth/oauth_client.js index f3d161e8c04..394ae00fbb6 100644 --- a/packages/accounts-oauth/oauth_client.js +++ b/packages/accounts-oauth/oauth_client.js @@ -9,7 +9,7 @@ Accounts.oauth.tryLoginAfterPopupClosed = function(credentialToken, callback) { // up with a more generic way to do this! if (err && err instanceof Meteor.Error && err.error === Accounts.LoginCancelledError.numericError) { - callback(new Accounts.LoginCancelledError(err.details)); + callback(new Accounts.LoginCancelledError(err.reason)); } else { callback(err); } diff --git a/packages/accounts-oauth/oauth_server.js b/packages/accounts-oauth/oauth_server.js index 39884ae843a..7854f025162 100644 --- a/packages/accounts-oauth/oauth_server.js +++ b/packages/accounts-oauth/oauth_server.js @@ -23,7 +23,8 @@ Accounts.registerLoginHandler(function (options) { // XXX we want `type` to be the service name such as "facebook" return { type: "oauth", error: new Meteor.Error( - Accounts.LoginCancelledError.numericError, "Login canceled") }; + Accounts.LoginCancelledError.numericError, + "No matching login attempt found") }; } var result = Oauth.retrieveCredential(options.oauth.credentialToken); if (result instanceof Error) From fc3cbd668930763c9ce1dea538f46134c9f622a7 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Thu, 20 Mar 2014 07:20:00 -0700 Subject: [PATCH 015/219] Remove extra 'login' help text. Looks like a mistake in a merge. Fixes #1934. --- tools/help.txt | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/tools/help.txt b/tools/help.txt index 43316989ed4..3c8131613f2 100644 --- a/tools/help.txt +++ b/tools/help.txt @@ -308,29 +308,6 @@ This command is for temporary, internal use, until we have a more mature system for building standalone command-line programs with Meteor. ->>> login -Log in to your Meteor account -Usage: meteor login [--email] [--galaxy ] - -Prompts for your username and password and logs you in to your -Meteor account. Pass --email to log in by email address instead of -username. Pass --galaxy to specify a galaxy to log in to. - -Builds the provided directory as a package, then loads the package and -calls the main() function inside the package. The function will receive -any remaining arguments. The exit status will be the return value of -main() (which is called inside a fiber). - -The arguments will be parsed by Meteor's option parser, which means that ---release will be effective (but not passed to the command), and that it will be -an error to pass any unknown options. If you want to pass options to your tool, -place them after a '--' argument (which turns off option parsing for the rest of -the arguments). - -This command is for temporary, internal use, until we have a more mature -system for building standalone command-line programs with Meteor. - - >>> self-test Run tests of the 'meteor' tool. Usage: meteor self-test [pattern] [--changed] [--slow] From 64f0c73b6b8caa01b4b2b70afb5126a0e3ac9823 Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Sat, 8 Mar 2014 09:07:49 -0800 Subject: [PATCH 016/219] Refactor Random.create() to be consistent with the global Random creation --- packages/random/random.js | 24 +++++++++++++++++------- packages/random/random_tests.js | 13 +++++++++++++ 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/packages/random/random.js b/packages/random/random.js index e0bc11d5a61..f01a1a7064a 100644 --- a/packages/random/random.js +++ b/packages/random/random.js @@ -109,6 +109,8 @@ RandomGenerator.prototype.fraction = function () { var array = new Uint32Array(1); window.crypto.getRandomValues(array); return array[0] * 2.3283064365386963e-10; // 2^-32 + } else { + throw new Error('No random generator available'); } }; @@ -180,13 +182,21 @@ var width = (typeof window !== 'undefined' && window.innerWidth) || var agent = (typeof navigator !== 'undefined' && navigator.userAgent) || ""; -if (nodeCrypto || - (typeof window !== "undefined" && - window.crypto && window.crypto.getRandomValues)) - Random = new RandomGenerator(); -else - Random = new RandomGenerator([new Date(), height, width, agent, Math.random()]); +function createRandomGenerator() { + if (nodeCrypto || + (typeof window !== "undefined" && + window.crypto && window.crypto.getRandomValues)) + return new RandomGenerator(); + else + return new RandomGenerator([new Date(), height, width, agent, Math.random()]); +}; + +Random = createRandomGenerator(); Random.create = function () { - return new RandomGenerator(arguments); + if (arguments.length == 0) { + return createRandomGenerator(); + } else { + return new RandomGenerator(arguments); + } }; diff --git a/packages/random/random_tests.js b/packages/random/random_tests.js index 940afbb6402..bf3f8c277cb 100644 --- a/packages/random/random_tests.js +++ b/packages/random/random_tests.js @@ -27,3 +27,16 @@ Tinytest.add('random - format', function (test) { test.isTrue(frac < 1.0); test.isTrue(frac >= 0.0); }); + +Tinytest.add('random - Alea is last resort', function (test) { + if (Meteor.isServer) { + test.isTrue(Random.alea === undefined); + test.isTrue(Random.create().alea === undefined); + } + if (Meteor.isClient) { + var useGetRandomValues = !!(typeof window !== "undefined" && + window.crypto && window.crypto.getRandomValues); + test.equal(Random.alea === undefined, useGetRandomValues); + test.equal(Random.create().alea === undefined, useGetRandomValues); + } +}); From a6e9ae392b49bab93236fad9d6072da99dbdc63b Mon Sep 17 00:00:00 2001 From: Justin SB Date: Fri, 14 Mar 2014 09:14:44 -0400 Subject: [PATCH 017/219] Rename create -> createWithSeeds, require parameters --- packages/random/random.js | 6 +++--- packages/random/random_tests.js | 10 +++++++--- packages/test-helpers/seeded_random.js | 2 +- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/random/random.js b/packages/random/random.js index f01a1a7064a..5e20f8f7216 100644 --- a/packages/random/random.js +++ b/packages/random/random.js @@ -193,9 +193,9 @@ function createRandomGenerator() { Random = createRandomGenerator(); -Random.create = function () { - if (arguments.length == 0) { - return createRandomGenerator(); +Random.createWithSeeds = function () { + if (arguments.length === 0) { + throw new Error('No seeds were provided'); } else { return new RandomGenerator(arguments); } diff --git a/packages/random/random_tests.js b/packages/random/random_tests.js index bf3f8c277cb..436a87daefd 100644 --- a/packages/random/random_tests.js +++ b/packages/random/random_tests.js @@ -7,7 +7,7 @@ Tinytest.add('random', function (test) { // algorithm being used and it starts generating a different // sequence for a seed, as long as the sequence is consistent for // a particular release. - var random = Random.create(0); + var random = Random.createWithSeeds(0); test.equal(random.id(), "cp9hWvhg8GSvuZ9os"); test.equal(random.id(), "3f3k6Xo7rrHCifQhR"); test.equal(random.id(), "shxDnjWWmnKPEoLhM"); @@ -31,12 +31,16 @@ Tinytest.add('random - format', function (test) { Tinytest.add('random - Alea is last resort', function (test) { if (Meteor.isServer) { test.isTrue(Random.alea === undefined); - test.isTrue(Random.create().alea === undefined); } if (Meteor.isClient) { var useGetRandomValues = !!(typeof window !== "undefined" && window.crypto && window.crypto.getRandomValues); test.equal(Random.alea === undefined, useGetRandomValues); - test.equal(Random.create().alea === undefined, useGetRandomValues); } }); + +Tinytest.add('random - createWithSeeds requires parameters', function (test) { + test.throws(function () { + Random.createWithSeeds(); + }); +}); diff --git a/packages/test-helpers/seeded_random.js b/packages/test-helpers/seeded_random.js index a950ef59a5e..6f525687810 100644 --- a/packages/test-helpers/seeded_random.js +++ b/packages/test-helpers/seeded_random.js @@ -3,7 +3,7 @@ SeededRandom = function(seed) { // seed may be a string or any type return new SeededRandom(seed); seed = seed || "seed"; - this.gen = Random.create(seed).alea; // from random.js + this.gen = Random.createWithSeeds(seed).alea; // from random.js }; SeededRandom.prototype.next = function() { return this.gen(); From f2a2b2eb74ea7ccae6e9706c741d018c3d03825c Mon Sep 17 00:00:00 2001 From: Justin SB Date: Fri, 14 Mar 2014 09:19:54 -0400 Subject: [PATCH 018/219] Revert createRandomGenerator function creation; no longer needed --- packages/random/random.js | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/packages/random/random.js b/packages/random/random.js index 5e20f8f7216..0c1397df5b1 100644 --- a/packages/random/random.js +++ b/packages/random/random.js @@ -182,21 +182,16 @@ var width = (typeof window !== 'undefined' && window.innerWidth) || var agent = (typeof navigator !== 'undefined' && navigator.userAgent) || ""; -function createRandomGenerator() { - if (nodeCrypto || - (typeof window !== "undefined" && - window.crypto && window.crypto.getRandomValues)) - return new RandomGenerator(); - else - return new RandomGenerator([new Date(), height, width, agent, Math.random()]); -}; - -Random = createRandomGenerator(); +if (nodeCrypto || + (typeof window !== "undefined" && + window.crypto && window.crypto.getRandomValues)) + Random = new RandomGenerator(); +else + Random = new RandomGenerator([new Date(), height, width, agent, Math.random()]); Random.createWithSeeds = function () { if (arguments.length === 0) { throw new Error('No seeds were provided'); - } else { - return new RandomGenerator(arguments); } + return new RandomGenerator(arguments); }; From 2e66a7f1a05d59e043a269bd20fd07aa5bfeab12 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Fri, 14 Feb 2014 16:15:35 -0800 Subject: [PATCH 019/219] Replace observe on all users with one observe per connection. --- packages/accounts-base/accounts_server.js | 136 +++++-------------- packages/accounts-password/password_tests.js | 8 +- 2 files changed, 40 insertions(+), 104 deletions(-) diff --git a/packages/accounts-base/accounts_server.js b/packages/accounts-base/accounts_server.js index 6f699126465..d219e03e1f7 100644 --- a/packages/accounts-base/accounts_server.js +++ b/packages/accounts-base/accounts_server.js @@ -557,26 +557,23 @@ Accounts._clearAllLoginTokens = function (userId) { ); }; - -// hashed token -> list of connection ids -var connectionsByLoginToken = {}; +// connection id -> observe handle for the login token that this +// connection is currently associated with +var userObservesForConnections = {}; // test hook -Accounts._getTokenConnections = function (token) { - return connectionsByLoginToken[token]; +Accounts._getUserObserve = function (connectionId) { + return userObservesForConnections[connectionId]; }; -// Remove the connection from the list of open connections for the connection's -// token. +// Clean up this connection's association with the token: that is, stop +// the observe that we started when we associated the connection with +// this token. var removeConnectionFromToken = function (connectionId) { - var token = Accounts._getLoginToken(connectionId); - if (token) { - connectionsByLoginToken[token] = _.without( - connectionsByLoginToken[token], - connectionId - ); - if (_.isEmpty(connectionsByLoginToken[token])) - delete connectionsByLoginToken[token]; + var observe = userObservesForConnections[connectionId]; + if (observe) { + observe.stop(); + delete userObservesForConnections[connectionId]; } }; @@ -590,33 +587,33 @@ Accounts._setLoginToken = function (userId, connection, newToken) { Accounts._setAccountData(connection.id, 'loginToken', newToken); if (newToken) { - if (! _.has(connectionsByLoginToken, newToken)) - connectionsByLoginToken[newToken] = []; - connectionsByLoginToken[newToken].push(connection.id); - - // Now that we've added the connection to the - // connectionsByLoginToken map for the token, the connection will - // be closed if the token is removed from the database. However - // at this point the token might have already been deleted, which - // wouldn't have closed the connection because it wasn't in the - // map yet. - // - // We also did need to first add the connection to the map above - // (and now remove it here if the token was deleted), because we - // could be getting a response from the database that the token - // still exists, but then it could be deleted in another fiber - // before our `findOne` call returns... and then that other fiber - // would need for the connection to be in the map for it to close - // the connection. - // - // We defer this check because there's no need for it to be on the critical - // path for login; we just need to ensure that the connection will get - // closed at some point if the token has been deleted. + // Set up an observe for this token. If the token goes away, we need + // to close the connection. We defer this observe because there's + // no need for it to be on the critical path for login; we just need + // to ensure that the connection will get closed at some point if + // the token gets deleted. Meteor.defer(function () { - if (! Meteor.users.findOne({ + var foundMatchingUser; + userObservesForConnections[connection.id] = Meteor.users.find({ _id: userId, - "services.resume.loginTokens.hashedToken": newToken - })) { + 'services.resume.loginTokens.hashedToken': newToken + }, { fields: { 'services.resume': 1 } }).observe({ + added: function () { + foundMatchingUser = true; + }, + removed: function () { + connection.close(); + // The onClose callback for the connection takes care of + // cleaning up the observe handle and any other state we have + // lying around. + } + }); + if (! foundMatchingUser) { + // We've set up an observe on the user associated with `newToken`, + // so if the new token is removed from the database, we'll close + // the connection. But the token might have already been deleted + // before we set up the observe, which wouldn't have closed the + // connection because the observe wasn't running yet. removeConnectionFromToken(connection.id); connection.close(); } @@ -624,24 +621,6 @@ Accounts._setLoginToken = function (userId, connection, newToken) { } }; -// Close all open connections associated with any of the tokens in -// `tokens`. -var closeConnectionsForTokens = function (tokens) { - _.each(tokens, function (token) { - if (_.has(connectionsByLoginToken, token)) { - // safety belt. close should defer potentially yielding callbacks. - Meteor._noYieldsAllowed(function () { - _.each(connectionsByLoginToken[token], function (connectionId) { - var connection = Accounts._getAccountData(connectionId, 'connection'); - if (connection) - connection.close(); - }); - }); - } - }); -}; - - // Login handler for resume tokens. Accounts.registerLoginHandler("resume", function(options) { if (!options.resume) @@ -1186,44 +1165,3 @@ Meteor.startup(function () { deleteSavedTokens(user._id, user.services.resume.loginTokensToDelete); }); }); - -/// -/// LOGGING OUT DELETED USERS -/// - -// When login tokens are removed from the database, close any sessions -// logged in with those tokens. -// -// Because we upgrade unhashed login tokens to hashed tokens at login -// time, sessions will only be logged in with a hashed token. Thus we -// only need to pull out hashed tokens here. -var closeTokensForUser = function (userTokens) { - closeConnectionsForTokens(_.compact(_.pluck(userTokens, "hashedToken"))); -}; - -// Like _.difference, but uses EJSON.equals to compute which values to return. -var differenceObj = function (array1, array2) { - return _.filter(array1, function (array1Value) { - return ! _.some(array2, function (array2Value) { - return EJSON.equals(array1Value, array2Value); - }); - }); -}; - -Meteor.users.find({}, { fields: { "services.resume": 1 }}).observe({ - changed: function (newUser, oldUser) { - var removedTokens = []; - if (newUser.services && newUser.services.resume && - oldUser.services && oldUser.services.resume) { - removedTokens = differenceObj(oldUser.services.resume.loginTokens || [], - newUser.services.resume.loginTokens || []); - } else if (oldUser.services && oldUser.services.resume) { - removedTokens = oldUser.services.resume.loginTokens || []; - } - closeTokensForUser(removedTokens); - }, - removed: function (oldUser) { - if (oldUser.services && oldUser.services.resume) - closeTokensForUser(oldUser.services.resume.loginTokens || []); - } -}); diff --git a/packages/accounts-password/password_tests.js b/packages/accounts-password/password_tests.js index 2daaac48673..2cd12672dff 100644 --- a/packages/accounts-password/password_tests.js +++ b/packages/accounts-password/password_tests.js @@ -766,7 +766,7 @@ if (Meteor.isServer) (function () { // XXX would be nice to test Accounts.config({forbidClientAccountCreation: true}) Tinytest.addAsync( - 'passwords - login tokens cleaned up', + 'passwords - login token observes get cleaned up', function (test, onComplete) { var username = Random.id(); Accounts.createUser({ @@ -778,8 +778,7 @@ if (Meteor.isServer) (function () { test, function (clientConn, serverConn) { serverConn.onClose(function () { - test.isFalse(_.contains( - Accounts._getTokenConnections(token), serverConn.id)); + test.isFalse(Accounts._getUserObserve(serverConn.id)); onComplete(); }); var result = clientConn.call('login', { @@ -789,8 +788,7 @@ if (Meteor.isServer) (function () { test.isTrue(result); var token = Accounts._getAccountData(serverConn.id, 'loginToken'); test.isTrue(token); - test.isTrue(_.contains( - Accounts._getTokenConnections(token), serverConn.id)); + test.isTrue(Accounts._getUserObserve(serverConn.id)); clientConn.disconnect(); }, onComplete From b79294a690c91b1ce3f97695b8a0bff3845ae1a9 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Tue, 18 Mar 2014 10:07:09 -0700 Subject: [PATCH 020/219] glasser review comments --- packages/accounts-base/accounts_server.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/accounts-base/accounts_server.js b/packages/accounts-base/accounts_server.js index d219e03e1f7..9d7a18e1aaf 100644 --- a/packages/accounts-base/accounts_server.js +++ b/packages/accounts-base/accounts_server.js @@ -498,7 +498,7 @@ Accounts._setAccountData = function (connectionId, field, value) { Meteor.server.onConnection(function (connection) { accountData[connection.id] = {connection: connection}; connection.onClose(function () { - removeConnectionFromToken(connection.id); + removeTokenFromConnection(connection.id); delete accountData[connection.id]; }); }); @@ -569,11 +569,11 @@ Accounts._getUserObserve = function (connectionId) { // Clean up this connection's association with the token: that is, stop // the observe that we started when we associated the connection with // this token. -var removeConnectionFromToken = function (connectionId) { +var removeTokenFromConnection = function (connectionId) { var observe = userObservesForConnections[connectionId]; if (observe) { - observe.stop(); delete userObservesForConnections[connectionId]; + observe.stop(); } }; @@ -583,7 +583,7 @@ Accounts._getLoginToken = function (connectionId) { // newToken is a hashed token. Accounts._setLoginToken = function (userId, connection, newToken) { - removeConnectionFromToken(connection.id); + removeTokenFromConnection(connection.id); Accounts._setAccountData(connection.id, 'loginToken', newToken); if (newToken) { @@ -594,10 +594,13 @@ Accounts._setLoginToken = function (userId, connection, newToken) { // the token gets deleted. Meteor.defer(function () { var foundMatchingUser; + // Because we upgrade unhashed login tokens to hashed tokens at + // login time, sessions will only be logged in with a hashed + // token. Thus we only need to observe hashed tokens here. userObservesForConnections[connection.id] = Meteor.users.find({ _id: userId, 'services.resume.loginTokens.hashedToken': newToken - }, { fields: { 'services.resume': 1 } }).observe({ + }, { fields: { _id: 1 } }).observeChanges({ added: function () { foundMatchingUser = true; }, @@ -614,7 +617,7 @@ Accounts._setLoginToken = function (userId, connection, newToken) { // the connection. But the token might have already been deleted // before we set up the observe, which wouldn't have closed the // connection because the observe wasn't running yet. - removeConnectionFromToken(connection.id); + removeTokenFromConnection(connection.id); connection.close(); } }); From b3e342ec7ef6d248dd5a00fca086d7d94cb54cc0 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 19 Mar 2014 21:31:11 -0700 Subject: [PATCH 021/219] Remove redundant call to 'removeTokenFromConnection' --- packages/accounts-base/accounts_server.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/accounts-base/accounts_server.js b/packages/accounts-base/accounts_server.js index 9d7a18e1aaf..51e85fa423a 100644 --- a/packages/accounts-base/accounts_server.js +++ b/packages/accounts-base/accounts_server.js @@ -617,7 +617,6 @@ Accounts._setLoginToken = function (userId, connection, newToken) { // the connection. But the token might have already been deleted // before we set up the observe, which wouldn't have closed the // connection because the observe wasn't running yet. - removeTokenFromConnection(connection.id); connection.close(); } }); From 051faf8895c15bbd6bb192ae3f3f200306e67e91 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 19 Mar 2014 22:01:48 -0700 Subject: [PATCH 022/219] Poll for the observe to appear in a test. --- packages/accounts-password/password_tests.js | 21 ++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/accounts-password/password_tests.js b/packages/accounts-password/password_tests.js index 2cd12672dff..614862502c7 100644 --- a/packages/accounts-password/password_tests.js +++ b/packages/accounts-password/password_tests.js @@ -788,8 +788,25 @@ if (Meteor.isServer) (function () { test.isTrue(result); var token = Accounts._getAccountData(serverConn.id, 'loginToken'); test.isTrue(token); - test.isTrue(Accounts._getUserObserve(serverConn.id)); - clientConn.disconnect(); + + // We poll here, instead of just checking `_getUserObserve` + // once, because the login method defers the creation of the + // observe, and setting up the observe yields, so we could end + // up here before the observe has been set up. + simplePoll( + function () { + return !! Accounts._getUserObserve(serverConn.id); + }, + function () { + test.isTrue(Accounts._getUserObserve(serverConn.id)); + clientConn.disconnect(); + }, + function () { + test.fail("timed out waiting for user observe for connection " + + serverConn.id); + onComplete(); + } + ); }, onComplete ); From efd044a00429bd7891d6428b617c28a732468e53 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 19 Mar 2014 22:02:13 -0700 Subject: [PATCH 023/219] Clean up observes created for connections that were closed before the observe was set up. --- packages/accounts-base/accounts_server.js | 36 +++++++++++++++++++---- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/packages/accounts-base/accounts_server.js b/packages/accounts-base/accounts_server.js index 51e85fa423a..872300b0d41 100644 --- a/packages/accounts-base/accounts_server.js +++ b/packages/accounts-base/accounts_server.js @@ -558,9 +558,16 @@ Accounts._clearAllLoginTokens = function (userId) { }; // connection id -> observe handle for the login token that this -// connection is currently associated with +// connection is currently associated with, or null. Null indicates that +// we are in the process of setting up the observe. var userObservesForConnections = {}; +// connection id -> boolean. Keeps track of connections that were closed +// before we had a chance to set up the observe on the user associated +// with this connection. To avoid leaking observes, we'll look in here +// immediately after setting up an observe. +var connectionsClosedBeforeObserve = {}; + // test hook Accounts._getUserObserve = function (connectionId) { return userObservesForConnections[connectionId]; @@ -571,9 +578,17 @@ Accounts._getUserObserve = function (connectionId) { // this token. var removeTokenFromConnection = function (connectionId) { var observe = userObservesForConnections[connectionId]; - if (observe) { - delete userObservesForConnections[connectionId]; - observe.stop(); + if (observe !== undefined) { + if (observe === null) { + // We're in the process of setting up an observe for this + // connection. We can't clean up that observe yet, but we can make + // note of it in `connectionsClosedBeforeObserve`, so that the + // observe will get torn down immediately after being set up. + connectionsClosedBeforeObserve[connectionId] = true; + } else { + delete userObservesForConnections[connectionId]; + observe.stop(); + } } }; @@ -588,10 +603,15 @@ Accounts._setLoginToken = function (userId, connection, newToken) { if (newToken) { // Set up an observe for this token. If the token goes away, we need - // to close the connection. We defer this observe because there's + // to close the connection. We defer the observe because there's // no need for it to be on the critical path for login; we just need // to ensure that the connection will get closed at some point if // the token gets deleted. + // + // Initially, we set the observe for this connection to null; this + // signifies to other code (which might run while we yield) that we + // are in the process of setting up an observe for this connection. + userObservesForConnections[connection.id] = null; Meteor.defer(function () { var foundMatchingUser; // Because we upgrade unhashed login tokens to hashed tokens at @@ -611,6 +631,12 @@ Accounts._setLoginToken = function (userId, connection, newToken) { // lying around. } }); + if (connectionsClosedBeforeObserve[connection.id]) { + // Oops, this connection was closed while we were setting up the + // observe. Clean it up now. + delete connectionsClosedBeforeObserve[connection.id]; + removeTokenFromConnection(connection.id); + } if (! foundMatchingUser) { // We've set up an observe on the user associated with `newToken`, // so if the new token is removed from the database, we'll close From e999ac782e2ecdcbaddf1798468a9a38aecf2832 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Fri, 21 Mar 2014 09:14:41 -0700 Subject: [PATCH 024/219] Remove `connectionsClosedBeforeObserve`. Instead of inserting into `connectionsClosedBeforeObserve`, we can just delete the null placeholder from `userObservesForConnections`, and then check for a deleted placeholder before storing the observe once it's been set up. --- packages/accounts-base/accounts_server.js | 35 +++++++++++++---------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/packages/accounts-base/accounts_server.js b/packages/accounts-base/accounts_server.js index 872300b0d41..a75c70134b1 100644 --- a/packages/accounts-base/accounts_server.js +++ b/packages/accounts-base/accounts_server.js @@ -562,12 +562,6 @@ Accounts._clearAllLoginTokens = function (userId) { // we are in the process of setting up the observe. var userObservesForConnections = {}; -// connection id -> boolean. Keeps track of connections that were closed -// before we had a chance to set up the observe on the user associated -// with this connection. To avoid leaking observes, we'll look in here -// immediately after setting up an observe. -var connectionsClosedBeforeObserve = {}; - // test hook Accounts._getUserObserve = function (connectionId) { return userObservesForConnections[connectionId]; @@ -581,10 +575,10 @@ var removeTokenFromConnection = function (connectionId) { if (observe !== undefined) { if (observe === null) { // We're in the process of setting up an observe for this - // connection. We can't clean up that observe yet, but we can make - // note of it in `connectionsClosedBeforeObserve`, so that the - // observe will get torn down immediately after being set up. - connectionsClosedBeforeObserve[connectionId] = true; + // connection. We can't clean up that observe yet, but if we + // delete the null placeholder for this connection, then the + // observe will get cleaned up as soon as it has been set up. + delete userObservesForConnections[connectionId]; } else { delete userObservesForConnections[connectionId]; observe.stop(); @@ -610,14 +604,18 @@ Accounts._setLoginToken = function (userId, connection, newToken) { // // Initially, we set the observe for this connection to null; this // signifies to other code (which might run while we yield) that we - // are in the process of setting up an observe for this connection. + // are in the process of setting up an observe for this + // connection. Once the observe is ready to go, we replace null with + // the real observe handle (unless the placeholder has been deleted, + // signifying that the connection was closed already -- in this case + // we just clean up the observe that we started). userObservesForConnections[connection.id] = null; Meteor.defer(function () { var foundMatchingUser; // Because we upgrade unhashed login tokens to hashed tokens at // login time, sessions will only be logged in with a hashed // token. Thus we only need to observe hashed tokens here. - userObservesForConnections[connection.id] = Meteor.users.find({ + var observe = Meteor.users.find({ _id: userId, 'services.resume.loginTokens.hashedToken': newToken }, { fields: { _id: 1 } }).observeChanges({ @@ -631,12 +629,19 @@ Accounts._setLoginToken = function (userId, connection, newToken) { // lying around. } }); - if (connectionsClosedBeforeObserve[connection.id]) { + + if (_.has(userObservesForConnections, connection.id)) { + if (userObservesForConnections[connection.id] !== null) { + throw new Error("Non-null user observe for connection " + + connection.id + " while observe was being set up?"); + } + userObservesForConnections[connection.id] = observe; + } else { // Oops, this connection was closed while we were setting up the // observe. Clean it up now. - delete connectionsClosedBeforeObserve[connection.id]; - removeTokenFromConnection(connection.id); + observe.stop(); } + if (! foundMatchingUser) { // We've set up an observe on the user associated with `newToken`, // so if the new token is removed from the database, we'll close From e872bc72e3e236b75364e6de78cfc1f04396f5bd Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Mon, 24 Mar 2014 10:14:26 -0700 Subject: [PATCH 025/219] Add 'meteor list-sites' command --- tools/commands.js | 19 +++++++++++++++++++ tools/deploy.js | 32 ++++++++++++++++++++++++++++++-- tools/help.txt | 9 +++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/tools/commands.js b/tools/commands.js index 3a003dd6c5b..385baf210ea 100644 --- a/tools/commands.js +++ b/tools/commands.js @@ -1295,6 +1295,25 @@ main.registerCommand({ }); +/////////////////////////////////////////////////////////////////////////////// +// list-sites +/////////////////////////////////////////////////////////////////////////////// + +main.registerCommand({ + name: 'list-sites', + minArgs: 0, + maxArgs: 0 +}, function (options) { + auth.pollForRegistrationCompletion(); + if (! auth.isLoggedIn()) { + process.stderr.write( + "You must be logged in for that. Try 'meteor login'.\n"); + } + + return deploy.listSites(); +}); + + /////////////////////////////////////////////////////////////////////////////// // dummy /////////////////////////////////////////////////////////////////////////////// diff --git a/tools/deploy.js b/tools/deploy.js index 84e82db94d9..d5061b3b91f 100644 --- a/tools/deploy.js +++ b/tools/deploy.js @@ -33,7 +33,7 @@ var Future = require('fibers/future'); // // Options include: // - method: GET, POST, or DELETE. default GET -// - operation: "info", "logs", "mongo", "deploy" +// - operation: "info", "logs", "mongo", "deploy", "authorized-apps" // - site: site name // - expectPayload: an array of key names. if present, then we expect // the server to return JSON content on success and to return an @@ -67,7 +67,8 @@ var deployRpc = function (options) { try { var result = httpHelpers.request(_.extend(options, { - url: config.getDeployUrl() + '/' + options.operation + '/' + options.site, + url: config.getDeployUrl() + '/' + options.operation + + (options.site ? ('/' + options.site) : ''), method: options.method || 'GET', bodyStream: options.bodyStream, useAuthHeader: true, @@ -708,6 +709,32 @@ site + ": " + "successfully transferred to your account.\n" + return 0; }; +var listSites = function () { + var result = deployRpc({ + method: "GET", + operation: "authorized-apps", + promptIfAuthFails: true, + expectPayload: ["sites"] + }); + + if (result.errorMessage) { + process.stderr.write("Couldn't list sites: " + + result.errorMessage + "\n"); + return 1; + } + + if (! result.payload || + ! result.payload.sites || + ! result.payload.sites.length) { + process.stdout.write("You don't have any sites yet.\n"); + } else { + _.each(result.payload.sites, function (site) { + process.stdout.write(site + "\n"); + }); + } + return 0; +}; + exports.bundleAndDeploy = bundleAndDeploy; exports.deleteApp = deleteApp; @@ -716,3 +743,4 @@ exports.logs = logs; exports.listAuthorized = listAuthorized; exports.changeAuthorized = changeAuthorized; exports.claim = claim; +exports.listSites = listSites; diff --git a/tools/help.txt b/tools/help.txt index 3c8131613f2..41c97628776 100644 --- a/tools/help.txt +++ b/tools/help.txt @@ -364,3 +364,12 @@ Grant a permission on an official service Usage: meteor admin grant [XXX] Not yet implemented + + +>>> list-sites +List sites for which you are authorized. +Usage: meteor list-sites + +List the sites that you have deployed with 'meteor deploy', and sites +for which other users have authorized you with the 'meteor authorized' +command. From bdbd6a2484e161f99367d8b09b999f6d81b8b01d Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Mon, 24 Mar 2014 10:42:06 -0700 Subject: [PATCH 026/219] Return immediately if not logged in on 'meteor list-sites' --- tools/commands.js | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/commands.js b/tools/commands.js index 385baf210ea..5d8627b4280 100644 --- a/tools/commands.js +++ b/tools/commands.js @@ -1308,6 +1308,7 @@ main.registerCommand({ if (! auth.isLoggedIn()) { process.stderr.write( "You must be logged in for that. Try 'meteor login'.\n"); + return 1; } return deploy.listSites(); From 74e80436874d38325ca2041d6a1caa0c9aa7ed28 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Tue, 18 Mar 2014 09:36:00 -0700 Subject: [PATCH 027/219] Split logoutOtherClients into two methods. One method call gives you a new token, and another method call removes all but the current token. Meteor.logoutOtherClients calls both these methods, and calls the user's callback when the second has returned. --- packages/accounts-base/accounts_client.js | 66 +++++++++-------- packages/accounts-base/accounts_server.js | 56 +++++++++++++- packages/accounts-base/accounts_tests.js | 76 +++++++++++++++++++ packages/accounts-password/password_tests.js | 77 +++++++++++--------- 4 files changed, 206 insertions(+), 69 deletions(-) diff --git a/packages/accounts-base/accounts_client.js b/packages/accounts-base/accounts_client.js index 18c8ed82dbc..140cdf1069e 100644 --- a/packages/accounts-base/accounts_client.js +++ b/packages/accounts-base/accounts_client.js @@ -215,38 +215,40 @@ Meteor.logout = function (callback) { }; Meteor.logoutOtherClients = function (callback) { - // Call the `logoutOtherClients` method. Store the login token that we get - // back and use it to log in again. The server is not supposed to close - // connections on the old token for 10 seconds, so we should have time to - // store our new token and log in with it before being disconnected. If we get - // disconnected, then we'll immediately reconnect with the new token. If for - // some reason we get disconnected before storing the new token, then the - // worst that will happen is that we'll have a flicker from trying to log in - // with the old token before storing and logging in with the new one. - Accounts.connection.apply('logoutOtherClients', [], { wait: true }, - function (error, result) { - if (error) { - callback && callback(error); - } else { - var userId = Meteor.userId(); - storeLoginToken(userId, result.token, result.tokenExpires); - // If the server hasn't disconnected us yet by deleting our - // old token, then logging in now with the new valid token - // will prevent us from getting disconnected. If the server - // has already disconnected us due to our old invalid token, - // then we would have already tried and failed to login with - // the old token on reconnect, and we have to make sure a - // login method gets sent here with the new token. - Meteor.loginWithToken(result.token, function (err) { - if (err && - storedLoginToken() && - storedLoginToken().token === result.token) { - makeClientLoggedOut(); - } - callback && callback(err); - }); - } - }); + // We need to make two method calls: one to replace our current token, + // and another to remove all tokens except the current one. We want to + // call these two methods one after the other, without any other + // methods running between them. For example, we don't want `logout` + // to be called in between our two method calls (otherwise the second + // method call would return an error). Another example: we don't want + // logout to be called before the callback for `getNewToken`; + // otherwise we would momentarily log the user out and then write a + // new token to localStorage. + // + // To accomplish this, we make both calls as wait methods, and queue + // them one after the other, without spinning off the event loop in + // between. Even though we queue `removeOtherTokens` before + // `getNewToken`, we won't actually send the `removeOtherTokens` call + // until the `getNewToken` callback has finished running, because they + // are both wait methods. + Accounts.connection.apply( + 'getNewToken', + [], + { wait: true }, + function (err, result) { + if (! err) { + storeLoginToken(Meteor.userId(), result.token, result.tokenExpires); + } + } + ); + Accounts.connection.apply( + 'removeOtherTokens', + [], + { wait: true }, + function (err) { + callback && callback(err); + } + ); }; diff --git a/packages/accounts-base/accounts_server.js b/packages/accounts-base/accounts_server.js index a75c70134b1..6b419604f6b 100644 --- a/packages/accounts-base/accounts_server.js +++ b/packages/accounts-base/accounts_server.js @@ -424,6 +424,17 @@ Meteor.methods({ // use. Tests set Accounts._noConnectionCloseDelayForTest to delete tokens // immediately instead of using a delay. // + // XXX COMPAT WITH 0.7.2 + // This single `logoutOtherClients` method has been replaced with two + // methods, one that you call to get a new token, and another that you + // call to remove all tokens except your own. The new design allows + // clients to know when other clients have actually been logged + // out. (The `logoutOtherClients` method guarantees the caller that + // the other clients will be logged out at some point, but makes no + // guarantees about when.) This method is left in for backwards + // compatibility, especially since application code might be calling + // this method directly. + // // @returns {Object} Object with token and tokenExpires keys. logoutOtherClients: function () { var self = this; @@ -441,7 +452,7 @@ Meteor.methods({ var tokens = user.services.resume.loginTokens; var newToken = Accounts._generateStampedLoginToken(); var userId = self.userId; - Meteor.users.update(self.userId, { + Meteor.users.update(userId, { $set: { "services.resume.loginTokensToDelete": tokens, "services.resume.haveLoginTokensToDelete": true @@ -462,8 +473,49 @@ Meteor.methods({ tokenExpires: Accounts._tokenExpiration(newToken.when) }; } else { - throw new Error("You are not logged in."); + throw new Meteor.Error("You are not logged in."); } + }, + + getNewToken: function () { + var self = this; + var user = Meteor.users.findOne(self.userId, { + fields: { "services.resume.loginTokens": 1 } + }); + if (! self.userId || ! user) { + throw new Meteor.Error("You are not logged in."); + } + // Be careful not to generate a new token that has a later + // expiration than the curren token. Otherwise, a bad guy with a + // stolen token could use this method to stop his stolen token from + // ever expiring. + var currentHashedToken = Accounts._getLoginToken(self.connection.id); + var currentStampedToken = _.find( + user.services.resume.loginTokens, + function (stampedToken) { + return stampedToken.hashedToken === currentHashedToken; + } + ); + if (! currentStampedToken) { // safety belt: this should never happen + throw new Meteor.Error("Invalid login token"); + } + var newStampedToken = Accounts._generateStampedLoginToken(); + newStampedToken.when = currentStampedToken.when; + Accounts._insertLoginToken(self.userId, newStampedToken); + return loginUser(self, self.userId, newStampedToken); + }, + + removeOtherTokens: function () { + var self = this; + if (! self.userId) { + throw new Meteor.Error("You are not logged in."); + } + var currentToken = Accounts._getLoginToken(self.connection.id); + Meteor.users.update(self.userId, { + $pull: { + "services.resume.loginTokens": { hashedToken: { $ne: currentToken } } + } + }); } }); diff --git a/packages/accounts-base/accounts_tests.js b/packages/accounts-base/accounts_tests.js index e3eeaecd0c6..1e49fd31f2f 100644 --- a/packages/accounts-base/accounts_tests.js +++ b/packages/accounts-base/accounts_tests.js @@ -1,3 +1,9 @@ +Meteor.methods({ + getCurrentLoginToken: function () { + return Accounts._getLoginToken(this.connection.id); + } +}); + // XXX it'd be cool to also test that the right thing happens if options // *are* validated, but Accounts._options is global state which makes this hard // (impossible?) @@ -297,3 +303,73 @@ Tinytest.addAsync( ); } ); + +Tinytest.add( + 'accounts - get new token', + function (test) { + // Test that the `getNewToken` method returns us a valid token, with + // the same expiration as our original token. + var userId = Accounts.insertUserDoc({}, { username: Random.id() }); + var stampedToken = Accounts._generateStampedLoginToken(); + Accounts._insertLoginToken(userId, stampedToken); + var conn = DDP.connect(Meteor.absoluteUrl()); + conn.call('login', { resume: stampedToken.token }); + test.equal(conn.call('getCurrentLoginToken'), + Accounts._hashLoginToken(stampedToken.token)); + + var newTokenResult = conn.call('getNewToken'); + test.equal(newTokenResult.tokenExpires, + Accounts._tokenExpiration(stampedToken.when)); + test.equal(conn.call('getCurrentLoginToken'), + Accounts._hashLoginToken(newTokenResult.token)); + conn.disconnect(); + + // A second connection should be able to log in with the new token + // we got. + var secondConn = DDP.connect(Meteor.absoluteUrl()); + secondConn.call('login', { resume: newTokenResult.token }); + secondConn.disconnect(); + } +); + +Tinytest.addAsync( + 'accounts - remove other tokens', + function (test, onComplete) { + // Test that the `removeOtherTokens` method removes all tokens other + // than the caller's token, thereby logging out and closing other + // connections. + var userId = Accounts.insertUserDoc({}, { username: Random.id() }); + var stampedTokens = []; + var conns = []; + + _.times(2, function (i) { + stampedTokens.push(Accounts._generateStampedLoginToken()); + Accounts._insertLoginToken(userId, stampedTokens[i]); + var conn = DDP.connect(Meteor.absoluteUrl()); + conn.call('login', { resume: stampedTokens[i].token }); + test.equal(conn.call('getCurrentLoginToken'), + Accounts._hashLoginToken(stampedTokens[i].token)); + conns.push(conn); + }); + + conns[0].call('removeOtherTokens'); + simplePoll( + function () { + var tokens = _.map(conns, function (conn) { + return conn.call('getCurrentLoginToken'); + }); + return ! tokens[1] && + tokens[0] === Accounts._hashLoginToken(stampedTokens[0].token); + }, + function () { // success + _.each(conns, function (conn) { + conn.disconnect(); + }); + onComplete(); + }, + function () { // timed out + throw new Error("accounts - remove other tokens timed out"); + } + ); + } +); diff --git a/packages/accounts-password/password_tests.js b/packages/accounts-password/password_tests.js index 614862502c7..45240231bc4 100644 --- a/packages/accounts-password/password_tests.js +++ b/packages/accounts-password/password_tests.js @@ -468,13 +468,53 @@ if (Meteor.isClient) (function () { test.equal(Meteor.userId(), null); })); }, + logoutStep, + function (test, expect) { + var self = this; + // Test that Meteor.logoutOtherClients logs out a second + // authentcated connection while leaving Accounts.connection + // logged in. + var secondConn = DDP.connect(Meteor.absoluteUrl()); + var token; + + var expectSecondConnLoggedOut = expect(function (err, result) { + test.isTrue(err); + }); + + var expectAccountsConnLoggedIn = expect(function (err, result) { + test.isFalse(err); + }); + + var expectSecondConnLoggedIn = expect(function (err, result) { + test.equal(result.token, token); + test.isFalse(err); + Meteor.logoutOtherClients(function (err) { + test.isFalse(err); + secondConn.call('login', { resume: token }, expectSecondConnLoggedOut); + Accounts.connection.call('login', { + resume: Accounts._storedLoginToken() + }, expectAccountsConnLoggedIn); + }); + }); + + Meteor.loginWithPassword(this.username, this.password, expect(function (err) { + test.isFalse(err); + token = Accounts._storedLoginToken(); + test.isTrue(token); + secondConn.call('login', { resume: token }, expectSecondConnLoggedIn); + })); + }, + logoutStep, + + // The tests below this point are for the deprecated + // `logoutOtherClients` method. + function (test, expect) { var self = this; // Test that Meteor.logoutOtherClients logs out a second authenticated // connection while leaving Accounts.connection logged in. var token; - var userId; self.secondConn = DDP.connect(Meteor.absoluteUrl()); var expectLoginError = expect(function (err) { @@ -502,7 +542,6 @@ if (Meteor.isClient) (function () { token = Accounts._storedLoginToken(); self.beforeLogoutOthersToken = token; test.isTrue(token); - userId = Meteor.userId(); self.secondConn.call("login", { resume: token }, expectSecondConnLoggedIn); })); @@ -536,41 +575,9 @@ if (Meteor.isClient) (function () { ); }, logoutStep, - function (test, expect) { - var self = this; - // Test that, when we call logoutOtherClients, if the server disconnects - // us before the logoutOtherClients callback runs, then we still end up - // logged in. - var expectServerLoggedIn = expect(function (err, result) { - test.isFalse(err); - test.isTrue(Meteor.userId()); - test.equal(result, Meteor.userId()); - }); - Meteor.loginWithPassword( - self.username, - self.password, - expect(function (err) { - test.isFalse(err); - test.isTrue(Meteor.userId()); - // The test is only useful if things interleave in the following order: - // - logoutOtherClients runs on the server - // - onReconnect fires and sends a login method with the old token, - // which results in an error - // - logoutOtherClients callback runs and stores the new token and - // logs in with it - // In practice they seem to interleave this way, but I'm not sure how - // to make sure that they do. - - Meteor.logoutOtherClients(function (err) { - test.isFalse(err); - Meteor.call("getUserId", expectServerLoggedIn); - }); - }) - ); - }, - logoutStep, + function (test, expect) { var self = this; // Test that deleting a user logs out that user's connections. From 3054b09790c2af74a511e17d2dc338370e053abc Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Tue, 18 Mar 2014 21:12:40 -0700 Subject: [PATCH 028/219] whitespace and use 'self' instead of 'this' --- packages/accounts-password/password_tests.js | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/accounts-password/password_tests.js b/packages/accounts-password/password_tests.js index 45240231bc4..adfde8594ea 100644 --- a/packages/accounts-password/password_tests.js +++ b/packages/accounts-password/password_tests.js @@ -490,19 +490,25 @@ if (Meteor.isClient) (function () { test.isFalse(err); Meteor.logoutOtherClients(function (err) { test.isFalse(err); - secondConn.call('login', { resume: token }, expectSecondConnLoggedOut); + secondConn.call('login', { resume: token }, + expectSecondConnLoggedOut); Accounts.connection.call('login', { resume: Accounts._storedLoginToken() }, expectAccountsConnLoggedIn); }); }); - Meteor.loginWithPassword(this.username, this.password, expect(function (err) { - test.isFalse(err); - token = Accounts._storedLoginToken(); - test.isTrue(token); - secondConn.call('login', { resume: token }, expectSecondConnLoggedIn); - })); + Meteor.loginWithPassword( + self.username, + self.password, + expect(function (err) { + test.isFalse(err); + token = Accounts._storedLoginToken(); + test.isTrue(token); + secondConn.call('login', { resume: token }, + expectSecondConnLoggedIn); + }) + ); }, logoutStep, From d5af69eca601308c6a26dc277d0ab70b6bc5e8a6 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Mon, 24 Mar 2014 13:54:05 -0700 Subject: [PATCH 029/219] Add comments to logoutOtherClients methods --- packages/accounts-base/accounts_server.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/accounts-base/accounts_server.js b/packages/accounts-base/accounts_server.js index 6b419604f6b..d31ab63a232 100644 --- a/packages/accounts-base/accounts_server.js +++ b/packages/accounts-base/accounts_server.js @@ -477,6 +477,14 @@ Meteor.methods({ } }, + // Generates a new login token with the same expiration as the + // connection's current token and saves it to the database. Associates + // the connection with this new token and returns it. Throws an error + // if called on a connection that isn't logged in. + // + // @returns Object + // If successful, returns { token: , id: , + // tokenExpires: }. getNewToken: function () { var self = this; var user = Meteor.users.findOne(self.userId, { @@ -505,6 +513,9 @@ Meteor.methods({ return loginUser(self, self.userId, newStampedToken); }, + // Removes all tokens except the token associated with the current + // connection. Throws an error if the connection is not logged + // in. Returns nothing on success. removeOtherTokens: function () { var self = this; if (! self.userId) { From b2632d45c57e5031f29bc87f2eb05044a64e4afb Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 17 Mar 2014 21:56:52 -0700 Subject: [PATCH 030/219] Move boilerplate HTML from tools to webapp This breaks a strong dependency between the webapp package and the bundler. Now we can change the standard page format, add more hooks, etc without changing the tools (and hot code push works properly). This is an incompatible change to the definition of the browser-program-pre1 format: it no longer contains a "page" field pointing to a boilerplate defined using ad hoc ##FOO## substitution. Instead, the manifest can contain "head" and "body" entries for data added with compileStep.appendDocument(). (Note that in Blaze, in a template file no longer calls appendDocument.) WebApp.addHtmlAttributeHook callbacks now return attribute objects (like those that Spacebars supports) rather than strings. --- History.md | 8 ++ packages/appcache/appcache-server.js | 2 +- packages/observe-sequence/package.js | 4 +- packages/star-translate/translator.js | 3 + packages/webapp/boilerplate.html | 22 ++++ packages/webapp/package.js | 9 +- packages/webapp/webapp_server.js | 128 +++++++++++++++--------- tools/bundler.js | 76 ++++---------- tools/tests/old/test-bundler-options.js | 50 ++++++--- 9 files changed, 179 insertions(+), 123 deletions(-) create mode 100644 packages/webapp/boilerplate.html diff --git a/History.md b/History.md index a067d9aec40..3d6eb26976b 100644 --- a/History.md +++ b/History.md @@ -2,6 +2,14 @@ * Log out a user's other sessions when they change their password. +* Move boilerplate HTML from tools to webapp. Changes internal + Webapp.addHtmlAttributeHook API incompatibly. + + +## v0.8.0 + +(Currently being stabilized. Features Blaze.) + ## v0.7.2 diff --git a/packages/appcache/appcache-server.js b/packages/appcache/appcache-server.js index ce579583d22..3d4bec1c4c8 100644 --- a/packages/appcache/appcache-server.js +++ b/packages/appcache/appcache-server.js @@ -57,7 +57,7 @@ var browserEnabled = function(request) { WebApp.addHtmlAttributeHook(function (request) { if (browserEnabled(request)) - return 'manifest="/app.manifest"'; + return { manifest: "/app.manifest" }; else return null; }); diff --git a/packages/observe-sequence/package.js b/packages/observe-sequence/package.js index 30be5f0ca58..5009db78d20 100644 --- a/packages/observe-sequence/package.js +++ b/packages/observe-sequence/package.js @@ -7,9 +7,7 @@ Package.on_use(function (api) { api.use('deps'); api.use('minimongo'); // for idStringify api.export('ObserveSequence'); - // XXX this does also run on the server but as long as deps is not - // documented to run there let's not try - api.add_files(['observe_sequence.js'], 'client'); + api.add_files(['observe_sequence.js']); }); Package.on_test(function (api) { diff --git a/packages/star-translate/translator.js b/packages/star-translate/translator.js index 77d08756def..bff404d0344 100644 --- a/packages/star-translate/translator.js +++ b/packages/star-translate/translator.js @@ -101,6 +101,9 @@ StarTranslator._writeClientProg = function (bundlePath, clientProgPath) { var clientManifest = { "format": "browser-program-pre1", "manifest": origClientManifest.manifest, + // XXX Haven't updated this for the app.html -> head/body change, but + // surely we don't need to because code in pre-star apps doesn't + // even read this file? "page": "app.html", "static": "static", "staticCacheable": "static_cacheable" diff --git a/packages/webapp/boilerplate.html b/packages/webapp/boilerplate.html new file mode 100644 index 00000000000..a4fb29f8fb2 --- /dev/null +++ b/packages/webapp/boilerplate.html @@ -0,0 +1,22 @@ + + +{{#each css}} {{/each}} + +{{#if inlineScriptsAllowed}} + +{{else}} + +{{/if}} +{{#each js}} +{{/each}} +{{#if inlineScriptsAllowed}} + +{{else}} + +{{/if}} +{{{head}}} + + +{{{body}}} + + diff --git a/packages/webapp/package.js b/packages/webapp/package.js index fab6a964347..2a50bf1aefa 100644 --- a/packages/webapp/package.js +++ b/packages/webapp/package.js @@ -8,7 +8,9 @@ Npm.depends({connect: "2.9.0", useragent: "2.0.7"}); Package.on_use(function (api) { - api.use(['logging', 'underscore', 'routepolicy'], 'server'); + api.use(['logging', 'underscore', 'routepolicy', 'spacebars-compiler', + 'spacebars', 'htmljs'], + 'server'); api.use(['underscore'], 'client'); api.use(['application-configuration', 'follower-livedata'], { unordered: true @@ -21,5 +23,10 @@ Package.on_use(function (api) { api.export(['WebApp', 'main', 'WebAppInternals'], 'server'); api.export(['WebApp'], 'client'); api.add_files('webapp_server.js', 'server'); + // This is a spacebars template, but we process it manually with the spacebars + // compiler rather than letting the 'templating' package (which isn't fully + // supported on the server yet) handle it. That also means that it doesn't + // contain the outer " + diff --git a/packages/spacebars-tests/template_tests.js b/packages/spacebars-tests/template_tests.js index 66443c8430b..f3da83e4377 100644 --- a/packages/spacebars-tests/template_tests.js +++ b/packages/spacebars-tests/template_tests.js @@ -1877,3 +1877,11 @@ Tinytest.add( } ); +Tinytest.add( + "spacebars - template - block comments should not be displayed", + function (test) { + var tmpl = Template.spacebars_test_block_comment; + var div = renderToDiv(tmpl); + test.equal(canonicalizeHtml(div.innerHTML), ''); + } +); From 89b087efb4dda6ee303fa16bce4226d37f714326 Mon Sep 17 00:00:00 2001 From: Jonathan Pidgeon Date: Wed, 9 Apr 2014 22:27:33 -0600 Subject: [PATCH 109/219] @font-face added to css minifiers --- packages/minifiers/minification.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/minifiers/minification.js b/packages/minifiers/minification.js index 567b8af8c9b..1637b516700 100644 --- a/packages/minifiers/minification.js +++ b/packages/minifiers/minification.js @@ -95,6 +95,13 @@ traverse.page = function(node) { + emit('}'); }; +traverse['font-face'] = function(node){ + return emit('@font-face', node.position, true) + + emit('{') + + mapVisit(node.declarations) + + emit('}'); +}; + traverse.rule = function(node) { var decls = node.declarations; if (!decls.length) return ''; From 5676dab4cc2d3f703386a14bf3ebdb1eac3c6838 Mon Sep 17 00:00:00 2001 From: Slava Kim Date: Tue, 15 Apr 2014 23:36:01 -0700 Subject: [PATCH 110/219] Use unreleased version of css-stringify that fixes #2028 --- packages/minifiers/.npm/package/npm-shrinkwrap.json | 4 ++-- packages/minifiers/package.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/minifiers/.npm/package/npm-shrinkwrap.json b/packages/minifiers/.npm/package/npm-shrinkwrap.json index bded1ec50e0..4b424c691b3 100644 --- a/packages/minifiers/.npm/package/npm-shrinkwrap.json +++ b/packages/minifiers/.npm/package/npm-shrinkwrap.json @@ -4,10 +4,10 @@ "version": "https://github.com/reworkcss/css-parse/tarball/aa7e23285375ca621dd20250bac0266c6d8683a5" }, "css-stringify": { - "version": "1.4.1", + "version": "https://github.com/reworkcss/css-stringify/tarball/a7fe6de82e055d41d1c5923ec2ccef06f2a45efa", "dependencies": { "source-map": { - "version": "0.1.31", + "version": "0.1.33", "dependencies": { "amdefine": { "version": "0.1.0" diff --git a/packages/minifiers/package.js b/packages/minifiers/package.js index fd97e290984..0815ea3fa48 100644 --- a/packages/minifiers/package.js +++ b/packages/minifiers/package.js @@ -6,7 +6,7 @@ Package.describe({ Npm.depends({ "uglify-js": "2.4.7", "css-parse": "https://github.com/reworkcss/css-parse/tarball/aa7e23285375ca621dd20250bac0266c6d8683a5", - "css-stringify": "1.4.1" + "css-stringify": "https://github.com/reworkcss/css-stringify/tarball/a7fe6de82e055d41d1c5923ec2ccef06f2a45efa" }); Package.on_use(function (api) { From a01833f74c560c6b3329b8b14e179c2617a09db8 Mon Sep 17 00:00:00 2001 From: David Greenspan Date: Wed, 16 Apr 2014 07:56:10 -0700 Subject: [PATCH 111/219] Fix Deps's broken _assign (harmless) You can't do ({hasOwnProperty: 12345}).hasOwnProperty("hasOwnProperty") -- think about it. Basically, `obj.hasOwnProperty(key)` is considered harmful. `Object.prototype.hasOwnProperty.call(obj, key)` is fine. We should standardize on `_.has` or something and do a pass through the codebase. This is my code so my bad. --- packages/deps/deps.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/deps/deps.js b/packages/deps/deps.js index db0c03744aa..d05ad800527 100644 --- a/packages/deps/deps.js +++ b/packages/deps/deps.js @@ -18,10 +18,12 @@ var setCurrentComputation = function (c) { // _assign is like _.extend or the upcoming Object.assign. // Copy src's own, enumerable properties onto tgt and return // tgt. +var _hasOwnProperty = Object.prototype.hasOwnProperty; var _assign = function (tgt, src) { - for (var k in src) - if (src.hasOwnProperty(k)) + for (var k in src) { + if (_hasOwnProperty.call(src, k)) tgt[k] = src[k]; + } return tgt; }; From 93154e2221fdbfc55805b645ae9143e6f8f1bc30 Mon Sep 17 00:00:00 2001 From: David Greenspan Date: Wed, 16 Apr 2014 07:57:30 -0700 Subject: [PATCH 112/219] Don't use _.pick in observe-sequence Trying to stick to a small set of _ methods for stand-alone Blaze --- packages/observe-sequence/observe_sequence.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/observe-sequence/observe_sequence.js b/packages/observe-sequence/observe_sequence.js index d7754ac671d..7af183f0840 100644 --- a/packages/observe-sequence/observe_sequence.js +++ b/packages/observe-sequence/observe_sequence.js @@ -218,11 +218,11 @@ var diffArray = function (lastSeqArray, seqArray, callbacks) { var lengthCur = lastSeqArray.length; _.each(seqArray, function (doc, i) { - newIdObjects.push(_.pick(doc, '_id')); + newIdObjects.push({_id: doc._id}); posNew[idStringify(doc._id)] = i; }); _.each(lastSeqArray, function (doc, i) { - oldIdObjects.push(_.pick(doc, '_id')); + oldIdObjects.push({_id: doc._id}); posOld[idStringify(doc._id)] = i; posCur[idStringify(doc._id)] = i; }); From 440ca4cd4d2039b1704788e9e8fa35e079d538ba Mon Sep 17 00:00:00 2001 From: David Glasser Date: Wed, 16 Apr 2014 14:18:32 -0700 Subject: [PATCH 113/219] Improve HTTP.call doc list of methods. --- docs/client/api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/client/api.js b/docs/client/api.js index 4d83217bb59..0bb610c3d7b 100644 --- a/docs/client/api.js +++ b/docs/client/api.js @@ -1678,7 +1678,7 @@ Template.api.httpcall = { args: [ {name: "method", type: "String", - descr: 'The HTTP method to use: "`GET`", "`POST`", "`PUT`", or "`DELETE`".'}, + descr: 'The [HTTP method](http://en.wikipedia.org/wiki/HTTP_method) to use, such as "`GET`", "`POST`", or "`HEAD`".'}, {name: "url", type: "String", descr: 'The URL to retrieve.'}, From f41223ba38385f7a59c88f158d626c21cfb3c70b Mon Sep 17 00:00:00 2001 From: Dan Dascalescu Date: Tue, 15 Apr 2014 17:52:24 -0700 Subject: [PATCH 114/219] Document that followRedirects defaults to true Per https://github.com/mikeal/request#requestoptions-callback --- docs/client/api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/client/api.js b/docs/client/api.js index 0bb610c3d7b..964184ba009 100644 --- a/docs/client/api.js +++ b/docs/client/api.js @@ -1713,7 +1713,7 @@ Template.api.httpcall = { descr: "Maximum time in milliseconds to wait for the request before failing. There is no timeout by default."}, {name: "followRedirects", type: "Boolean", - descr: "If true, transparently follow HTTP redirects. Cannot be set to false on the client."} + descr: "If `true`, transparently follow HTTP redirects. Cannot be set to `false` on the client. Default `true`."} ] }; From 797a080e7f4988cf4d9a0d1749266446ea96e99c Mon Sep 17 00:00:00 2001 From: Avital Oliver Date: Thu, 17 Apr 2014 01:34:56 -0700 Subject: [PATCH 115/219] Fix `UI.toHTML` on templates containing `{{#with}}` Originally reported at https://github.com/meteor/meteor/issues/2007#issuecomment-40530195 --- packages/htmljs/html.js | 2 +- packages/spacebars-tests/template_tests.html | 25 ++++++++++++++++++++ packages/spacebars-tests/template_tests.js | 21 ++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/packages/htmljs/html.js b/packages/htmljs/html.js index 71c2feb5515..797c53c55ae 100644 --- a/packages/htmljs/html.js +++ b/packages/htmljs/html.js @@ -181,7 +181,7 @@ stopWithLater = function (instance) { if (Deps.active) instance.materialized(); else - instance.data.stop(); + instance.data && instance.data.stop(); } }; diff --git a/packages/spacebars-tests/template_tests.html b/packages/spacebars-tests/template_tests.html index 2eceba5ddfb..28b0739b9c5 100644 --- a/packages/spacebars-tests/template_tests.html +++ b/packages/spacebars-tests/template_tests.html @@ -676,3 +676,28 @@ + + + + + + + + + diff --git a/packages/spacebars-tests/template_tests.js b/packages/spacebars-tests/template_tests.js index 66443c8430b..adc4a01d03f 100644 --- a/packages/spacebars-tests/template_tests.js +++ b/packages/spacebars-tests/template_tests.js @@ -1877,3 +1877,24 @@ Tinytest.add( } ); +Tinytest.add("spacebars - templates - UI.toHTML", function (test) { + Template.spacebars_test_tohtml_basic.foo = "foo"; + test.equal(canonicalizeHtml( + UI.toHTML(Template.spacebars_test_tohtml_basic)), "foo"); + + Template.spacebars_test_tohtml_if.foo = "foo"; + test.equal(canonicalizeHtml( + UI.toHTML(Template.spacebars_test_tohtml_if)), "foo"); + + Template.spacebars_test_tohtml_with.foo = "foo"; + test.equal(canonicalizeHtml( + UI.toHTML(Template.spacebars_test_tohtml_with)), "foo"); + + Template.spacebars_test_tohtml_each.foos = ["foo"]; + test.equal(canonicalizeHtml( + UI.toHTML(Template.spacebars_test_tohtml_each)), "foo"); + + test.equal(canonicalizeHtml( + UI.toHTML(Template.spacebars_test_tohtml_inclusion)), "foo"); +}); + From ca46407e5d082067716400ec8cb3f00dae2f2905 Mon Sep 17 00:00:00 2001 From: Avital Oliver Date: Thu, 17 Apr 2014 02:57:56 -0700 Subject: [PATCH 116/219] Properly clean up autoruns on `UI.toHTML`. The prior commit didn't stop components of type `Spacebars.With` correctly. --- packages/htmljs/html.js | 10 +++-- packages/spacebars-tests/template_tests.html | 8 +++- packages/spacebars-tests/template_tests.js | 45 +++++++++++++------- 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/packages/htmljs/html.js b/packages/htmljs/html.js index 797c53c55ae..f8348d6793b 100644 --- a/packages/htmljs/html.js +++ b/packages/htmljs/html.js @@ -178,10 +178,14 @@ callReactiveFunction = function (func) { stopWithLater = function (instance) { if (instance.materialized && instance.materialized.isWith) { - if (Deps.active) + if (Deps.active) { instance.materialized(); - else - instance.data && instance.data.stop(); + } else { + if (instance.data) // `UI.With` + instance.data.stop(); + else if (instance.v) // `Spacebars.With` + instance.v.stop(); + } } }; diff --git a/packages/spacebars-tests/template_tests.html b/packages/spacebars-tests/template_tests.html index 28b0739b9c5..37b63a45ee3 100644 --- a/packages/spacebars-tests/template_tests.html +++ b/packages/spacebars-tests/template_tests.html @@ -698,6 +698,10 @@ {{/each}} - + + diff --git a/packages/spacebars-tests/template_tests.js b/packages/spacebars-tests/template_tests.js index e6b570e6fb1..93f1b646ff8 100644 --- a/packages/spacebars-tests/template_tests.js +++ b/packages/spacebars-tests/template_tests.js @@ -1877,7 +1877,7 @@ Tinytest.add( } ); -Tinytest.add("spacebars - templates - UI.toHTML", function (test) { +Tinytest.add("spacebars - template - UI.toHTML", function (test) { // run once, verifying that autoruns are stopped var once = function (tmplToRender, tmplForHelper, helper, val) { var count = 0; @@ -1919,3 +1919,24 @@ Tinytest.add( test.equal(canonicalizeHtml(div.innerHTML), ''); } ); + +// Originally reported at https://github.com/meteor/meteor/issues/2046 +Tinytest.add( + "spacebars - template - {{#with}} with mutated data context", + function (test) { + var tmpl = Template.spacebars_test_with_mutated_data_context; + var foo = {value: 0}; + var dep = new Deps.Dependency; + tmpl.foo = function () { + dep.depend(); + return foo; + }; + + var div = renderToDiv(tmpl); + test.equal(canonicalizeHtml(div.innerHTML), '0'); + + foo.value = 1; + dep.changed(); + Deps.flush(); + test.equal(canonicalizeHtml(div.innerHTML), '1'); + }); diff --git a/packages/spacebars/spacebars-runtime.js b/packages/spacebars/spacebars-runtime.js index 120356753bd..59c2f37a47c 100644 --- a/packages/spacebars/spacebars-runtime.js +++ b/packages/spacebars/spacebars-runtime.js @@ -216,7 +216,7 @@ Spacebars.dot = function (value, id1/*, id2, ...*/) { Spacebars.With = function (argFunc, contentBlock, elseContentBlock) { return UI.Component.extend({ init: function () { - this.v = UI.emboxValue(argFunc); + this.v = UI.emboxValue(argFunc, UI.safeEquals); }, render: function () { return UI.If(this.v, UI.With(this.v, contentBlock), elseContentBlock); diff --git a/packages/ui/builtins.js b/packages/ui/builtins.js index a97bead0673..2ba0da6f48b 100644 --- a/packages/ui/builtins.js +++ b/packages/ui/builtins.js @@ -38,7 +38,7 @@ UI.Unless = function (argFunc, contentBlock, elseContentBlock) { // (Because then, they may be equal references to an object that was mutated, // and we'll never know. We save only a reference to the old object; we don't // do any deep-copying or diffing.) -var safeEquals = function (a, b) { +UI.safeEquals = function (a, b) { if (a !== b) return false; else @@ -67,7 +67,7 @@ UI.With = function (argFunc, contentBlock) { }; block.init = function () { - this.data = UI.emboxValue(argFunc, safeEquals); + this.data = UI.emboxValue(argFunc, UI.safeEquals); }; block.materialized = function () { From 568b0f929ca513c33f6d1347d6bd5461ae3587cc Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Sun, 20 Apr 2014 17:23:03 -0700 Subject: [PATCH 134/219] Change 'port' to 'proxyPort' in test-packages and update run-all comment. run-all interface was changed in 9b8bd31a. --- tools/commands.js | 2 +- tools/run-all.js | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/tools/commands.js b/tools/commands.js index 81b87d5b2db..f5b3ccbee96 100644 --- a/tools/commands.js +++ b/tools/commands.js @@ -1104,7 +1104,7 @@ main.registerCommand({ // sure the user doesn't 'meteor update' in the app, requiring // a switch to a different release appDirForVersionCheck: options.appDir, - port: options.port, + proxyPort: options.port, disableOplog: options['disable-oplog'], settingsFile: options.settings, banner: "Tests", diff --git a/tools/run-all.js b/tools/run-all.js index c4419e39ad1..0f00937549b 100644 --- a/tools/run-all.js +++ b/tools/run-all.js @@ -11,9 +11,10 @@ var AppRunner = require('./run-app.js').AppRunner; var MongoRunner = require('./run-mongo.js').MongoRunner; var Updater = require('./run-updater.js').Updater; -// options: port, buildOptions, settingsFile, banner, program, -// onRunEnd, onFailure, watchForChanges, quiet, rootUrl, mongoUrl, -// oplogUrl, disableOplog, appDirForVersionCheck +// options: proxyPort, proxyHost, appPort, appHost, buildOptions, +// settingsFile, banner, program, onRunEnd, onFailure, watchForChanges, +// quiet, rootUrl, mongoUrl, oplogUrl, disableOplog, +// appDirForVersionCheck var Runner = function (appDir, options) { var self = this; self.appDir = appDir; From 6b95113d99f664f99946260f94838c6b3d5021e6 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Sun, 20 Apr 2014 17:16:39 -0700 Subject: [PATCH 135/219] Make Blaze refuse to render javascript: URLs. Call UI._allowJavascriptUrls() to enable them again. In the future, we should have a per-element way to enable this, not a global configuration, and maybe someday even a similar feature for other policies (like enabling or disabling inline event handlers). --- packages/spacebars-tests/template_tests.html | 8 ++ packages/spacebars-tests/template_tests.js | 56 ++++++++++++++ packages/ui/attrs.js | 78 ++++++++++++++++++++ packages/ui/base.js | 8 ++ 4 files changed, 150 insertions(+) diff --git a/packages/spacebars-tests/template_tests.html b/packages/spacebars-tests/template_tests.html index dbec7e2fbb7..0f2e733dcdc 100644 --- a/packages/spacebars-tests/template_tests.html +++ b/packages/spacebars-tests/template_tests.html @@ -718,3 +718,11 @@ {{value}} {{/with}} + + diff --git a/packages/spacebars-tests/template_tests.js b/packages/spacebars-tests/template_tests.js index 93f1b646ff8..4a68ae7c8c4 100644 --- a/packages/spacebars-tests/template_tests.js +++ b/packages/spacebars-tests/template_tests.js @@ -1940,3 +1940,59 @@ Tinytest.add( Deps.flush(); test.equal(canonicalizeHtml(div.innerHTML), '1'); }); + +Tinytest.add( + "spacebars - template - javascript scheme urls", + function (test) { + var tmpl = Template.spacebars_test_url_attribute; + var sessionKey = "foo-" + Random.id(); + tmpl.foo = function () { + return Session.get(sessionKey); + }; + + var div = renderToDiv(tmpl); + document.body.appendChild(div); + + var checkAttrs = function (url, isJavascriptProtocol) { + Session.set(sessionKey, url); + Deps.flush(); + _.each( + // [tag name, attr name, is a url attribute] + [["A", "href", true], ["FORM", "action", true], + ["IMG", "src", true], ["INPUT", "value", false]], + function (attrInfo) { + + var normalizedUrl; + var elem = document.createElement(attrInfo[0]); + elem[attrInfo[1]] = url; + document.body.appendChild(elem); + normalizedUrl = elem[attrInfo[1]]; + document.body.removeChild(elem); + + _.each( + div.getElementsByTagName(attrInfo[0]), + function (elem) { + test.equal( + elem[attrInfo[1]], + isJavascriptProtocol && attrInfo[2] ? "" : normalizedUrl + ); + } + ); + } + ); + }; + + test.equal(UI._javascriptUrlsAllowed(), false); + checkAttrs("http://www.meteor.com", false); + checkAttrs("javascript:alert(1)", true); + checkAttrs("jAvAsCrIpT:alert(1)", true); + checkAttrs(" javascript:alert(1)", true); + UI._allowJavascriptUrls(); + test.equal(UI._javascriptUrlsAllowed(), true); + checkAttrs("http://www.meteor.com", false); + checkAttrs("javascript:alert(1)", false); + checkAttrs("jAvAsCrIpT:alert(1)", false); + checkAttrs(" javascript:alert(1)", false); + document.body.removeChild(div); + } +); diff --git a/packages/ui/attrs.js b/packages/ui/attrs.js index 7025c88ccf9..000ae986f6d 100644 --- a/packages/ui/attrs.js +++ b/packages/ui/attrs.js @@ -160,6 +160,82 @@ var isSVGElement = function (elem) { return 'ownerSVGElement' in elem; }; +var isUrlAttribute = function (tagName, attrName) { + // Compiled from http://www.w3.org/TR/REC-html40/index/attributes.html + // and + // http://www.w3.org/html/wg/drafts/html/master/index.html#attributes-1 + var urlAttrs = { + FORM: ['action'], + BODY: ['background'], + BLOCKQUOTE: ['cite'], + Q: ['cite'], + DEL: ['cite'], + INS: ['cite'], + OBJECT: ['classid', 'codebase', 'data', 'usemap'], + APPLET: ['codebase'], + A: ['href'], + AREA: ['href'], + LINK: ['href'], + BASE: ['href'], + IMG: ['longdesc', 'src', 'usemap'], + FRAME: ['longdesc', 'src'], + IFRAME: ['longdesc', 'src'], + HEAD: ['profile'], + SCRIPT: ['src'], + INPUT: ['src', 'usemap', 'formaction'], + BUTTON: ['formaction'], + BASE: ['href'], + MENUITEM: ['icon'], + HTML: ['manifest'], + VIDEO: ['poster'] + }; + + if (attrName === 'itemid') { + return true; + } + + var urlAttrNames = urlAttrs[tagName] || []; + return _.contains(urlAttrNames, attrName); +}; + +// UrlHandler is an attribute handler for all HTML attributes that take +// URL values. It disallows javascript: URLs, unless +// UI._allowJavascriptUrls() has been called. To detect javascript: +// urls, we set the attribute and then reads the attribute out of the +// DOM, in order to avoid writing our own URL normalization code. (We +// don't want to be fooled by ' javascript:alert(1)' or +// 'jAvAsCrIpT:alert(1)'.) In future, when the URL interface is more +// widely supported, we can use that, which will be +// cleaner. https://developer.mozilla.org/en-US/docs/Web/API/URL +var origUpdate = AttributeHandler.prototype.update; +var UrlHandler = AttributeHandler.extend({ + update: function (element, oldValue, value) { + var self = this; + var args = arguments; + var isJavascriptProtocol = false; + origUpdate.apply(self, args); + + if (! UI._javascriptUrlsAllowed()) { + // If we're updating an A tag, we can read the 'protocol' property + // of the element itself. Otherwise, get the string value of the + // attribute and look for 'javascript:' at the beginning. + if (element.tagName === 'A') { + isJavascriptProtocol = element.protocol && + element.protocol === 'javascript:'; + } else { + isJavascriptProtocol = element[self.name] && + element[self.name].indexOf('javascript:') === 0; + } + + if (isJavascriptProtocol) { + Meteor._debug("javascript: URLs are not allowed. " + + "Use UI._allowJavascriptUrls() to enable them."); + origUpdate.apply(self, [element, value, null]); + } + } + } +}); + // XXX make it possible for users to register attribute handlers! makeAttributeHandler = function (elem, name, value) { // generally, use setAttribute but certain attributes need to be set @@ -180,6 +256,8 @@ makeAttributeHandler = function (elem, name, value) { return new ValueHandler(name, value); } else if (name.substring(0,6) === 'xlink:') { return new XlinkHandler(name.substring(6), value); + } else if (isUrlAttribute(elem.tagName, name)) { + return new UrlHandler(name, value); } else { return new AttributeHandler(name, value); } diff --git a/packages/ui/base.js b/packages/ui/base.js index da515003a94..4f4d3f0bea8 100644 --- a/packages/ui/base.js +++ b/packages/ui/base.js @@ -344,3 +344,11 @@ UI.getElementData = function (el) { var comp = UI.DomRange.getContainingComponent(el); return comp && getComponentData(comp); }; + +var jsUrlsAllowed = false; +UI._allowJavascriptUrls = function () { + jsUrlsAllowed = true; +}; +UI._javascriptUrlsAllowed = function () { + return jsUrlsAllowed; +}; From 92a7e9a30d67d8f0a3c6dd5be13e5fa6ddc76759 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Mon, 21 Apr 2014 11:17:13 -0700 Subject: [PATCH 136/219] Reindent `OAuthEncryption.isSealed` --- packages/oauth-encryption/encrypt.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/oauth-encryption/encrypt.js b/packages/oauth-encryption/encrypt.js index c29b7bfbff1..528c0e169f7 100644 --- a/packages/oauth-encryption/encrypt.js +++ b/packages/oauth-encryption/encrypt.js @@ -150,10 +150,10 @@ OAuthEncryption.open = function (ciphertext, userId) { OAuthEncryption.isSealed = function (maybeCipherText) { - return maybeCipherText && - OAuthEncryption._isBase64(maybeCipherText.iv) && - OAuthEncryption._isBase64(maybeCipherText.ciphertext) && - _.isString(maybeCipherText.algorithm); + return maybeCipherText && + OAuthEncryption._isBase64(maybeCipherText.iv) && + OAuthEncryption._isBase64(maybeCipherText.ciphertext) && + _.isString(maybeCipherText.algorithm); }; From f8036fc1f4494ade188288d9e06fd2a342a8f7e5 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Mon, 21 Apr 2014 11:17:37 -0700 Subject: [PATCH 137/219] Check `authTag` in `isSealed` --- packages/oauth-encryption/encrypt.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/oauth-encryption/encrypt.js b/packages/oauth-encryption/encrypt.js index 528c0e169f7..43f8318443f 100644 --- a/packages/oauth-encryption/encrypt.js +++ b/packages/oauth-encryption/encrypt.js @@ -153,6 +153,7 @@ OAuthEncryption.isSealed = function (maybeCipherText) { return maybeCipherText && OAuthEncryption._isBase64(maybeCipherText.iv) && OAuthEncryption._isBase64(maybeCipherText.ciphertext) && + OAuthEncryption._isBase64(maybeCipherText.authTag) && _.isString(maybeCipherText.algorithm); }; From a7a21e53659e5eb2688a1c14ca081601917e0e09 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Mon, 21 Apr 2014 11:19:03 -0700 Subject: [PATCH 138/219] Add some curly braces --- packages/oauth-encryption/encrypt.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/oauth-encryption/encrypt.js b/packages/oauth-encryption/encrypt.js index 43f8318443f..67223ba1e94 100644 --- a/packages/oauth-encryption/encrypt.js +++ b/packages/oauth-encryption/encrypt.js @@ -129,16 +129,17 @@ OAuthEncryption.open = function (ciphertext, userId) { var data; try { data = EJSON.parse(result.plaintext.toString()); - } - catch (e) { - if (e instanceof SyntaxError && Meteor._printDecryptionFailure) + } catch (e) { + if (e instanceof SyntaxError && Meteor._printDecryptionFailure) { Meteor._debug("OAuth decryption unsuccessful: probably wrong key"); + } throw new Error(); } if (! result.auth_ok) { - if (Meteor._printDecryptionFailure) + if (Meteor._printDecryptionFailure) { Meteor._debug("userId does not match in OAuth decryption"); + } throw new Error(); } From 4c8567067639daca72704bb3b71a3f6d06720c5d Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Mon, 21 Apr 2014 11:40:38 -0700 Subject: [PATCH 139/219] Check `auth_ok` before parsing decrypted ciphertext --- packages/oauth-encryption/encrypt.js | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/oauth-encryption/encrypt.js b/packages/oauth-encryption/encrypt.js index 67223ba1e94..4213742602a 100644 --- a/packages/oauth-encryption/encrypt.js +++ b/packages/oauth-encryption/encrypt.js @@ -122,23 +122,19 @@ OAuthEncryption.open = function (ciphertext, userId) { new Buffer(ciphertext.authTag, "base64") ); - // If we can't parse the decrypted text, it's probably because we - // decrypted with the wrong key. Check this before checking - // auth_ok because if decryption fails then auth_ok will also be - // false. + if (! result.auth_ok) { + if (Meteor._printDecryptionFailure) { + Meteor._debug("OAuth decryption unsuccessful"); + } + throw new Error(); + } + var data; try { data = EJSON.parse(result.plaintext.toString()); } catch (e) { if (e instanceof SyntaxError && Meteor._printDecryptionFailure) { - Meteor._debug("OAuth decryption unsuccessful: probably wrong key"); - } - throw new Error(); - } - - if (! result.auth_ok) { - if (Meteor._printDecryptionFailure) { - Meteor._debug("userId does not match in OAuth decryption"); + Meteor._debug("OAuth decryption unsuccessful"); } throw new Error(); } From a251255001d1a289c7beb2d570ba866d4cda262e Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 21 Apr 2014 11:56:06 -0700 Subject: [PATCH 140/219] Revert "Meteor.loginWith now waits for config" This reverts commit 76ded8feb27a0d83f0b1857aa07e3a7c3d00c7d3. --- History.md | 6 +- packages/accounts-base/accounts_client.js | 13 --- packages/facebook/facebook_client.js | 41 ++++----- packages/facebook/package.js | 5 -- packages/github/github_client.js | 41 +++++---- packages/github/package.js | 5 -- packages/google/google_client.js | 87 +++++++++---------- packages/google/package.js | 5 -- packages/meetup/meetup_client.js | 50 +++++------ packages/meetup/package.js | 5 -- .../meteor_developer_client.js | 49 ++++++----- packages/meteor-developer/package.js | 5 -- packages/twitter/package.js | 5 -- packages/twitter/twitter_client.js | 45 +++++----- packages/weibo/package.js | 5 -- packages/weibo/weibo_client.js | 36 ++++---- 16 files changed, 169 insertions(+), 234 deletions(-) diff --git a/History.md b/History.md index 061b12dd1bd..b2919c34c60 100644 --- a/History.md +++ b/History.md @@ -18,13 +18,9 @@ * Add `Random.secret()` for generating security-critical secrets like login tokens. -* If any of the `Meteor.loginWith[ExternalService]` functions are called before - login service configuration is loaded, clients will wait for the configuration - to be ready rather than fail. #1911 #2048. - * `Meteor.logoutOtherClients` now calls the user callback when other login tokens have actually been removed from the database, not when - they have been marked for eventual removal. #1915. + they have been marked for eventual removal. Fixes #1915. * Add `meteor list-sites` command for listing the sites that you have deployed to meteor.com with your Meteor developer account. diff --git a/packages/accounts-base/accounts_client.js b/packages/accounts-base/accounts_client.js index d0cb26c5aec..140cdf1069e 100644 --- a/packages/accounts-base/accounts_client.js +++ b/packages/accounts-base/accounts_client.js @@ -267,19 +267,6 @@ Accounts.loginServicesConfigured = function () { return loginServicesHandle.ready(); }; -Accounts.withLoginServiceConfiguration = function (serviceName, callback) { - Deps.autorun(function (handle) { - if (Accounts.loginServicesConfigured()) { - handle.stop(); - Deps.nonreactive(function () { - callback( - Package['service-configuration'].ServiceConfiguration.configurations.findOne( - {service: serviceName})); - }); - } - }); -}; - /// /// HANDLEBARS HELPERS /// diff --git a/packages/facebook/facebook_client.js b/packages/facebook/facebook_client.js index c5584f707d9..ed86eed72d5 100644 --- a/packages/facebook/facebook_client.js +++ b/packages/facebook/facebook_client.js @@ -13,30 +13,27 @@ Facebook.requestCredential = function (options, credentialRequestCompleteCallbac options = {}; } - Accounts.withLoginServiceConfiguration("facebook", function (config) { - if (!config) { - credentialRequestCompleteCallback && credentialRequestCompleteCallback( - new ServiceConfiguration.ConfigError("Service not configured")); - return; - } + var config = ServiceConfiguration.configurations.findOne({service: 'facebook'}); + if (!config) { + credentialRequestCompleteCallback && credentialRequestCompleteCallback(new ServiceConfiguration.ConfigError("Service not configured")); + return; + } - var credentialToken = Random.secret(); - var mobile = /Android|webOS|iPhone|iPad|iPod|BlackBerry|Windows Phone/i.test( - navigator.userAgent); - var display = mobile ? 'touch' : 'popup'; + var credentialToken = Random.secret(); + var mobile = /Android|webOS|iPhone|iPad|iPod|BlackBerry|Windows Phone/i.test(navigator.userAgent); + var display = mobile ? 'touch' : 'popup'; - var scope = "email"; - if (options && options.requestPermissions) - scope = options.requestPermissions.join(','); + var scope = "email"; + if (options && options.requestPermissions) + scope = options.requestPermissions.join(','); - var loginUrl = - 'https://www.facebook.com/dialog/oauth?client_id=' + config.appId + - '&redirect_uri=' + Meteor.absoluteUrl('_oauth/facebook?close') + - '&display=' + display + '&scope=' + scope + '&state=' + credentialToken; + var loginUrl = + 'https://www.facebook.com/dialog/oauth?client_id=' + config.appId + + '&redirect_uri=' + Meteor.absoluteUrl('_oauth/facebook?close') + + '&display=' + display + '&scope=' + scope + '&state=' + credentialToken; - Oauth.showPopup( - loginUrl, - _.bind(credentialRequestCompleteCallback, null, credentialToken) - ); - }); + Oauth.showPopup( + loginUrl, + _.bind(credentialRequestCompleteCallback, null, credentialToken) + ); }; diff --git a/packages/facebook/package.js b/packages/facebook/package.js index 1bb8434680f..534ed18a287 100644 --- a/packages/facebook/package.js +++ b/packages/facebook/package.js @@ -13,11 +13,6 @@ Package.on_use(function(api) { api.use('underscore', 'server'); api.use('random', 'client'); api.use('service-configuration', ['client', 'server']); - // XXX We intended to keep this module separated from accounts-*. But - // service-configuration pulls in accounts-base anyway (for - // Accounts.connection), and accounts-base seemed like the best place to - // put Accounts.withLoginServiceConfiguration for now. - api.use('accounts-base', 'client'); api.export('Facebook'); diff --git a/packages/github/github_client.js b/packages/github/github_client.js index 1b35e3c2117..ccf06b2a5d0 100644 --- a/packages/github/github_client.js +++ b/packages/github/github_client.js @@ -12,28 +12,27 @@ Github.requestCredential = function (options, credentialRequestCompleteCallback) options = {}; } - Accounts.withLoginServiceConfiguration("github", function (config) { - if (!config) { - credentialRequestCompleteCallback && credentialRequestCompleteCallback( - new ServiceConfiguration.ConfigError("Service not configured")); - return; - } - var credentialToken = Random.secret(); + var config = ServiceConfiguration.configurations.findOne({service: 'github'}); + if (!config) { + credentialRequestCompleteCallback && credentialRequestCompleteCallback(new ServiceConfiguration.ConfigError("Service not configured")); + return; + } + var credentialToken = Random.secret(); + + var scope = (options && options.requestPermissions) || []; + var flatScope = _.map(scope, encodeURIComponent).join('+'); - var scope = (options && options.requestPermissions) || []; - var flatScope = _.map(scope, encodeURIComponent).join('+'); + var loginUrl = + 'https://github.com/login/oauth/authorize' + + '?client_id=' + config.clientId + + '&scope=' + flatScope + + '&redirect_uri=' + Meteor.absoluteUrl('_oauth/github?close') + + '&state=' + credentialToken; - var loginUrl = - 'https://github.com/login/oauth/authorize' + - '?client_id=' + config.clientId + - '&scope=' + flatScope + - '&redirect_uri=' + Meteor.absoluteUrl('_oauth/github?close') + - '&state=' + credentialToken; - Oauth.showPopup( - loginUrl, - _.bind(credentialRequestCompleteCallback, null, credentialToken), - {width: 900, height: 450} - ); - }); + Oauth.showPopup( + loginUrl, + _.bind(credentialRequestCompleteCallback, null, credentialToken), + {width: 900, height: 450} + ); }; diff --git a/packages/github/package.js b/packages/github/package.js index 9f8182814ba..c8b6ac92aad 100644 --- a/packages/github/package.js +++ b/packages/github/package.js @@ -13,11 +13,6 @@ Package.on_use(function(api) { api.use('templating', 'client'); api.use('random', 'client'); api.use('service-configuration', ['client', 'server']); - // XXX We intended to keep this module separated from accounts-*. But - // service-configuration pulls in accounts-base anyway (for - // Accounts.connection), and accounts-base seemed like the best place to - // put Accounts.withLoginServiceConfiguration for now. - api.use('accounts-base', 'client'); api.export('Github'); diff --git a/packages/google/google_client.js b/packages/google/google_client.js index 4e18392414c..b080264669b 100644 --- a/packages/google/google_client.js +++ b/packages/google/google_client.js @@ -14,51 +14,48 @@ Google.requestCredential = function (options, credentialRequestCompleteCallback) options = {}; } - Accounts.withLoginServiceConfiguration("google", function (config) { - if (!config) { - credentialRequestCompleteCallback && credentialRequestCompleteCallback( - new ServiceConfiguration.ConfigError("Service not configured")); - return; - } - - var credentialToken = Random.secret(); - - // always need this to get user id from google. - var requiredScope = ['profile']; - var scope = ['email']; - if (options.requestPermissions) - scope = options.requestPermissions; - scope = _.union(scope, requiredScope); - var flatScope = _.map(scope, encodeURIComponent).join('+'); - - // https://developers.google.com/accounts/docs/OAuth2WebServer#formingtheurl - var accessType = options.requestOfflineToken ? 'offline' : 'online'; - var approvalPrompt = options.forceApprovalPrompt ? 'force' : 'auto'; - - var loginUrl = - 'https://accounts.google.com/o/oauth2/auth' + - '?response_type=code' + - '&client_id=' + config.clientId + - '&scope=' + flatScope + - '&redirect_uri=' + Meteor.absoluteUrl('_oauth/google?close') + - '&state=' + credentialToken + - '&access_type=' + accessType + - '&approval_prompt=' + approvalPrompt; + var config = ServiceConfiguration.configurations.findOne({service: 'google'}); + if (!config) { + credentialRequestCompleteCallback && credentialRequestCompleteCallback(new ServiceConfiguration.ConfigError("Service not configured")); + return; + } - // Use Google's domain-specific login page if we want to restrict creation - // to a particular email domain. (Don't use it if - // restrictCreationByEmailDomain is a function.) Note that all this does is - // change Google's UI --- accounts-base/accounts_server.js still checks - // server-side that the server has the proper email address after the OAuth - // conversation. - if (typeof Accounts._options.restrictCreationByEmailDomain === 'string') { - loginUrl += '&hd=' + encodeURIComponent(Accounts._options.restrictCreationByEmailDomain); - } + var credentialToken = Random.secret(); + + // always need this to get user id from google. + var requiredScope = ['profile']; + var scope = ['email']; + if (options.requestPermissions) + scope = options.requestPermissions; + scope = _.union(scope, requiredScope); + var flatScope = _.map(scope, encodeURIComponent).join('+'); + + // https://developers.google.com/accounts/docs/OAuth2WebServer#formingtheurl + var accessType = options.requestOfflineToken ? 'offline' : 'online'; + var approvalPrompt = options.forceApprovalPrompt ? 'force' : 'auto'; + + var loginUrl = + 'https://accounts.google.com/o/oauth2/auth' + + '?response_type=code' + + '&client_id=' + config.clientId + + '&scope=' + flatScope + + '&redirect_uri=' + Meteor.absoluteUrl('_oauth/google?close') + + '&state=' + credentialToken + + '&access_type=' + accessType + + '&approval_prompt=' + approvalPrompt; + + // Use Google's domain-specific login page if we want to restrict creation to + // a particular email domain. (Don't use it if restrictCreationByEmailDomain + // is a function.) Note that all this does is change Google's UI --- + // accounts-base/accounts_server.js still checks server-side that the server + // has the proper email address after the OAuth conversation. + if (typeof Accounts._options.restrictCreationByEmailDomain === 'string') { + loginUrl += '&hd=' + encodeURIComponent(Accounts._options.restrictCreationByEmailDomain); + } - Oauth.showPopup( - loginUrl, - _.bind(credentialRequestCompleteCallback, null, credentialToken), - { height: 406 } - ); - }); + Oauth.showPopup( + loginUrl, + _.bind(credentialRequestCompleteCallback, null, credentialToken), + { height: 406 } + ); }; diff --git a/packages/google/package.js b/packages/google/package.js index bb56b95cb1f..8bd523c2a22 100644 --- a/packages/google/package.js +++ b/packages/google/package.js @@ -10,11 +10,6 @@ Package.on_use(function(api) { api.use('oauth', ['client', 'server']); api.use('http', ['server']); api.use(['underscore', 'service-configuration'], ['client', 'server']); - // XXX We intended to keep this module separated from accounts-*. But - // service-configuration pulls in accounts-base anyway (for - // Accounts.connection), and accounts-base seemed like the best place to - // put Accounts.withLoginServiceConfiguration for now. - api.use('accounts-base', 'client'); api.use(['random', 'templating'], 'client'); api.export('Google'); diff --git a/packages/meetup/meetup_client.js b/packages/meetup/meetup_client.js index d6e55289926..277811d208d 100644 --- a/packages/meetup/meetup_client.js +++ b/packages/meetup/meetup_client.js @@ -11,34 +11,32 @@ Meetup.requestCredential = function (options, credentialRequestCompleteCallback) options = {}; } - Accounts.withLoginServiceConfiguration("meetup", function (config) { - if (!config) { - credentialRequestCompleteCallback && credentialRequestCompleteCallback( - new ServiceConfiguration.ConfigError("Service not configured")); - return; - } - var credentialToken = Random.secret(); + var config = ServiceConfiguration.configurations.findOne({service: 'meetup'}); + if (!config) { + credentialRequestCompleteCallback && credentialRequestCompleteCallback(new ServiceConfiguration.ConfigError("Service not configured")); + return; + } + var credentialToken = Random.secret(); - var scope = (options && options.requestPermissions) || []; - var flatScope = _.map(scope, encodeURIComponent).join('+'); + var scope = (options && options.requestPermissions) || []; + var flatScope = _.map(scope, encodeURIComponent).join('+'); - var loginUrl = - 'https://secure.meetup.com/oauth2/authorize' + - '?client_id=' + config.clientId + - '&response_type=code' + - '&scope=' + flatScope + - '&redirect_uri=' + Meteor.absoluteUrl('_oauth/meetup?close') + - '&state=' + credentialToken; + var loginUrl = + 'https://secure.meetup.com/oauth2/authorize' + + '?client_id=' + config.clientId + + '&response_type=code' + + '&scope=' + flatScope + + '&redirect_uri=' + Meteor.absoluteUrl('_oauth/meetup?close') + + '&state=' + credentialToken; - // meetup box gets taller when permissions requested. - var height = 620; - if (_.without(scope, 'basic').length) - height += 130; + // meetup box gets taller when permissions requested. + var height = 620; + if (_.without(scope, 'basic').length) + height += 130; - Oauth.showPopup( - loginUrl, - _.bind(credentialRequestCompleteCallback, null, credentialToken), - {width: 900, height: height} - ); - }); + Oauth.showPopup( + loginUrl, + _.bind(credentialRequestCompleteCallback, null, credentialToken), + {width: 900, height: height} + ); }; diff --git a/packages/meetup/package.js b/packages/meetup/package.js index 6654ef228bc..8a739fe3e51 100644 --- a/packages/meetup/package.js +++ b/packages/meetup/package.js @@ -13,11 +13,6 @@ Package.on_use(function(api) { api.use('underscore', 'client'); api.use('random', 'client'); api.use('service-configuration', ['client', 'server']); - // XXX We intended to keep this module separated from accounts-*. But - // service-configuration pulls in accounts-base anyway (for - // Accounts.connection), and accounts-base seemed like the best place to - // put Accounts.withLoginServiceConfiguration for now. - api.use('accounts-base', 'client'); api.export('Meetup'); diff --git a/packages/meteor-developer/meteor_developer_client.js b/packages/meteor-developer/meteor_developer_client.js index 47199672342..81a57d58c8d 100644 --- a/packages/meteor-developer/meteor_developer_client.js +++ b/packages/meteor-developer/meteor_developer_client.js @@ -5,33 +5,34 @@ MeteorDeveloperAccounts = {}; // completion. Takes one argument, credentialToken on success, or Error on // error. var requestCredential = function (credentialRequestCompleteCallback) { - Accounts.withLoginServiceConfiguration("meteor-developer", function (config) { - if (!config) { - credentialRequestCompleteCallback && - credentialRequestCompleteCallback( - new ServiceConfiguration.ConfigError("Service not configured") - ); - return; - } + var config = ServiceConfiguration.configurations.findOne({ + service: 'meteor-developer' + }); + if (!config) { + credentialRequestCompleteCallback && + credentialRequestCompleteCallback( + new ServiceConfiguration.ConfigError("Service not configured") + ); + return; + } - var credentialToken = Random.secret(); + var credentialToken = Random.secret(); - var loginUrl = - METEOR_DEVELOPER_URL + "/oauth2/authorize?" + - "state=" + credentialToken + - "&response_type=code&" + - "client_id=" + config.clientId + - "&redirect_uri=" + Meteor.absoluteUrl("_oauth/meteor-developer?close"); + var loginUrl = + METEOR_DEVELOPER_URL + "/oauth2/authorize?" + + "state=" + credentialToken + + "&response_type=code&" + + "client_id=" + config.clientId + + "&redirect_uri=" + Meteor.absoluteUrl("_oauth/meteor-developer?close"); - Oauth.showPopup( - loginUrl, - _.bind(credentialRequestCompleteCallback, null, credentialToken), - { - width: 470, - height: 420 - } - ); - }); + Oauth.showPopup( + loginUrl, + _.bind(credentialRequestCompleteCallback, null, credentialToken), + { + width: 470, + height: 420 + } + ); }; MeteorDeveloperAccounts.requestCredential = requestCredential; diff --git a/packages/meteor-developer/package.js b/packages/meteor-developer/package.js index 644a2205fe5..39691f01e95 100644 --- a/packages/meteor-developer/package.js +++ b/packages/meteor-developer/package.js @@ -8,11 +8,6 @@ Package.on_use(function (api) { api.use('oauth', ['client', 'server']); api.use('http', ['server']); api.use(['underscore', 'service-configuration'], ['client', 'server']); - // XXX We intended to keep this module separated from accounts-*. But - // service-configuration pulls in accounts-base anyway (for - // Accounts.connection), and accounts-base seemed like the best place to - // put Accounts.withLoginServiceConfiguration for now. - api.use('accounts-base', 'client'); api.use(['random', 'templating'], 'client'); api.export('MeteorDeveloperAccounts'); diff --git a/packages/twitter/package.js b/packages/twitter/package.js index 974c089bab8..5180af2fd8c 100644 --- a/packages/twitter/package.js +++ b/packages/twitter/package.js @@ -13,11 +13,6 @@ Package.on_use(function(api) { api.use('random', 'client'); api.use('underscore', 'server'); api.use('service-configuration', ['client', 'server']); - // XXX We intended to keep this module separated from accounts-*. But - // service-configuration pulls in accounts-base anyway (for - // Accounts.connection), and accounts-base seemed like the best place to - // put Accounts.withLoginServiceConfiguration for now. - api.use('accounts-base', 'client'); api.export('Twitter'); diff --git a/packages/twitter/twitter_client.js b/packages/twitter/twitter_client.js index a22ac27a8b9..f1553c9e960 100644 --- a/packages/twitter/twitter_client.js +++ b/packages/twitter/twitter_client.js @@ -12,32 +12,29 @@ Twitter.requestCredential = function (options, credentialRequestCompleteCallback options = {}; } - Accounts.withLoginServiceConfiguration("twitter", function (config) { - if (!config) { - credentialRequestCompleteCallback && credentialRequestCompleteCallback( - new ServiceConfiguration.ConfigError("Service not configured")); - return; - } + var config = ServiceConfiguration.configurations.findOne({service: 'twitter'}); + if (!config) { + credentialRequestCompleteCallback && credentialRequestCompleteCallback(new ServiceConfiguration.ConfigError("Service not configured")); + return; + } - var credentialToken = Random.secret(); - // We need to keep credentialToken across the next two 'steps' so we're - // adding a credentialToken parameter to the url and the callback url that - // we'll be returned to by oauth provider + var credentialToken = Random.secret(); + // We need to keep credentialToken across the next two 'steps' so we're adding + // a credentialToken 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/oauth1/oauth1_server.js - var callbackUrl = Meteor.absoluteUrl('_oauth/twitter?close&state=' + - credentialToken); + // url back to app, enters "step 2" as described in + // packages/accounts-oauth1-helper/oauth1_server.js + var callbackUrl = Meteor.absoluteUrl('_oauth/twitter?close&state=' + credentialToken); - // url to app, enters "step 1" as described in - // packages/oauth1/oauth1_server.js - var loginUrl = '/_oauth/twitter/?requestTokenAndRedirect=' - + encodeURIComponent(callbackUrl) - + '&state=' + credentialToken; + // url to app, enters "step 1" as described in + // packages/accounts-oauth1-helper/oauth1_server.js + var loginUrl = '/_oauth/twitter/?requestTokenAndRedirect=' + + encodeURIComponent(callbackUrl) + + '&state=' + credentialToken; - Oauth.showPopup( - loginUrl, - _.bind(credentialRequestCompleteCallback, null, credentialToken) - ); - }); + Oauth.showPopup( + loginUrl, + _.bind(credentialRequestCompleteCallback, null, credentialToken) + ); }; diff --git a/packages/weibo/package.js b/packages/weibo/package.js index 7b787ff4d84..9ea42e1875a 100644 --- a/packages/weibo/package.js +++ b/packages/weibo/package.js @@ -12,11 +12,6 @@ Package.on_use(function(api) { api.use('templating', 'client'); api.use('random', 'client'); api.use('service-configuration', ['client', 'server']); - // XXX We intended to keep this module separated from accounts-*. But - // service-configuration pulls in accounts-base anyway (for - // Accounts.connection), and accounts-base seemed like the best place to - // put Accounts.withLoginServiceConfiguration for now. - api.use('accounts-base', 'client'); api.export('Weibo'); diff --git a/packages/weibo/weibo_client.js b/packages/weibo/weibo_client.js index 643e2721367..feff8f29208 100644 --- a/packages/weibo/weibo_client.js +++ b/packages/weibo/weibo_client.js @@ -12,25 +12,23 @@ Weibo.requestCredential = function (options, credentialRequestCompleteCallback) options = {}; } - Accounts.withLoginServiceConfiguration("weibo", function (config) { - if (!config) { - credentialRequestCompleteCallback && credentialRequestCompleteCallback( - new ServiceConfiguration.ConfigError("Service not configured")); - return; - } + var config = ServiceConfiguration.configurations.findOne({service: 'weibo'}); + if (!config) { + credentialRequestCompleteCallback && credentialRequestCompleteCallback(new ServiceConfiguration.ConfigError("Service not configured")); + return; + } - var credentialToken = Random.secret(); - // XXX need to support configuring access_type and scope - var loginUrl = - 'https://api.weibo.com/oauth2/authorize' + - '?response_type=code' + - '&client_id=' + config.clientId + - '&redirect_uri=' + Meteor.absoluteUrl('_oauth/weibo?close', {replaceLocalhost: true}) + - '&state=' + credentialToken; + var credentialToken = Random.secret(); + // XXX need to support configuring access_type and scope + var loginUrl = + 'https://api.weibo.com/oauth2/authorize' + + '?response_type=code' + + '&client_id=' + config.clientId + + '&redirect_uri=' + Meteor.absoluteUrl('_oauth/weibo?close', {replaceLocalhost: true}) + + '&state=' + credentialToken; - Oauth.showPopup( - loginUrl, - _.bind(credentialRequestCompleteCallback, null, credentialToken) - ); - }); + Oauth.showPopup( + loginUrl, + _.bind(credentialRequestCompleteCallback, null, credentialToken) + ); }; From 8c3855031519c3b6d6a4a54337bb67e26f2053ff Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 21 Apr 2014 13:35:54 -0700 Subject: [PATCH 141/219] Improve error message for pre-config login Addresses #2048. An earlier attempt (to wait for the config to load) ran into popup blockers. It would be nice to load the config statically with something like Arunoda's fast-render. That said, even that's not good enough to allow OAuth logins that bypass the popup blocker that aren't a result of a user action, and for user actions it's easy enough to gate your login button on `Accounts.loginServicesConfigured()`. Longer term solutions include non-popup methods of OAuth login (see Issue #438). --- packages/facebook/facebook_client.js | 3 ++- packages/facebook/facebook_server.js | 2 +- packages/github/github_client.js | 3 ++- packages/github/github_server.js | 2 +- packages/google/google_client.js | 3 ++- packages/google/google_server.js | 2 +- packages/meetup/meetup_client.js | 3 ++- packages/meetup/meetup_server.js | 2 +- packages/meteor-developer/meteor_developer_client.js | 4 +--- packages/meteor-developer/meteor_developer_server.js | 2 +- packages/oauth/oauth_server.js | 2 +- packages/oauth1/oauth1_server.js | 2 +- .../service_configuration_common.js | 10 ++++++++-- packages/twitter/twitter_client.js | 3 ++- packages/weibo/weibo_client.js | 3 ++- packages/weibo/weibo_server.js | 2 +- 16 files changed, 29 insertions(+), 19 deletions(-) diff --git a/packages/facebook/facebook_client.js b/packages/facebook/facebook_client.js index ed86eed72d5..5ea383f5a1e 100644 --- a/packages/facebook/facebook_client.js +++ b/packages/facebook/facebook_client.js @@ -15,7 +15,8 @@ Facebook.requestCredential = function (options, credentialRequestCompleteCallbac var config = ServiceConfiguration.configurations.findOne({service: 'facebook'}); if (!config) { - credentialRequestCompleteCallback && credentialRequestCompleteCallback(new ServiceConfiguration.ConfigError("Service not configured")); + credentialRequestCompleteCallback && credentialRequestCompleteCallback( + new ServiceConfiguration.ConfigError()); return; } diff --git a/packages/facebook/facebook_server.js b/packages/facebook/facebook_server.js index f9f7c1d6964..31bc9335fec 100644 --- a/packages/facebook/facebook_server.js +++ b/packages/facebook/facebook_server.js @@ -44,7 +44,7 @@ var isJSON = function (str) { var getTokenResponse = function (query) { var config = ServiceConfiguration.configurations.findOne({service: 'facebook'}); if (!config) - throw new ServiceConfiguration.ConfigError("Service not configured"); + throw new ServiceConfiguration.ConfigError(); var responseContent; try { diff --git a/packages/github/github_client.js b/packages/github/github_client.js index ccf06b2a5d0..4dd13fb7123 100644 --- a/packages/github/github_client.js +++ b/packages/github/github_client.js @@ -14,7 +14,8 @@ Github.requestCredential = function (options, credentialRequestCompleteCallback) var config = ServiceConfiguration.configurations.findOne({service: 'github'}); if (!config) { - credentialRequestCompleteCallback && credentialRequestCompleteCallback(new ServiceConfiguration.ConfigError("Service not configured")); + credentialRequestCompleteCallback && credentialRequestCompleteCallback( + new ServiceConfiguration.ConfigError()); return; } var credentialToken = Random.secret(); diff --git a/packages/github/github_server.js b/packages/github/github_server.js index 78e342f421f..071aa9340c0 100644 --- a/packages/github/github_server.js +++ b/packages/github/github_server.js @@ -24,7 +24,7 @@ if (Meteor.release) var getAccessToken = function (query) { var config = ServiceConfiguration.configurations.findOne({service: 'github'}); if (!config) - throw new ServiceConfiguration.ConfigError("Service not configured"); + throw new ServiceConfiguration.ConfigError(); var response; try { diff --git a/packages/google/google_client.js b/packages/google/google_client.js index b080264669b..51b80b8edbe 100644 --- a/packages/google/google_client.js +++ b/packages/google/google_client.js @@ -16,7 +16,8 @@ Google.requestCredential = function (options, credentialRequestCompleteCallback) var config = ServiceConfiguration.configurations.findOne({service: 'google'}); if (!config) { - credentialRequestCompleteCallback && credentialRequestCompleteCallback(new ServiceConfiguration.ConfigError("Service not configured")); + credentialRequestCompleteCallback && credentialRequestCompleteCallback( + new ServiceConfiguration.ConfigError()); return; } diff --git a/packages/google/google_server.js b/packages/google/google_server.js index 27c98321bbb..c9467b33bef 100644 --- a/packages/google/google_server.js +++ b/packages/google/google_server.js @@ -38,7 +38,7 @@ Oauth.registerService('google', 2, null, function(query) { var getTokens = function (query) { var config = ServiceConfiguration.configurations.findOne({service: 'google'}); if (!config) - throw new ServiceConfiguration.ConfigError("Service not configured"); + throw new ServiceConfiguration.ConfigError(); var response; try { diff --git a/packages/meetup/meetup_client.js b/packages/meetup/meetup_client.js index 277811d208d..b30c1a7cea5 100644 --- a/packages/meetup/meetup_client.js +++ b/packages/meetup/meetup_client.js @@ -13,7 +13,8 @@ Meetup.requestCredential = function (options, credentialRequestCompleteCallback) var config = ServiceConfiguration.configurations.findOne({service: 'meetup'}); if (!config) { - credentialRequestCompleteCallback && credentialRequestCompleteCallback(new ServiceConfiguration.ConfigError("Service not configured")); + credentialRequestCompleteCallback && credentialRequestCompleteCallback( + new ServiceConfiguration.ConfigError()); return; } var credentialToken = Random.secret(); diff --git a/packages/meetup/meetup_server.js b/packages/meetup/meetup_server.js index fe4aa80e8e4..e4d278753a2 100644 --- a/packages/meetup/meetup_server.js +++ b/packages/meetup/meetup_server.js @@ -17,7 +17,7 @@ Oauth.registerService('meetup', 2, null, function(query) { var getAccessToken = function (query) { var config = ServiceConfiguration.configurations.findOne({service: 'meetup'}); if (!config) - throw new ServiceConfiguration.ConfigError("Service not configured"); + throw new ServiceConfiguration.ConfigError(); var response; try { diff --git a/packages/meteor-developer/meteor_developer_client.js b/packages/meteor-developer/meteor_developer_client.js index 81a57d58c8d..0520143ee51 100644 --- a/packages/meteor-developer/meteor_developer_client.js +++ b/packages/meteor-developer/meteor_developer_client.js @@ -10,9 +10,7 @@ var requestCredential = function (credentialRequestCompleteCallback) { }); if (!config) { credentialRequestCompleteCallback && - credentialRequestCompleteCallback( - new ServiceConfiguration.ConfigError("Service not configured") - ); + credentialRequestCompleteCallback(new ServiceConfiguration.ConfigError()); return; } diff --git a/packages/meteor-developer/meteor_developer_server.js b/packages/meteor-developer/meteor_developer_server.js index 0a04d8a050b..12767fa331f 100644 --- a/packages/meteor-developer/meteor_developer_server.js +++ b/packages/meteor-developer/meteor_developer_server.js @@ -35,7 +35,7 @@ var getTokens = function (query) { service: 'meteor-developer' }); if (!config) - throw new ServiceConfiguration.ConfigError("Service not configured"); + throw new ServiceConfiguration.ConfigError(); var response; try { diff --git a/packages/oauth/oauth_server.js b/packages/oauth/oauth_server.js index 0c74f9d1587..bda3f5861c3 100644 --- a/packages/oauth/oauth_server.js +++ b/packages/oauth/oauth_server.js @@ -137,7 +137,7 @@ var oauthServiceName = function (req) { // Make sure we're configured var ensureConfigured = function(serviceName) { if (!ServiceConfiguration.configurations.findOne({service: serviceName})) { - throw new ServiceConfiguration.ConfigError("Service not configured"); + throw new ServiceConfiguration.ConfigError(); } }; diff --git a/packages/oauth1/oauth1_server.js b/packages/oauth1/oauth1_server.js index a75d88bb866..32334a0de33 100644 --- a/packages/oauth1/oauth1_server.js +++ b/packages/oauth1/oauth1_server.js @@ -3,7 +3,7 @@ Oauth._requestHandlers['1'] = function (service, query, res) { var config = ServiceConfiguration.configurations.findOne({service: service.serviceName}); if (!config) { - throw new ServiceConfiguration.ConfigError("Service " + service.serviceName + " not configured"); + throw new ServiceConfiguration.ConfigError(service.serviceName); } var urls = service.urls; diff --git a/packages/service-configuration/service_configuration_common.js b/packages/service-configuration/service_configuration_common.js index 93e1db08006..a9f6cb64fe2 100644 --- a/packages/service-configuration/service_configuration_common.js +++ b/packages/service-configuration/service_configuration_common.js @@ -18,8 +18,14 @@ ServiceConfiguration.configurations = new Meteor.Collection( // Thrown when trying to use a login service which is not configured -ServiceConfiguration.ConfigError = function(description) { - this.message = description; +ServiceConfiguration.ConfigError = function (serviceName) { + if (Meteor.isClient && !Accounts.loginServicesConfigured()) { + this.message = "Login service configuration not yet loaded"; + } else if (serviceName) { + this.message = "Service " + serviceName + " not configured"; + } else { + this.message = "Service not configured"; + } }; ServiceConfiguration.ConfigError.prototype = new Error(); ServiceConfiguration.ConfigError.prototype.name = 'ServiceConfiguration.ConfigError'; diff --git a/packages/twitter/twitter_client.js b/packages/twitter/twitter_client.js index f1553c9e960..5d8da022a21 100644 --- a/packages/twitter/twitter_client.js +++ b/packages/twitter/twitter_client.js @@ -14,7 +14,8 @@ Twitter.requestCredential = function (options, credentialRequestCompleteCallback var config = ServiceConfiguration.configurations.findOne({service: 'twitter'}); if (!config) { - credentialRequestCompleteCallback && credentialRequestCompleteCallback(new ServiceConfiguration.ConfigError("Service not configured")); + credentialRequestCompleteCallback && credentialRequestCompleteCallback( + new ServiceConfiguration.ConfigError()); return; } diff --git a/packages/weibo/weibo_client.js b/packages/weibo/weibo_client.js index feff8f29208..5a14a52a193 100644 --- a/packages/weibo/weibo_client.js +++ b/packages/weibo/weibo_client.js @@ -14,7 +14,8 @@ Weibo.requestCredential = function (options, credentialRequestCompleteCallback) var config = ServiceConfiguration.configurations.findOne({service: 'weibo'}); if (!config) { - credentialRequestCompleteCallback && credentialRequestCompleteCallback(new ServiceConfiguration.ConfigError("Service not configured")); + credentialRequestCompleteCallback && credentialRequestCompleteCallback( + new ServiceConfiguration.ConfigError()); return; } diff --git a/packages/weibo/weibo_server.js b/packages/weibo/weibo_server.js index 1d2f5256389..356cb2541be 100644 --- a/packages/weibo/weibo_server.js +++ b/packages/weibo/weibo_server.js @@ -33,7 +33,7 @@ Oauth.registerService('weibo', 2, null, function(query) { var getTokenResponse = function (query) { var config = ServiceConfiguration.configurations.findOne({service: 'weibo'}); if (!config) - throw new ServiceConfiguration.ConfigError("Service not configured"); + throw new ServiceConfiguration.ConfigError(); var response; try { From 3ad16722828ad351e1bfd2ace2718d3e0008c677 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Mon, 21 Apr 2014 14:20:24 -0700 Subject: [PATCH 142/219] Fix open redirector in oauth1 login flow. Clients are no longer allowed to specify callback URLs. --- History.md | 6 ++++++ packages/oauth1/oauth1_server.js | 6 ++++-- packages/twitter/twitter_client.js | 7 +------ 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/History.md b/History.md index ea5b25a44f9..2fc99261536 100644 --- a/History.md +++ b/History.md @@ -1,5 +1,11 @@ ## v.NEXT +## v0.8.0.1 + +* Fix security flaw in OAuth1 implementation. Clients can no longer + choose the callback_url for OAuth1 logins. + + ## v0.8.0 Meteor 0.8.0 introduces Blaze, a total rewrite of our live templating engine, diff --git a/packages/oauth1/oauth1_server.js b/packages/oauth1/oauth1_server.js index 6a61fbd6c71..c01b32a6541 100644 --- a/packages/oauth1/oauth1_server.js +++ b/packages/oauth1/oauth1_server.js @@ -16,13 +16,15 @@ Oauth._requestHandlers['1'] = function (service, query, res) { if (query.requestTokenAndRedirect) { // step 1 - get and store a request token + var callbackUrl = Meteor.absoluteUrl("_oauth/twitter?close&state=" + + query.state); // Get a request token to start auth process - oauthBinding.prepareRequestToken(query.requestTokenAndRedirect); + oauthBinding.prepareRequestToken(callbackUrl); // Keep track of request token so we can verify it on the next step requestTokens[query.state] = { - requestToken: oauthBinding.requestToken, + requestToken: oauthBinding.requestToken, requestTokenSecret: oauthBinding.requestTokenSecret }; diff --git a/packages/twitter/twitter_client.js b/packages/twitter/twitter_client.js index c8ca6fd44e5..d2d919a51b9 100644 --- a/packages/twitter/twitter_client.js +++ b/packages/twitter/twitter_client.js @@ -23,14 +23,9 @@ Twitter.requestCredential = function (options, credentialRequestCompleteCallback // a credentialToken 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.absoluteUrl('_oauth/twitter?close&state=' + credentialToken); - // url to app, enters "step 1" as described in // packages/accounts-oauth1-helper/oauth1_server.js - var loginUrl = '/_oauth/twitter/?requestTokenAndRedirect=' - + encodeURIComponent(callbackUrl) + var loginUrl = '/_oauth/twitter/?requestTokenAndRedirect=true' + '&state=' + credentialToken; Oauth.showPopup( From dd7c90d3aebbb376364288a8b602226e40d7396c Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Mon, 21 Apr 2014 14:47:00 -0700 Subject: [PATCH 143/219] Update docs and examples --- docs/.meteor/release | 2 +- docs/lib/release-override.js | 2 +- examples/clock/.meteor/release | 2 +- examples/leaderboard/.meteor/release | 2 +- examples/parties/.meteor/release | 2 +- examples/todos/.meteor/release | 2 +- examples/wordplay/.meteor/release | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/.meteor/release b/docs/.meteor/release index 6b184fbe05d..16d04896987 100644 --- a/docs/.meteor/release +++ b/docs/.meteor/release @@ -1 +1 @@ -0.8.0-rc3 +0.8.0.1-rc1 diff --git a/docs/lib/release-override.js b/docs/lib/release-override.js index b99159cbf3c..6ca2e658be9 100644 --- a/docs/lib/release-override.js +++ b/docs/lib/release-override.js @@ -1,5 +1,5 @@ // While galaxy apps are on their own special meteor releases, override // Meteor.release here. if (Meteor.isClient) { - Meteor.release = Meteor.release ? "0.7.2" : undefined; + Meteor.release = Meteor.release ? "0.8.0.1" : undefined; } diff --git a/examples/clock/.meteor/release b/examples/clock/.meteor/release index 621e94f0ec9..16d04896987 100644 --- a/examples/clock/.meteor/release +++ b/examples/clock/.meteor/release @@ -1 +1 @@ -none +0.8.0.1-rc1 diff --git a/examples/leaderboard/.meteor/release b/examples/leaderboard/.meteor/release index 7486fdbc50b..16d04896987 100644 --- a/examples/leaderboard/.meteor/release +++ b/examples/leaderboard/.meteor/release @@ -1 +1 @@ -0.7.2 +0.8.0.1-rc1 diff --git a/examples/parties/.meteor/release b/examples/parties/.meteor/release index 7486fdbc50b..16d04896987 100644 --- a/examples/parties/.meteor/release +++ b/examples/parties/.meteor/release @@ -1 +1 @@ -0.7.2 +0.8.0.1-rc1 diff --git a/examples/todos/.meteor/release b/examples/todos/.meteor/release index 7486fdbc50b..16d04896987 100644 --- a/examples/todos/.meteor/release +++ b/examples/todos/.meteor/release @@ -1 +1 @@ -0.7.2 +0.8.0.1-rc1 diff --git a/examples/wordplay/.meteor/release b/examples/wordplay/.meteor/release index 7486fdbc50b..16d04896987 100644 --- a/examples/wordplay/.meteor/release +++ b/examples/wordplay/.meteor/release @@ -1 +1 @@ -0.7.2 +0.8.0.1-rc1 From 41d36b671a2bbe3538b353738b2186607c3024c7 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Mon, 21 Apr 2014 14:50:56 -0700 Subject: [PATCH 144/219] Update banner text --- scripts/admin/banner.txt | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/scripts/admin/banner.txt b/scripts/admin/banner.txt index cb26735a338..92f42a493ea 100644 --- a/scripts/admin/banner.txt +++ b/scripts/admin/banner.txt @@ -1,7 +1,4 @@ -=> Meteor 0.8.0: Introducing Blaze, Meteor's new live templating engine! - Better integration with jQuery plugins, fine-grained updates, - reactive SVG support, and more! - https://github.com/meteor/meteor/wiki/Using-Blaze +=> Meteor 0.8.0.1: Fix security problem in Twitter OAuth flow. This release is being downloaded in the background. Update your - project to Meteor 0.8.0 by running 'meteor update'. + project to Meteor 0.8.0.1 by running 'meteor update'. From 700673592ce95fdcc8faea4302a7308ad9a8763e Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Mon, 21 Apr 2014 14:51:59 -0700 Subject: [PATCH 145/219] Update notices --- scripts/admin/notices.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/admin/notices.json b/scripts/admin/notices.json index a0374986de1..2a6e4d880fd 100644 --- a/scripts/admin/notices.json +++ b/scripts/admin/notices.json @@ -111,6 +111,9 @@ "http://madewith.meteor.com/ no longer supports app badges."] } }, + { + "release": "0.8.0.1" + }, { "release": "NEXT" } From 0e5e38f0066f3b85a3e3264ea6e0fc7c299982c1 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Mon, 21 Apr 2014 15:21:25 -0700 Subject: [PATCH 146/219] Update docs and examples to 0.8.0.1 --- docs/.meteor/release | 2 +- examples/clock/.meteor/release | 2 +- examples/leaderboard/.meteor/release | 2 +- examples/parties/.meteor/release | 2 +- examples/todos/.meteor/release | 2 +- examples/wordplay/.meteor/release | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/.meteor/release b/docs/.meteor/release index 16d04896987..4b324f2d8fa 100644 --- a/docs/.meteor/release +++ b/docs/.meteor/release @@ -1 +1 @@ -0.8.0.1-rc1 +0.8.0.1 diff --git a/examples/clock/.meteor/release b/examples/clock/.meteor/release index 16d04896987..4b324f2d8fa 100644 --- a/examples/clock/.meteor/release +++ b/examples/clock/.meteor/release @@ -1 +1 @@ -0.8.0.1-rc1 +0.8.0.1 diff --git a/examples/leaderboard/.meteor/release b/examples/leaderboard/.meteor/release index 16d04896987..4b324f2d8fa 100644 --- a/examples/leaderboard/.meteor/release +++ b/examples/leaderboard/.meteor/release @@ -1 +1 @@ -0.8.0.1-rc1 +0.8.0.1 diff --git a/examples/parties/.meteor/release b/examples/parties/.meteor/release index 16d04896987..4b324f2d8fa 100644 --- a/examples/parties/.meteor/release +++ b/examples/parties/.meteor/release @@ -1 +1 @@ -0.8.0.1-rc1 +0.8.0.1 diff --git a/examples/todos/.meteor/release b/examples/todos/.meteor/release index 16d04896987..4b324f2d8fa 100644 --- a/examples/todos/.meteor/release +++ b/examples/todos/.meteor/release @@ -1 +1 @@ -0.8.0.1-rc1 +0.8.0.1 diff --git a/examples/wordplay/.meteor/release b/examples/wordplay/.meteor/release index 16d04896987..4b324f2d8fa 100644 --- a/examples/wordplay/.meteor/release +++ b/examples/wordplay/.meteor/release @@ -1 +1 @@ -0.8.0.1-rc1 +0.8.0.1 From 25e3428132ea29293f9bb283c3dc58e6f075c275 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Mon, 21 Apr 2014 15:21:53 -0700 Subject: [PATCH 147/219] Add 0.7.2.1 to notices --- scripts/admin/notices.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/admin/notices.json b/scripts/admin/notices.json index 2a6e4d880fd..3d232543db0 100644 --- a/scripts/admin/notices.json +++ b/scripts/admin/notices.json @@ -94,6 +94,9 @@ { "release": "0.7.2" }, + { + "release": "0.7.2.1" + }, { "release": "0.8.0", "notices": [ From a9f65f0759ba87ab0eae86028f851e562757d6f7 Mon Sep 17 00:00:00 2001 From: Stephen Darnell Date: Sun, 12 May 2013 21:43:27 +0100 Subject: [PATCH 148/219] Stop node tar from including proprietary tags The linux tar whinges about unrecognised headers (though newer versions include an option to not warn). By building tar files without proprietary, we will be able to use files.createTarball() in more places. For example, undo commits 1c36bbaa79d76efd4817c5e1faeeed0c9429e0a3 and 1e2a40ef2bab5134ff2d6d813cd3ea5a0822a748 --- tools/files.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/files.js b/tools/files.js index 71dd92956e8..b90f293fec1 100644 --- a/tools/files.js +++ b/tools/files.js @@ -409,7 +409,7 @@ files.createTarGzStream = function (dirPath) { var fstream = require('fstream'); var zlib = require("zlib"); return fstream.Reader({ path: dirPath, type: 'Directory' }).pipe( - tar.Pack()).pipe(zlib.createGzip()); + tar.Pack({ noProprietary: true })).pipe(zlib.createGzip()); }; // Tar-gzips a directory into a tarball on disk, synchronously. From 3740d42f2d3aef3ab3d3882358ee278afcdf38d3 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 21 Apr 2014 18:50:44 -0700 Subject: [PATCH 149/219] Document Accounts.loginServicesConfigured Fixes #1051. See also #2048. --- docs/client/api.html | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/client/api.html b/docs/client/api.html index 02df066a033..ff1f70e0016 100644 --- a/docs/client/api.html +++ b/docs/client/api.html @@ -1710,6 +1710,11 @@

Accounts

Session.set('errorMessage', err.reason || 'Unknown error'); }); +Login service configuration is sent from the server to the client over DDP when +your app starts up; you may not call the login function until the configuration +is loaded. The function `Accounts.loginServicesConfigured()` is a reactive data +source that will return true once the login service is configured; you should +not make login buttons visible or active until it is true. {{> api_box currentUser}} From ce201682f0efaf48104082ad8e628a615d9ec385 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 21 Apr 2014 19:17:46 -0700 Subject: [PATCH 150/219] Warn about unready publications to spiderable docs Fixes #1149. --- docs/client/packages/spiderable.html | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/client/packages/spiderable.html b/docs/client/packages/spiderable.html index 9afade0eacd..d3adbd63393 100644 --- a/docs/client/packages/spiderable.html +++ b/docs/client/packages/spiderable.html @@ -33,6 +33,14 @@ `$PATH`. If you use `meteor deploy` this is already taken care of. {{/warning}} +{{#warning}} +When running your page, `spiderable` will wait for all publications +to be ready. Make sure that all of your [`publish functions`](#meteor_publish) +either return a cursor (or an array of cursors), or eventually call +[`this.ready()`](#publish_ready). Otherwise, the `phantomjs` executions +will fail. +{{/warning}} + {{/markdown}} From 3229f24ad029867de35d7300e4c56df19c09ff34 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 21 Apr 2014 19:27:27 -0700 Subject: [PATCH 151/219] Update Twitter configuration instructions again Also, advise turning on "Sign in with Twitter", which means that users won't get an authorization question every time they log in. Fixes #1164. --- packages/twitter/twitter_configure.html | 10 ++++++++++ packages/twitter/twitter_configure.js | 6 +++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/twitter/twitter_configure.html b/packages/twitter/twitter_configure.html index 67195547dbc..32820e4505e 100644 --- a/packages/twitter/twitter_configure.html +++ b/packages/twitter/twitter_configure.html @@ -9,5 +9,15 @@
  • Set Callback URL to: {{siteUrl}}_oauth/twitter?close
    • +
  • + Select "Create your Twitter application". +
    • +
  • + On the Settings tab, enable "Allow this application to be used to Sign in with Twitter" and click + "Update settings". +
    • +
  • + Switch to the API Keys tab. +
    • diff --git a/packages/twitter/twitter_configure.js b/packages/twitter/twitter_configure.js index e41329cbdec..c40ca0808d0 100644 --- a/packages/twitter/twitter_configure.js +++ b/packages/twitter/twitter_configure.js @@ -5,7 +5,7 @@ Template.configureLoginServiceDialogForTwitter.siteUrl = function () { Template.configureLoginServiceDialogForTwitter.fields = function () { return [ - {property: 'consumerKey', label: 'Consumer key'}, - {property: 'secret', label: 'Consumer secret'} + {property: 'consumerKey', label: 'API key'}, + {property: 'secret', label: 'API secret'} ]; -}; \ No newline at end of file +}; From af7aab41b675594fdb0bc57a636c30dc3d9ed6bc Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Mon, 21 Apr 2014 21:55:59 -0700 Subject: [PATCH 152/219] Remove unnecessary DOM update in test. --- packages/spacebars-tests/template_tests.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/spacebars-tests/template_tests.js b/packages/spacebars-tests/template_tests.js index 4a68ae7c8c4..5163676d24e 100644 --- a/packages/spacebars-tests/template_tests.js +++ b/packages/spacebars-tests/template_tests.js @@ -1951,7 +1951,6 @@ Tinytest.add( }; var div = renderToDiv(tmpl); - document.body.appendChild(div); var checkAttrs = function (url, isJavascriptProtocol) { Session.set(sessionKey, url); @@ -1993,6 +1992,5 @@ Tinytest.add( checkAttrs("javascript:alert(1)", false); checkAttrs("jAvAsCrIpT:alert(1)", false); checkAttrs(" javascript:alert(1)", false); - document.body.removeChild(div); } ); From 775ff19345667318667f615bfc331fd043d60669 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Mon, 21 Apr 2014 21:56:16 -0700 Subject: [PATCH 153/219] Only update URL attribute values after checking protocol. --- packages/ui/attrs.js | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/packages/ui/attrs.js b/packages/ui/attrs.js index 000ae986f6d..8a5bae05b5e 100644 --- a/packages/ui/attrs.js +++ b/packages/ui/attrs.js @@ -198,6 +198,23 @@ var isUrlAttribute = function (tagName, attrName) { return _.contains(urlAttrNames, attrName); }; +// To get the protocol for a URL, we let the browser normalize it for +// us, by setting it as the href for an anchor tag and then reading out +// the 'protocol' property. +if (Meteor.isClient) { + var anchorForNormalization = document.createElement('A'); +} + +var getProtocol = function (url) { + if (Meteor.isClient) { + anchorForNormalization.href = url; + return anchorForNormalization.protocol; + } else { + var parsed = Npm.require('url').parse(url); + return parsed.protocol; + } +}; + // UrlHandler is an attribute handler for all HTML attributes that take // URL values. It disallows javascript: URLs, unless // UI._allowJavascriptUrls() has been called. To detect javascript: @@ -212,25 +229,17 @@ var UrlHandler = AttributeHandler.extend({ update: function (element, oldValue, value) { var self = this; var args = arguments; - var isJavascriptProtocol = false; - origUpdate.apply(self, args); - - if (! UI._javascriptUrlsAllowed()) { - // If we're updating an A tag, we can read the 'protocol' property - // of the element itself. Otherwise, get the string value of the - // attribute and look for 'javascript:' at the beginning. - if (element.tagName === 'A') { - isJavascriptProtocol = element.protocol && - element.protocol === 'javascript:'; - } else { - isJavascriptProtocol = element[self.name] && - element[self.name].indexOf('javascript:') === 0; - } + if (UI._javascriptUrlsAllowed()) { + origUpdate.apply(self, args); + } else { + var isJavascriptProtocol = (getProtocol(value) === 'javascript:'); if (isJavascriptProtocol) { Meteor._debug("javascript: URLs are not allowed. " + "Use UI._allowJavascriptUrls() to enable them."); - origUpdate.apply(self, [element, value, null]); + origUpdate.apply(self, [element, oldValue, null]); + } else { + origUpdate.apply(self, args); } } } From c445b571349b4584cd5653e030cc7f370fd473ef Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Mon, 21 Apr 2014 22:11:02 -0700 Subject: [PATCH 154/219] Avoid relying on HTMLAnchorElement.protocol; browser support not clear. Also avoid url.format, since we don't actually need server-side URL normalization yet and it's not clear what, if any, normalization url.format does. --- packages/ui/attrs.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/ui/attrs.js b/packages/ui/attrs.js index 8a5bae05b5e..7961c603372 100644 --- a/packages/ui/attrs.js +++ b/packages/ui/attrs.js @@ -205,13 +205,12 @@ if (Meteor.isClient) { var anchorForNormalization = document.createElement('A'); } -var getProtocol = function (url) { +var normalizeUrl = function (url) { if (Meteor.isClient) { anchorForNormalization.href = url; - return anchorForNormalization.protocol; + return anchorForNormalization.href; } else { - var parsed = Npm.require('url').parse(url); - return parsed.protocol; + throw new Error('normalizeUrl not implemented on the server'); } }; @@ -233,7 +232,8 @@ var UrlHandler = AttributeHandler.extend({ if (UI._javascriptUrlsAllowed()) { origUpdate.apply(self, args); } else { - var isJavascriptProtocol = (getProtocol(value) === 'javascript:'); + var isJavascriptProtocol = + (normalizeUrl(value).indexOf('javascript:') === 0); if (isJavascriptProtocol) { Meteor._debug("javascript: URLs are not allowed. " + "Use UI._allowJavascriptUrls() to enable them."); From c68c77d51788822f4c528393072e072a3f6968df Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Tue, 22 Apr 2014 09:44:21 -0700 Subject: [PATCH 155/219] Add History entry for javascript: hrefs --- History.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/History.md b/History.md index 061b12dd1bd..0ad1621b0bc 100644 --- a/History.md +++ b/History.md @@ -29,6 +29,10 @@ * Add `meteor list-sites` command for listing the sites that you have deployed to meteor.com with your Meteor developer account. +* Blaze no longer renders javascript: URLs in attribute values by + default, to help prevent cross-site scripting bugs. Use + `UI._allowJavascriptUrls()` to allow them. + * Upgraded dependencies: - Node.js from 0.10.25 to 0.10.26. - MongoDB driver from 1.3.19 to 1.4.1 From 54f6d3654ae5fa0feedca97aa26888ca54bdf8a5 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Tue, 22 Apr 2014 10:28:27 -0700 Subject: [PATCH 156/219] Move userId out of AAD and into plaintext. We want to maintain compatibility with the node crypto module, which doesn't currently have an interface for specifying AAD. --- packages/accounts-base/accounts_server.js | 4 +-- packages/oauth-encryption/encrypt.js | 42 +++++++++++++---------- packages/oauth/oauth_server.js | 5 ++- 3 files changed, 27 insertions(+), 24 deletions(-) diff --git a/packages/accounts-base/accounts_server.js b/packages/accounts-base/accounts_server.js index a941612386b..9acd22c1bd4 100644 --- a/packages/accounts-base/accounts_server.js +++ b/packages/accounts-base/accounts_server.js @@ -900,9 +900,9 @@ var usingOAuthEncryption = function () { // OAuth service data is temporarily stored in the pending credentials // collection during the oauth authentication process. Sensitive data -// such as access tokens are encrypted without the user id AAD because +// such as access tokens are encrypted without the user id because // we don't know the user id yet. We re-encrypt these fields with the -// user id AAD included when storing the service data permanently in +// user id included when storing the service data permanently in // the users collection. // var pinEncryptedFieldsToUser = function (serviceData, userId) { diff --git a/packages/oauth-encryption/encrypt.js b/packages/oauth-encryption/encrypt.js index 4213742602a..1ad88cef11d 100644 --- a/packages/oauth-encryption/encrypt.js +++ b/packages/oauth-encryption/encrypt.js @@ -45,30 +45,30 @@ OAuthEncryption.loadKey = function (key) { }; -var userIdToAAD = function (userId) { - if (userId !== undefined) - return new Buffer(EJSON.stringify({userId: userId})); - else - return new Buffer([]); -}; - - // Encrypt `data`, which may be any EJSON-compatible object, using the // previously loaded OAuth secret key. // -// The `userId` argument is optional. If specified, it is used as the -// "additional authenticated data" (AAD). The encrypted ciphertext -// can only be decrypted by supplying the same user id, which prevents -// user specific credentials such as access tokens from being used by -// a different user. +// The `userId` argument is optional. The data is encrypted as { data: +// *, userId: * }. When the result of `seal` is passed to `open`, the +// same user id must be supplied, which prevents user specific +// credentials such as access tokens from being used by a different +// user. +// +// We would actually like the user id to be AAD (additional +// authenticated data), but the node crypto API does not currently have +// support for specifying AAD. // OAuthEncryption.seal = function (data, userId) { - if (! gcmKey) + if (! gcmKey) { throw new Error("No OAuth encryption key loaded"); - var plaintext = new Buffer(EJSON.stringify(data)); + } + + var plaintext = new Buffer(EJSON.stringify({ + data: data, + userId: userId + })); var iv = crypto.randomBytes(12); - var aad = userIdToAAD(userId); - var result = gcm.encrypt(gcmKey, iv, plaintext, aad); + var result = gcm.encrypt(gcmKey, iv, plaintext, new Buffer([]) /* aad */); return { iv: iv.toString("base64"), ciphertext: result.ciphertext.toString("base64"), @@ -118,7 +118,7 @@ OAuthEncryption.open = function (ciphertext, userId) { gcmKey, new Buffer(ciphertext.iv, "base64"), new Buffer(ciphertext.ciphertext, "base64"), - userIdToAAD(userId), + new Buffer([]), new Buffer(ciphertext.authTag, "base64") ); @@ -139,7 +139,11 @@ OAuthEncryption.open = function (ciphertext, userId) { throw new Error(); } - return data; + if (data.userId !== userId) { + throw new Error(); + } + + return data.data; } catch (e) { throw new Error("decryption failed"); } diff --git a/packages/oauth/oauth_server.js b/packages/oauth/oauth_server.js index 7fef3f850e8..57cc1bd6307 100644 --- a/packages/oauth/oauth_server.js +++ b/packages/oauth/oauth_server.js @@ -190,9 +190,8 @@ var usingOAuthEncryption = function () { // The user id is not specified because the user isn't known yet at // this point in the oauth authentication process. After the oauth // authentication process completes the encrypted service data fields -// will be re-encrypted with the user id as the AAD (additional -// authenticated data) before inserting the service data into the user -// document. +// will be re-encrypted with the user id included before inserting the +// service data into the user document. // OAuth.sealSecret = function (plaintext) { if (usingOAuthEncryption()) From 24e42e715b418f1d507ca84438bf0bbdae497841 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Tue, 22 Apr 2014 11:23:34 -0700 Subject: [PATCH 157/219] Remove Meteor._printDecryptionFailures. We can add it back in if users want it; for now it makes me nervous. --- packages/oauth-encryption/encrypt.js | 38 ++++++++-------------------- 1 file changed, 10 insertions(+), 28 deletions(-) diff --git a/packages/oauth-encryption/encrypt.js b/packages/oauth-encryption/encrypt.js index 1ad88cef11d..5ea6f04db61 100644 --- a/packages/oauth-encryption/encrypt.js +++ b/packages/oauth-encryption/encrypt.js @@ -87,30 +87,12 @@ OAuthEncryption.seal = function (data, userId) { // To prevent an attacker from breaking the encryption key by // observing the result of sending manipulated ciphertexts, `open` // throws "decryption unsuccessful" on any error. -// -// For developers working on new code which uses oauth-encryption -// (such as working on a new login service), it's painful not to be -// able to see the actual cause of failure. Setting -// `Meteor._printDecryptionFailure` displays the reason the decryption -// failed. This should never be set in production. (Developers who -// are simply using existing oauth and accounts packages wouldn't need -// to use this). -// -// XXX `Meteor._printDecryptionFailure` parallels livedata's -// `Meteor._printSentDDP` and `Meteor._printReceivedDDP`: debugging -// utilities very useful for development but ones we wouldn't want to -// run in production. We might like to have an API such as -// `Meteor.dev` which would be guaranteed to be an empty -// object in production. -// OAuthEncryption.open = function (ciphertext, userId) { if (! gcmKey) throw new Error("No OAuth encryption key loaded"); try { if (ciphertext.algorithm !== "aes-128-gcm") { - if (Meteor._printDecryptionFailure) - Meteor._debug("unsupported algorithm in OAuth ciphertext"); throw new Error(); } @@ -118,32 +100,32 @@ OAuthEncryption.open = function (ciphertext, userId) { gcmKey, new Buffer(ciphertext.iv, "base64"), new Buffer(ciphertext.ciphertext, "base64"), - new Buffer([]), + new Buffer([]), /* aad */ new Buffer(ciphertext.authTag, "base64") ); if (! result.auth_ok) { - if (Meteor._printDecryptionFailure) { - Meteor._debug("OAuth decryption unsuccessful"); - } throw new Error(); } + var err; var data; + try { data = EJSON.parse(result.plaintext.toString()); } catch (e) { - if (e instanceof SyntaxError && Meteor._printDecryptionFailure) { - Meteor._debug("OAuth decryption unsuccessful"); - } - throw new Error(); + err = new Error(); } if (data.userId !== userId) { - throw new Error(); + err = new Error(); } - return data.data; + if (err) { + throw err; + } else { + return data.data; + } } catch (e) { throw new Error("decryption failed"); } From 739cb078f79295e485284f95c6031b10d8a43c94 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Tue, 22 Apr 2014 11:24:06 -0700 Subject: [PATCH 158/219] Add a couple more oauth encryption tests --- packages/oauth-encryption/encrypt_tests.js | 29 ++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/packages/oauth-encryption/encrypt_tests.js b/packages/oauth-encryption/encrypt_tests.js index cafe17dbe97..a41d15897aa 100644 --- a/packages/oauth-encryption/encrypt_tests.js +++ b/packages/oauth-encryption/encrypt_tests.js @@ -91,6 +91,35 @@ Tinytest.add("oauth-encryption - open with wrong userId", function (test) { OAuthEncryption.loadKey(null); }); +Tinytest.add("oauth-encryption - seal and open with no userId", function (test) { + OAuthEncryption.loadKey( + new Buffer([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]). + toString("base64") + ); + var ciphertext = OAuthEncryption.seal({a: 1, b: 2}); + var decrypted = OAuthEncryption.open(ciphertext); + test.equal(decrypted, {a: 1, b: 2}); +}); + +Tinytest.add("oauth-encryption - open modified ciphertext", function (test) { + OAuthEncryption.loadKey( + new Buffer([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]). + toString("base64") + ); + var ciphertext = OAuthEncryption.seal({a: 1, b: 2}); + + var b = new Buffer(ciphertext.ciphertext, "base64"); + b[0] = b[0] ^ 1; + ciphertext.ciphertext = b.toString("base64"); + + test.throws( + function () { + OAuthEncryption.open(ciphertext); + }, + "decryption failed" + ); +}); + Tinytest.add("oauth-encryption - isSealed", function (test) { OAuthEncryption.loadKey( From 20f2ec4226ad54e6723edfd8f9fddb704c7866ee Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Tue, 22 Apr 2014 11:41:55 -0700 Subject: [PATCH 159/219] Add OAuth token encryption to pending token collections --- packages/oauth/pending_credentials.js | 16 +++++----------- packages/oauth1/oauth1_pending_request_tokens.js | 14 ++++++-------- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/packages/oauth/pending_credentials.js b/packages/oauth/pending_credentials.js index 11c5bdfd896..e841b06124f 100644 --- a/packages/oauth/pending_credentials.js +++ b/packages/oauth/pending_credentials.js @@ -32,22 +32,16 @@ var _cleanupHandle = Meteor.setInterval(_cleanStaleResults, 60 * 1000); // Stores the key and credential in the _pendingCredentials collection -// XXX After oauth token encryption is added to Meteor, apply it here too -var OAuthEncryption = Package["oauth-encryption"] && Package["oauth-encryption"].OAuthEncryption; - -var usingOAuthEncryption = function () { - return OAuthEncryption && OAuthEncryption.keyIsLoaded(); -}; - - -// Stores the token and credential in the _pendingCredentials collection // // @param key {string} // @param credential {string} The credential to store // OAuth._storePendingCredential = function (key, credential) { - if (credential instanceof Error) + if (credential instanceof Error) { credential = storableError(credential); + } else { + credential = OAuth.sealSecret(credential); + } OAuth._pendingCredentials.insert({ key: key, @@ -70,7 +64,7 @@ OAuth._retrievePendingCredential = function (key) { if (pendingCredential.credential.error) return recreateError(pendingCredential.credential.error); else - return pendingCredential.credential; + return OAuth.openSecret(pendingCredential.credential); } else { return undefined; } diff --git a/packages/oauth1/oauth1_pending_request_tokens.js b/packages/oauth1/oauth1_pending_request_tokens.js index 3d7fd3be337..217d7a6b3e7 100644 --- a/packages/oauth1/oauth1_pending_request_tokens.js +++ b/packages/oauth1/oauth1_pending_request_tokens.js @@ -40,7 +40,6 @@ var _cleanupHandle = Meteor.setInterval(_cleanStaleResults, 60 * 1000); // Stores the key and request token in the _pendingRequestTokens collection -// XXX After oauth token encryption is added to Meteor, apply it here too // // @param key {string} // @param requestToken {string} @@ -49,8 +48,8 @@ var _cleanupHandle = Meteor.setInterval(_cleanStaleResults, 60 * 1000); Oauth._storeRequestToken = function (key, requestToken, requestTokenSecret) { Oauth._pendingRequestTokens.insert({ key: key, - requestToken: requestToken, - requestTokenSecret: requestTokenSecret, + requestToken: OAuth.sealSecret(requestToken), + requestTokenSecret: OAuth.sealSecret(requestTokenSecret), createdAt: new Date() }); }; @@ -59,8 +58,6 @@ Oauth._storeRequestToken = function (key, requestToken, requestTokenSecret) { // Retrieves and removes a request token from the _pendingRequestTokens collection // Returns an object containing requestToken and requestTokenSecret properties // -// XXX After oauth token encryption is added to Meteor, apply it here too -// // @param key {string} // Oauth._retrieveRequestToken = function (key) { @@ -70,10 +67,11 @@ Oauth._retrieveRequestToken = function (key) { if (pendingRequestToken) { Oauth._pendingRequestTokens.remove({ _id: pendingRequestToken._id }); return { - requestToken: pendingRequestToken.requestToken, - requestTokenSecret: pendingRequestToken.requestTokenSecret + requestToken: OAuth.openSecret(pendingRequestToken.requestToken), + requestTokenSecret: OAuth.openSecret( + pendingRequestToken.requestTokenSecret) }; } else { return undefined; } -}; \ No newline at end of file +}; From 326180ce0a07984e821001f7aede652a76cc68a6 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Tue, 22 Apr 2014 11:44:13 -0700 Subject: [PATCH 160/219] Oauth -> OAuth --- .../oauth1/oauth1_pending_request_tokens.js | 18 +++++++++--------- packages/oauth1/oauth1_server.js | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/oauth1/oauth1_pending_request_tokens.js b/packages/oauth1/oauth1_pending_request_tokens.js index 217d7a6b3e7..76e8d88401e 100644 --- a/packages/oauth1/oauth1_pending_request_tokens.js +++ b/packages/oauth1/oauth1_pending_request_tokens.js @@ -19,13 +19,13 @@ // Collection containing pending request tokens // Has key, requestToken, requestTokenSecret, and createdAt fields. -Oauth._pendingRequestTokens = new Meteor.Collection( +OAuth._pendingRequestTokens = new Meteor.Collection( "meteor_oauth_pendingRequestTokens", { _preventAutopublish: true }); -Oauth._pendingRequestTokens._ensureIndex('key', {unique: 1}); -Oauth._pendingRequestTokens._ensureIndex('createdAt'); +OAuth._pendingRequestTokens._ensureIndex('key', {unique: 1}); +OAuth._pendingRequestTokens._ensureIndex('createdAt'); @@ -34,7 +34,7 @@ var _cleanStaleResults = function() { // Remove request tokens older than 5 minute var timeCutoff = new Date(); timeCutoff.setMinutes(timeCutoff.getMinutes() - 5); - Oauth._pendingRequestTokens.remove({ createdAt: { $lt: timeCutoff } }); + OAuth._pendingRequestTokens.remove({ createdAt: { $lt: timeCutoff } }); }; var _cleanupHandle = Meteor.setInterval(_cleanStaleResults, 60 * 1000); @@ -45,8 +45,8 @@ var _cleanupHandle = Meteor.setInterval(_cleanStaleResults, 60 * 1000); // @param requestToken {string} // @param requestTokenSecret {string} // -Oauth._storeRequestToken = function (key, requestToken, requestTokenSecret) { - Oauth._pendingRequestTokens.insert({ +OAuth._storeRequestToken = function (key, requestToken, requestTokenSecret) { + OAuth._pendingRequestTokens.insert({ key: key, requestToken: OAuth.sealSecret(requestToken), requestTokenSecret: OAuth.sealSecret(requestTokenSecret), @@ -60,12 +60,12 @@ Oauth._storeRequestToken = function (key, requestToken, requestTokenSecret) { // // @param key {string} // -Oauth._retrieveRequestToken = function (key) { +OAuth._retrieveRequestToken = function (key) { check(key, String); - var pendingRequestToken = Oauth._pendingRequestTokens.findOne({ key: key }); + var pendingRequestToken = OAuth._pendingRequestTokens.findOne({ key: key }); if (pendingRequestToken) { - Oauth._pendingRequestTokens.remove({ _id: pendingRequestToken._id }); + OAuth._pendingRequestTokens.remove({ _id: pendingRequestToken._id }); return { requestToken: OAuth.openSecret(pendingRequestToken.requestToken), requestTokenSecret: OAuth.openSecret( diff --git a/packages/oauth1/oauth1_server.js b/packages/oauth1/oauth1_server.js index 9085ab77c70..ab1d58c8a8a 100644 --- a/packages/oauth1/oauth1_server.js +++ b/packages/oauth1/oauth1_server.js @@ -18,7 +18,7 @@ OAuth._requestHandlers['1'] = function (service, query, res) { oauthBinding.prepareRequestToken(callbackUrl); // Keep track of request token so we can verify it on the next step - Oauth._storeRequestToken(query.state, + OAuth._storeRequestToken(query.state, oauthBinding.requestToken, oauthBinding.requestTokenSecret ); @@ -39,7 +39,7 @@ OAuth._requestHandlers['1'] = function (service, query, res) { // and close the window to allow the login handler to proceed // Get the user's request token so we can verify it and clear it - var requestTokenInfo = Oauth._retrieveRequestToken(query.state); + var requestTokenInfo = OAuth._retrieveRequestToken(query.state); // Verify user authorized access and the oauth_token matches // the requestToken from previous step From a8869d07ec4fc7771b82e83d632023a331be076a Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 22 Apr 2014 15:52:52 -0700 Subject: [PATCH 161/219] Provide a better error if ROOT_URL is not an URL Fixes #1404. --- packages/meteor/url_server.js | 13 +++++++++++-- tools/main.js | 11 +++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/packages/meteor/url_server.js b/packages/meteor/url_server.js index 81b55e776f0..9eee42f0c3a 100644 --- a/packages/meteor/url_server.js +++ b/packages/meteor/url_server.js @@ -1,6 +1,15 @@ if (process.env.ROOT_URL && typeof __meteor_runtime_config__ === "object") { __meteor_runtime_config__.ROOT_URL = process.env.ROOT_URL; - var pathPrefix = Npm.require('url').parse(__meteor_runtime_config__.ROOT_URL).pathname; - __meteor_runtime_config__.ROOT_URL_PATH_PREFIX = pathPrefix === "/" ? "" : pathPrefix; + if (__meteor_runtime_config__.ROOT_URL) { + var parsedUrl = Npm.require('url').parse(__meteor_runtime_config__.ROOT_URL); + // Sometimes users try to pass, eg, ROOT_URL=mydomain.com. + if (!parsedUrl.host) { + throw Error("$ROOT_URL, if specified, must be an URL"); + } + var pathPrefix = parsedUrl.pathname; + __meteor_runtime_config__.ROOT_URL_PATH_PREFIX = pathPrefix === "/" ? "" : pathPrefix; + } else { + __meteor_runtime_config__.ROOT_URL_PATH_PREFIX = ""; + } } diff --git a/tools/main.js b/tools/main.js index 5ec8a8622bb..4521e2b9f5e 100644 --- a/tools/main.js +++ b/tools/main.js @@ -337,6 +337,17 @@ Fiber(function () { process.exit(1); } + // This is a bit of a hack, but: if we don't check this in the tool, then the + // first time we do a unipackage.load, it will fail due to the check in the + // meteor package, and that'll look a lot uglier. + if (process.env.ROOT_URL) { + var parsedUrl = require('url').parse(process.env.ROOT_URL); + if (!parsedUrl.host) { + process.stderr.write('$ROOT_URL, if specified, must be an URL.\n'); + process.exit(1); + } + } + // Parse the arguments. // // We must first identify which options are boolean and which take From 3ad2a70a03228a9a7681dc64fd7d286c6accaee6 Mon Sep 17 00:00:00 2001 From: Avital Oliver Date: Tue, 22 Apr 2014 21:29:32 -0700 Subject: [PATCH 162/219] Truncate HTTP errors at 500 characters, not 180. I hit this personally with a misconfigured OAuth service and I couldn't diagnose the problem due to the message being too short. --- packages/http/httpcall_common.js | 2 +- packages/http/httpcall_tests.js | 4 ++-- packages/http/test_responder.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/http/httpcall_common.js b/packages/http/httpcall_common.js index a2fe4bdd2b0..5b6c4f9b660 100644 --- a/packages/http/httpcall_common.js +++ b/packages/http/httpcall_common.js @@ -1,5 +1,5 @@ makeErrorByStatus = function(statusCode, content) { - var MAX_LENGTH = 160; // if you change this, also change the appropriate test + var MAX_LENGTH = 500; // if you change this, also change the appropriate test var truncate = function(str, length) { return str.length > length ? str.slice(0, length) + '...' : str; diff --git a/packages/http/httpcall_tests.js b/packages/http/httpcall_tests.js index c05ad924825..08492544081 100644 --- a/packages/http/httpcall_tests.js +++ b/packages/http/httpcall_tests.js @@ -116,8 +116,8 @@ testAsyncMulti("httpcall - errors", [ // in test_responder.js we make a very long response body, to make sure // that we truncate messages. first of all, make sure we didn't make that // message too short, so that we can be sure we're verifying that we truncate. - test.isTrue(error.response.content.length > 180); - test.isTrue(error.message.length < 180); // make sure we truncate. + test.isTrue(error.response.content.length > 520); + test.isTrue(error.message.length < 520); // make sure we truncate. }; HTTP.call("GET", url_prefix()+"/fail", expect(error500Callback)); diff --git a/packages/http/test_responder.js b/packages/http/test_responder.js index 90940c1b53c..e3cc6575831 100644 --- a/packages/http/test_responder.js +++ b/packages/http/test_responder.js @@ -11,7 +11,7 @@ var respond = function(req, res) { } else if (req.url === "/fail") { res.statusCode = 500; res.end("SOME SORT OF SERVER ERROR. " + - "MAKE THIS VERY LONG TO MAKE SURE WE TRUNCATE. MAKE THIS VERY LONG TO MAKE SURE WE TRUNCATE. MAKE THIS VERY LONG TO MAKE SURE WE TRUNCATE. MAKE THIS VERY LONG TO MAKE SURE WE TRUNCATE. MAKE THIS VERY LONG TO MAKE SURE WE TRUNCATE. MAKE THIS VERY LONG TO MAKE SURE WE TRUNCATE. MAKE THIS VERY LONG TO MAKE SURE WE TRUNCATE. MAKE THIS VERY LONG TO MAKE SURE WE TRUNCATE. MAKE THIS VERY LONG TO MAKE SURE WE TRUNCATE. MAKE THIS VERY LONG TO MAKE SURE WE TRUNCATE. MAKE THIS VERY LONG TO MAKE SURE WE TRUNCATE. MAKE THIS VERY LONG TO MAKE SURE WE TRUNCATE. MAKE THIS VERY LONG TO MAKE SURE WE TRUNCATE. MAKE THIS VERY LONG TO MAKE SURE WE TRUNCATE. "); + _.join(_.times(100, "MAKE THIS LONG TO TEST THAT WE TRUNCATE"))); return; } else if (req.url === "/redirect") { res.statusCode = 301; From 471f09cbce1e8cf49d69a1d20fef47d42f9bdb97 Mon Sep 17 00:00:00 2001 From: Avital Oliver Date: Tue, 22 Apr 2014 21:39:46 -0700 Subject: [PATCH 163/219] Oops. The test shouldn't throw errors on the server. --- packages/http/test_responder.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/http/test_responder.js b/packages/http/test_responder.js index e3cc6575831..47239e6672e 100644 --- a/packages/http/test_responder.js +++ b/packages/http/test_responder.js @@ -10,8 +10,10 @@ var respond = function(req, res) { return; } else if (req.url === "/fail") { res.statusCode = 500; - res.end("SOME SORT OF SERVER ERROR. " + - _.join(_.times(100, "MAKE THIS LONG TO TEST THAT WE TRUNCATE"))); + res.end("SOME SORT OF SERVER ERROR. foo" + + _.times(100, function () { + return "MAKE THIS LONG TO TEST THAT WE TRUNCATE"; + }).join(' ')); return; } else if (req.url === "/redirect") { res.statusCode = 301; From d7a4e65226a7fa585c41a04991a59ef3c6741e3c Mon Sep 17 00:00:00 2001 From: David Glasser Date: Wed, 23 Apr 2014 12:53:12 -0700 Subject: [PATCH 164/219] rough draft of History update for 0.8.1 --- .mailmap | 9 ++++++ History.md | 83 +++++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 78 insertions(+), 14 deletions(-) diff --git a/.mailmap b/.mailmap index b50bae079a4..42f74158abd 100644 --- a/.mailmap +++ b/.mailmap @@ -8,7 +8,10 @@ # For any emails that show up in the shortlog that aren't in one of # these lists, figure out their GitHub username and add them. +GITHUB: aldeed GITHUB: AlexeyMK +GITHUB: apendua +GITHUB: arbesfeld GITHUB: DenisGorbachev GITHUB: EOT GITHUB: FooBarWidget @@ -20,10 +23,12 @@ GITHUB: awwx GITHUB: cmather GITHUB: codeinthehole GITHUB: dandv +GITHUB: davegonzalez GITHUB: emgee3 GITHUB: icellan GITHUB: jacott GITHUB: jfhamlin +GITHUB: justinsb GITHUB: marcandre GITHUB: mart-jansink GITHUB: meawoppl @@ -33,7 +38,9 @@ GITHUB: mitar GITHUB: mizzao GITHUB: mquandalle GITHUB: nathan-muir +GITHUB: Neftedollar GITHUB: paulswartz +GITHUB: Pent GITHUB: queso GITHUB: rdickert GITHUB: rgould @@ -50,6 +57,7 @@ METEOR: dgreensp METEOR: estark37 METEOR: estark37 METEOR: glasser +METEOR: glasser METEOR: gschmidt METEOR: karayu METEOR: n1mmy @@ -57,3 +65,4 @@ METEOR: sixolet METEOR: Slava METEOR: stubailo METEOR: ekatek + diff --git a/History.md b/History.md index d77fb1e4d0c..3301cc638a3 100644 --- a/History.md +++ b/History.md @@ -1,9 +1,11 @@ ## v.NEXT +#### Meteor Accounts + * Log out a user's other sessions when they change their password. -* Move boilerplate HTML from tools to webapp. Changes internal - Webapp.addHtmlAttributeHook API incompatibly. +* XXX emily: does remove-users-observe warrant an entry here? or is this what + the previous thing is? * Store pending OAuth login results in the database instead of in-memory, so that an OAuth flow succeeds even if different requests @@ -12,9 +14,6 @@ * When validateLoginAttempt callbacks return false, don't override a more specific error message. -* The oplog observe driver handles errors communicating with Mongo better and - knows to re-poll all queries during Mongo failovers. - * Add `Random.secret()` for generating security-critical secrets like login tokens. @@ -22,22 +21,78 @@ login tokens have actually been removed from the database, not when they have been marked for eventual removal. Fixes #1915. -* Add `meteor list-sites` command for listing the sites that you have - deployed to meteor.com with your Meteor developer account. +* Rename `Oauth` to `OAuth`. `Oauth` is now an alias for backwards + compatibility. + +* Add `oauth-encryption` package for encrypting sensitive account + credentials in the database. + +#### Blaze * Blaze no longer renders javascript: URLs in attribute values by default, to help prevent cross-site scripting bugs. Use `UI._allowJavascriptUrls()` to allow them. -* Upgraded dependencies: - - Node.js from 0.10.25 to 0.10.26. - - MongoDB driver from 1.3.19 to 1.4.1 +* Fix `UI.toHTML` on templates containing `{{#with}}` -* Rename `Oauth` to `OAuth`. `Oauth` is now an alias for backwards - compatibility. +* Fix {{#with}} over a data context that is mutated + +* Properly clean up autoruns on `UI.toHTML` + +* Add support for `{{!-- block comments --}}` in Spacebars. Block comments may + contain `}}`, so they are more useful than `{{! normal comments}}` for + commenting out sections of Spacebars templates. + XXX shouldn't this be in spacebars/README.md? + +* Kill TBODY special case in DomRange (XXX 45ac9b1a6d needs a better + description) + +* Do not ignore jquery-event extra parameters (? XXX b2193f5) + + +#### DDP and Mongo + +* DDP heartbeats + +* The oplog observe driver handles errors communicating with Mongo better and + knows to re-poll all queries during Mongo failovers. + +* Fix bugs involving mutating DDP method arguments. + + +#### meteor command-line tool + +* Move boilerplate HTML from tools to webapp. Changes internal + Webapp.addHtmlAttributeHook API incompatibly. + +* Add `meteor list-sites` command for listing the sites that you have + deployed to meteor.com with your Meteor developer account. + +* Third-party template languages can request that their generated source loads + before other JavaScript files, just like *.html files, by passing the + isTemplate option to Plugin.registerSourceHandler. + +* You can specify a particular interface for the dev mode runner to bind to with + `meteor -p host:port`. + +* Don't include proprietary tar tags in bundle tarballs. + +* Convert relative urls to absolute url when merging CSS files + + +#### Upgraded dependencies + +* Node.js from 0.10.25 to 0.10.26. +* MongoDB driver from 1.3.19 to 1.4.1 +* stylus: 0.42.3 (from 0.42.2) +* showdown: XXX (from XXX) +* css-parse: an unreleased version (from 1.7.0) +* css-stringify: an unreleased version (from 1.4.1) + + +Patches contributed by GitHub users aldeed, apendua, arbesfeld, awwx, dandv, +davegonzalez, justinsb, mquandalle, Neftedollar, Pent, sdarnell, and timhaines. -* Add `oauth-encryption` package for encrypting sensitive account - credentials in the database. ## v0.8.0.1 From 47b022841b40f5ca37adccc778ade373559519e5 Mon Sep 17 00:00:00 2001 From: Justin SB Date: Wed, 23 Apr 2014 16:04:15 -0700 Subject: [PATCH 165/219] Generalize reproducible inserted ID generation For example, calling `insert` inside a method body will now return consistent IDs on the client and the server, and latency compensation will work properly instead of producing flicker. Code that wants a random stream that is consistent between method stub and real method execution can get one with `DDP.randomStream`. --- History.md | 9 +- packages/livedata/DDP.md | 47 ++++ packages/livedata/livedata_common.js | 6 + packages/livedata/livedata_connection.js | 66 ++++-- .../livedata/livedata_connection_tests.js | 68 +++--- packages/livedata/livedata_server.js | 14 +- packages/livedata/package.js | 2 + packages/livedata/random_stream.js | 103 ++++++++ packages/livedata/random_stream_tests.js | 44 ++++ packages/mongo-livedata/collection.js | 37 ++- .../mongo-livedata/mongo_livedata_tests.js | 220 +++++++++++++++++- 11 files changed, 560 insertions(+), 56 deletions(-) create mode 100644 packages/livedata/random_stream.js create mode 100644 packages/livedata/random_stream_tests.js diff --git a/History.md b/History.md index 3301cc638a3..2aa9694303d 100644 --- a/History.md +++ b/History.md @@ -52,7 +52,14 @@ #### DDP and Mongo -* DDP heartbeats +* DDP heartbeats XXX + +* Generalize the mechanism by which client-side inserts generated IDs to support + latency compensation of generation of multiple random values. For example, + calling `insert` inside a method body will now return consistent IDs on the + client and the server. Code that wants a random stream that is consistent + between method stub and real method execution can get one with + `DDP.randomStream`. * The oplog observe driver handles errors communicating with Mongo better and knows to re-poll all queries during Mongo failovers. diff --git a/packages/livedata/DDP.md b/packages/livedata/DDP.md index 7337e90abf5..0cb9d137f95 100644 --- a/packages/livedata/DDP.md +++ b/packages/livedata/DDP.md @@ -187,6 +187,8 @@ a `pong` message. If the received `ping` message includes an `id` field, the - `method`: string (method name) - `params`: optional array of EJSON items (parameters to the method) - `id`: string (an arbitrary client-determined identifier for this method call) + - `randomSeed`: optional JSON value (an arbitrary client-determined seed + for pseudo-random generators) * `result` (server -> client): - `id`: string (the id passed to 'method') - `error`: optional Error (an error thrown by the method (or method-not-found) @@ -210,6 +212,19 @@ a `pong` message. If the received `ping` message includes an `id` field, the * There is no particular required ordering between `result` and `updated` messages for a method call. + * The client may provide a randomSeed JSON value. If provided, this value is + used to seed pseudo-random number generation. By using the same seed with + the same algorithm, the same pseudo-random values can be generated on the + client and the server. In particular, this is used for generating ids for + newly created documents. If randomSeed is not provided, then values + generated on the server and the client will not be identical. + + * Currently randomSeed is expected to be a string, and the algorithm by which + values are produced from this is not yet documented. It will likely be + formally specified in future when we are confident that the complete + requirements are known, or when a compatible implementation requires this + to be specified. + ## Errors: Errors appear in `result` and `nosub` messages in an optional error field. An @@ -277,3 +292,35 @@ of EJSON should not rely on key order, if possible. > MongoDB relies on key order. When using EJSON with MongoDB, the > implementation of EJSON must preserve key order. + + +## Appendix 2: randomSeed backward/forward compatibility + +randomSeed was added into DDP pre2, but it does not break backward or forward +compatibility. + +If the client stub and the server produce documents that are different in any +way, Meteor will reconcile this difference. This may cause 'flicker' in the UI +as the values change on the client to reflect what happened on the server, but +the final result will be correct: the server and the client will agree. + +Consistent id generation / randomSeed does not alter the syncing process, and +thus will (at worst) be the same: + +* If neither the server nor the client support randomSeed, we will get the +classical/flicker behavior. + +* If the client supports randomSeed, but the server does not, the server will +ignore randomSeed, as it ignores any unknown properties in a DDP method call. +Different ids will be generated, but this will be fixed by syncing. + +* If the server supports randomSeed, but the client does not, the server will +generate unseeded random values (providing a randomSeed is optional); different +ids will be generated; and again this will be fixed by syncing. + +* If both client and server support randomSeed, but different ids are +generated, either because the generation procedure is buggy, or the stub +behaves differently to the server, then syncing will fix this. + +* If both client and server support randomSeed, in the normal case the ids +generated will be the same, and syncing will be a no-op. diff --git a/packages/livedata/livedata_common.js b/packages/livedata/livedata_common.js index 9c59d0e5108..7bacf467c45 100644 --- a/packages/livedata/livedata_common.js +++ b/packages/livedata/livedata_common.js @@ -31,6 +31,12 @@ MethodInvocation = function (options) { // On the server, the connection this method call came in on. this.connection = options.connection; + + // The seed for randomStream value generation + this.randomSeed = options.randomSeed; + + // This is set by RandomStream.get; and holds the random stream state + this.randomStream = null; }; _.extend(MethodInvocation.prototype, { diff --git a/packages/livedata/livedata_connection.js b/packages/livedata/livedata_connection.js index e32a39e3fa4..abb717f0a5d 100644 --- a/packages/livedata/livedata_connection.js +++ b/packages/livedata/livedata_connection.js @@ -622,6 +622,17 @@ _.extend(Connection.prototype, { // onResultReceived: Function - a callback to call as soon as the method // result is received. the data written by // the method may not yet be in the cache! + // returnStubValue: Boolean - If true then in cases where we would have + // otherwise discarded the stub's return value + // and returned undefined, instead we go ahead + // and return it. Specifically, this is any + // time other than when (a) we are already + // inside a stub or (b) we are in Node and no + // callback was provided. Currently we require + // this flag to be explicitly passed to reduce + // the likelihood that stub return values will + // be confused with server return values; we + // may improve this in future. // @param callback {Optional Function} apply: function (name, args, options, callback) { var self = this; @@ -658,6 +669,27 @@ _.extend(Connection.prototype, { }; })(); + var enclosing = DDP._CurrentInvocation.get(); + var alreadyInSimulation = enclosing && enclosing.isSimulation; + + // Lazily generate a randomSeed, only if it is requested by the stub. + // The random streams only have utility if they're used on both the client + // and the server; if the client doesn't generate any 'random' values + // then we don't expect the server to generate any either. + // Less commonly, the server may perform different actions from the client, + // and may in fact generate values where the client did not, but we don't + // have any client-side values to match, so even here we may as well just + // use a random seed on the server. In that case, we don't pass the + // randomSeed to save bandwidth, and we don't even generate it to save a + // bit of CPU and to avoid consuming entropy. + var randomSeed = null; + var randomSeedGenerator = function () { + if (randomSeed === null) { + randomSeed = makeRpcSeed(enclosing, name); + } + return randomSeed; + }; + // Run the stub, if we have one. The stub is supposed to make some // temporary writes to the database to give the user a smooth experience // until the actual result of executing the method comes back from the @@ -670,18 +702,17 @@ _.extend(Connection.prototype, { // to do a RPC, so we use the return value of the stub as our return // value. - var enclosing = DDP._CurrentInvocation.get(); - var alreadyInSimulation = enclosing && enclosing.isSimulation; - var stub = self._methodHandlers[name]; if (stub) { var setUserId = function(userId) { self.setUserId(userId); }; + var invocation = new MethodInvocation({ isSimulation: true, userId: self.userId(), - setUserId: setUserId + setUserId: setUserId, + randomSeed: function () { return randomSeedGenerator(); } }); if (!alreadyInSimulation) @@ -690,7 +721,7 @@ _.extend(Connection.prototype, { try { // Note that unlike in the corresponding server code, we never audit // that stubs check() their arguments. - var ret = DDP._CurrentInvocation.withValue(invocation, function () { + var stubReturnValue = DDP._CurrentInvocation.withValue(invocation, function () { if (Meteor.isServer) { // Because saveOriginals and retrieveOriginals aren't reentrant, // don't allow stubs to yield. @@ -716,12 +747,12 @@ _.extend(Connection.prototype, { // we'll end up returning undefined. if (alreadyInSimulation) { if (callback) { - callback(exception, ret); + callback(exception, stubReturnValue); return undefined; } if (exception) throw exception; - return ret; + return stubReturnValue; } // If an exception occurred in a stub, and we're ignoring it @@ -756,18 +787,25 @@ _.extend(Connection.prototype, { // Send the RPC. Note that on the client, it is important that the // stub have finished before we send the RPC, so that we know we have // a complete list of which local documents the stub wrote. + var message = { + msg: 'method', + method: name, + params: args, + id: methodId() + }; + + // Send the randomSeed only if we used it + if (randomSeed !== null) { + message.randomSeed = randomSeed; + } + var methodInvoker = new MethodInvoker({ methodId: methodId(), callback: callback, connection: self, onResultReceived: options.onResultReceived, wait: !!options.wait, - message: { - msg: 'method', - method: name, - params: args, - id: methodId() - } + message: message }); if (options.wait) { @@ -792,7 +830,7 @@ _.extend(Connection.prototype, { if (future) { return future.wait(); } - return undefined; + return options.returnStubValue ? stubReturnValue : undefined; }, // Before calling a method stub, prepare all stores to track changes and allow diff --git a/packages/livedata/livedata_connection_tests.js b/packages/livedata/livedata_connection_tests.js index ea84baa2987..1c404669b60 100644 --- a/packages/livedata/livedata_connection_tests.js +++ b/packages/livedata/livedata_connection_tests.js @@ -17,12 +17,16 @@ var makeConnectMessage = function (session) { return msg; } +// Tests that stream got a message that matches expected. +// Expected is normally an object, and allows a wildcard value of '*', +// which will then match any value. +// Returns the message (parsed as a JSON object if expected is an object); +// which is particularly handy if you want to extract a value that was +// matched as a wildcard. var testGotMessage = function (test, stream, expected) { - var retVal = undefined; - if (stream.sent.length === 0) { test.fail({error: 'no message received', expected: expected}); - return retVal; + return undefined; } var got = stream.sent.shift(); @@ -42,17 +46,10 @@ var testGotMessage = function (test, stream, expected) { _.each(keysWithStarValues, function (k) { expected[k] = got[k]; }); - if (keysWithStarValues.length === 1) { - retVal = got[keysWithStarValues[0]]; - } else { - retVal = _.map(keysWithStarValues, function (k) { - return got[k]; - }); - } } test.equal(got, expected); - return retVal; + return got; }; var startAndConnect = function(test, stream) { @@ -275,6 +272,7 @@ Tinytest.add("livedata stub - this", function (test) { conn.call('test_this', _.identity); // satisfy method, quiesce connection var message = JSON.parse(stream.sent.shift()); + test.isUndefined(message.randomSeed); test.equal(message, {msg: 'method', method: 'test_this', params: [], id:message.id}); test.length(stream.sent, 0); @@ -322,9 +320,11 @@ if (Meteor.isClient) { test.equal(counts, {added: 1, removed: 0, changed: 0, moved: 0}); // get response from server - var message = JSON.parse(stream.sent.shift()); - test.equal(message, {msg: 'method', method: 'do_something', - params: ['friday!'], id:message.id}); + var message = testGotMessage(test, stream, {msg: 'method', + method: 'do_something', + params: ['friday!'], + id: '*', + randomSeed: '*'}); test.equal(coll.find({}).count(), 1); test.equal(coll.find({value: 'friday!'}).count(), 1); @@ -357,6 +357,7 @@ if (Meteor.isClient) { // test we still send a method request to server var message2 = JSON.parse(stream.sent.shift()); + test.isUndefined(message2.randomSeed); test.equal(message2, {msg: 'method', method: 'do_something_else', params: ['monday'], id: message2.id}); @@ -401,6 +402,7 @@ Tinytest.add("livedata stub - mutating method args", function (test) { // Method should be called with original arg, not mutated arg. var message = JSON.parse(stream.sent.shift()); + test.isUndefined(message.randomSeed); test.equal(message, {msg: 'method', method: 'mutateArgs', params: [{foo: 50}], id: message.id}); test.length(stream.sent, 0); @@ -453,9 +455,11 @@ if (Meteor.isClient) { conn.call('do_something', _.identity); // see we only send message for outer methods - var message = JSON.parse(stream.sent.shift()); - test.equal(message, {msg: 'method', method: 'do_something', - params: [], id:message.id}); + var message = testGotMessage(test, stream, {msg: 'method', + method: 'do_something', + params: [], + id: '*', + randomSeed: '*'}); test.length(stream.sent, 0); // but inner method runs locally. @@ -566,6 +570,7 @@ Tinytest.add("livedata stub - reconnect", function (test) { // The non-wait method should send, but not the wait method. var methodMessage = JSON.parse(stream.sent.shift()); + test.isUndefined(methodMessage.randomSeed); test.equal(methodMessage, {msg: 'method', method: 'do_something', params: [], id:methodMessage.id}); test.equal(stream.sent.length, 0); @@ -623,6 +628,7 @@ Tinytest.add("livedata stub - reconnect", function (test) { o.expectCallbacks({added: 1, changed: 1}); var waitMethodMessage = JSON.parse(stream.sent.shift()); + test.isUndefined(waitMethodMessage.randomSeed); test.equal(waitMethodMessage, {msg: 'method', method: 'do_something_else', params: [], id: waitMethodMessage.id}); test.equal(stream.sent.length, 0); @@ -633,6 +639,7 @@ Tinytest.add("livedata stub - reconnect", function (test) { // wait method done means we can send the third method test.equal(stream.sent.length, 1); var laterMethodMessage = JSON.parse(stream.sent.shift()); + test.isUndefined(laterMethodMessage.randomSeed); test.equal(laterMethodMessage, {msg: 'method', method: 'do_something_later', params: [], id: laterMethodMessage.id}); @@ -677,7 +684,7 @@ if (Meteor.isClient) { // Method sent. var methodId = testGotMessage( test, stream, {msg: 'method', method: 'writeSomething', - params: [], id: '*'}); + params: [], id: '*', randomSeed: '*'}).id; test.equal(stream.sent.length, 0); // Get some data. @@ -744,7 +751,7 @@ if (Meteor.isClient) { // Method sent. var methodId2 = testGotMessage( test, stream, {msg: 'method', method: 'writeSomething', - params: [], id: '*'}); + params: [], id: '*', randomSeed: '*'}).id; test.equal(stream.sent.length, 0); // Get some data. @@ -777,7 +784,7 @@ if (Meteor.isClient) { testGotMessage(test, stream, makeConnectMessage(SESSION_ID + 1)); var slowMethodId = testGotMessage( test, stream, - {msg: 'method', method: 'slowMethod', params: [], id: '*'}); + {msg: 'method', method: 'slowMethod', params: [], id: '*'}).id; // Still holding out hope for session resumption, so nothing updated yet. test.equal(coll.find().count(), 2); test.equal(coll.findOne(stubWrittenId2), {_id: stubWrittenId2, foo: 'bar'}); @@ -840,7 +847,7 @@ Tinytest.add("livedata stub - reconnect method which only got data", function (t // Method sent. var methodId = testGotMessage( test, stream, {msg: 'method', method: 'doLittle', - params: [], id: '*'}); + params: [], id: '*'}).id; test.equal(stream.sent.length, 0); // Get some data. @@ -930,7 +937,7 @@ if (Meteor.isClient) { // Method sent. var insertMethodId = testGotMessage( test, stream, {msg: 'method', method: 'insertSomething', - params: [], id: '*'}); + params: [], id: '*', randomSeed: '*'}).id; test.equal(stream.sent.length, 0); // Call update method. @@ -943,7 +950,7 @@ if (Meteor.isClient) { // Method sent. var updateMethodId = testGotMessage( test, stream, {msg: 'method', method: 'updateIt', - params: [stubWrittenId], id: '*'}); + params: [stubWrittenId], id: '*'}).id; test.equal(stream.sent.length, 0); // Get some data... slightly different than what we wrote. @@ -1019,7 +1026,7 @@ if (Meteor.isClient) { // first method sent var firstMethodId = testGotMessage( test, stream, {msg: 'method', method: 'no-op', - params: [], id: '*'}); + params: [], id: '*'}).id; test.equal(stream.sent.length, 0); // ack the first method @@ -1029,7 +1036,7 @@ if (Meteor.isClient) { // Wait method sent. var waitMethodId = testGotMessage( test, stream, {msg: 'method', method: 'no-op', - params: [], id: '*'}); + params: [], id: '*'}).id; test.equal(stream.sent.length, 0); // ack the wait method @@ -1039,7 +1046,7 @@ if (Meteor.isClient) { // insert method sent. var insertMethodId = testGotMessage( test, stream, {msg: 'method', method: 'insertSomething', - params: [], id: '*'}); + params: [], id: '*', randomSeed: '*'}).id; test.equal(stream.sent.length, 0); // ack the insert method @@ -1448,7 +1455,7 @@ Tinytest.add("livedata connection - onReconnect with sent messages", function(te // Test that we sent just the login message. var loginId = testGotMessage( test, stream, {msg: 'method', method: 'do_something', - params: ['login'], id: '*'}); + params: ['login'], id: '*'}).id; // we connect. stream.receive({msg: 'connected', session: Random.id()}); @@ -1463,7 +1470,7 @@ Tinytest.add("livedata connection - onReconnect with sent messages", function(te testGotMessage( test, stream, {msg: 'method', method: 'do_something', - params: ['one'], id: '*'}); + params: ['one'], id: '*'}).id; }); @@ -1488,7 +1495,7 @@ Tinytest.add("livedata stub - reconnect double wait method", function (test) { // Method sent. var halfwayId = testGotMessage( test, stream, {msg: 'method', method: 'halfwayMethod', - params: [], id: '*'}); + params: [], id: '*'}).id; test.equal(stream.sent.length, 0); // Get the result. This means it will not be resent. @@ -1502,7 +1509,7 @@ Tinytest.add("livedata stub - reconnect double wait method", function (test) { testGotMessage(test, stream, makeConnectMessage(SESSION_ID)); var reconnectId = testGotMessage( test, stream, {msg: 'method', method: 'reconnectMethod', - params: [], id: '*'}); + params: [], id: '*'}).id; test.length(stream.sent, 0); // Still holding out hope for session resumption, so no callbacks yet. test.equal(output, []); @@ -1593,6 +1600,7 @@ if (Meteor.isClient) { test.equal(coll.findOne(), {_id: "foo", bar: 42}); // It also sends the method message. var methodMessage = JSON.parse(stream.sent.shift()); + test.isUndefined(methodMessage.randomSeed); test.equal(methodMessage, {msg: 'method', method: '/' + collName + '/insert', params: [{_id: "foo", bar: 42}], id: methodMessage.id}); diff --git a/packages/livedata/livedata_server.js b/packages/livedata/livedata_server.js index 74d4206db15..fff6114866d 100644 --- a/packages/livedata/livedata_server.js +++ b/packages/livedata/livedata_server.js @@ -595,14 +595,18 @@ _.extend(Session.prototype, { var self = this; // reject malformed messages - // XXX should also reject messages with unknown attributes? + // For now, we silently ignore unknown attributes, + // for forwards compatibility. if (typeof (msg.id) !== "string" || typeof (msg.method) !== "string" || - (('params' in msg) && !(msg.params instanceof Array))) { + (('params' in msg) && !(msg.params instanceof Array)) || + (('randomSeed' in msg) && (typeof msg.randomSeed !== "string"))) { self.sendError("Malformed method invocation", msg); return; } + var randomSeed = msg.randomSeed || null; + // set up to mark the method as satisfied once all observers // (and subscriptions) have reacted to any writes that were // done. @@ -637,7 +641,8 @@ _.extend(Session.prototype, { userId: self.userId, setUserId: setUserId, unblock: unblock, - connection: self.connectionHandle + connection: self.connectionHandle, + randomSeed: randomSeed }); try { var result = DDPServer._CurrentWriteFence.withValue(fence, function () { @@ -1394,7 +1399,8 @@ _.extend(Server.prototype, { isSimulation: false, userId: userId, setUserId: setUserId, - connection: connection + connection: connection, + randomSeed: makeRpcSeed(currentInvocation, name) }); try { var result = DDP._CurrentInvocation.withValue(invocation, function () { diff --git a/packages/livedata/package.js b/packages/livedata/package.js index 3fe9b835e4a..8eaa60f32b2 100644 --- a/packages/livedata/package.js +++ b/packages/livedata/package.js @@ -57,6 +57,7 @@ Package.on_use(function (api) { api.add_files('crossbar.js', 'server'); api.add_files('livedata_common.js', ['client', 'server']); + api.add_files('random_stream.js', ['client', 'server']); api.add_files('livedata_connection.js', ['client', 'server']); @@ -78,6 +79,7 @@ Package.on_test(function (api) { api.add_files('livedata_test_service.js', ['client', 'server']); api.add_files('session_view_tests.js', ['server']); api.add_files('crossbar_tests.js', ['server']); + api.add_files('random_stream_tests.js', ['client', 'server']); api.use('http', 'client'); api.add_files(['stream_tests.js'], 'client'); diff --git a/packages/livedata/random_stream.js b/packages/livedata/random_stream.js new file mode 100644 index 00000000000..b09a6eda2e5 --- /dev/null +++ b/packages/livedata/random_stream.js @@ -0,0 +1,103 @@ +// RandomStream allows for generation of pseudo-random values, from a seed. +// +// We use this for consistent 'random' numbers across the client and server. +// We want to generate probably-unique IDs on the client, and we ideally want +// the server to generate the same IDs when it executes the method. +// +// For generated values to be the same, we must seed ourselves the same way, +// and we must keep track of the current state of our pseudo-random generators. +// We call this state the scope. By default, we use the current DDP method +// invocation as our scope. DDP now allows the client to specify a randomSeed. +// If a randomSeed is provided it will be used to seed our random sequences. +// In this way, client and server method calls will generate the same values. +// +// We expose multiple named streams; each stream is independent +// and is seeded differently (but predictably from the name). +// By using multiple streams, we support reordering of requests, +// as long as they occur on different streams. +// +// @param options {Optional Object} +// seed: Array or value - Seed value(s) for the generator. +// If an array, will be used as-is +// If a value, will be converted to a single-value array +// If omitted, a random array will be used as the seed. +RandomStream = function (options) { + var self = this; + + this.seed = [].concat(options.seed || randomToken()); + + this.sequences = {}; +}; + +// Returns a random string of sufficient length for a random seed. +// This is a placeholder function; a similar function is planned +// for Random itself; when that is added we should remove this function, +// and call Random's randomToken instead. +function randomToken() { + return Random.hexString(20); +}; + +// Returns the random stream with the specified name, in the specified scope. +// If scope is null (or otherwise falsey) then we will use Random, which will +// give us as random numbers as possible, but won't produce the same +// values across client and server. +// However, scope will normally be the current DDP method invocation, so +// we'll use the stream with the specified name, and we should get consistent +// values on the client and server sides of a method call. +RandomStream.get = function (scope, name) { + if (!name) { + name = "default"; + } + if (!scope) { + // There was no scope passed in; + // the sequence won't actually be reproducible. + return Random; + } + var randomStream = scope.randomStream; + if (!randomStream) { + scope.randomStream = randomStream = new RandomStream({ + seed: scope.randomSeed + }); + } + return randomStream._sequence(name); +}; + +// Returns the named sequence of pseudo-random values. +// The scope will be DDP._CurrentInvocation.get(), so the stream will produce +// consistent values for method calls on the client and server. +DDP.randomStream = function (name) { + var scope = DDP._CurrentInvocation.get(); + return RandomStream.get(scope, name); +}; + +// Creates a randomSeed for passing to a method call. +// Note that we take enclosing as an argument, +// though we expect it to be DDP._CurrentInvocation.get() +// However, we often evaluate makeRpcSeed lazily, and thus the relevant +// invocation may not be the one currently in scope. +// If enclosing is null, we'll use Random and values won't be repeatable. +makeRpcSeed = function (enclosing, methodName) { + var stream = RandomStream.get(enclosing, '/rpc/' + methodName); + return stream.hexString(20); +}; + +_.extend(RandomStream.prototype, { + // Get a random sequence with the specified name, creating it if does not exist. + // New sequences are seeded with the seed concatenated with the name. + // By passing a seed into Random.create, we use the Alea generator. + _sequence: function (name) { + var self = this; + + var sequence = self.sequences[name] || null; + if (sequence === null) { + var sequenceSeed = self.seed.concat(name); + for (var i = 0; i < sequenceSeed.length; i++) { + if (_.isFunction(sequenceSeed[i])) { + sequenceSeed[i] = sequenceSeed[i](); + } + } + self.sequences[name] = sequence = Random.createWithSeeds.apply(null, sequenceSeed); + } + return sequence; + } +}); diff --git a/packages/livedata/random_stream_tests.js b/packages/livedata/random_stream_tests.js new file mode 100644 index 00000000000..67f95f1e8e1 --- /dev/null +++ b/packages/livedata/random_stream_tests.js @@ -0,0 +1,44 @@ +Tinytest.add("livedata - DDP.randomStream", function (test) { + var randomSeed = Random.id(); + var context = { randomSeed: randomSeed }; + + var sequence = DDP._CurrentInvocation.withValue(context, function () { + return DDP.randomStream('1'); + }); + + var seeds = sequence.alea.args; + + test.equal(seeds.length, 2); + test.equal(seeds[0], randomSeed); + test.equal(seeds[1], '1'); + + var id1 = sequence.id(); + + // Clone the sequence by building it the same way RandomStream.get does + var sequenceClone = Random.createWithSeeds.apply(null, seeds); + var id1Cloned = sequenceClone.id(); + var id2Cloned = sequenceClone.id(); + test.equal(id1, id1Cloned); + + // We should get the same sequence when we use the same key + sequence = DDP._CurrentInvocation.withValue(context, function () { + return DDP.randomStream('1'); + }); + seeds = sequence.alea.args; + test.equal(seeds.length, 2); + test.equal(seeds[0], randomSeed); + test.equal(seeds[1], '1'); + + // But we should be at the 'next' position in the stream + var id2 = sequence.id(); + + // Technically these could be equal, but likely to be a bug if hit + // http://search.dilbert.com/comic/Random%20Number%20Generator + test.notEqual(id1, id2); + + test.equal(id2, id2Cloned); +}); + +Tinytest.add("livedata - DDP.randomStream with no-args", function (test) { + DDP.randomStream().id(); +}); diff --git a/packages/mongo-livedata/collection.js b/packages/mongo-livedata/collection.js index 4eac2acd322..14794f59d23 100644 --- a/packages/mongo-livedata/collection.js +++ b/packages/mongo-livedata/collection.js @@ -40,13 +40,15 @@ Meteor.Collection = function (name, options) { switch (options.idGeneration) { case 'MONGO': self._makeNewID = function () { - return new Meteor.Collection.ObjectID(); + var src = name ? DDP.randomStream('/collection/' + name) : Random; + return new Meteor.Collection.ObjectID(src.hexString(24)); }; break; case 'STRING': default: self._makeNewID = function () { - return Random.id(); + var src = name ? DDP.randomStream('/collection/' + name) : Random; + return src.id(); }; break; } @@ -374,7 +376,19 @@ _.each(["insert", "update", "remove"], function (name) { || insertId instanceof Meteor.Collection.ObjectID)) throw new Error("Meteor requires document _id fields to be non-empty strings or ObjectIDs"); } else { - insertId = args[0]._id = self._makeNewID(); + var generateId = true; + // Don't generate the id if we're the client and the 'outermost' call + // This optimization saves us passing both the randomSeed and the id + // Passing both is redundant. + if (self._connection && self._connection !== Meteor.server) { + var enclosing = DDP._CurrentInvocation.get(); + if (!enclosing) { + generateId = false; + } + } + if (generateId) { + insertId = args[0]._id = self._makeNewID(); + } } } else { args[0] = Meteor.Collection._rewriteSelector(args[0]); @@ -401,10 +415,14 @@ _.each(["insert", "update", "remove"], function (name) { // On inserts, always return the id that we generated; on all other // operations, just return the result from the collection. var chooseReturnValueFromCollectionResult = function (result) { - if (name === "insert") + if (name === "insert") { + if (!insertId && result) { + insertId = result; + } return insertId; - else + } else { return result; + } }; var wrappedCallback; @@ -444,7 +462,7 @@ _.each(["insert", "update", "remove"], function (name) { } ret = chooseReturnValueFromCollectionResult( - self._connection.apply(self._prefix + name, args, wrappedCallback) + self._connection.apply(self._prefix + name, args, {returnStubValue: true}, wrappedCallback) ); } else { @@ -638,6 +656,13 @@ Meteor.Collection.prototype._defineMutationMethods = function() { // All the methods do their own validation, instead of using check(). check(arguments, [Match.Any]); try { + if (method === "insert") { + // Ensure that we have an id on an insert + if (!_.has(arguments[0], '_id')) { + arguments[0]._id = self._makeNewID(); + } + } + if (this.isSimulation) { // In a client simulation, you can do any mutation (even with a diff --git a/packages/mongo-livedata/mongo_livedata_tests.js b/packages/mongo-livedata/mongo_livedata_tests.js index 8f76c4965d7..4ec23a95a39 100644 --- a/packages/mongo-livedata/mongo_livedata_tests.js +++ b/packages/mongo-livedata/mongo_livedata_tests.js @@ -2,6 +2,10 @@ // the selector (or inserted document) contains fail: true. var TRANSFORMS = {}; + +// We keep track of the collections, so we can refer to them by name +var COLLECTIONS = {}; + if (Meteor.isServer) { Meteor.methods({ createInsecureCollection: function (name, options) { @@ -15,6 +19,7 @@ if (Meteor.isServer) { options.transform = TRANSFORMS[options.transformName]; } var c = new Meteor.Collection(name, options); + COLLECTIONS[name] = c; c._insecure = true; Meteor.publish('c-' + name, function () { return c.find(); @@ -23,6 +28,32 @@ if (Meteor.isServer) { }); } +// We store the generated id, keyed by collection, for each insert +// This is so we can test the stub and the server generate the same id +var INSERTED_IDS = {}; + +Meteor.methods({ + insertObjects: function (collectionName, doc, count) { + var c = COLLECTIONS[collectionName]; + var ids = []; + for (var i = 0; i < count; i++) { + var id = c.insert(doc); + INSERTED_IDS[collectionName] = (INSERTED_IDS[collectionName] || []).concat([id]); + ids.push(id); + } + return ids; + }, + upsertObject: function (collectionName, selector, modifier) { + var c = COLLECTIONS[collectionName]; + return c.upsert(selector, modifier); + }, + doMeteorCall: function (name /*, arguments */) { + var args = Array.prototype.slice.call(arguments); + + return Meteor.call.apply(null, args); + } +}); + var runInFence = function (f) { if (Meteor.isClient) { f(); @@ -1239,7 +1270,7 @@ testAsyncMulti('mongo-livedata - document with length, ' + idGeneration, [ var self = this; var collectionName = Random.id(); if (Meteor.isClient) { - Meteor.call('createInsecureCollection', collectionName); + Meteor.call('createInsecureCollection', collectionName, collectionOptions); Meteor.subscribe('c-' + collectionName); } @@ -2125,6 +2156,193 @@ testAsyncMulti('mongo-livedata - specified _id', [ } ]); + +// Consistent id generation tests +function collectionInsert (test, expect, coll, index) { + var clientSideId = coll.insert({name: "foo"}, expect(function (err1, id) { + test.equal(id, clientSideId); + var o = coll.findOne(id); + test.isTrue(_.isObject(o)); + test.equal(o.name, 'foo'); + })); +}; + +function collectionUpsert (test, expect, coll, index) { + var upsertId = '123456' + index; + + coll.upsert(upsertId, {$set: {name: "foo"}}, expect(function (err1, result) { + test.equal(result.insertedId, upsertId); + test.equal(result.numberAffected, 1); + + var o = coll.findOne(upsertId); + test.isTrue(_.isObject(o)); + test.equal(o.name, 'foo'); + })); +}; + +function collectionUpsertExisting (test, expect, coll, index) { + var clientSideId = coll.insert({name: "foo"}, expect(function (err1, id) { + test.equal(id, clientSideId); + + var o = coll.findOne(id); + test.isTrue(_.isObject(o)); + // We're not testing sequencing/visibility rules here, so skip this check + // test.equal(o.name, 'foo'); + })); + + coll.upsert(clientSideId, {$set: {name: "bar"}}, expect(function (err1, result) { + test.equal(result.insertedId, clientSideId); + test.equal(result.numberAffected, 1); + + var o = coll.findOne(clientSideId); + test.isTrue(_.isObject(o)); + test.equal(o.name, 'bar'); + })); +}; + +function functionCallsInsert (test, expect, coll, index) { + Meteor.call("insertObjects", coll._name, {name: "foo"}, 1, expect(function (err1, ids) { + test.notEqual((INSERTED_IDS[coll._name] || []).length, 0); + var stubId = INSERTED_IDS[coll._name][index]; + + test.equal(ids.length, 1); + test.equal(ids[0], stubId); + + var o = coll.findOne(stubId); + test.isTrue(_.isObject(o)); + test.equal(o.name, 'foo'); + })); +}; + +function functionCallsUpsert (test, expect, coll, index) { + var upsertId = '123456' + index; + Meteor.call("upsertObject", coll._name, upsertId, {$set:{name: "foo"}}, expect(function (err1, result) { + test.equal(result.insertedId, upsertId); + test.equal(result.numberAffected, 1); + + var o = coll.findOne(upsertId); + test.isTrue(_.isObject(o)); + test.equal(o.name, 'foo'); + })); +}; + +function functionCallsUpsertExisting (test, expect, coll, index) { + var id = coll.insert({name: "foo"}); + + var o = coll.findOne(id); + test.notEqual(null, o); + test.equal(o.name, 'foo'); + + Meteor.call("upsertObject", coll._name, id, {$set:{name: "bar"}}, expect(function (err1, result) { + test.equal(result.numberAffected, 1); + test.equal(result.insertedId, undefined); + + var o = coll.findOne(id); + test.isTrue(_.isObject(o)); + test.equal(o.name, 'bar'); + })); +}; + +function functionCalls3Inserts (test, expect, coll, index) { + Meteor.call("insertObjects", coll._name, {name: "foo"}, 3, expect(function (err1, ids) { + test.notEqual((INSERTED_IDS[coll._name] || []).length, 0); + test.equal(ids.length, 3); + + for (var i = 0; i < 3; i++) { + var stubId = INSERTED_IDS[coll._name][(3 * index) + i]; + test.equal(ids[i], stubId); + + var o = coll.findOne(stubId); + test.isTrue(_.isObject(o)); + test.equal(o.name, 'foo'); + } + })); +}; + +function functionChainInsert (test, expect, coll, index) { + Meteor.call("doMeteorCall", "insertObjects", coll._name, {name: "foo"}, 1, expect(function (err1, ids) { + test.notEqual((INSERTED_IDS[coll._name] || []).length, 0); + var stubId = INSERTED_IDS[coll._name][index]; + + test.equal(ids.length, 1); + test.equal(ids[0], stubId); + + var o = coll.findOne(stubId); + test.isTrue(_.isObject(o)); + test.equal(o.name, 'foo'); + })); +}; + +function functionChain2Insert (test, expect, coll, index) { + Meteor.call("doMeteorCall", "doMeteorCall", "insertObjects", coll._name, {name: "foo"}, 1, expect(function (err1, ids) { + test.notEqual((INSERTED_IDS[coll._name] || []).length, 0); + var stubId = INSERTED_IDS[coll._name][index]; + + test.equal(ids.length, 1); + test.equal(ids[0], stubId); + + var o = coll.findOne(stubId); + test.isTrue(_.isObject(o)); + test.equal(o.name, 'foo'); + })); +}; + +function functionChain2Upsert (test, expect, coll, index) { + var upsertId = '123456' + index; + Meteor.call("doMeteorCall", "doMeteorCall", "upsertObject", coll._name, upsertId, {$set:{name: "foo"}}, expect(function (err1, result) { + test.equal(result.insertedId, upsertId); + test.equal(result.numberAffected, 1); + + var o = coll.findOne(upsertId); + test.isTrue(_.isObject(o)); + test.equal(o.name, 'foo'); + })); +}; + +_.each( [collectionInsert, collectionUpsert, + functionCallsInsert, functionCallsUpsert, functionCallsUpsertExisting, + functionCalls3Inserts, + functionChainInsert, + functionChain2Insert, functionChain2Upsert], function (fn) { +_.each( [1, 3], function (repetitions) { +_.each( [1, 3], function (collectionCount) { +_.each( ['STRING', 'MONGO'], function (idGeneration) { + + testAsyncMulti('mongo-livedata - consistent _id generation ' + fn.name + ', ' + repetitions + ' repetitions on ' + collectionCount + ' collections, idGeneration=' + idGeneration, [ function (test, expect) { + var collectionOptions = { idGeneration: idGeneration }; + + var collections = []; + + for (var i = 0; i < collectionCount; i++) { + var collectionName = "consistentid_" + Random.id(); + if (Meteor.isClient) { + Meteor.call('createInsecureCollection', collectionName, collectionOptions); + Meteor.subscribe('c-' + collectionName); + } + + var coll = new Meteor.Collection(collectionName, collectionOptions); + COLLECTIONS[collectionName] = coll; + + collections.push(coll); + } + + // Reset state before actual test + INSERTED_IDS = {}; + + for (var i = 0; i < repetitions; i++) { + for (var j = 0; j < collectionCount; j++) { + fn(test, expect, collections[j], i); + } + } + }]); + +}); +}); +}); +}); + + + testAsyncMulti('mongo-livedata - empty string _id', [ function (test, expect) { var self = this; From ab08191ac3c2f48c122d9c90acb34d3aed55bc28 Mon Sep 17 00:00:00 2001 From: Andrew Wilcox Date: Tue, 22 Apr 2014 13:55:47 -0400 Subject: [PATCH 166/219] Allow validate login hook to override error from beginPasswordExchange Fixes #2058 --- History.md | 3 ++ packages/accounts-base/accounts_server.js | 1 + packages/accounts-password/password_server.js | 5 ++-- packages/accounts-password/password_tests.js | 29 ++++++++++++++++++- .../accounts-password/password_tests_setup.js | 24 ++++++++++----- 5 files changed, 52 insertions(+), 10 deletions(-) diff --git a/History.md b/History.md index 2aa9694303d..21ed9821bb8 100644 --- a/History.md +++ b/History.md @@ -101,6 +101,9 @@ Patches contributed by GitHub users aldeed, apendua, arbesfeld, awwx, dandv, davegonzalez, justinsb, mquandalle, Neftedollar, Pent, sdarnell, and timhaines. +* A validate login hook can now override the exception thrown from + `beginPasswordExchange` like it can for other login methods. + ## v0.8.0.1 diff --git a/packages/accounts-base/accounts_server.js b/packages/accounts-base/accounts_server.js index 9acd22c1bd4..214974e457b 100644 --- a/packages/accounts-base/accounts_server.js +++ b/packages/accounts-base/accounts_server.js @@ -309,6 +309,7 @@ Accounts._reportLoginFailure = function (methodInvocation, methodName, methodArg validateLogin(methodInvocation.connection, attempt); failedLogin(methodInvocation.connection, attempt); + return attempt; }; diff --git a/packages/accounts-password/password_server.js b/packages/accounts-password/password_server.js index 6a019f2d4d0..a758da4a1c0 100644 --- a/packages/accounts-password/password_server.js +++ b/packages/accounts-password/password_server.js @@ -73,12 +73,13 @@ Meteor.methods({beginPasswordExchange: function (request) { // the second step method ('login') is called. If a user calls // 'beginPasswordExchange' but then never calls the second step // 'login' method, no login hook will fire. - Accounts._reportLoginFailure(self, 'beginPasswordExchange', arguments, { + // The validate login hooks can mutate the exception to be thrown. + var attempt = Accounts._reportLoginFailure(self, 'beginPasswordExchange', arguments, { type: 'password', error: err, userId: user && user._id }); - throw err; + throw attempt.error; } // Save results so we can verify them later. diff --git a/packages/accounts-password/password_tests.js b/packages/accounts-password/password_tests.js index adfde8594ea..bd1ce9a6d13 100644 --- a/packages/accounts-password/password_tests.js +++ b/packages/accounts-password/password_tests.js @@ -33,7 +33,12 @@ if (Meteor.isClient) (function () { }, 10 * 1000, 100); }; var invalidateLoginsStep = function (test, expect) { - Meteor.call("testInvalidateLogins", true, expect(function (error) { + Meteor.call("testInvalidateLogins", 'fail', expect(function (error) { + test.isFalse(error); + })); + }; + var hideActualLoginErrorStep = function (test, expect) { + Meteor.call("testInvalidateLogins", 'hide', expect(function (error) { test.isFalse(error); })); }; @@ -616,6 +621,28 @@ if (Meteor.isClient) (function () { }) ); }, + validateLoginsStep, + function (test, expect) { + Meteor.loginWithPassword( + "no such user", + "some password", + expect(function (error) { + test.isTrue(error); + test.equal(error.reason, 'User not found'); + }) + ); + }, + hideActualLoginErrorStep, + function (test, expect) { + Meteor.loginWithPassword( + "no such user", + "some password", + expect(function (error) { + test.isTrue(error); + test.equal(error.reason, 'hide actual error'); + }) + ); + }, validateLoginsStep ]); diff --git a/packages/accounts-password/password_tests_setup.js b/packages/accounts-password/password_tests_setup.js index 8f420321f1e..e9d110c936a 100644 --- a/packages/accounts-password/password_tests_setup.js +++ b/packages/accounts-password/password_tests_setup.js @@ -15,14 +15,14 @@ Accounts.onCreateUser(function (options, user) { }); -// connection id -> true +// connection id -> action var invalidateLogins = {}; Meteor.methods({ - testInvalidateLogins: function (flag) { - if (flag) - invalidateLogins[this.connection.id] = true; + testInvalidateLogins: function (action) { + if (action) + invalidateLogins[this.connection.id] = action; else delete invalidateLogins[this.connection.id]; } @@ -30,9 +30,19 @@ Meteor.methods({ Accounts.validateLoginAttempt(function (attempt) { - return ! (attempt && - attempt.connection && - invalidateLogins[attempt.connection.id]); + var action = + attempt && + attempt.connection && + invalidateLogins[attempt.connection.id]; + + if (! action) + return true; + else if (action === 'fail') + return false; + else if (action === 'hide') + throw new Meteor.Error(403, 'hide actual error'); + else + throw new Error('unknown action: ' + action); }); From a496fcab4cfa6598da2e583e59725ec1e5e003fd Mon Sep 17 00:00:00 2001 From: Nick Martin Date: Wed, 23 Apr 2014 19:35:23 -0700 Subject: [PATCH 167/219] one more comment. --- packages/accounts-base/accounts_server.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/accounts-base/accounts_server.js b/packages/accounts-base/accounts_server.js index 214974e457b..5f4de80932c 100644 --- a/packages/accounts-base/accounts_server.js +++ b/packages/accounts-base/accounts_server.js @@ -309,6 +309,8 @@ Accounts._reportLoginFailure = function (methodInvocation, methodName, methodArg validateLogin(methodInvocation.connection, attempt); failedLogin(methodInvocation.connection, attempt); + // validateLogin may mutate attempt to set a new error message. Return + // the modified version. return attempt; }; From 9fb63da3c7a2898b0ace22107c7268bb984e69bb Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Thu, 24 Apr 2014 09:56:42 -0700 Subject: [PATCH 168/219] Handle unexpected keys for pending OAuth credentials. Duplicate keys aren't expected, but in case something weird happens, just override the previous information associated with that key. We simply insert nothing for non-string keys (e.g. an OAuth flow with no `state` parameter, which should never happen normally). --- packages/oauth/oauth_tests.js | 19 +++++++++++++++ packages/oauth/pending_credentials.js | 12 ++++++++-- .../oauth1/oauth1_pending_request_tokens.js | 15 +++++++++--- packages/oauth1/oauth1_tests.js | 24 +++++++++++++++++++ 4 files changed, 65 insertions(+), 5 deletions(-) diff --git a/packages/oauth/oauth_tests.js b/packages/oauth/oauth_tests.js index d560ae10d3e..0984ab57add 100644 --- a/packages/oauth/oauth_tests.js +++ b/packages/oauth/oauth_tests.js @@ -28,3 +28,22 @@ Tinytest.add("oauth - pendingCredential handles Meteor.Errors", function (test) test.equal(result.stack, testError.stack); test.isUndefined(result.meteorError); }); + +Tinytest.add("oauth - null, undefined key for pendingCredential", function (test) { + var cred = Random.id(); + test.throws(function () { + OAuth._storePendingCredential(null, cred); + }); + test.throws(function () { + OAuth._storePendingCredential(undefined, cred); + }); +}); + +Tinytest.add("oauth - pendingCredential handles duplicate key", function (test) { + var key = Random.id(); + var cred = Random.id(); + OAuth._storePendingCredential(key, cred); + var newCred = Random.id(); + OAuth._storePendingCredential(key, newCred); + test.equal(OAuth._retrievePendingCredential(key), newCred); +}); diff --git a/packages/oauth/pending_credentials.js b/packages/oauth/pending_credentials.js index e841b06124f..d5197a8d53d 100644 --- a/packages/oauth/pending_credentials.js +++ b/packages/oauth/pending_credentials.js @@ -31,19 +31,27 @@ var _cleanStaleResults = function() { var _cleanupHandle = Meteor.setInterval(_cleanStaleResults, 60 * 1000); -// Stores the key and credential in the _pendingCredentials collection +// Stores the key and credential in the _pendingCredentials collection. +// Will throw an exception if `key` is not a string. // // @param key {string} // @param credential {string} The credential to store // OAuth._storePendingCredential = function (key, credential) { + check(key, String); + if (credential instanceof Error) { credential = storableError(credential); } else { credential = OAuth.sealSecret(credential); } - OAuth._pendingCredentials.insert({ + // We do an upsert here instead of an insert in case the user happens + // to somehow send the same `state` parameter twice during an OAuth + // login; we don't want a duplicate key error. + OAuth._pendingCredentials.upsert({ + key: key + }, { key: key, credential: credential, createdAt: new Date() diff --git a/packages/oauth1/oauth1_pending_request_tokens.js b/packages/oauth1/oauth1_pending_request_tokens.js index 76e8d88401e..ff025a577f0 100644 --- a/packages/oauth1/oauth1_pending_request_tokens.js +++ b/packages/oauth1/oauth1_pending_request_tokens.js @@ -15,7 +15,8 @@ // For this reason, the _pendingRequestTokens are stored in the database // so they can be shared across Meteor App servers. // - +// XXX This code is fairly similar to oauth/pending_credentials.js -- +// maybe we can combine them somehow. // Collection containing pending request tokens // Has key, requestToken, requestTokenSecret, and createdAt fields. @@ -39,14 +40,22 @@ var _cleanStaleResults = function() { var _cleanupHandle = Meteor.setInterval(_cleanStaleResults, 60 * 1000); -// Stores the key and request token in the _pendingRequestTokens collection +// Stores the key and request token in the _pendingRequestTokens collection. +// Will throw an exception if `key` is not a string. // // @param key {string} // @param requestToken {string} // @param requestTokenSecret {string} // OAuth._storeRequestToken = function (key, requestToken, requestTokenSecret) { - OAuth._pendingRequestTokens.insert({ + check(key, String); + + // We do an upsert here instead of an insert in case the user happens + // to somehow send the same `state` parameter twice during an OAuth + // login; we don't want a duplicate key error. + OAuth._pendingRequestTokens.upsert({ + key: key + }, { key: key, requestToken: OAuth.sealSecret(requestToken), requestTokenSecret: OAuth.sealSecret(requestTokenSecret), diff --git a/packages/oauth1/oauth1_tests.js b/packages/oauth1/oauth1_tests.js index f85ead956f0..3820a337e82 100644 --- a/packages/oauth1/oauth1_tests.js +++ b/packages/oauth1/oauth1_tests.js @@ -84,3 +84,27 @@ Tinytest.add("oauth1 - pendingCredential is stored and can be retrieved (with oa OAuthEncryption.loadKey(null); } }); + +Tinytest.add("oauth1 - duplicate key for request token", function (test) { + var key = Random.id(); + var token = Random.id(); + var secret = Random.id(); + OAuth._storeRequestToken(key, token, secret); + var newToken = Random.id(); + var newSecret = Random.id(); + OAuth._storeRequestToken(key, newToken, newSecret); + var result = OAuth._retrieveRequestToken(key); + test.equal(result.requestToken, newToken); + test.equal(result.requestTokenSecret, newSecret); +}); + +Tinytest.add("oauth1 - null, undefined key for request token", function (test) { + var token = Random.id(); + var secret = Random.id(); + test.throws(function () { + OAuth._storeRequestToken(null, token, secret); + }); + test.throws(function () { + OAuth._storeRequestToken(undefined, token, secret); + }); +}); From cbd55698fd3374ed0fc5f4e902983de09dabb466 Mon Sep 17 00:00:00 2001 From: emgee3 Date: Wed, 23 Apr 2014 14:07:02 -0700 Subject: [PATCH 169/219] Add collapsing TOC to Meteor Docs On small devices (< 768 px) make the Table of Contents hide by default. --- docs/client/docs.css | 91 +++++++++++++++++++++++++++++-------------- docs/client/docs.html | 3 +- docs/client/docs.js | 24 +++++++++++- 3 files changed, 86 insertions(+), 32 deletions(-) diff --git a/docs/client/docs.css b/docs/client/docs.css index 70afcfd9cde..52403510840 100644 --- a/docs/client/docs.css +++ b/docs/client/docs.css @@ -173,7 +173,7 @@ a:hover { /** Main pane **/ #main { - margin: 10px; + margin: 10px 10px 10px 60px; line-height: 1.3; color: #333333; } @@ -407,49 +407,80 @@ dl.callbacks { /** layout control **/ -/* default to no sidebar */ -#nav { - display: none; +#menu-ico { + font-size: 30px; + float: right; + position: fixed; + top: 3px; + left: 6px; } -.github-ribbon { + +#menu-ico.hidden { display: none; } -pre { - white-space: pre-wrap; -} -@media (min-width: 768px) { -/* ipad portrait or better */ -#main { - width: 440px; - height: 100%; - margin-left: 260px; /* nav width + padding */ - padding: 30px; -} +/* default to no sidebar */ #nav { display: block; - width: 200px; + background: #FFF; position: fixed; - overflow: auto; + width: 260px; height: 100%; top: 0; + left: -220px; +} + +#nav.show { left: 0; + overflow: auto; } -.main-headline { + +.github-ribbon { display: none; } +pre { + white-space: pre-wrap; } -@media (min-width: 1024px) { -/* ipad landscape and desktop */ -#main { - width: 610px; - margin-left: 330px; /* nav width + padding */ -} -#nav { - width: 270px; -} -.github-ribbon { - display: block; +@media (min-width: 768px) { + /* ipad portrait or better */ + #main { + width: 440px; + height: 100%; + margin-left: 260px; /* nav width + padding */ + padding: 30px; + } + #nav { + display: block; + width: 200px; + position: fixed; + overflow: auto; + height: 100%; + top: 0; + left: 0; + } + .main-headline { + display: none; + } + + #menu-ico { + display: none; + } } + +@media (min-width: 1024px) { + /* ipad landscape and desktop */ + #main { + width: 610px; + margin-left: 330px; /* nav width + padding */ + } + #nav { + width: 270px; + } + .github-ribbon { + display: block; + } + #menu-ico { + display: none; + } } diff --git a/docs/client/docs.html b/docs/client/docs.html index c67d0b6139c..b2bcfe2f780 100644 --- a/docs/client/docs.html +++ b/docs/client/docs.html @@ -8,7 +8,7 @@ -