paulvanbladel / aurelia-auth

:key: Authentication plugin for aurelia
200 stars 74 forks source link

Bearer present twice in header #116

Closed michalru3ch closed 8 years ago

michalru3ch commented 8 years ago

Hi Paul / the team.

Yesterday I've upgraded aurelia-auth in my project from 0.12.15 to 2.1.3, did the necessary changes based on documentation and sample app and I'm experiencing quite strange behavior:

1) Bearer is present twice in every request causing it to fail (HEADER: authorization:Bearer xyzValue,Bearer xyzValue)

2) When I comment following line .withInterceptor(auth.tokenInterceptor) it's present just once and everything works fine. Bearer shouldn't be present though, right? Btw, I'm not injecting it myself.

I've installed this library via jspm. Could you please comment on this? Am I missing something?

Thank you in advance!

michalru3ch commented 8 years ago

One more thing I forgot to mention: the only difference between docs & sample app compared to my project is that I didn't create CustomHttpClient. I'm configuring Aurelia's HttpClient, because all my calls require authentication.

paulvanbladel commented 8 years ago

Hi, Are you sure you are calling this.fetchConfig.configure(); Only once? Or maybe combining with explicit config on http client?

paulvanbladel commented 8 years ago

Anyhow, we can build in something to allow only one config.

michalru3ch commented 8 years ago

Yes, I'm calling this.fetchConfig.configure() - in activate method of app.js which is executed just once for whole life cycle of the application.

What about point #2. What/who is responsible for adding the Bearer?

paulvanbladel commented 8 years ago

You wrote that you did als upgrade of the config up to the documentation, but there is no real change necessary when going to 2.x

michalru3ch commented 8 years ago

Based on documentation I changed following:

1) added headers: { 'Accept': 'application/json', 'X-Requested-With': 'Fetch' }

2) Instead of including 'fetch' I followed recommendation to include 'isomorphic-fetch' instead

These are the only changes I've made.

michalru3ch commented 8 years ago

.withInterceptor(_this.auth.tokenInterceptor) was causing problems so I left it out

mbroadst commented 8 years ago

@paulvanbladel @michalru3ch I think this line needs to change from .append to .set: https://github.com/paulvanbladel/aurelia-auth/blob/master/src/authentication.js#L163

michalru3ch commented 8 years ago

Hmmm, so once I've created CustomHttpClient as describe in documentation and present in sample app, everything started to work correctly - I can use .withInterceptor(auth.tokenInterceptor), Bearer is present once. When I left this line out, no Bearer was attached as expected.

So it means that configuring/modifying Aurelia's httpClient is not supported anymore; you have to derive from it and configure/modify that object.

Sorry for keeping you busy with this issue. I'm not sure whether my initial approach should be supported or not. I do not have any reservations. Thank you!

paulvanbladel commented 8 years ago

No worries, Maybe the docs are not clear enough. You can modify standard http client, but when doing so you should not call this.fetchConfig.configure();

paulvanbladel commented 8 years ago

@mbroadst yep definitely. That makes things "idempotent".

michalru3ch commented 8 years ago

One last question: Is it necessary to import HttpClient and 'isomorphic-fetch' into every model where I want to use CustomHttpClient? CustomHttpClient imports them so it seems redundant to me.

paulvanbladel commented 8 years ago

Indeed.

mbroadst commented 8 years ago

@michalru3ch no you shouldn't have to. isomorphic-fetch one time will be sufficient to "install" the polyfill, and your CustomHttpClient should be injecting HttpClient and therefore you're dealing with singletons there.

michalru3ch commented 8 years ago

Yes, I thought so... Paul, please update aurelia-auth-sample, so other developers won't repeat it everywhere.

Thank you both; it was pleasure to code with you :)