github/meteor
1
1
Fork 0
mirror of https://github.com/meteor/meteor.git synced 2026-05-02 03:01:46 -04:00
Code Issues Packages Projects Releases Wiki Activity

Avoid overwriting fresh tokens from other tabs.

Browse Source 
* Before wiping a bad token from storage on reconnect, make sure that
  we're wiping the token that we tried and failed to log in with. Avoids
  logging out another tab that might have gotten a fresh valid token
  while we were logging in with the old, invalid one (though it is still
  theoretically possible).
* In the logoutOtherClients callback, try to log in with the token that
  we get in the response. Accounts for the situation where the server
  disconnects us before the callback runs.
* If we fail to log in with a token found during a localStorage poll,
  make the client logged out.
* Add a test that attempts to simulate one tab getting a fresh new token
  while another tab logs in with an old invalid token on reconnect.
This commit is contained in:
Emily Stark
2013-11-18 15:26:08 -08:00
parent 56d60907cc
commit 44629cf800
3 changed files with 102 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

60
packages/accounts-base/accounts_client.js
View File

@@ -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
///

10
packages/accounts-base/localstorage_token.js
View File

@@ -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;
};

49
packages/accounts-password/password_tests.js
View File

@@ -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.