diff --git a/packages/accounts-base/accounts_client.js b/packages/accounts-base/accounts_client.js index 0e7df5582b..d21bdad805 100644 --- a/packages/accounts-base/accounts_client.js +++ b/packages/accounts-base/accounts_client.js @@ -115,8 +115,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 @@ -194,23 +214,41 @@ Meteor.logout = function (callback) { }; Meteor.logoutOtherClients = function (callback) { - // Call the `logoutOtherClients` method and store the login token that we get - // back. 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 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. - Meteor.apply('logoutOtherClients', [], + // 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. + Meteor.apply('logoutOtherClients', [], { wait: true }, function (error, result) { - if (! error) { + 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); + }); } }); }; + /// /// LOGIN SERVICES /// diff --git a/packages/accounts-base/localstorage_token.js b/packages/accounts-base/localstorage_token.js index 24c6e25318..e30d4514c7 100644 --- a/packages/accounts-base/localstorage_token.js +++ b/packages/accounts-base/localstorage_token.js @@ -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; }; diff --git a/packages/accounts-password/password_tests.js b/packages/accounts-password/password_tests.js index 59ae584d98..73f7ea1db0 100644 --- a/packages/accounts-password/password_tests.js +++ b/packages/accounts-password/password_tests.js @@ -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 Meteor.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); @@ -477,6 +485,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.