Commit 001adbbc authored by Ethan Urie's avatar Ethan Urie

Add support for the url scheme 'tls://' to force encryption

Gitaly does something simiilar and supports `tcp` and `unix` where we
support `grpc` for unencrypted communications.
parent 60010802
...@@ -410,7 +410,7 @@ class ApplicationSetting < ApplicationRecord ...@@ -410,7 +410,7 @@ class ApplicationSetting < ApplicationRecord
if: :external_authorization_service_enabled if: :external_authorization_service_enabled
validates :spam_check_endpoint_url, validates :spam_check_endpoint_url,
addressable_url: { schemes: %w(grpc) }, allow_blank: true addressable_url: { schemes: %w(tls grpc) }, allow_blank: true
validates :spam_check_endpoint_url, validates :spam_check_endpoint_url,
presence: true, presence: true,
......
...@@ -24,11 +24,11 @@ module Gitlab ...@@ -24,11 +24,11 @@ module Gitlab
def initialize def initialize
@endpoint_url = Gitlab::CurrentSettings.current_application_settings.spam_check_endpoint_url @endpoint_url = Gitlab::CurrentSettings.current_application_settings.spam_check_endpoint_url
# remove the `grpc://` as it's only useful to ensure we're expecting to @creds = client_creds(@endpoint_url)
# connect with Spamcheck
@endpoint_url = @endpoint_url.gsub(%r(^grpc:\/\/), '')
@creds = stub_creds # remove the `grpc://` or 'tls://' as it's only useful to ensure we're expecting to
# connect with Spamcheck
@endpoint_url = @endpoint_url.sub(%r{^grpc://|^tls://}, '')
end end
def issue_spam?(spam_issue:, user:, context: {}) def issue_spam?(spam_issue:, user:, context: {})
...@@ -96,11 +96,11 @@ module Gitlab ...@@ -96,11 +96,11 @@ module Gitlab
nanos: ar_timestamp.to_time.nsec) nanos: ar_timestamp.to_time.nsec)
end end
def stub_creds def client_creds(url)
if Rails.env.development? || Rails.env.test? if URI(url).scheme == 'tls'
:this_channel_is_insecure GRPC::Core::ChannelCredentials.new(::Gitlab::X509::Certificate.ca_certs_bundle)
else else
GRPC::Core::ChannelCredentials.new ::Gitlab::X509::Certificate.ca_certs_bundle :this_channel_is_insecure
end end
end end
......
...@@ -32,6 +32,34 @@ RSpec.describe Gitlab::Spamcheck::Client do ...@@ -32,6 +32,34 @@ RSpec.describe Gitlab::Spamcheck::Client do
stub_application_setting(spam_check_endpoint_url: endpoint) stub_application_setting(spam_check_endpoint_url: endpoint)
end end
describe 'url scheme' do
before do
allow_next_instance_of(::Spamcheck::SpamcheckService::Stub) do |instance|
allow(instance).to receive(:check_for_spam_issue).and_return(response)
end
end
context 'is tls ' do
let(:endpoint) { 'tls://spamcheck.example.com'}
it 'uses secure connection' do
expect(Spamcheck::SpamcheckService::Stub).to receive(:new).with(endpoint.sub(%r{^tls://}, ''),
instance_of(GRPC::Core::ChannelCredentials),
anything)
subject
end
end
context 'is grpc' do
it 'uses insecure connection' do
expect(Spamcheck::SpamcheckService::Stub).to receive(:new).with(endpoint.sub(%r{^grpc://}, ''),
:this_channel_is_insecure,
anything)
subject
end
end
end
describe '#issue_spam?' do describe '#issue_spam?' do
before do before do
allow_next_instance_of(::Spamcheck::SpamcheckService::Stub) do |instance| allow_next_instance_of(::Spamcheck::SpamcheckService::Stub) do |instance|
......
...@@ -247,6 +247,7 @@ RSpec.describe ApplicationSetting do ...@@ -247,6 +247,7 @@ RSpec.describe ApplicationSetting do
end end
it { is_expected.to allow_value('grpc://example.org/spam_check').for(:spam_check_endpoint_url) } it { is_expected.to allow_value('grpc://example.org/spam_check').for(:spam_check_endpoint_url) }
it { is_expected.to allow_value('tls://example.org/spam_check').for(:spam_check_endpoint_url) }
it { is_expected.not_to allow_value('https://example.org/spam_check').for(:spam_check_endpoint_url) } it { is_expected.not_to allow_value('https://example.org/spam_check').for(:spam_check_endpoint_url) }
it { is_expected.not_to allow_value('nonsense').for(:spam_check_endpoint_url) } it { is_expected.not_to allow_value('nonsense').for(:spam_check_endpoint_url) }
it { is_expected.not_to allow_value(nil).for(:spam_check_endpoint_url) } it { is_expected.not_to allow_value(nil).for(:spam_check_endpoint_url) }
...@@ -259,6 +260,7 @@ RSpec.describe ApplicationSetting do ...@@ -259,6 +260,7 @@ RSpec.describe ApplicationSetting do
end end
it { is_expected.to allow_value('grpc://example.org/spam_check').for(:spam_check_endpoint_url) } it { is_expected.to allow_value('grpc://example.org/spam_check').for(:spam_check_endpoint_url) }
it { is_expected.to allow_value('tls://example.org/spam_check').for(:spam_check_endpoint_url) }
it { is_expected.not_to allow_value('https://example.org/spam_check').for(:spam_check_endpoint_url) } it { is_expected.not_to allow_value('https://example.org/spam_check').for(:spam_check_endpoint_url) }
it { is_expected.not_to allow_value('nonsense').for(:spam_check_endpoint_url) } it { is_expected.not_to allow_value('nonsense').for(:spam_check_endpoint_url) }
it { is_expected.to allow_value(nil).for(:spam_check_endpoint_url) } it { is_expected.to allow_value(nil).for(:spam_check_endpoint_url) }
......
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