paulvanbladel / aurelia-auth

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

Unsupported BodyInit type #105

Open jasonhjohnson opened 8 years ago

jasonhjohnson commented 8 years ago

So, in IE 9 when I tried using this plugin I got the following error after authenticating in the popup: Blob is undefined

So, I figured a Blob polyfill might be needed and threw this in my pages header: https://rawgit.com/eligrey/Blob.js/master/Blob.js

That seemed to fix the Blob error. But then, got a different error: Unsupported BodyInit type

I thought it might have something to do with the fetch polyfil so I tried using the fetch-ie8 polyfill instead but it didn't fix it. Any help is appreciated.

Thanks, Jason

jasonhjohnson commented 8 years ago

I think it may be related to this issue: https://github.com/facebook/react-native/issues/2538.

Perhaps adjust the POST in your login() method to something like:

return this.http.fetch(loginUrl, {
      method: 'post',
      headers: typeof(content)==='string' ? {'Content-Type': 'application/x-www-form-urlencoded', 'Accept': 'application/json'} : {},
      body: typeof(content)==='string' ? JSON.stringify(content) : JSON.stringify(content)
    })     
  }
jasonhjohnson commented 8 years ago

Actually, it's probably needed in the exchangeForToken method of oauth2.js

jasonhjohnson commented 8 years ago

I got this to work in IE 9 (still works in other browsers) by changing the POST in exchangeForToken to the following:

return this.http.fetch(exchangeForTokenUrl, {
      method: 'post',
      headers: { 'Content-Type': 'application/json; charset=utf-8' },
      body: JSON.stringify(data),
      credentials: credentials
    })

Could you incorporate this change?

Thanks, J

mbroadst commented 8 years ago

@jasonhjohnson put together a PR we'll get it in

jasonhjohnson commented 8 years ago

@mbroadst Sure thing. Also, you guys may not care about IE 9 (that sounded snotty but I didn't mean it that way), but in addition to the change above I had to use two polyfills:

<script src="//cdn.rawgit.com/eligrey/Blob.js/master/Blob.js"></script>
<script src="//cdn.rawgit.com/davidchambers/Base64.js/master/base64.min.js"></script>
mbroadst commented 8 years ago

@jasonhjohnson yeah I think that information is probably better placed directly on the README, I don't think it makes sense to include the polyfill since most browsers already support those (plus this is more related to fetch rather than aurelia-auth)

jasonhjohnson commented 8 years ago

@mbroadst Agreed. https://github.com/paulvanbladel/aurelia-auth/pull/107

ghiscoding commented 7 years ago

I actually got this problem with the Twitter and OAuth1.js and Koa (server side). It seems that Koa doesn't like the POST with missing Content-Type, I get the following error

  InternalServerError: Missing content-type
      at Object.throw (C:\OwnSyncDev\aurelia-auth-sample\server\node_modules\koa\lib\context.js:91:23)
      at Object.exports.authenticate (C:\OwnSyncDev\aurelia-auth-sample\server\auths\twitter.js:110:22)
      at exports.authenticate.throw (<anonymous>)

I traced down the problem to be in the oauth1.js file.

OAuth1.prototype.exchangeForToken = function exchangeForToken(oauthData, userData, current) {
      var data = (0, _authUtilities.extend)({}, userData, oauthData);
      var exchangeForTokenUrl = this.config.baseUrl ? (0, _authUtilities.joinUrl)(this.config.baseUrl, current.url) : current.url;
      var credentials = this.config.withCredentials ? 'include' : 'same-origin';

      return this.http.fetch(exchangeForTokenUrl, {
        method: 'post',
        body: (0, _aureliaFetchClient.json)(data),
        credentials: credentials
      }).then(_authUtilities.status);

Adding a Content-Type on that http.fetch fixes the problem. However like @paulvanbladel suggested in the PR #107, the Content-Type could also be added in the auth-fetch-config.

So here's the final fix which works for me

this.httpClient.configure(function (httpConfig) {
        httpConfig.withDefaults({
          headers: {
            'Accept': 'application/json',
            'Content-Type': 'application/json' // <<-- this line -->>
          }
        }).withInterceptor(_this.auth.tokenInterceptor);
      });

I can make a PR if really required

alexsaare commented 7 years ago

This fix breaks the default behaviour when posting multipart data. The boundary information of the Content Type header should be dynamically added by the browser. Forcing the content type to appication/json means that you can not remove it and allow aurelia-fetch to do it's own thing

import {HttpClient, json} from 'aurelia-fetch-client';

@inject(HttpClient)
export class MyClass {

constructor(private client : HttpClient){} 

public createDocument(fileKey :string, files : FileList) : Promise<MyResponse> {
    var form = new FormData();

    for (let i = 0; i < files.length; i++) {
       form.append(fileKey, files[i]);
     }

    return this.client
       .fetch('someurl', { method: 'post', body: form })
       .then(response => {
             ...process response
       });
}

Setting it to undefined as part of the fetch (as suggested here) actually outputs the string "undefined" in the header - same for null. I.E. the below doesn't work.

    .fetch('someurl', { method: 'post', body: form, headers: {'ContentType': undefined } })

I can create a new HttpClient but then i can't add the interceptor as the Authentication module is not exposed.

Let me know if i'm missing something!

ielcoro commented 6 years ago

Please revert the merged Pull Request #168 and set the headers explicitly on request instead of messing with the defaults.

Versión 3.0.5 totally broke various ongoing applications, because as @alexsaare said, the defaults prevent multipart header from being calculated automatically when using FormData.

For the time being we don't use FetchConfig for configuring the library and instead we rely on manual configuration:

function configureHttpClient(aurelia: Aurelia): void {
    let httpClient: HttpClient = aurelia.container.get(HttpClient);
    let auth = aurelia.container.get(AuthService);

     // Do not use FetchConfig as #105 broke all multipart requests, instead configure it manually
    // let httpClientAuth: FetchConfig = aurelia.container.get(FetchConfig);

    // httpClientAuth.configure();

    //manually configure httpclient auth interceptor
    httpClient.configure(config => {
        config.withInterceptor(auth.tokenInterceptor)
ielcoro commented 6 years ago

Just uploaded a pull request that reverts this behavior and instead uses a explicit per-request headers approach.

alexsaare commented 6 years ago

Just FYI -

To work around this issue, I extended HttpClient like this

import {HttpClient} from 'aurelia-fetch-client';
import {inject} from 'aurelia-framework';
import {AuthService} from 'aurelia-auth';

@inject(AuthService)
export class NonJsonHttpClient extends HttpClient {
  constructor(auth) {
    super();
    this.configure(config => {
      config
        .withInterceptor(auth.tokenInterceptor);
    });
  }
}

So for the classes that need MultiPart headers I inject NonJsonHttpClient rather than HttpClient.

This gives me a different instance which works as expected.

paulvanbladel commented 6 years ago

@ielcoro are you interested to join as admin and handle the PR?

ielcoro commented 6 years ago

@paulvanbladel it would be a pleasure. However I have to be honest, I do not have much time to spare on side projects, at least not as much as I would love, so I can do some code reviews and issue filtering, pr mergin etc, but I don't have the time to take the ownership of the repo, if that is ok with you, yes I can start handling this PR.

paulvanbladel commented 6 years ago

Thanks.