Introduced httpUpdate parameter for loginConfirmed function

[Astrac]

Proposed implementation for

https://github.com/witoldsz/angular-http-auth/issues/30

[witoldsz]

Hi, can you show us some code using this new feature? I am asking, because you are providing a function to alter buffered requests. What about all further requests? Is this function applied also to default configuration of $http service? If so, I would suggest trying to make buffered requests to adapt to the new configuration of $http service instead of altering buffered requests independently. Does it make any sense?

[Astrac]

@stu-salsbury I thought about updating the default $http config (in my case the default headers), but then I thought that I will also need the $http service to communicate with another API that doesn't need the same configuration as google does.

@witoldsz I have a service that has these code:

var request = function (feed, parameters) {
  if (!parameters) {
    parameters = {};
  }
  parameters.alt = 'json';

  return $http({
    method: 'GET',
    url: feed,
    params: parameters,
    headers: {
      Authorization: 'Bearer ' + msGoogleAuth.getToken()
    }
  }).error(errorHandler);
};

return {
  'request': request
};

The "msGoogleAuth" service is listening to the login-required events, performing oauth authentication using the google gapi library, storing the token and calling the loginConfirmed method. Since the $http objects have already been created without the Authorization header when retrying they will fail over and over again (despite the token is now available).

Using my patch I do something like that when confirming the login:

authService.loginConfirmed(function(config) {
  return _.extend(config, {'headers': {'Authorization': 'Bearer ' + msGoogleAuth.getToken() }});
});

I understand that there is a bit of code duplication here (the header is added in two different places), so any alternative suggestion will be much appreciated!

[laurelnaiad]

Are the OAuth headers so expensive that they can't live in the default headers even for requests that don't need the OAuth token?

If so, maybe you want to find a way to decorate $http to allow it to have a set of different configs from which you can choose when any given request is made? Perhaps based on the presence of absense of some session objects?

It seems to me that forcing them into subsequent retries this way is an unclean way of accomplishing the objective. Won't those same headers be necessary in future requests that need this authentication?

I think if there's room for angular-http-auth to help, it's in getting involved with sessions, which might be outside the bounds of angular-http-auth's mission?

[witoldsz]

@Astrac Is your case related to #24? If it is, does the solution work?

[Astrac]

@witoldsz Yes, it's definitely related to #24. I'm using it in my project and it does work. As you can see I use the httpUpdate function to extend the config objects and to add an authentication header before retrying. In my mind the function mechanism is a way to intercept the request before executing it so that additional logic in general can be added.

@stu-salsbury The problem is not the expensiveness of the header, but I think that it would be more unclean in an application to mix the headers that you're sending to different backends. Moreover some backends may fail to accept the request if there are headers that are not permitted (e.g. if the request is being done in a CORS way).

The idea to decorate the $http service is something I was thinking about, but the problem is that you should be able to overwrite the $http service since angular-http-auth uses it and not the decorated one that I could create.

[laurelnaiad]

@Astrac I see your points. I think think the cleaner solution would be to have retries use updated defaults as their defaults with any overrides coming from the initial defaults at the time they became bufferred b) let the user push additional overrides when the user does whatever it is that lets the retries happen (presently that's loginConfirmed()).

If an http service is rejecting a request because there are headers it doesn't understand and shouldn't care about in the context of the resource being requested, then isn't that service malfunctioning?

[Astrac]

@stu-salsbury In the case of CORS requests headers are to be treated in a strict manner, it's the protocol that enforces it. That means that requests done from a different origin than the target domain (in my case my website is the origin and google the target) may be refused if there are headers that are not explicitly allowed by the target itself. I had also to remove from the defaults the X-Requested-With headers to get my CORS requests working from angular.

[laurelnaiad]

I am aware of the X-Requested-With header and how that needs special handling for angular.

But is it really true that if I walk around with an OAuth token for gmail that I won't be able to log in to stackoverflow? Somehow they're all staying authenticated for me, and I doubt it's because they're conniving together to make my requests all have only the unique headers that they individually need or expect.

[witoldsz]

@Astrac If you say the solution of #24 works, this is to use $http.get, $http.post, etc..., so the altered $http config is applied by angular itself when retrying, then is this pull request still relevant?

[Astrac]

@witoldsz Sorry, I meant that my solution works, not #24. Anyway, I didn't try it but I'm quite sure that it will work in that use case. But mine is a bit more complex: I use $http to deal with multiple backends and I don't want to add the authorization header to the default config for these reasons:

• Different backends will have different tokens for the same "Authorization" header.
• I don't feel comfortable sending my google token to another service.
• CORS will be refused by the "x" service if there is an header that was meant for the "y" service.

Is there any other way you can think about to address my use case?

[witoldsz]

It is merged already: 10dd81a9ffdca4e10abe79e1141fe349ba161401 (since 2013-08-22), closing.