witoldsz / angular-http-auth

MIT License
2.38k stars 417 forks source link

Include status code 419? #91

Open jsuiker opened 9 years ago

jsuiker commented 9 years ago

It certainly has been useful for me when working with tokens. Are people wary of supporting a non-standard status code like 419?

.config(['$httpProvider', function($httpProvider) {
  $httpProvider.interceptors.push(['$rootScope', '$q', 'httpBuffer', function($rootScope, $q, httpBuffer) {
    return {
      responseError: function(rejection) {
        if (!rejection.config.ignoreAuthModule) {
          switch (rejection.status) {
            case 419:
            case 401:
              var deferred = $q.defer();
              httpBuffer.append(rejection.config, deferred);
              $rootScope.$broadcast('event:auth-loginRequired', rejection);
              return deferred.promise;
            case 403:
              $rootScope.$broadcast('event:auth-forbidden', rejection);
              break;
          }
        }
        // otherwise, default behaviour
        return $q.reject(rejection);
      }
    };
  }]);
}]);
witoldsz commented 9 years ago

Hi, we need to find some better way of handling such custom codes. I was thinking about something like adding an option like forceAuthModule; it would work contrary to ignoreAuthModule.

Now, with option like this, you could just insert an interceptor:

.config(['$httpProvider', function($httpProvider) {
  $httpProvider.interceptors.push(['$q', function($q) {
    return {
      responseError: function(rejection) {
        rejection.config.forceAuthModule = (rejection.status === 419);
        return $q.reject(rejection);
      }
    };
  }]);
}]);

If such an interceptor would be registered before the one from this module, it should make it work with 419 codes, I guess.

Another solution would be to provide an option to add some kind of extra "rejection resolver" function, so everyone could amend the way this module triggers, but it would require more changes in code than the first option with simple flag.

What do you think?

wcyrek-comrise commented 9 years ago

Hi, I too am using 419 for expired JWT token much in the same way as OP. It could be useful to have configuration for nonstandard codes, but coding that in myself was not too difficult. If anything, supporting JWTs out of the box would be a much better addition.

wcyrek-comrise commented 9 years ago

Continuing on this issue. I am adding yet another code in addition to 419 (I need something to handle accounts that were logged in but are not yet confirmed accounts, which forces the user to enter confirmation code from email). This seems like also a common use-case for deferment and should have it's own set of events. I now feel that adding both of these use-cases to this module would be useful, since i end up adding the necessary code to app.js and my controller instead of http-auth-interceptor.js, and not having all the code that does similar things in one place is a headache and triggers my OCD :)

Dzienki,

Witek

wcyrek-comrise commented 9 years ago

Ok one more thing that I found useful is using call to authService.loginConfirmed() in the event listener for my custom event auth-confirmConfirmed. But this is not ideal because that call also triggers auth-loginConfirmed event.

Anyway we can split off the part of loginConfirmed that will configure the updater and call httpBuffer.retryAll(updater) from the part that broadcasts the event? That way we can reuse the former without the latter causing possibly unintended behavior. Or maybe we can have the event to be broadcast passed as an optional parameter?

vlapo commented 8 years ago

@witoldsz What about config property with HTTP status codes for loginRequired and forbidden (401/403 default configuration)? Call it support non-standard HTTP status codes.

Now module support only standardized 401 and 403 HTTP statuses. But I could imagine some project use non-standard HTTP codes.

Of course it is necessary create some kind of module configuration and so on.

witoldsz commented 8 years ago

Yes, I like that idea. The only thing to figure out is how exactly the configuration should look like, i.e. what should be hard-coded and what should be open for change.

vlapo commented 8 years ago

Configuration is very powerful tool and it give us space to create new functionalities. But angular-http-auth is simple module, so I think configuration dont have to be complicated.

loginConfirmed: function(data, configUpdater) {
    var updater = configUpdater || function(config) {return config;};
    $rootScope.$broadcast(config.eventPrefix + 'loginConfirmed', data);
    httpBuffer.retryAll(updater);
},
loginConfirmed: function(data, configUpdater) {
    var updater = configUpdater || function(config) {return config;};
    $rootScope.$broadcast('event:auth-loginConfirmed', data);
    if(config.retryOnLoginConfirmed) {
        httpBuffer.retryAll(updater);
    }
},

IMHO non-standard HTTP codes and event prefix are enough.

kofronpi commented 8 years ago

What if you want to include additional interceptors on errors such as 500, etc. ?