fixed usage of node auth with special characters

[devpaul]

Type: bug

request/provider/node was url encoding user names and passwords for basic authentication. Node's http/s request() handles this already by doing a base64 encoding. This is breaking usernames and passwords for Basic auth with special characters due to the extra url encoding.

• There is a related issue
• All code matches the style guide
• Unit or Functional tests are included in the PR

[devpaul]

In the meantime, if anyone needs a workaround for this:

import { RequestOptions, Response } from '@dojo/core/request/interfaces';
import request, { Provider } from '@dojo/core/request';
import nodeRequest from '@dojo/core/request/providers/node';
import Task from '@dojo/core/async/Task';

export function fixAuth(options: RequestOptions) {
	if (options.password || options.user) {
		const credentials = new Buffer(`${ options.user || '' }:${ options.password || ''}`).toString('base64');
		const headers = options.headers = <{ [key: string]: string }> options.headers || {};
		headers['Authorization'] = `Basic ${ credentials }`;
		delete options.password;
		delete options.user;
	}
	return options;
}

const provider: Provider = function (url: string, options: RequestOptions): Task<Response> {
	return nodeRequest(url, fixAuth(options));
};

request.setDefaultProvider(provider);

export default request;

[codecov]

Codecov Report

Merging #323 into master will increase coverage by 0.43%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #323      +/-   ##
==========================================
+ Coverage   93.88%   94.32%   +0.43%     
==========================================
  Files          39       39              
  Lines        2144     2257     +113     
  Branches      408      457      +49     
==========================================
+ Hits         2013     2129     +116     
+ Misses         52       51       -1     
+ Partials       79       77       -2

Impacted Files	Coverage Δ
src/request/providers/node.ts	92.49% <100%> (+4.45%)	⬆️
src/lang.ts	100% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee34d42...6887110. Read the comment docs.

[kitsonk]

Was this tied to any particular version of Node.js. It seems odd we would have been doing something (though untested) only to have it break later on. If there was a change in Node.js, we should identify it so we make sure we have the right compatibility expectations.

[devpaul]

Nothing came up in a search of issues from nodejs/node that looked like it would apply.

If authentication was done in the URL rather than as an auth header then it's possible that you'd want to url encode the values, but then you wouldn't base64 encode it.