Several issues with adal5-http.service

[jo-me]

Hey there

I've integrated your adal wrapper and found several issues with the Adal5HTTPService.

1. Contrary to this.http.request throws error in http service #1 the angular common http client's sendrequest has 3 arguments: method, url, options, see the debugger output here:
   

If the method is omitted, then the request object becomes messed up. The URL is in the method field...
Not sure where the difference comes from, but I'm using Angular 5.1.

2. The authorization header is not added if there are already headers present in the request.
   adal-angular4 does not have this issue.

Both issues can be fixed by replacing the block in sendRequest method with:

authenticatedCall = this.service.acquireToken(resource)
	.flatMap(function (token) {
		if (options.headers == null) {
			options.headers = new http_1.HttpHeaders();
		}
		options.headers = options.headers.append('Authorization', 'Bearer ' + token);
		return _this.http.request(method, url, options)
			.catch(_this.handleError);
	});

[grumar]

Fixed in d021fed, please let me now if everything works like expected.

[geerzo]

This works for me. I would change
options.headers = options.headers.append('Authorization', 'Bearer ' + token);
to
options.headers = options.headers.set('Authorization', 'Bearer ' + token);

This will ensure that if someone set that field already it's overwritten. Otherwise you wind up with 2 Authorization headers.

[pandianramalingam]

Hi @grumar ,

Set authorization header code throwing error if there are already headers present in the request.

BR,
PR

[geerzo]

Yes, the existing code has an error, this change fixes it.

[pandianramalingam]

when we can expect updated version ?

[grumar]

@geerzo @pandianramalingam
I made some changes in d021fed few days ago. Did You try lastest version?

[geerzo]

@grumar yes I have and it doesn’t work if the headers is already set because the local headers variable never gets set. You if block checks if options.headers is null then sets the headers variable but if that check fails the headers variable is never set but the next line sets the Authorization header and it fails because headers is null.

[jo-me]

The original issue is resolved. THanks for that!

For more fault tolerance, I'd check if the options parameter is actually set because otherwise the request will fail altogether. If no special http options are needed the parameter could be omitted.

[stamler]

@jo-me Making observe: optional in the options object and using the HttpObserve type (like HttpClient does) should solve the problem. I did it but haven't tested it yet. May be a nice update to @grumar 's code.

[grumar]

Fixed in #22 & #23

[grumar]

published as beta
npm install adal-angular5@2.1.1