New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LogoutOtherClients calls back before others are actually logged out #1915

Closed
davidworkman9 opened this Issue Mar 13, 2014 · 6 comments

Comments

Projects
None yet
3 participants
@davidworkman9
Contributor

davidworkman9 commented Mar 13, 2014

@estark37

This comment has been minimized.

Contributor

estark37 commented Mar 13, 2014

Hey @davidworkman9. Thanks for the clear repro. This is actually working as designed. The logoutOtherClients method marks the currently existing login tokens for deletion but doesn't actually delete them, and then returns a fresh new token to the caller. A few seconds later, the server actually deletes the marked tokens and closes the associated connections. The point of this dance is to avoid flicker on the calling client: we don't want the caller to be temporarily logged out when all the tokens get deleted and then have to re-login when it gets the result of the logoutOtherClients method. So a success callback is not an indication that all the clients have been logged out, but rather that the server got the message and will eventually log out all other clients.

Do you have a particular use case in mind where you need to know when all clients have actually been logged out? Or maybe clearer docs would be helpful?

@davidworkman9

This comment has been minimized.

Contributor

davidworkman9 commented Mar 13, 2014

I'm in the process of implementing MFA on all database modifications, sort of like sudo in unix. You enter your MFA token and you're good for edits for the next x hours. I discussed this here before in this issue: #1384.

Originally I was going to attach the timestamp to the login token, but it isn't accessible on the server in all the places I need it to be. So I decided I'd log everyone out first, then attach it to the user, when a user logs in, the timestamp gets nuked, so they'd be prompted again the next time they went to do a modification.

I suppose I can hack around this in a Meteor method that doesn't return until it gets the right response from the database (loginToken size = 1) but it's not optimum. Any suggestions?

@davidworkman9

This comment has been minimized.

Contributor

davidworkman9 commented Mar 13, 2014

Thinking about this more and I have some concerns...

Let's say, (tiny chance of this actually happening but still a chance) that you call logoutOtherClients. The server returns the new token, the client invokes the callback, and the server decides in 2 or 3 seconds it will log everyone else out. At that point the connection between the application server and database server dies for a small amount of time. Is the code (right now) smart enough to keep trying? If it doesn't, the client will never be notified, and no one will actually be logged out (until the server restarts and clears the queued tokens, or the client calls logOutOtherClients again).

I think the proper solution to this is two calls to the server, behind the scenes, with the callback from logoutOtherClients not being called until the second one returns, or either one returns an error.

@glasser

This comment has been minimized.

Member

glasser commented Mar 14, 2014

If nothing else, the docs probably should be more specific about when the callback is called.

@estark37

This comment has been minimized.

Contributor

estark37 commented Mar 14, 2014

@davidworkman9 I think the concern you describe shouldn't be possible; if the app can ever recover its connection to Mongo, then it should reconnect and execute the query. Otherwise, we could never rely on any queries inside a Meteor.setTimeout or Meteor.defer!

However, I think your suggestion of using two methods might be a good one. The first method could be markExistingTokensForDeletion (and return a fresh token) and the second method could be flushMarkedTokens, or something like that. In addition to allowing the callback to be called at a more reasonable time, that would also remove the need to have a somewhat arbitrary timeout for when we think the calling client has received the new token.

@davidworkman9

This comment has been minimized.

Contributor

davidworkman9 commented Mar 14, 2014

@estark37 wouldn't an exception be thrown if the connection to mongo was not available whether it's in a setTimeout or not? I think I'm going to try this out.

Anyhow, I'm glad you agree two calls makes more sense. I'm sure other use cases other than mine are sure to crop up where a developer might want everyone but the current user logged out before performing some action.

I don't know for sure if taking out the timeout for flushMarkedTokens is a good idea though. It might be better to do both, in case the user disconnects before the second call is made? I don't know, maybe that's overkill. Either way still beneficial.

@estark37 estark37 closed this in f68908b Apr 17, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment