Commit 598d3969 authored by John Mason's avatar John Mason

Pass URI to elasticsearch client instead of string

Because we were URL encoding username and password
but passing `uri.to_s` to the elasticsearch client,
elasticsearch client was URL encoding the username
and password a second time which resulted in 401
errors.

Passing a URI to elasticsearch client fixes this.

Changelog: fixed
EE: true
parent bb4e5bea
...@@ -299,7 +299,9 @@ module EE ...@@ -299,7 +299,9 @@ module EE
end end
def elasticsearch_url def elasticsearch_url
read_attribute(:elasticsearch_url).split(',').map(&:strip) read_attribute(:elasticsearch_url).split(',').map do |s|
URI.parse(s.strip)
end
end end
def elasticsearch_url=(values) def elasticsearch_url=(values)
...@@ -317,12 +319,10 @@ module EE ...@@ -317,12 +319,10 @@ module EE
def elasticsearch_url_with_credentials def elasticsearch_url_with_credentials
return elasticsearch_url if elasticsearch_username.blank? return elasticsearch_url if elasticsearch_username.blank?
elasticsearch_url.map do |url| elasticsearch_url.map do |uri|
uri = URI.parse(url)
uri.user = URI.encode_www_form_component(elasticsearch_username) uri.user = URI.encode_www_form_component(elasticsearch_username)
uri.password = URI.encode_www_form_component(elasticsearch_password) uri.password = URI.encode_www_form_component(elasticsearch_password)
uri.to_s uri
end end
end end
......
...@@ -351,7 +351,7 @@ RSpec.describe Admin::ApplicationSettingsController do ...@@ -351,7 +351,7 @@ RSpec.describe Admin::ApplicationSettingsController do
context 'advanced search settings' do context 'advanced search settings' do
it 'updates the advanced search settings' do it 'updates the advanced search settings' do
settings = { settings = {
elasticsearch_url: 'http://my-elastic.search:9200', elasticsearch_url: URI.parse('http://my-elastic.search:9200'),
elasticsearch_indexing: false, elasticsearch_indexing: false,
elasticsearch_aws: true, elasticsearch_aws: true,
elasticsearch_aws_access_key: 'elasticsearch_aws_access_key', elasticsearch_aws_access_key: 'elasticsearch_aws_access_key',
......
...@@ -311,25 +311,25 @@ RSpec.describe ApplicationSetting do ...@@ -311,25 +311,25 @@ RSpec.describe ApplicationSetting do
it 'presents a single URL as a one-element array' do it 'presents a single URL as a one-element array' do
setting.elasticsearch_url = 'http://example.com' setting.elasticsearch_url = 'http://example.com'
expect(setting.elasticsearch_url).to eq(%w[http://example.com]) expect(setting.elasticsearch_url).to match_array([URI.parse('http://example.com')])
end end
it 'presents multiple URLs as a many-element array' do it 'presents multiple URLs as a many-element array' do
setting.elasticsearch_url = 'http://example.com,https://invalid.invalid:9200' setting.elasticsearch_url = 'http://example.com,https://invalid.invalid:9200'
expect(setting.elasticsearch_url).to eq(%w[http://example.com https://invalid.invalid:9200]) expect(setting.elasticsearch_url).to match_array([URI.parse('http://example.com'), URI.parse('https://invalid.invalid:9200')])
end end
it 'strips whitespace from around URLs' do it 'strips whitespace from around URLs' do
setting.elasticsearch_url = ' http://example.com, https://invalid.invalid:9200 ' setting.elasticsearch_url = ' http://example.com, https://invalid.invalid:9200 '
expect(setting.elasticsearch_url).to eq(%w[http://example.com https://invalid.invalid:9200]) expect(setting.elasticsearch_url).to match_array([URI.parse('http://example.com'), URI.parse('https://invalid.invalid:9200')])
end end
it 'strips trailing slashes from URLs' do it 'strips trailing slashes from URLs' do
setting.elasticsearch_url = 'http://example.com/, https://example.com:9200/, https://example.com:9200/prefix//' setting.elasticsearch_url = 'http://example.com/, https://example.com:9200/, https://example.com:9200/prefix//'
expect(setting.elasticsearch_url).to eq(%w[http://example.com https://example.com:9200 https://example.com:9200/prefix]) expect(setting.elasticsearch_url).to match_array([URI.parse('http://example.com'), URI.parse('https://example.com:9200'), URI.parse('https://example.com:9200/prefix')])
end end
end end
...@@ -339,7 +339,7 @@ RSpec.describe ApplicationSetting do ...@@ -339,7 +339,7 @@ RSpec.describe ApplicationSetting do
setting.elasticsearch_username = 'foo' setting.elasticsearch_username = 'foo'
setting.elasticsearch_password = 'bar' setting.elasticsearch_password = 'bar'
expect(setting.elasticsearch_url_with_credentials).to eq(%w[http://foo:bar@example.com https://foo:bar@example.org:9200]) expect(setting.elasticsearch_url_with_credentials).to match_array([URI.parse('http://foo:bar@example.com'), URI.parse('https://foo:bar@example.org:9200')])
end end
it 'embeds username only' do it 'embeds username only' do
...@@ -347,7 +347,7 @@ RSpec.describe ApplicationSetting do ...@@ -347,7 +347,7 @@ RSpec.describe ApplicationSetting do
setting.elasticsearch_username = 'foo' setting.elasticsearch_username = 'foo'
setting.elasticsearch_password = '' setting.elasticsearch_password = ''
expect(setting.elasticsearch_url_with_credentials).to eq(%w[http://foo:@example.com https://foo:@example.org:9200]) expect(setting.elasticsearch_url_with_credentials).to match_array([URI.parse('http://foo:@example.com'), URI.parse('https://foo:@example.org:9200')])
end end
it 'overrides existing embedded credentials' do it 'overrides existing embedded credentials' do
...@@ -355,7 +355,7 @@ RSpec.describe ApplicationSetting do ...@@ -355,7 +355,7 @@ RSpec.describe ApplicationSetting do
setting.elasticsearch_username = 'foo' setting.elasticsearch_username = 'foo'
setting.elasticsearch_password = 'bar' setting.elasticsearch_password = 'bar'
expect(setting.elasticsearch_url_with_credentials).to eq(%w[http://foo:bar@example.com https://foo:bar@example.org:9200]) expect(setting.elasticsearch_url_with_credentials).to match_array([URI.parse('http://foo:bar@example.com'), URI.parse('https://foo:bar@example.org:9200')])
end end
it 'returns original url if credentials blank' do it 'returns original url if credentials blank' do
...@@ -363,7 +363,7 @@ RSpec.describe ApplicationSetting do ...@@ -363,7 +363,7 @@ RSpec.describe ApplicationSetting do
setting.elasticsearch_username = '' setting.elasticsearch_username = ''
setting.elasticsearch_password = '' setting.elasticsearch_password = ''
expect(setting.elasticsearch_url_with_credentials).to eq(%w[http://username:password@example.com https://test:test@example.org:9200]) expect(setting.elasticsearch_url_with_credentials).to match_array([URI.parse('http://username:password@example.com'), URI.parse('https://test:test@example.org:9200')])
end end
it 'encodes the credentials' do it 'encodes the credentials' do
...@@ -371,9 +371,9 @@ RSpec.describe ApplicationSetting do ...@@ -371,9 +371,9 @@ RSpec.describe ApplicationSetting do
setting.elasticsearch_username = 'foo/admin' setting.elasticsearch_username = 'foo/admin'
setting.elasticsearch_password = 'b@r' setting.elasticsearch_password = 'b@r'
expect(setting.elasticsearch_url_with_credentials).to eq(%w[ expect(setting.elasticsearch_url_with_credentials).to match_array([
http://foo%2Fadmin:b%40r@example.com URI.parse('http://foo%2Fadmin:b%40r@example.com'),
https://foo%2Fadmin:b%40r@example.org:9200 URI.parse('https://foo%2Fadmin:b%40r@example.org:9200')
]) ])
end end
end end
...@@ -404,7 +404,7 @@ RSpec.describe ApplicationSetting do ...@@ -404,7 +404,7 @@ RSpec.describe ApplicationSetting do
) )
expect(setting.elasticsearch_config).to eq( expect(setting.elasticsearch_config).to eq(
url: ['http://foo:bar@example.com:9200'], url: [URI.parse('http://foo:bar@example.com:9200')],
aws: false, aws: false,
aws_region: 'test-region', aws_region: 'test-region',
aws_access_key: 'test-access-key', aws_access_key: 'test-access-key',
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment