Skip to content

Commit

Permalink
Merge branch 'two-methods-for-logout-others' into devel
Browse files Browse the repository at this point in the history
Fixes #1915.
  • Loading branch information
Emily Stark committed Apr 17, 2014
2 parents a66ed34 + d5af69e commit f68908b
Show file tree
Hide file tree
Showing 4 changed files with 223 additions and 69 deletions.
66 changes: 34 additions & 32 deletions packages/accounts-base/accounts_client.js
Expand Up @@ -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);
}
);
};


Expand Down
67 changes: 65 additions & 2 deletions packages/accounts-base/accounts_server.js
Expand Up @@ -431,6 +431,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;
Expand All @@ -448,7 +459,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
Expand All @@ -469,8 +480,60 @@ 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.");
}
},

// 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: <new token>, id: <user id>,
// tokenExpires: <expiration date> }.
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);
},

// 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) {
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 } }
}
});
}
});

Expand Down
76 changes: 76 additions & 0 deletions 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?)
Expand Down Expand Up @@ -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");
}
);
}
);
83 changes: 48 additions & 35 deletions packages/accounts-password/password_tests.js
Expand Up @@ -468,13 +468,59 @@ 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(
self.username,
self.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) {
Expand Down Expand Up @@ -502,7 +548,6 @@ if (Meteor.isClient) (function () {
token = Accounts._storedLoginToken();
self.beforeLogoutOthersToken = token;
test.isTrue(token);
userId = Meteor.userId();
self.secondConn.call("login", { resume: token },
expectSecondConnLoggedIn);
}));
Expand Down Expand Up @@ -536,41 +581,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.
Expand Down

0 comments on commit f68908b

Please sign in to comment.