Merge branch '346397-eurie-configure-spamcheck-tls-via-url-schema' into 'master'

Add support for the url scheme 'tls' to force encryption for Spamcheck

See merge request gitlab-org/gitlab!75677

app/models/application_setting.rb

@@ -410,7 +410,7 @@ class ApplicationSetting < ApplicationRecord
             if: :external_authorization_service_enabled
 
   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,
             presence: true,

doc/api/settings.md

@@ -405,9 +405,9 @@ listed in the descriptions of the relevant settings.
 | `sourcegraph_enabled`                    | boolean          | no                                   | Enables Sourcegraph integration. Default is `false`. **If enabled, requires** `sourcegraph_url`. |
 | `sourcegraph_public_only`                | boolean          | no                                   | Blocks Sourcegraph from being loaded on private and internal projects. Default is `true`. |
 | `sourcegraph_url`                        | string           | required by: `sourcegraph_enabled`   | The Sourcegraph instance URL for integration. |
-| `spam_check_endpoint_enabled`            | boolean          | no                                   | Enables Spam Check via external API endpoint. Default is `false`. |
-| `spam_check_endpoint_url`                | string           | no                                   | URL of the external Spam Check service endpoint. |
-| `spam_check_api_key`                      | string           | no                                   | The API key used by GitLab for accessing the Spam Check service endpoint. |
+| `spam_check_endpoint_enabled`            | boolean          | no                                   | Enables spam checking using external Spam Check API endpoint. Default is `false`. |
+| `spam_check_endpoint_url`                | string           | no                                   | URL of the external Spamcheck service endpoint. Valid URI schemes are `grpc` or `tls`. Specifying `tls` forces communication to be encrypted.|
+| `spam_check_api_key`                     | string           | no                                   | API key used by GitLab for accessing the Spam Check service endpoint. |
 | `suggest_pipeline_enabled`               | boolean          | no                                   | Enable pipeline suggestion banner. |
 | `terminal_max_session_time`              | integer          | no                                   | Maximum time for web terminal websocket connection (in seconds). Set to `0` for unlimited time. |
 | `terms`                                  | text             | required by: `enforce_terms`         | (**Required by:** `enforce_terms`) Markdown content for the ToS. |

lib/gitlab/spamcheck/client.rb

@@ -21,14 +21,16 @@ module Gitlab
         update: ::Spamcheck::Action::UPDATE
       }.freeze
 
+      URL_SCHEME_REGEX = %r{^grpc://|^tls://}.freeze
+
       def initialize
         @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
-        # connect with Spamcheck
-        @endpoint_url = @endpoint_url.gsub(%r(^grpc:\/\/), '')
+        @creds = client_creds(@endpoint_url)
 
-        @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(URL_SCHEME_REGEX, '')
       end
 
       def issue_spam?(spam_issue:, user:, context: {})
@@ -96,11 +98,11 @@ module Gitlab
                                         nanos: ar_timestamp.to_time.nsec)
       end
 
-      def stub_creds
-        if Rails.env.development? || Rails.env.test?
-          :this_channel_is_insecure
+      def client_creds(url)
+        if URI(url).scheme == 'tls' || Rails.env.production?
+          GRPC::Core::ChannelCredentials.new(::Gitlab::X509::Certificate.ca_certs_bundle)
         else
-          GRPC::Core::ChannelCredentials.new ::Gitlab::X509::Certificate.ca_certs_bundle
+          :this_channel_is_insecure
         end
       end
 

spec/lib/gitlab/spamcheck/client_spec.rb

@@ -32,6 +32,60 @@ RSpec.describe Gitlab::Spamcheck::Client do
     stub_application_setting(spam_check_endpoint_url: endpoint)
   end
 
+  describe 'url scheme' do
+    let(:stub) { double(:spamcheck_stub, check_for_spam_issue: response) }
+
+    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).and_return(stub)
+        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).and_return(stub)
+        subject
+      end
+    end
+  end
+
+  describe "Rails environment" do
+    let(:stub) { double(:spamcheck_stub, check_for_spam_issue: response) }
+
+    context "production" do
+      before do
+        allow(Rails.env).to receive(:production?).and_return(true)
+      end
+
+      it 'uses secure connection' do
+        expect(Spamcheck::SpamcheckService::Stub).to receive(:new).with(endpoint.sub(%r{^grpc://}, ''),
+                                                                        instance_of(GRPC::Core::ChannelCredentials),
+                                                                        anything).and_return(stub)
+        subject
+      end
+    end
+
+    context "not production" do
+      before do
+        allow(Rails.env).to receive(:production?).and_return(false)
+      end
+
+      it 'uses insecure connection' do
+        expect(Spamcheck::SpamcheckService::Stub).to receive(:new).with(endpoint.sub(%r{^grpc://}, ''),
+                                                                        :this_channel_is_insecure,
+                                                                        anything).and_return(stub)
+        subject
+      end
+    end
+  end
+
   describe '#issue_spam?' do
     before do
       allow_next_instance_of(::Spamcheck::SpamcheckService::Stub) do |instance|

spec/models/application_setting_spec.rb

@@ -247,6 +247,7 @@ RSpec.describe ApplicationSetting do
         end
 
         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('nonsense').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
         end
 
         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('nonsense').for(:spam_check_endpoint_url) }
         it { is_expected.to allow_value(nil).for(:spam_check_endpoint_url) }