Reputation: 4374
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.
Upvotes: 122
Views: 46983
Reputation: 418
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)
Upvotes: 0
Reputation: 4374
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.
Upvotes: 67
Reputation: 5802
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');
});
});
Upvotes: 15
Reputation: 999
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.
Upvotes: 13
Reputation: 6066
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:
run()
block instead of a config()
block might already do the trick). But can you guarantee that $http hasn't been called already?AuthService.setHttp()
or something.Upvotes: 43