Skip to content

Commit

Permalink
fix: add non-spec username parsing back (#277)
Browse files Browse the repository at this point in the history
  • Loading branch information
szmarczak authored Aug 3, 2022
1 parent 0a24d1e commit b3ccf3d
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 4 deletions.
5 changes: 4 additions & 1 deletion src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,10 @@ export class Server extends EventEmitter {
throw new RequestError('Invalid "Proxy-Authorization" header', 400);
}

if (auth.type !== 'Basic') {
// https://datatracker.ietf.org/doc/html/rfc7617#page-3
// Note that both scheme and parameter names are matched case-
// insensitively.
if (auth.type.toLowerCase() !== 'basic') {
throw new RequestError('The "Proxy-Authorization" header must have the "Basic" type.', 400);
}

Expand Down
27 changes: 26 additions & 1 deletion src/utils/parse_authorization_header.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Buffer } from 'node:buffer';

const splitAt = (string: string, index: number) => {
return [
index === -1 ? '' : string.substring(0, index),
Expand All @@ -23,12 +25,35 @@ export const parseAuthorizationHeader = (header: string): Authorization | null =

const [type, data] = splitAt(header, header.indexOf(' '));

// https://datatracker.ietf.org/doc/html/rfc7617#page-3
// Note that both scheme and parameter names are matched case-
// insensitively.
if (type.toLowerCase() !== 'basic') {
return { type, data };
}

const auth = Buffer.from(data, 'base64').toString();
const [username, password] = splitAt(auth, auth.indexOf(':'));

// https://datatracker.ietf.org/doc/html/rfc7617#page-5
// To receive authorization, the client
//
// 1. obtains the user-id and password from the user,
//
// 2. constructs the user-pass by concatenating the user-id, a single
// colon (":") character, and the password,
//
// 3. encodes the user-pass into an octet sequence (see below for a
// discussion of character encoding schemes),
//
// 4. and obtains the basic-credentials by encoding this octet sequence
// using Base64 ([RFC4648], Section 4) into a sequence of US-ASCII
// characters ([RFC0020]).

// Note:
// If there's a colon : missing, we imply that the user-pass string is just a username.
// This is a non-spec behavior. At Apify there are clients that rely on this.
// If you want this behavior changed, please open an issue.
const [username, password] = auth.includes(':') ? splitAt(auth, auth.indexOf(':')) : [auth, ''];

return {
type,
Expand Down
36 changes: 35 additions & 1 deletion test/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,9 @@ const createTestSuite = ({
}

if (mainProxyAuth) {
if (mainProxyAuth.username !== username || mainProxyAuth.password !== password) {
const authDoesNotMatch = mainProxyAuth.username !== username || mainProxyAuth.password !== password;
const nopassword = username === 'nopassword' && password === '';
if (authDoesNotMatch && !nopassword) {
result.requestAuthentication = true;
addToMainProxyServerConnectionIds = false;
// Now that authentication is requested, upstream proxy should not get used,
Expand Down Expand Up @@ -965,6 +967,38 @@ const createTestSuite = ({
});

if (mainProxyAuth) {
it('implies username if colon missing', (done) => {
const server = net.createServer((socket) => {
socket.end();
});

server.once('error', (error) => {
done(error);
});

server.listen(0, () => {
const req = http.request(mainProxyUrl, {
method: 'CONNECT',
path: `127.0.0.1:${server.address().port}`,
headers: {
host: `127.0.0.1:${server.address().port}`,
'proxy-authorization': `Basic ${Buffer.from('nopassword').toString('base64')}`,
},
});
req.once('connect', (response, socket, head) => {
expect(response.statusCode).to.equal(200);
expect(head.length).to.equal(0);

socket.destroy();
server.close(() => {
done();
});
});

req.end();
});
});

it('returns 407 for invalid credentials', () => {
return Promise.resolve()
.then(() => {
Expand Down
3 changes: 2 additions & 1 deletion test/tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,10 @@ describe('tools.parseAuthorizationHeader()', () => {
data: 'dXNlcm5hbWU6',
});

// Do not alter this test, see comment in src/utils/parse_authorization_header.ts
expect(parse(authStr('Basic', 'username'))).to.eql({
type: 'Basic',
username: '',
username: 'username',
password: '',
data: 'dXNlcm5hbWU=',
});
Expand Down

0 comments on commit b3ccf3d

Please sign in to comment.