From 7f468d2064c4fc91abd92761670e1d50cb8aa0a3 Mon Sep 17 00:00:00 2001 From: Spencer Alger Date: Wed, 8 Apr 2015 15:05:51 -0700 Subject: [PATCH] Unify auth handling in the Host class - flatten auth config to an Authorization header when the Host is created - remove individual Authorization handling from connectors - removed incomplete support for per-request auth - use per-request headers to provide your own Authorization header per request --- src/lib/connectors/angular.js | 23 +++-------------- src/lib/connectors/http.js | 1 - src/lib/connectors/jquery.js | 6 ----- src/lib/connectors/xhr.js | 27 +++++++++---------- src/lib/host.js | 26 +++++++++++-------- test/unit/browser_builds/angular.js | 40 ----------------------------- test/unit/browser_builds/generic.js | 2 ++ test/unit/browser_builds/jquery.js | 2 ++ test/unit/specs/host.js | 13 ++++------ test/unit/specs/http_connector.js | 3 +-- 10 files changed, 40 insertions(+), 103 deletions(-) diff --git a/src/lib/connectors/angular.js b/src/lib/connectors/angular.js index dfc1351a0..98ceb813c 100644 --- a/src/lib/connectors/angular.js +++ b/src/lib/connectors/angular.js @@ -10,37 +10,20 @@ var _ = require('../utils'); var ConnectionAbstract = require('../connection'); var ConnectionFault = require('../errors').ConnectionFault; -function makeAuthHeader(auth) { - return 'Basic ' + (new Buffer(auth, 'utf8')).toString('base64'); -} - function AngularConnector(host, config) { ConnectionAbstract.call(this, host, config); + var self = this; - self.headerDefaults = {}; - - if (self.host.auth) { - self.headerDefaults.Authorization = makeAuthHeader(self.host.auth); - } - config.$injector.invoke(['$http', '$q', function ($http, $q) { self.$q = $q; self.$http = $http; }]); + } _.inherits(AngularConnector, ConnectionAbstract); -AngularConnector.prototype.request = function (userParams, cb) { +AngularConnector.prototype.request = function (params, cb) { var abort = this.$q.defer(); - var params = _.cloneDeep(userParams); - - params.headers = _.defaults(params.headers || {}, this.headerDefaults); - if (params.auth) { - params.headers.Authorization = makeAuthHeader(params.auth); - } - - // inform the host not to use the auth, by overriding it in the params - params.auth = false; this.$http({ method: params.method, diff --git a/src/lib/connectors/http.js b/src/lib/connectors/http.js index d2e6b409a..fcaf91f3c 100644 --- a/src/lib/connectors/http.js +++ b/src/lib/connectors/http.js @@ -106,7 +106,6 @@ HttpConnector.prototype.makeReqParams = function (params) { var reqParams = { method: params.method || 'GET', protocol: host.protocol + ':', - auth: host.auth, hostname: host.host, port: host.port, path: (host.path || '') + (params.path || ''), diff --git a/src/lib/connectors/jquery.js b/src/lib/connectors/jquery.js index 892bc07fe..d02ce7e24 100644 --- a/src/lib/connectors/jquery.js +++ b/src/lib/connectors/jquery.js @@ -26,12 +26,6 @@ JqueryConnector.prototype.request = function (params, cb) { done: cb }; - if (params.auth) { - var auths = params.auth.split(':'); - ajax.username = auths[0]; - ajax.password = auths[1]; - } - var jqXHR = jQuery.ajax(ajax) .done(function (data, textStatus, jqXHR) { cb(null, data, jqXHR.statusCode(), { diff --git a/src/lib/connectors/xhr.js b/src/lib/connectors/xhr.js index 1fa8962e3..d4508efd8 100644 --- a/src/lib/connectors/xhr.js +++ b/src/lib/connectors/xhr.js @@ -53,16 +53,21 @@ if (!getXhr) { XhrConnector.prototype.request = function (params, cb) { var xhr = getXhr(); var timeoutId; - var url = this.host.makeUrl(params); - var headers = this.host.getHeaders(params.headers); - + var host = this.host; var log = this.log; + + var url = host.makeUrl(params); + var headers = host.getHeaders(params.headers); var async = params.async === false ? false : asyncDefault; - if (params.auth) { - xhr.open(params.method || 'GET', url, async, params.auth.user, params.auth.pass); - } else { - xhr.open(params.method || 'GET', url, async); + xhr.open(params.method || 'GET', url, async); + + if (headers) { + for (var key in headers) { + if (headers[key] !== void 0) { + xhr.setRequestHeader(key, headers[key]); + } + } } xhr.onreadystatechange = function () { @@ -74,14 +79,6 @@ XhrConnector.prototype.request = function (params, cb) { } }; - if (headers) { - for (var key in headers) { - if (headers[key] !== void 0) { - xhr.setRequestHeader(key, headers[key]); - } - } - } - xhr.send(params.body || void 0); return function () { diff --git a/src/lib/host.js b/src/lib/host.js index 05e30091b..8ec3be5e2 100644 --- a/src/lib/host.js +++ b/src/lib/host.js @@ -10,13 +10,19 @@ var _ = require('./utils'); var startsWithProtocolRE = /^([a-z]+:)?\/\//; var defaultProto = 'http:'; +var btoa; /* jshint ignore:start */ if (typeof window !== 'undefined') { defaultProto = window.location.protocol; + btoa = window.btoa; } /* jshint ignore:end */ +btoa = btoa || function (data) { + return (new Buffer(data, 'utf8')).toString('base64'); +}; + var urlParseFields = [ 'protocol', 'hostname', 'pathname', 'port', 'auth', 'query' ]; @@ -42,7 +48,7 @@ Host.defaultPorts = { }; function Host(config, globalConfig) { - config = config || {}; + config = _.clone(config || {}); globalConfig = globalConfig || {}; // defaults @@ -50,7 +56,6 @@ function Host(config, globalConfig) { this.host = 'localhost'; this.path = ''; this.port = 9200; - this.auth = null; this.query = null; this.headers = null; this.suggestCompression = !!globalConfig.suggestCompression; @@ -97,8 +102,14 @@ function Host(config, globalConfig) { config = {}; } + if (config.auth) { + config.headers = config.headers || {}; + config.headers.Authorization = 'Basic ' + btoa(config.auth); + delete config.auth; + } + _.forOwn(config, function (val, prop) { - if (val != null) this[prop] = val; + if (val != null) this[prop] = _.clone(val); }, this); // make sure the query string is parsed @@ -149,15 +160,8 @@ Host.prototype.makeUrl = function (params) { // build the query string var query = qs.stringify(this.getQuery(params.query)); - var auth = ''; - if (params.auth) { - auth = params.auth + '@'; - } else if (this.auth && params.auth !== false) { - auth = this.auth + '@'; - } - if (this.host) { - return this.protocol + '://' + auth + this.host + port + path + (query ? '?' + query : ''); + return this.protocol + '://' + this.host + port + path + (query ? '?' + query : ''); } else { return path + (query ? '?' + query : ''); } diff --git a/test/unit/browser_builds/angular.js b/test/unit/browser_builds/angular.js index 540b8fffd..559b09edb 100644 --- a/test/unit/browser_builds/angular.js +++ b/test/unit/browser_builds/angular.js @@ -82,44 +82,4 @@ describe('Angular esFactory', function () { return prom; }); }); - - describe('$http', function () { - bootstrap({ - bluebirdPromises: true - }); - - it('uses the auth header provided', function () { - var authString = 'user:password'; - var authHeader = 'Basic ' + (new Buffer(authString, 'utf8')).toString('base64'); - var $httpParams = null; - var client = esFactory({ - host: { - host: 'some-other-es-host.com', - auth: authString - } - }); - - // once the client calls the $http method, flush the requests and trigger an - // error if the expected request was not made - var connection = client.transport.connectionPool.getConnections().pop(); - var stub = sinon.stub(connection, '$http', function (params) { - $httpParams = params; - return Promise.resolve({ - data: null, - status: 200, - headers: function () { - return {}; - } - }); - }); - - var prom = client.ping({ - requestTimeout: 1000 - }); - return prom.then(function () { - expect($httpParams).to.have.property('headers'); - expect($httpParams.headers).to.have.property('Authorization', authHeader); - }); - }); - }); }); \ No newline at end of file diff --git a/test/unit/browser_builds/generic.js b/test/unit/browser_builds/generic.js index 5bc6b22a5..4deec0163 100644 --- a/test/unit/browser_builds/generic.js +++ b/test/unit/browser_builds/generic.js @@ -6,9 +6,11 @@ describe('elasticsearch namespace', function () { it('is defined on the window', function () { expect(es).to.be.ok(); }); + it('has Client, ConnectionPool, Transport, and errors keys', function () { expect(es).to.have.keys('Client', 'ConnectionPool', 'Transport', 'errors'); }); + it('can create a client', function () { var client = new es.Client({ hosts: null }); expect(client).to.have.keys('transport'); diff --git a/test/unit/browser_builds/jquery.js b/test/unit/browser_builds/jquery.js index d82a27c1e..0a0e3724e 100644 --- a/test/unit/browser_builds/jquery.js +++ b/test/unit/browser_builds/jquery.js @@ -5,9 +5,11 @@ describe('jQuery.es namespace', function () { it('is defined on the global jQuery', function () { expect($.es).to.be.ok(); }); + it('has Client, ConnectionPool, Transport, and errors keys', function () { expect($.es).to.have.keys('Client', 'ConnectionPool', 'Transport', 'errors'); }); + it('can create a client', function () { var client = new $.es.Client({ hosts: null }); expect(client).to.have.keys('transport'); diff --git a/test/unit/specs/host.js b/test/unit/specs/host.js index 025e34a8c..d02444b1a 100644 --- a/test/unit/specs/host.js +++ b/test/unit/specs/host.js @@ -9,7 +9,6 @@ var hostDefaults = { host: 'localhost', port: 9200, path: '', - auth: null, query: {}, headers: null, suggestCompression: false, @@ -44,7 +43,7 @@ describe('Host class', function () { var headers = { 'X-Special-Routing-Header': 'pie' }; var host = new Host({ headers: headers }); - expect(host.headers).to.be(headers); + expect(host.headers).to.eql(headers); }); describe('from a string', function () { @@ -56,7 +55,6 @@ describe('Host class', function () { host: 'pizza.com', port: 420, path: '/pizza/cheese', - auth: 'john:dude', query: { shrooms: 'true' } @@ -122,7 +120,7 @@ describe('Host class', function () { expect(host.host).to.eql('pizza.com'); expect(host.port).to.eql(888); expect(host.path).to.eql('/path'); - expect(host.auth).to.eql('joe:diner'); + expect(host.headers).to.eql({ Authorization: 'Basic ' + (new Buffer('joe:diner')).toString('base64') }); expect(host.query).to.eql({ query: 'yes' }); @@ -151,9 +149,8 @@ describe('Host class', function () { path: '/this and that', query: { param: 1 - }, - auth: 'user:pass' - })).to.be('http://user:pass@localhost:9200/prefix/this and that?user_id=123¶m=1'); + } + })).to.be('http://localhost:9200/prefix/this and that?user_id=123¶m=1'); }); it('ensures that path starts with a forward-slash', function () { @@ -179,7 +176,7 @@ describe('Host class', function () { expect(host.makeUrl()).to.be('http://john/'); host = new Host({ host: 'italy', path: '/pie', auth: 'user:pass'}); - expect(host.makeUrl()).to.be('http://user:pass@italy:9200/pie'); + expect(host.makeUrl()).to.be('http://italy:9200/pie'); }); it('outputs valid relative urls when the host is empty', function () { diff --git a/test/unit/specs/http_connector.js b/test/unit/specs/http_connector.js index b9778b277..1a9b3c9e0 100644 --- a/test/unit/specs/http_connector.js +++ b/test/unit/specs/http_connector.js @@ -72,10 +72,10 @@ describe('Http Connector', function () { var con = new HttpConnection(host, {}); var reqParams = con.makeReqParams(); + expect(reqParams).to.not.have.property('auth'); expect(reqParams).to.eql({ method: 'GET', protocol: 'http:', - auth: 'john:dude', hostname: 'pizza.com', port: 9200, path: '/pizza/cheese?shrooms=true', @@ -142,7 +142,6 @@ describe('Http Connector', function () { expect(reqParams).to.eql({ method: 'PUT', protocol: 'http:', - auth: null, hostname: 'google.com', port: 80, path: '/stuff',