Skip to content

Commit

Permalink
Clear up credentials handling.
Browse files Browse the repository at this point in the history
After revisiting the specification
https://www.rabbitmq.com/uri-spec.html and thus
http://www.ietf.org/rfc/rfc3986.txt, I've made credentials handling
more consistent for both stringly URLs and object URLs.

 - iff both parts are missing, use defaults
 - if anything is supplied:
   - the username ends before the first colon
   - the password starts after the first colon
   - absent values are treated as empty

On that last: the spec says "do what you want with absent values", and
another option is to replace absent values with a default. But I think
it makes more sense to treat username-and-password as a unit you
supply all or none of.

This commit adds some tests for credentials parsing, and makes the
parsing more explicit (if a bit longer).
  • Loading branch information
squaremo committed Mar 7, 2017
1 parent 615f870 commit 85d2996
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 10 deletions.
29 changes: 20 additions & 9 deletions lib/connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,22 @@ function openFrames(vhost, query, credentials, extraClientProperties) {
};
}

// Decide on credentials based on what we're supplied
// Decide on credentials based on what we're supplied. Note that in a
// parsed URL, the auth part is already URL-decoded, so e.g., '%3a' in
// the URL is already decoded to ':'. This is a bit unhelpful, as it
// means we can't tell whether a colon is a separator, or part of the
// username. Assume no colons in usernames.
function credentialsFromUrl(parts) {
var user = 'guest', passwd = 'guest';
if (parts.auth) {
var auth = parts.auth.split(/:(.+)/);
user = auth[0];
passwd = auth[1];
var colon = parts.auth.indexOf(':')
if (colon == -1) {
user = parts.auth;
passwd = '';
} else {
user = parts.auth.substring(0, colon);
passwd = parts.auth.substring(colon+1);
}
}
return credentials.plain(user, passwd);
}
Expand Down Expand Up @@ -110,12 +119,13 @@ function connect(url, socketOptions, openCallback) {
sockopts.port = url.port || ((protocol === 'amqp:') ? 5672 : 5671);

var user, pass;
if (!url.username) {
user = 'guest';
pass = url.password || 'guest';
// Only default if both are missing, to have the same behaviour as
// the stringly URL.
if (url.username == undefined && url.password == undefined) {
user = 'guest'; pass = 'guest';
} else {
user = url.username;
pass = url.password;
user = url.username || '';
pass = url.password || '';
}

var config = {
Expand Down Expand Up @@ -177,3 +187,4 @@ function connect(url, socketOptions, openCallback) {
}

module.exports.connect = connect;
module.exports.credentialsFromUrl = credentialsFromUrl;
4 changes: 3 additions & 1 deletion lib/credentials.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ module.exports.plain = function(user, passwd) {
mechanism: 'PLAIN',
response: function() {
return new Buffer(['', user, passwd].join(String.fromCharCode(0)))
}
},
username: user,
password: passwd
}
}

Expand Down
42 changes: 42 additions & 0 deletions test/connect.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,57 @@
'use strict';

var connect = require('../lib/connect').connect;
var credentialsFromUrl = require('../lib/connect').credentialsFromUrl;
var assert = require('assert');
var util = require('./util');
var net = require('net');
var fail = util.fail, succeed = util.succeed,
kCallback = util.kCallback,
succeedIfAttributeEquals = util.succeedIfAttributeEquals;
var format = require('util').format;

var URL = process.env.URL || 'amqp://localhost';

suite("Credentials", function() {

function checkCreds(creds, user, pass, done) {
if (creds.mechanism != 'PLAIN') {
return done('expected mechanism PLAIN');
}
if (creds.username != user || creds.password != pass) {
return done(format("expected '%s', '%s'; got '%s', '%s'",
user, pass, creds.username, creds.password));
}
done();
}

test("no creds", function(done) {
var parts = {auth: ''};
var creds = credentialsFromUrl(parts);
checkCreds(creds, 'guest', 'guest', done);
});
test("usual user:pass", function(done) {
var parts = {auth: 'user:pass'};
var creds = credentialsFromUrl(parts);
checkCreds(creds, 'user', 'pass', done);
});
test("missing user", function(done) {
var parts = {auth: ':password'};
var creds = credentialsFromUrl(parts);
checkCreds(creds, '', 'password', done);
});
test("missing password", function(done) {
var parts = {auth: 'username'};
var creds = credentialsFromUrl(parts);
checkCreds(creds, 'username', '', done);
});
test("colon in password", function(done) {
var parts = {auth: 'username:pass:word'};
var creds = credentialsFromUrl(parts);
checkCreds(creds, 'username', 'pass:word', done);
});
});

suite("Connect API", function() {

test("Connection refused", function(done) {
Expand Down

0 comments on commit 85d2996

Please sign in to comment.