dojo / core

:rocket: Dojo 2 - language helpers and utilities.
http://dojo.io
Other
213 stars 62 forks source link

fixed usage of node auth with special characters #323

Closed devpaul closed 7 years ago

devpaul commented 7 years ago

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.

devpaul commented 7 years ago

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[bot] commented 7 years ago

Codecov Report

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

Impacted file tree graph

@@            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%) :arrow_up:
src/lang.ts 100% <0%> (ø) :arrow_up:

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.

devpaul commented 7 years ago

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.