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

Accounts.onLogin clientside not firing when user re-opens page and is already logged in #5127

Closed
joaobarcia opened this issue Sep 9, 2015 · 17 comments

Comments

@joaobarcia
Copy link

@joaobarcia joaobarcia commented Sep 9, 2015

onLogin is not firing when user opens the page and is already logged in, only when user actively goes from logged out to logged in state.

On the server side it works on both situations and it seems that it should work that way as well for the client side, at least for my use case. Is there any way around this? Is it possible to implement?

Thanks

@stubailo
Copy link
Contributor

@stubailo stubailo commented Sep 15, 2015

Thanks for reporting! I agree that it should be consistent on the client and server.

I suspect that a workaround on the client could be:

Tracker.autorun(function () {
  nowLoggedIn = !! Meteor.userId();

  if (nowLoggedIn) {
    alert("just logged in!");
  }
});

This should fire both when the user first logs in, and when they load any other page. Let me know if that works for you.

@luisreyes
Copy link

@luisreyes luisreyes commented Sep 16, 2015

Should the onLogin event only fire when the action has taken place? In my application Accounts.onLogin is firing on page refresh and I wish it wasn't since im trying to redirect them to the user's profile page when they successfully log in.

If I use the Accounts.onLogin it fires every time the user hits refresh or manually enters a url. Am I missing something?

@stubailo
Copy link
Contributor

@stubailo stubailo commented Sep 16, 2015

Hmm, I didn't actually test it. It would make a lot of sense for onLogin to only fire when the user actually initiates a login.

@stubailo stubailo added the backlog label Oct 23, 2015
@skifaster
Copy link

@skifaster skifaster commented Nov 24, 2015

Is there any progress on the issue? Any updates on what should be expected? In our current use case, we need to be able to log when people "login". Right now we define "login" as when someone provides a username and password, or provides a login token; not when a resume token is being re-negotiated on client/server.

Having a parameter that provides the difference between resume token and other login methods would also work for our current scenario.

Any feedback is much appreciated.

@LDubya
Copy link

@LDubya LDubya commented Jan 26, 2016

Bump. This is annoying. I want to redirect new logins to the dashboard. However it's also redirecting page-refreshes and URL links to the dashboard, since it counts those as logging in. This is not how anyone expects this to work.

@craigdrayton
Copy link

@craigdrayton craigdrayton commented Feb 3, 2016

I'm also encountering this issue - Accounts.onLogin is triggering every time the user changes page. I'm unable to simply redirect a newly logged in user to a particular route.

@rvrv
Copy link

@rvrv rvrv commented Feb 17, 2016

Im having same issue here. I want to call some redirects only when user successfuly logging in, and Accounts.onLogin is called also when logged user refresh page.

As a workaround i call my redirects from onSubmitHook delivered by useraccounts:core as mentioned here https://github.com/meteor-useraccounts/core/blob/master/Guide.md#configuration-api

Hope this helps someone. Cheers.

@tyleha
Copy link

@tyleha tyleha commented Mar 4, 2016

I am discovering that a code reload fires the client-side onLogin, but closing the tab and re-opening it does not. In both cases, the server-side onLogin hook fires. I am thoroughly confused...while I'm sure there's an explanation, it certainly is not expected behavior for a user or a dev.

I would expect onLogin to be associated solely with logging in. Right now, it's functioning like some kind of onConnection hook, but not reliably.

@lorensr
Copy link
Member

@lorensr lorensr commented Mar 10, 2016

On the server, onLogin fires during any login, including a resume login. On the client, onLogin does not fire during a resume login. It would be nice if it did.

onLoginFailure does fire on both the server and client during a failed resume login.

@lorensr
Copy link
Member

@lorensr lorensr commented Mar 12, 2016

Here are two workarounds: one that watches client-side DDP, and one that sends the client an event from server-side onLogin/onLoginFailure:

http://stackoverflow.com/a/35929513/627729

@GeorgeCalvert
Copy link

@GeorgeCalvert GeorgeCalvert commented Apr 29, 2016

First — not to conflate the two issues, but note this forum post re onLogin firing on Firefox but not on Chrome. @joaobarcia, you're seeing something different, right?

Second — my own expectation is that onLogin should fire any time users initiate a log in, they deliberately (re)load the page (i.e. an F5 or button refresh, a return in the address bar or clicking a bookmark) or a hot codepush refreshes the page. For me, the router should manage redirects and catch links so onLogin doesn't fire for navigation within the SPA.

I want it firing on reloads because it makes debugging far easier. I expect it on hot pushes because I want a full refresh of the auth (including the user in Minimongo and any settings I might derive) if I change the code (though this would count as a login for @skifaster). I don't need it fired when the socket is only lost and renegotiated as long as I'm guaranteed the user has not changed.

FWIW, if you've got a requirement to default to some route upon login, I'd suggest looking at the URL and only redirecting if the URL is the root. Thus you'll enable deep linking/bookmarking.

@joaobarcia
Copy link
Author

@joaobarcia joaobarcia commented Apr 29, 2016

Disclaimer: I haven't looked into the issue for 6 months, results might be different by now

First — not to conflate the two issues, but note this forum post re onLogin firing on Firefox but not on Chrome. @joaobarcia, you're seeing something different, right?

I believe my issue was different. In my case:

  • Client-side fired on hard login. Did NOT fire on page load/refresh. Only tested Firefox on Ubuntu
  • Server-side fired as expected

Second — my own expectation is that onLogin should fire any time users initiate a log in, they deliberately (re)load the page (i.e. an F5 or button refresh, a return in the address bar or clicking a bookmark) or a hot codepush refreshes the page. For me, the router should manage redirects and catch links so onLogin doesn't fire for navigation within the SPA.

Agree! This is what already happens on the server-side btw

@zol zol removed the backlog label May 3, 2016
@ksorskiy
Copy link

@ksorskiy ksorskiy commented Sep 19, 2016

Guys! onLogin function takes 1 parameter: user. This is a complex object, and it has a type field. so when you just refresh the page (and already logged in), then type = resume. and when you have logged in by password, then type will be password. all you need is to write something like

Accounts.onLogin(function(user) {
    //console.log(user.user.log[user.user.log.length - 1]);
    if(user.type == 'password') {
        // logged in from logged out status!
    } else if(user.type == 'password') {
        // just refreshed
        }
});

Hope it will help.

@pricetula
Copy link

@pricetula pricetula commented Dec 22, 2016

The onLogin() method fires on page refresh
Accounts.onLogin(function(user) {
console.log(user) //<------ The user argument passed is always undefined and not an object if on client side
});

@sunlee-newyork
Copy link

@sunlee-newyork sunlee-newyork commented May 29, 2017

Same as @pricetula, user param is undefined for me on page refresh.

@hwillson
Copy link
Member

@hwillson hwillson commented Jan 2, 2018

Hi all - just reviewing this issue thread. It seems as though some people expect registered Accounts.onLogin hooks to fire whenever an authenticated user refreshes a page (which happens currently), whereas other people expect the registered hooks to only fire when a user actually logs in. This means we can't really change this to be one way or the other exclusively, without upsetting at least some people.

#5127 (comment) mentions the possibility of checking user.type to figure out if the hook is being called on a page refresh versus a true login. While this is possible on the server (since registered Accounts.onLogin hooks on the server receive a user parameter), it is not currently possible on the client side (Accounts.onLogin hooks don't receive a user param client side). Adjusting client side Accounts.onLogin hooks so that they get back user.type information (where type equals password for logins, and type equals resume for page reloads) is fairly straightforward, and would provide a flexible way to address the original issue here (and is backwards compatible).

Marking as pull-requests-encouraged. If anyone is interested in working on this, please chime in, otherwise I'll get a PR ready for this shortly.

hwillson added a commit to hwillson/meteor that referenced this issue Jan 3, 2018
Client side `Accounts.onLogin` callbacks are triggered after a
successful login, but they're also triggered after a successful
DDP reconnect (if already logged in). As discussed in meteor#5127,
some people think this is the correct behaviour, while others
feel that `onLogin` callbacks should really only fire after a
user has actually logged in (e.g. after using something like
`Meteor.loginWithPassword`). Since people are split on the
proper approach here, an alternative solution would be to
provide client side `Accounts.onLogin` callbacks with a way
to tell if they're being called after a login or after a
reconnect, then let them decide what to do.

Server side `Accounts.onLogin` callbacks receive an object that
contains various login details. One of those items is a login
`type`, that can hold values like `password`, `resume`, etc.
When a user completes a true password based login, the returned
login `type` is `password`. When the user is already logged in
but undergoes a DDP disconnect + reconnect, the returned login
`type` is `resume`. This means server side `Accounts.onLogin`
callbacks have a way to tell which type of login is happening,
allowing them to respond accordingly.

This PR adjusts client side `Accounts.onLogin` callbacks such
that they also receive a single login details object, that
contains login type information. Unlike the server side login
details object, the client side login details object only
contains the login type (to help reduce network transfer
overhead, make sure we're not exposing server details on the
client we shouldn't be, etc.).

This approach should give developers a better way to respond to
different client side login types, and act accordingly.

Fixes meteor#5127.
@hwillson
Copy link
Member

@hwillson hwillson commented Jan 3, 2018

PR #9512 should help with this.

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
You can’t perform that action at this time.