122

I'm trying to write a HTTP interceptor for my AngularJS app to handle authentication.

This code works, but I'm concerned about manually injecting a service since I thought Angular is supposed to handle this automatically:

    app.config(['$httpProvider', function ($httpProvider) {
    $httpProvider.interceptors.push(function ($location, $injector) {
        return {
            'request': function (config) {
                //injected manually to get around circular dependency problem.
                var AuthService = $injector.get('AuthService');
                console.log(AuthService);
                console.log('in request interceptor');
                if (!AuthService.isAuthenticated() && $location.path != '/login') {
                    console.log('user is not logged in.');
                    $location.path('/login');
                }
                return config;
            }
        };
    })
}]);

What I started out doing, but ran into circular dependency problems:

    app.config(function ($provide, $httpProvider) {
    $provide.factory('HttpInterceptor', function ($q, $location, AuthService) {
        return {
            'request': function (config) {
                console.log('in request interceptor.');
                if (!AuthService.isAuthenticated() && $location.path != '/login') {
                    console.log('user is not logged in.');
                    $location.path('/login');
                }
                return config;
            }
        };
    });

    $httpProvider.interceptors.push('HttpInterceptor');
});

Another reason why I'm concerned is that the section on $http in the Angular Docs seem to show a way to get dependencies injected the "regular way" into a Http interceptor. See their code snippet under "Interceptors":

// register the interceptor as a service
$provide.factory('myHttpInterceptor', function($q, dependency1, dependency2) {
  return {
    // optional method
    'request': function(config) {
      // do something on success
      return config || $q.when(config);
    },

    // optional method
   'requestError': function(rejection) {
      // do something on error
      if (canRecover(rejection)) {
        return responseOrNewPromise
      }
      return $q.reject(rejection);
    },



    // optional method
    'response': function(response) {
      // do something on success
      return response || $q.when(response);
    },

    // optional method
   'responseError': function(rejection) {
      // do something on error
      if (canRecover(rejection)) {
        return responseOrNewPromise
      }
      return $q.reject(rejection);
    };
  }
});

$httpProvider.interceptors.push('myHttpInterceptor');

Where should the above code go?

I guess my question is what's the right way to go about doing this?

Thanks, and I hope my question was clear enough.

shaunlim
  • 4,384
  • 6
  • 33
  • 38
  • 1
    Just out of curiosity, what dependencies - if any - where you using in your AuthService? I was having circular dependency issues using the request method in the http interceptor, which brought me here. I'm using angularfire's $firebaseAuth. When I removed the block of code that uses $route from the injector (line 510), everything started working. There's an issue [here](https://github.com/angular/angular.js/issues/2367) but it's about using $http in the interceptor. Off to git! – slamborne Jan 08 '14 at 08:32
  • Hm, for what it's worth, AuthService in my case depends on $window, $http, $location, $q – shaunlim Jan 09 '14 at 07:36
  • I've got a case that retries the request in the interceptor in some circumstances, so there is an even shorter circular dependency on `$http`. The only way around it I've found is to use `$injector.get`, but it would be great to know if there is good way to structure the code to avoid this. – Michal Charemza Jan 14 '14 at 14:16
  • 1
    Take a look at the response from @rewritten: https://github.com/angular/angular.js/issues/2367 that fixed a similar problem for me. What he is doing is like this: $http = $http || $injector.get("$http"); of course you can replace $http with your own service you are trying to use. – Jonathan Mar 12 '15 at 18:09

5 Answers5

67

This is what I ended up doing

  .config(['$httpProvider', function ($httpProvider) {
        //enable cors
        $httpProvider.defaults.useXDomain = true;

        $httpProvider.interceptors.push(['$location', '$injector', '$q', function ($location, $injector, $q) {
            return {
                'request': function (config) {

                    //injected manually to get around circular dependency problem.
                    var AuthService = $injector.get('Auth');

                    if (!AuthService.isAuthenticated()) {
                        $location.path('/login');
                    } else {
                        //add session_id as a bearer token in header of all outgoing HTTP requests.
                        var currentUser = AuthService.getCurrentUser();
                        if (currentUser !== null) {
                            var sessionId = AuthService.getCurrentUser().sessionId;
                            if (sessionId) {
                                config.headers.Authorization = 'Bearer ' + sessionId;
                            }
                        }
                    }

                    //add headers
                    return config;
                },
                'responseError': function (rejection) {
                    if (rejection.status === 401) {

                        //injected manually to get around circular dependency problem.
                        var AuthService = $injector.get('Auth');

                        //if server returns 401 despite user being authenticated on app side, it means session timed out on server
                        if (AuthService.isAuthenticated()) {
                            AuthService.appLogOut();
                        }
                        $location.path('/login');
                        return $q.reject(rejection);
                    }
                }
            };
        }]);
    }]);

Note: The $injector.get calls should be within the methods of the interceptor, if you try to use them elsewhere you will continue to get a circular dependency error in JS.

KreepN
  • 8,528
  • 1
  • 40
  • 58
