Skip to content
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 also logout current session in same browser #1616

Closed
zapaz opened this issue Nov 16, 2013 · 14 comments
Closed

LogoutOtherClients also logout current session in same browser #1616

zapaz opened this issue Nov 16, 2013 · 14 comments

Comments

@zapaz
Copy link

@zapaz zapaz commented Nov 16, 2013

_3 Upvotes_ LogoutOtherClients on new Login works fine on other browsers or other devices loggued, good !

But when opening a Meteor App in a second tabs on the same browser, both windows are logout. Would be better to logout only the other one (or none !?).

Another way to get the same problem, being in a Meteor app, click on an external link, then comme back with the back button of the navigator, here only one tab but still logout (seems always two tabs in browser memory)

@aadrian
Copy link

@aadrian aadrian commented Nov 16, 2013

But when opening a Meteor App in a second tabs on the same browser, both windows are logout. Would be better to logout only the other one

Isn't this the correct behavior? AFAIK browser tabs share the same session.

@zapaz
Copy link
Author

@zapaz zapaz commented Nov 16, 2013

@aadrian : seems contradictory to documentation that says

"Log out other clients logged in as the current user, but does not log out the client that calls this function"

anyway this case is not the problem, the second case is the real problem

@zapaz
Copy link
Author

@zapaz zapaz commented Nov 16, 2013

here is some code to get the issue : https://github.com/zapaz/meteor-issue-1606

also available on http://issue-1606.meteor.com

@estark37
Copy link
Contributor

@estark37 estark37 commented Nov 18, 2013

Hi @zapaz -- thanks for the bug report and reproduction. I'm working on this. Turns out there are a couple race conditions when you have login methods and logoutOtherClients methods running at the same time. The particular race that you have in your repro is easy to fix (and partially sort-of fixed on devel already), but there are others that are trickier and I'd like to fix them all at once. :)

In the meantime, a temporary workaround would be to not call Meteor.logoutOtherClients while Meteor.loggingIn() is true -- in other words, stick if (Meteor.loggingIn()) return; in front of your autorun function in your example.

@zapaz
Copy link
Author

@zapaz zapaz commented Nov 18, 2013

In the meantime, a temporary workaround would be to not call Meteor.logoutOtherClients while Meteor.loggingIn() is true -- in other words, stick if (Meteor.loggingIn()) return; in front of your autorun function in your example.

Thanks, I will do that, waiting for the full fix

estark37 pushed a commit that referenced this issue Nov 18, 2013
If we called `login` and then called `logoutOtherClients` before the login
result was recieved, then we would end up with no `onReconnect` callback. Fixed
by just leaving `onReconnect` as it is when calling `logoutOtherClients` -- we
were only replacing `onReconnect` for the sake of tests that have since been
rewritten much more cleanly.

Fixes #1616.
estark37 pushed a commit that referenced this issue Jan 4, 2014
If we called `login` and then called `logoutOtherClients` before the login
result was recieved, then we would end up with no `onReconnect` callback. Fixed
by just leaving `onReconnect` as it is when calling `logoutOtherClients` -- we
were only replacing `onReconnect` for the sake of tests that have since been
rewritten much more cleanly.

Fixes #1616.
estark37 pushed a commit that referenced this issue Jan 6, 2014
If we called `login` and then called `logoutOtherClients` before the login
result was recieved, then we would end up with no `onReconnect` callback. Fixed
by just leaving `onReconnect` as it is when calling `logoutOtherClients` -- we
were only replacing `onReconnect` for the sake of tests that have since been
rewritten much more cleanly.

Fixes #1616.
@estark37 estark37 closed this in 56d6090 Jan 9, 2014
estark37 pushed a commit that referenced this issue Jan 9, 2014
estark37 pushed a commit that referenced this issue Jan 9, 2014
@estark37
Copy link
Contributor

@estark37 estark37 commented Jan 9, 2014

Hi @zapaz, I believe this issue is fully fixed on devel now, as well as some other related race conditions. Thanks so much for the bug report and reproduction recipe!

@Alhaza
Copy link

@Alhaza Alhaza commented Jul 1, 2014

I'm still having the issue with all tabs being logged out when the user click on "Open Link in a new tab." Is the right way of using it is by putting the following code in a js file in the client folder?

Deps.autorun(function(){
  if(Meteor.loggingIn()){
    return;
  }
  if(Meteor.user()){
    Meteor.logoutOtherClients();
  }
})
@estark37
Copy link
Contributor

@estark37 estark37 commented Jul 7, 2014

Hi @Alhaza -- can you please file a new issue with a full reproduction recipe, per our contribution guidelines?

@Alhaza
Copy link

@Alhaza Alhaza commented Aug 5, 2014

@estark37, sorry for the delay, I was preoccupied.

Here is the example code: https://github.com/Alhaza/meteor-issue-1616

Deployed here: http://meteor-issue-1616.meteor.com/

@estark37
Copy link
Contributor

@estark37 estark37 commented Aug 20, 2014

Thanks @Alhaza -- I'm looking into it.

@cesarve77
Copy link

@cesarve77 cesarve77 commented Feb 20, 2016

+1

2 similar comments
@guncebektas
Copy link

@guncebektas guncebektas commented Mar 19, 2016

+1

@MichaelJCole
Copy link
Contributor

@MichaelJCole MichaelJCole commented Apr 11, 2016

+1

@abernix
Copy link
Member

@abernix abernix commented Nov 16, 2016

Closing as duplicate per my comments here: #6777 (comment)

@abernix abernix closed this Nov 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
9 participants