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

Fixing Authorization header with basic auth #397

Merged
merged 1 commit into from Aug 4, 2016

Conversation

@madebyherzblut
Copy link
Contributor

commented Aug 3, 2016

The http adapter did not remove a custom Authorization header when auth is set (as it is state in the documentation).

@coveralls

This comment has been minimized.

Copy link

commented Aug 3, 2016

Coverage Status

Coverage remained the same at 92.876% when pulling 4415221 on madebyherzblut:fix-auth-header into 8332392 on mzabriskie:master.

@nickuraltsev

This comment has been minimized.

Copy link
Member

commented Aug 3, 2016

Thank you for the PR! Should we delete the Authorization header in this case as well?

    var parsed = url.parse(config.url);
    if (!auth && parsed.auth) {
      var urlAuth = parsed.auth.split(':');
      var urlUsername = urlAuth[0] || '';
      var urlPassword = urlAuth[1] || '';
      auth = urlUsername + ':' + urlPassword;
    }

I would suggest to check whether auth is defined (after the code block above) and if it is, delete the header:

if (auth) {
  delete headers.Authorization;
}
Fixing Authorization header with basic auth
The http adapater did not remove a custom Authorization header when auth is set.

@madebyherzblut madebyherzblut force-pushed the madebyherzblut:fix-auth-header branch from 4415221 to 1ffcbb0 Aug 4, 2016

@coveralls

This comment has been minimized.

Copy link

commented Aug 4, 2016

Coverage Status

Coverage remained the same at 92.876% when pulling 1ffcbb0 on madebyherzblut:fix-auth-header into 8332392 on mzabriskie:master.

@madebyherzblut

This comment has been minimized.

Copy link
Contributor Author

commented Aug 4, 2016

Thank you and nice catch @nickuraltsev 👍 I made the changes and amended the original commit to keep the history clean.

@nickuraltsev nickuraltsev merged commit 8abe0d4 into axios:master Aug 4, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.