shaunlim
  • 4,384
  • 6
  • 33
  • 38
  • 4
    Using manual injection ($injector.get('Auth')) solved problem. Good work! – Robert Apr 28 '15 at 06:54
  • To avoid circular dependency i'm checking which url is called. if(!config.url.includes('/oauth/v2/token') && config.url.includes('/api')){ // Call OAuth Service }. Therefor there is no more circular dependency. At least for myself it worked ;). – Brieuc Jun 02 '15 at 14:46
  • Perfect. This is exactly what I needed to fix a similar problem. Thanks @shaunlim! – Martyn Chamberlin Nov 21 '15 at 04:18
  • I don't really like this solution because then this service is anonymous and not as easy to get a handle on for tests. Much better solution to inject on runtime. – kmanzana May 04 '16 at 16:17
  • That worked for me. Basically injecting the service that was using $http. – Thomas Jun 04 '16 at 08:39
  • Ultimately the trick for me here (and to avoid putting the interceptor logic in the config block as the above) was the array syntax with dependencies on interceptors.push. The downside (I am using typescript) was the elimination of the factory pattern in favor of a local method that news up the interceptor with dependencies. – Shawn Aug 09 '16 at 14:58
43

You have a circular dependency between $http and your AuthService.

What you are doing by using the $injector service is solving the chicken-and-egg problem by delaying the dependency of $http on the AuthService.

I believe that what you did is actually the simplest way of doing it.

You could also do this by:

  • Registering the interceptor later (doing so in a run() block instead of a config() block might already do the trick). But can you guarantee that $http hasn't been called already?
  • "Injecting" $http manually into the AuthService when you're registering the interceptor by calling AuthService.setHttp() or something.
  • ...
Pieter Herroelen
  • 5,977
  • 2
  • 29
  • 37
  • 15
    How this answer is solving the problem, I didn't see? @shaunlim – Inanc Gumus Jun 23 '14 at 13:22
  • 1
    Actually its not solving it, its just points out that algorithm flow was badly. – Roman M. Koss Jul 11 '14 at 11:54
  • 13
    You cannot register the interceptor in a ```run()``` block, because you cannot inject $httpProvider to a run block. You can only do that in the configuration phase. – eekboom May 09 '16 at 08:41
  • 2
    Good point re circular reference, but otherwise it should not be an accepted answer. Neither of the bullet points makes any sense – Nikolai Dec 08 '16 at 17:57
15

I think using the $injector directly is an antipattern.

A way to break the circular dependency is to use an event: Instead of injecting $state, inject $rootScope. Instead of redirecting directly, do 

this.$rootScope.$emit("unauthorized");

plus

angular
    .module('foo')
    .run(function($rootScope, $state) {
        $rootScope.$on('unauthorized', () => {
            $state.transitionTo('login');
        });
    });
eekboom
  • 5,551
  • 1
  • 30
  • 39
  • 2
    I think this is more elegant solution, as it will not have any dependency, we can also listen to this event at many places wherever relevant – Basav Jun 12 '16 at 11:20
  • This won't work for my needs, as I cannot have a return value after dispatching an event. – xabitrigo Jun 14 '17 at 16:31
13

Bad logic made such results

Actually there is no point of seeking is user authored or not in Http Interceptor. I would recomend to wrap your all HTTP requests into single .service (or .factory, or into .provider), and use it for ALL requests. On each time you call function, you can check is user logged in or not. If all is ok, allow send request.

In your case, Angular application will send request in any case, you just checking authorization there, and after that JavaScript will send request.

Core of your problem

myHttpInterceptor is called under $httpProvider instance. Your AuthService uses $http, or $resource, and here you have dependency recursion, or circular dependency. If your remove that dependency from AuthService, than you will not see that error.

Also as @Pieter Herroelen pointed, you could place this interceptor in your module module.run, but this will be more like a hack, not a solution.

If your up to do clean and self descriptive code, you must go with some of SOLID principles.

At least Single Responsibility principle will help you a lot in such situations.

Roman M. Koss
  • 970
  • 1
  • 15
  • 33
  • 5
    I don't think this answer is well worded, but I **do** think it gets to the root of the issue. The issue with an auth service that stores current user data *and* the means of logging in (an http request) is that it is responsible for *two* things. If that is instead divided into one service for storing current user data, and another service for logging in, then the http interceptor need only depend on the "current user service", and no longer creates a circular dependency. – Snixtor May 10 '16 at 06:35
  • @Snixtor Thank you! I need to learn English more, to be more clear. – Roman M. Koss May 10 '16 at 12:41
0

If you're just checking for the Auth state (isAuthorized()) I would recommend to put that state in a separate module, say "Auth", which just holds the state and doesn't use $http itself.

app.config(['$httpProvider', function ($httpProvider) {
  $httpProvider.interceptors.push(function ($location, Auth) {
    return {
      'request': function (config) {
        if (!Auth.isAuthenticated() && $location.path != '/login') {
          console.log('user is not logged in.');
          $location.path('/login');
        }
        return config;
      }
    }
  })
}])

Auth Module:

angular
  .module('app')
  .factory('Auth', Auth)

function Auth() {
  var $scope = {}
  $scope.sessionId = localStorage.getItem('sessionId')
  $scope.authorized = $scope.sessionId !== null
  //... other auth relevant data

  $scope.isAuthorized = function() {
    return $scope.authorized
  }

  return $scope
}

(i used localStorage to store the sessionId on client side here, but you can also set this inside your AuthService after a $http call for example)

joewhite86
  • 428
  • 3
  • 6