Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include status code 419? #91

Open
jsuiker opened this issue Mar 10, 2015 · 8 comments
Open

Include status code 419? #91

jsuiker opened this issue Mar 10, 2015 · 8 comments

Comments

@jsuiker
Copy link

jsuiker commented Mar 10, 2015

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
Copy link
Owner

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
Copy link

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
Copy link

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
Copy link

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
Copy link

vlapo commented Jul 20, 2015

@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
Copy link
Owner

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
Copy link

vlapo commented Jul 20, 2015

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
Copy link

kofronpi commented Aug 6, 2015

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants