angular-fullstack / generator-angular-fullstack

Yeoman generator for an Angular app with an Express server
https://awk34.gitbook.io/generator-angular-fullstack
6.12k stars 1.24k forks source link

Why Auth.isLoggedInAsync() in app.js? #847

Closed honkskillet closed 9 years ago

honkskillet commented 9 years ago

Just looking for a little clarification. What is the point of using 'Auth.isLoggedInAsync()' inside app.js?

.run(function ($rootScope, $location, Auth) {
    $rootScope.$on('$routeChangeStart', function (event, next) {
      Auth.isLoggedInAsync(function(loggedIn) { ...

By checking logged in status in an async manner, it allows a request for a route that the user may not be authenticated for to be send, incurring unneeded overhead, no? I'm sure I'm just in need of sleep, but what is the use for check logged in status asynchronously?

Thanks.

ramdog commented 9 years ago

In addition to the unneeded overhead there is another issue: if a logged out user tries to go to a protected URL in the app directly (i.e. they type the URL into the browser or click a link they have) then $stateChangeStart (in the case of using ui-router in the generator configuration) will complete, the new state will load, and only when the promise is resolved will the user then get kicked back to the login page. This can result in a pretty ugly UI blip with the new template loading for a moment before redirecting the user.

It would be good if somehow there was a way to wait for the promise to resolve before completing the $state change. I'll try to look into it.

ramdog commented 9 years ago

Did a quick search: looks like a pattern I've seen is to check if it's a protected route, and if so call event.preventDefault() then wait for the Auth promise to resolve, and only then call $state.go() with the next state and toParams. Thoughts on if this is worth it to submit a PR?

ramdog commented 9 years ago

@honkskillet - I just did this in app.js and it seems to work quite well. It sounds like you aren't configured to use ui-router, but your app.js probably has a section that looks like this. Figured I'd send this your way in case it could be useful for you. I'll try to figure out if this is worth creating a PR for this generator.

.run(function ($rootScope, $state, Auth) {
    var bypassAuthCheck = false;

    // Redirect to login if route requires auth and you're not logged in
    $rootScope.$on('$stateChangeStart', function (event, next, toParams) {
      if (!next.authenticate || (next.authenticate && bypassAuthCheck)) {
        return;
      }

      event.preventDefault();

      Auth.isLoggedInAsync(function (loggedIn) {
        if (!loggedIn) {
          bypassAuthCheck = false;
          $state.go('login');
        } else {
          bypassAuthCheck = true;
          $state.go(next, toParams);
        }
      });

    });
  });
honkskillet commented 9 years ago

@ ramdog Thanks Yes, when I generated my app I didn't include ui-router so I'm just using vanilla ngRouter.
Another issue with the app.js for ngRouter is that after going to an unauthorized route it redirects to the login page with

$location.path('/login');

This means that if the user decides he does not want to log in and hits the back button he will go back to the unauthorized page and get immediately redirected back to the login page. Unless the user spams the back button, he will end up in an ugly loop ping ponging between the unauthorized page and the login page. Instead, when redirecting to /login, this should be called

$location.replace().path('/login');