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

Timing issue in isLoggedIn #294

Closed
Rcjuk opened this issue Jul 2, 2014 · 4 comments
Closed

Timing issue in isLoggedIn #294

Rcjuk opened this issue Jul 2, 2014 · 4 comments
Labels

Comments

@Rcjuk
Copy link

Rcjuk commented Jul 2, 2014

in app.js

$rootScope.$on('$stateChangeStart', function (event, next) {
      if (next.authenticate && !Auth.isLoggedIn()) {
        $location.path('/login');
      }
    });

Auth.isLoggedIn is returning false when the app is first run and the user is already logged in. Only when you switch state a second time does the function return true as the currentuser has then been set in the auth service. Could there is a timing issue of some sort?

@DaftMonk
Copy link
Member

DaftMonk commented Jul 2, 2014

Hmm, what might be happening is that the currentUser object hasn't been returned from the server by the time that function is called.

Maybe a solution would to have isLoggedIn return a promise based on requesting the currentUser, and only redirect after that's been resolved.

@cameri
Copy link

cameri commented Jul 9, 2014

I had the same problem when programming authentication on my application... I had to make my auth service return a promise to check the validity of the current session.

@tarunkurapati
Copy link

I have the same problem.

@fedfigca
Copy link

Looking at how the auth service is setup, what I did was use getCurrentUser() instead of isLoggedIn(), and run the check on the property in the $promise of the returned object (actually inside that $promise you can safely use isLoggedIn ) , as far as I can tell this works until a better solution is provided.

An extra check to make sure getCurrentUser didn't give us undefined was needed. It might not be the best, but is something.

DaftMonk added a commit that referenced this issue Jul 13, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants