move authentication check from app.js

[honkskillet]

I think (and correct me if I'm wrong here) that the client side check of user authentication should be moved out of client/app/app.js . If you look in that file, currently you will see

.run(function ($rootScope, $location, Auth) {
    // Redirect to login if route requires auth and you're not logged in
    $rootScope.$on('$routeChangeStart', function (event, next) {
      Auth.isLoggedInAsync(function(loggedIn) {
        if (next.authenticate && !loggedIn) {
          $location.path('/login');
        }
      });
    });
  });

Auth.isLoggedInAsync is, as the name suggests, asynchronous. Under the hood it calls User.get(), an http request. So there is a very real chance that you can have the new route loaded before this is resolved. This could cause two bad outcomes,

1. The route loads before it is confirmed that the user isn't authenticated. This might result in the route briefly flickering into view before being redirect to /login .
2. The route loads before the user is authenticated. This mean that if you call Auth.getCurrentUser() while setting up your controller, the current user object will just be a promise.

Might it not be better to move authentication into the route's $routeProvider.config function? Currently .config for a route might look like.

.config(function ($routeProvider) {
    $routeProvider
      .when('/settings', {
        templateUrl: 'app/account/settings/settings.html',
        controller: 'SettingsCtrl',
        authenticate: true
      });

Instead, use resolve to make sure the user is authenticated prior to actually loading the route. (Double check this as I am coding it in the browser right now.)

.config(function ($routeProvider, Auth) {
    $routeProvider
      .when('/settings', {
        /*...*/
        resolve: {
          authenticatedUser: function($q, $location){
            var deferred =$q.defer();
            Auth.isLoggedInAsync(function(loggedIn) {
              if(loggedIn){
                deferred.resolve(Auth.getCurrentUser() );
              } else  {  
                 deferred.reject(); 
                 $location.path('/login');
              }
            });
            return deferred.promise;
          },
        },
      });

Now, the route won't load until the user's authentication status has actually been determined. Better yet would be to add function($q,$location){...} to the Auth service, making it about as simple to check authentication this way. Say, call it Auth.authenticateUserAsync(). Then .config would look something like,

.config(function ($routeProvider, Auth) {
    $routeProvider
      .when('/settings', {
        templateUrl: 'app/account/settings/settings.html',
        controller: 'SettingsCtrl',
        resolve: {
          authenticatedUser: function(Auth){
            return Auth.getAuthenticatedUserAsync();
          },
        },
      });

Some added bonuses from this approach (besides fixing the issues above),

1. authenticatedUser would be injected into the route controller ready to use.
2. Under the old approach, when you open a config and see authenticate:true you have zero idea where this value is actually being used. I had to hunt through the entire client side code to find that it was being used in app.js. With the new approach, the logic flow is right there plain to see.

Thoughts?

[honkskillet]

PS, this is for ngRouter. I've never used ui-router and haven't looked at that code.

[honkskillet]

I went ahead and made these changes in an active app I've been working on. I added a new function to the Auth factory which is basically identical to the promise making function I showed above.

     getAuthenticatedUserAsync: function(){
        var deferred =$q.defer();
        console.log(deferred)
        this.isLoggedInAsync(function(loggedIn) {
          if(loggedIn){
            deferred.resolve(currentUser );
          } else  {  
             deferred.reject(); 
             $location.path('/login');
          }
        });
        return deferred.promise;
      },