Replace observe on all users with one observe per connection.

This commit is contained in:
Emily Stark
2014-02-14 16:15:35 -08:00
parent 5b4ecd9a71
commit 2e66a7f1a0
2 changed files with 40 additions and 104 deletions

View File

@@ -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 || []);
}
});

View File

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