Skip to content

Commit

Permalink
fix: safe join for urls (#265)
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanshortiss
evanshortiss authored May 26, 2021
1 parent b77d888 commit 16ac479
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 31 deletions.
2 changes: 1 addition & 1 deletion lib/authorization-server-request.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const buildError = (requestError) => {
const getAuthUrlFromOCP = async (url, insecureSkipTlsVerify = true) => {
const req = {
method: 'GET',
url: `${url}/.well-known/oauth-authorization-server`,
url: new URL('/.well-known/oauth-authorization-server', url).toString(),
strictSSL: insecureSkipTlsVerify
};

Expand Down
2 changes: 1 addition & 1 deletion lib/basic-auth-request.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ async function getUserFromAuthToken (settings) {
return new Promise((resolve, reject) => {
const req = {
method: 'GET',
url: `${settings.url}/apis/user.openshift.io/v1/users/~`,
url: new URL('/apis/user.openshift.io/v1/users/~', settings.url).toString(),
auth: {
bearer: settings.token
},
Expand Down
47 changes: 35 additions & 12 deletions test/authorization-server-request-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

const test = require('tape');
const proxyquire = require('proxyquire');
const BASE_URL = 'http://some.cluster.com:6443/';

test('authorization server request', (t) => {
const authorizationServerRequest = proxyquire('../lib/authorization-server-request', {
Expand All @@ -12,16 +13,38 @@ test('authorization server request', (t) => {
return cb(null, {
statusCode: 200
},
'{"authorization_endpoint": "http://"}');
`{"authorization_endpoint": "${BASE_URL}"}`);
}
});

const p = authorizationServerRequest.getAuthUrlFromOCP('http://', false);
const p = authorizationServerRequest.getAuthUrlFromOCP(BASE_URL, false);

t.equal(p instanceof Promise, true, 'is an Promise');

p.then((url) => {
t.equal(url, 'http://?response_type=token&client_id=openshift-challenging-client', 'should be equal');
t.equal(url, `${BASE_URL}?response_type=token&client_id=openshift-challenging-client`, 'should be equal');
t.end();
});
});

test('authorization server request URL join safety', (t) => {
const authorizationServerRequest = proxyquire('../lib/authorization-server-request', {
request: (requestObject, cb) => {
t.equal(requestObject.strictSSL, false, 'should be false');
t.equal(requestObject.url, `${BASE_URL}.well-known/oauth-authorization-server`);
return cb(null, {
statusCode: 200
},
`{"authorization_endpoint": "${BASE_URL}"}`);
}
});

const p = authorizationServerRequest.getAuthUrlFromOCP(BASE_URL, false);

t.equal(p instanceof Promise, true, 'is an Promise');

p.then((url) => {
t.equal(url, `${BASE_URL}?response_type=token&client_id=openshift-challenging-client`, 'should be equal');
t.end();
});
});
Expand All @@ -33,16 +56,16 @@ test('authorization server request without insecureSkipTlsVerify', (t) => {
return cb(null, {
statusCode: 200
},
'{"authorization_endpoint": "http://"}');
`{"authorization_endpoint": "${BASE_URL}"}`);
}
});

const p = authorizationServerRequest.getAuthUrlFromOCP('http://');
const p = authorizationServerRequest.getAuthUrlFromOCP(BASE_URL);

t.equal(p instanceof Promise, true, 'is an Promise');

p.then((url) => {
t.equal(url, 'http://?response_type=token&client_id=openshift-challenging-client', 'should be equal');
t.equal(url, `${BASE_URL}?response_type=token&client_id=openshift-challenging-client`, 'should be equal');
t.end();
});
});
Expand All @@ -55,21 +78,21 @@ test('authorization server request with empty body', (t) => {
statusCode: 200,
request: {
uri: {
host: 'https://'
host: BASE_URL
}
}
},
'{}');
}
});

const p = authorizationServerRequest.getAuthUrlFromOCP('http://', false);
const p = authorizationServerRequest.getAuthUrlFromOCP(BASE_URL, false);

t.equal(p instanceof Promise, true, 'is an Promise');

p.catch((error) => {
t.equal(error.message,
'Unable to retrieve the token_endpoint for https://. Cannot obtain token_endpoint from response.',
`Unable to retrieve the token_endpoint for ${BASE_URL}. Cannot obtain token_endpoint from response.`,
'should be equal');
t.end();
});
Expand All @@ -83,14 +106,14 @@ test('authorization server request with 404 status code', (t) => {
statusCode: 404,
request: {
uri: {
host: 'https://'
host: BASE_URL
}
}
});
}
});

const p = authorizationServerRequest.getAuthUrlFromOCP('http://', false);
const p = authorizationServerRequest.getAuthUrlFromOCP(BASE_URL, false);

t.equal(p instanceof Promise, true, 'is an Promise');

Expand All @@ -113,7 +136,7 @@ test('authorization server request with error', (t) => {
}
});

const p = authorizationServerRequest.getAuthUrlFromOCP('http://', false);
const p = authorizationServerRequest.getAuthUrlFromOCP(BASE_URL, false);

