Permalink
Browse files

Merge remote-tracking branch 'origin/devel' into sso

Conflicts:
	packages/accounts-base/accounts_client.js
  • Loading branch information...
2 parents e445a4a + 32768a8 commit d19e1f7b3d18f550f84a9c15c1599aeaf5fdb9c0 Emily Stark committed Jan 10, 2014
View
@@ -11,6 +11,10 @@
with numeric `length` fields as arrays. Among other things, this allows you
to use documents with numeric `length` fields with Mongo. #594 #1737
+* Fix races when calling login and/or logoutOtherClients from multiple
+ tabs. #1616
+
+
## v0.7.0.1
* Two fixes to `meteor run` Mongo startup bugs that could lead to hangs with the
@@ -116,8 +116,28 @@ Accounts.callLoginMethod = function (options) {
// need to show a logging-in animation.
_suppressLoggingIn: true,
userCallback: function (error) {
+ var storedTokenNow = storedLoginToken();
if (error) {
- makeClientLoggedOut();
+ // If we had a login error AND the current stored token is the
+ // one that we tried to log in with, then declare ourselves
+ // logged out. If there's a token in storage but it's not the
+ // token that we tried to log in with, we don't know anything
+ // about whether that token is valid or not, so do nothing. The
+ // periodic localStorage poll will decide if we are logged in or
+ // out with this token, if it hasn't already. Of course, even
+ // with this check, another tab could insert a new valid token
+ // immediately before we clear localStorage here, which would
+ // lead to both tabs being logged out, but by checking the token
+ // in storage right now we hope to make that unlikely to happen.
+ //
+ // If there is no token in storage right now, we don't have to
+ // do anything; whatever code removed the token from storage was
+ // responsible for calling `makeClientLoggedOut()`, or the
+ // periodic localStorage poll will call `makeClientLoggedOut`
+ // eventually if another tab wiped the token from storage.
+ if (storedTokenNow && storedTokenNow === result.token) {
+ makeClientLoggedOut();
+ }
}
// Possibly a weird callback to call, but better than nothing if
// there is a reconnect between "login result received" and "data
@@ -195,25 +215,41 @@ Meteor.logout = function (callback) {
};
Meteor.logoutOtherClients = function (callback) {
- // Our connection is going to be closed, but we don't want to call the
- // onReconnect handler until the result comes back for this method, because
- // the token will have been deleted on the server. Instead, wait until we get
- // a new token and call the reconnect handler with that.
- // XXX this is messy.
- // XXX what if login gets called before the callback runs?
- var origOnReconnect = Accounts.connection.onReconnect;
- var userId = Meteor.userId();
- Accounts.connection.onReconnect = null;
+ // 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) {
- Accounts.connection.onReconnect = origOnReconnect;
- if (! error)
+ if (error) {
+ callback && callback(error);
+ } else {
+ var userId = Meteor.userId();
storeLoginToken(userId, result.token, result.tokenExpires);
- Accounts.connection.onReconnect();
- callback && callback(error);
+ // 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);
+ });
+ }
});
};
+
///
/// LOGIN SERVICES
///
@@ -115,10 +115,14 @@ var pollStoredLoginToken = function() {
// != instead of !== just to make sure undefined and null are treated the same
if (lastLoginTokenWhenPolled != currentLoginToken) {
- if (currentLoginToken)
- Meteor.loginWithToken(currentLoginToken); // XXX should we pass a callback here?
- else
+ if (currentLoginToken) {
+ Meteor.loginWithToken(currentLoginToken, function (err) {
+ if (err)
+ makeClientLoggedOut();
+ });
+ } else {
Meteor.logout();
+ }
}
lastLoginTokenWhenPolled = currentLoginToken;
};
@@ -1,5 +1,13 @@
Accounts._noConnectionCloseDelayForTest = true;
+if (Meteor.isServer) {
+ Meteor.methods({
+ getUserId: function () {
+ return this.userId;
+ }
+ });
+}
+
if (Meteor.isClient) (function () {
// XXX note, only one test can do login/logout things at once! for
@@ -404,10 +412,10 @@ if (Meteor.isClient) (function () {
var self = this;
// copied from livedata/client_convenience.js
- var ddpUrl = '/';
+ self.ddpUrl = '/';
if (typeof __meteor_runtime_config__ !== "undefined") {
if (__meteor_runtime_config__.DDP_DEFAULT_CONNECTION_URL)
- ddpUrl = __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?
@@ -416,7 +424,7 @@ if (Meteor.isClient) (function () {
// connection while leaving Accounts.connection logged in.
var token;
var userId;
- self.secondConn = DDP.connect(ddpUrl);
+ self.secondConn = DDP.connect(self.ddpUrl);
var expectLoginError = expect(function (err) {
test.isTrue(err);
@@ -479,6 +487,41 @@ 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.
Meteor.loginWithPassword(this.username, this.password, expect(function (err) {
test.isFalse(err);
@@ -20,7 +20,7 @@ Meteor.absoluteUrl = function (path, options) {
if (path)
url += path;
- // turn http to http if secure option is set, and we're not talking
+ // turn http to https if secure option is set, and we're not talking
// to localhost.
if (options.secure &&
/^http:/.test(url) && // url starts with 'http:'

0 comments on commit d19e1f7

Please sign in to comment.