t.equal(p instanceof Promise, true, 'is an Promise');

Expand Down
72 changes: 55 additions & 17 deletions test/basic-auth-request-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@

const test = require('tape');
const proxyquire = require('proxyquire');
const BASE_URL = 'http://some.cluster.com:6443/';

test('basic auth request', (t) => {
const basicAuthRequest = proxyquire('../lib/basic-auth-request', {
'./authorization-server-request': {
getAuthUrlFromOCP: (url, insecureSkipTlsVerify) => {
t.equal(url, 'http://');
return Promise.resolve('https://');
t.equal(url, BASE_URL);
return Promise.resolve(BASE_URL);
}
},
request: (requestObject, cb) => {
Expand All @@ -27,7 +28,7 @@ test('basic auth request', (t) => {
});

const settings = {
url: 'http://',
url: BASE_URL,
user: 'username',
password: 'password',
insecureSkipTlsVerify: true
Expand All @@ -47,8 +48,8 @@ test('basic auth request with 404 status code', (t) => {
const basicAuthRequest = proxyquire('../lib/basic-auth-request', {
'./authorization-server-request': {
getAuthUrlFromOCP: (url, insecureSkipTlsVerify) => {
t.equal(url, 'http://');
return Promise.resolve('https://');
t.equal(url, BASE_URL);
return Promise.resolve(BASE_URL);
}
},
request: (requestObject, cb) => {
Expand All @@ -57,15 +58,15 @@ test('basic auth request with 404 status code', (t) => {
statusCode: 404,
request: {
uri: {
host: 'https://'
host: BASE_URL
}
}
});
}
});

const settings = {
url: 'http://',
url: BASE_URL,
user: 'username',
password: 'password',
insecureSkipTlsVerify: true
Expand All @@ -77,7 +78,7 @@ test('basic auth request with 404 status code', (t) => {

p.catch((error) => {
t.equal(error.message,
'Unable to authenticate user username to https://. Cannot obtain access token from response.',
`Unable to authenticate user username to ${BASE_URL}. Cannot obtain access token from response.`,
'should be equal');
t.end();
});
Expand All @@ -87,8 +88,8 @@ test('basic auth request with 401 status code', (t) => {
const basicAuthRequest = proxyquire('../lib/basic-auth-request', {
'./authorization-server-request': {
getAuthUrlFromOCP: (url, insecureSkipTlsVerify) => {
t.equal(url, 'http://');
return Promise.resolve('https://');
t.equal(url, BASE_URL);
return Promise.resolve(BASE_URL);
}
},
request: (requestObject, cb) => {
Expand All @@ -97,15 +98,15 @@ test('basic auth request with 401 status code', (t) => {
statusCode: 401,
request: {
uri: {
host: 'https://'
host: BASE_URL
}
}
});
}
});

const settings = {
url: 'http://',
url: BASE_URL,
user: 'username',
password: 'password',
insecureSkipTlsVerify: true
Expand All @@ -128,7 +129,7 @@ test('basic auth request with request error', (t) => {
'./authorization-server-request': {
getAuthUrlFromOCP: (url, insecureSkipTlsVerify) => {
t.equal(url, undefined);
return Promise.resolve('https://');
return Promise.resolve(BASE_URL);
}
},
request: (requestObject, cb) => {
Expand Down Expand Up @@ -168,7 +169,42 @@ test('get user from token', (t) => {
});

const settings = {
url: 'http://',
url: BASE_URL,
token: '12346',
insecureSkipTlsVerify: true
};

const p = basicAuthRequest.getUserFromAuthToken(settings);

t.equal(p instanceof Promise, true, 'is an Promise');

p.then((userObject) => {
t.equal(userObject.metadata.name, 'developer', 'user should be equal');
t.end();
});
});

test('get user from token URL join safety', (t) => {
const basicAuthRequest = proxyquire('../lib/basic-auth-request', {
request: (requestObject, cb) => {
t.equal(requestObject.strictSSL, false, 'should be false');
t.equal(requestObject.url, `${BASE_URL}apis/user.openshift.io/v1/users/~`);

return cb(null, {
statusCode: 200
},
JSON.stringify({
kind: 'User',
metadata: {
name: 'developer'
}
})
);
}
});

const settings = {
url: BASE_URL, // note the trailing slash
token: '12346',
insecureSkipTlsVerify: true
};
Expand All @@ -191,15 +227,15 @@ test('get user from token with 401 status code', (t) => {
statusCode: 401,
request: {
uri: {
host: 'https://'
host: BASE_URL
}
}
});
}
});

const settings = {
url: 'http://',
url: BASE_URL,
token: '12346',
insecureSkipTlsVerify: true
};
Expand All @@ -223,7 +259,9 @@ test('get user from token with request error', (t) => {
}
});

const settings = {};
const settings = {
url: BASE_URL
};

const p = basicAuthRequest.getUserFromAuthToken(settings);

Expand Down

0 comments on commit 16ac479

Please sign in to comment.