Merge branch 'jdsalaro-spam-check-apikey-setting-and-textbox' into 'master'

add spam_check_api_key textbox to admin / application_settings / reporting

See merge request gitlab-org/gitlab!60385

app/helpers/application_settings_helper.rb

@@ -302,6 +302,7 @@ module ApplicationSettingsHelper
       :sourcegraph_public_only,
       :spam_check_endpoint_enabled,
       :spam_check_endpoint_url,
+      :spam_check_api_key,
       :terminal_max_session_time,
       :terms,
       :throttle_authenticated_api_enabled,

app/models/application_setting.rb

@@ -134,6 +134,14 @@ class ApplicationSetting < ApplicationRecord
             presence: true,
             if: :akismet_enabled
 
+  validates :spam_check_api_key,
+            length: { maximum: 2000, message: _('is too long (maximum is %{count} characters)') },
+            allow_blank: true
+
+  validates :spam_check_api_key,
+            presence: true,
+            if: :spam_check_endpoint_enabled
+
   validates :unique_ips_limit_per_user,
             numericality: { greater_than_or_equal_to: 1 },
             presence: true,
@@ -516,6 +524,7 @@ class ApplicationSetting < ApplicationRecord
   attr_encrypted :lets_encrypt_private_key, encryption_options_base_32_aes_256_gcm
   attr_encrypted :eks_secret_access_key, encryption_options_base_32_aes_256_gcm
   attr_encrypted :akismet_api_key, encryption_options_base_32_aes_256_gcm
+  attr_encrypted :spam_check_api_key, encryption_options_base_32_aes_256_gcm
   attr_encrypted :elasticsearch_aws_secret_access_key, encryption_options_base_32_aes_256_gcm
   attr_encrypted :recaptcha_private_key, encryption_options_base_32_aes_256_gcm
   attr_encrypted :recaptcha_site_key, encryption_options_base_32_aes_256_gcm

app/models/application_setting_implementation.rb

@@ -38,6 +38,7 @@ module ApplicationSettingImplementation
         admin_mode: false,
         after_sign_up_text: nil,
         akismet_enabled: false,
+        akismet_api_key: nil,
         allow_local_requests_from_system_hooks: true,
         allow_local_requests_from_web_hooks_and_services: false,
         asset_proxy_enabled: false,
@@ -149,6 +150,7 @@ module ApplicationSettingImplementation
         sourcegraph_url: nil,
         spam_check_endpoint_enabled: false,
         spam_check_endpoint_url: nil,
+        spam_check_api_key: nil,
         terminal_max_session_time: 0,
         throttle_authenticated_api_enabled: false,
         throttle_authenticated_api_period_in_seconds: 3600,

app/views/admin/application_settings/_spam.html.haml

@@ -78,5 +78,9 @@
     .form-group
       = f.label :spam_check_endpoint_url, _('URL of the external Spam Check endpoint'), class: 'label-bold'
       = f.text_field :spam_check_endpoint_url, class: 'form-control gl-form-input'
+    .form-group
+      = f.label :spam_check_api_key, _('Spam Check API Key'), class: 'gl-font-weight-bold'
+      = f.text_field :spam_check_api_key, class: 'form-control gl-form-input'
+      .form-text.text-muted= _('The API key used by GitLab for accessing the Spam Check service endpoint')
 
   = f.submit _('Save changes'), class: "gl-button btn btn-confirm"

changelogs/unreleased/jdsalaro-spam-check-apikey-setting-and-textbox.yml

+---
+title: Add a Spamcheck API key application setting
+merge_request: 60385
+author: jdsalaro
+type: changed

db/migrate/20210427125400_add_spamcheck_api_key_to_application_setting.rb

+# frozen_string_literal: true
+
+class AddSpamcheckApiKeyToApplicationSetting < ActiveRecord::Migration[6.0]
+  def up
+    add_column :application_settings, :encrypted_spam_check_api_key, :binary
+    add_column :application_settings, :encrypted_spam_check_api_key_iv, :binary
+  end
+
+  def down
+    remove_column :application_settings, :encrypted_spam_check_api_key
+    remove_column :application_settings, :encrypted_spam_check_api_key_iv
+  end
+end

db/schema_migrations/20210427125400

+7632c2442c5cb9194c177df9b9106efdb433d43b34250fc434c1e1ff2f8ed8f0
\ No newline at end of file

db/structure.sql

@@ -9511,6 +9511,8 @@ CREATE TABLE application_settings (
     throttle_authenticated_packages_api_enabled boolean DEFAULT false NOT NULL,
     deactivate_dormant_users boolean DEFAULT false NOT NULL,
     whats_new_variant smallint DEFAULT 0,
+    encrypted_spam_check_api_key bytea,
+    encrypted_spam_check_api_key_iv bytea,
     CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)),
     CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)),
     CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)),

doc/api/settings.md

@@ -388,6 +388,7 @@ listed in the descriptions of the relevant settings.
 | `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. |
 | `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. |
 | `throttle_authenticated_api_enabled`     | boolean          | no                                   | (**If enabled, requires:** `throttle_authenticated_api_period_in_seconds` and `throttle_authenticated_api_requests_per_period`) Enable authenticated API request rate limit. Helps reduce request volume (for example, from crawlers or abusive bots). |

lib/api/settings.rb

@@ -154,6 +154,7 @@ module API
       optional :spam_check_endpoint_enabled, type: Boolean, desc: 'Enable Spam Check via external API endpoint'
       given spam_check_endpoint_enabled: ->(val) { val } do
         requires :spam_check_endpoint_url, type: String, desc: 'The URL of the external Spam Check service endpoint'
+        requires :spam_check_api_key, type: String, desc: 'The API key used by GitLab for accessing the Spam Check service endpoint'
       end
       optional :terminal_max_session_time, type: Integer, desc: 'Maximum time for web terminal websocket connection (in seconds). Set to 0 for unlimited time.'
       optional :usage_ping_enabled, type: Boolean, desc: 'Every week GitLab will report license usage back to GitLab, Inc.'

locale/gitlab.pot

@@ -30320,6 +30320,9 @@ msgstr ""
 msgid "SourcegraphPreferences|Uses a custom %{linkStart}Sourcegraph instance%{linkEnd}."
 msgstr ""
 
+msgid "Spam Check API Key"
+msgstr ""
+
 msgid "Spam Logs"
 msgstr ""
 
@@ -31668,6 +31671,9 @@ msgid_plural "The %{type} contains the following errors:"
 msgstr[0] ""
 msgstr[1] ""
 
+msgid "The API key used by GitLab for accessing the Spam Check service endpoint"
+msgstr ""
+
 msgid "The Advanced Search in GitLab is a powerful search service that saves you time. Instead of creating duplicate code and wasting time, you can now search for code within other teams that can help your own project."
 msgstr ""
 

spec/features/admin/admin_settings_spec.rb

@@ -426,6 +426,7 @@ RSpec.describe 'Admin updates settings' do
           fill_in 'IPs per user', with: 15
           check 'Enable Spam Check via external API endpoint'
           fill_in 'URL of the external Spam Check endpoint', with: 'https://www.example.com/spamcheck'
+          fill_in 'Spam Check API Key', with: 'SPAM_CHECK_API_KEY'
           click_button 'Save changes'
         end
 

spec/features/issues/spam_issues_spec.rb

@@ -14,6 +14,7 @@ RSpec.describe 'New issue', :js do
     Gitlab::CurrentSettings.update!(
       akismet_enabled: true,
       akismet_api_key: 'testkey',
+      spam_check_api_key: 'testkey',
       recaptcha_enabled: true,
       recaptcha_site_key: 'test site key',
       recaptcha_private_key: 'test private key'

spec/features/snippets/spam_snippets_spec.rb

@@ -18,6 +18,7 @@ RSpec.describe 'snippet editor with spam', skip: "Will be handled in https://git
     Gitlab::CurrentSettings.update!(
       akismet_enabled: true,
       akismet_api_key: 'testkey',
+      spam_check_api_key: 'testkey',
       recaptcha_enabled: true,
       recaptcha_site_key: 'test site key',
       recaptcha_private_key: 'test private key'

spec/requests/api/settings_spec.rb

@@ -41,6 +41,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do
       expect(json_response['snippet_size_limit']).to eq(50.megabytes)
       expect(json_response['spam_check_endpoint_enabled']).to be_falsey
       expect(json_response['spam_check_endpoint_url']).to be_nil
+      expect(json_response['spam_check_api_key']).to be_nil
       expect(json_response['wiki_page_max_content_bytes']).to be_a(Integer)
       expect(json_response['require_admin_approval_after_user_signup']).to eq(true)
       expect(json_response['personal_access_token_prefix']).to be_nil
@@ -123,6 +124,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do
             raw_blob_request_limit: 300,
             spam_check_endpoint_enabled: true,
             spam_check_endpoint_url: 'https://example.com/spam_check',
+            spam_check_api_key: 'SPAM_CHECK_API_KEY',
             disabled_oauth_sign_in_sources: 'unknown',
             import_sources: 'github,bitbucket',
             wiki_page_max_content_bytes: 12345,
@@ -168,6 +170,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do
         expect(json_response['raw_blob_request_limit']).to eq(300)
         expect(json_response['spam_check_endpoint_enabled']).to be_truthy
         expect(json_response['spam_check_endpoint_url']).to eq('https://example.com/spam_check')
+        expect(json_response['spam_check_api_key']).to eq('SPAM_CHECK_API_KEY')
         expect(json_response['disabled_oauth_sign_in_sources']).to eq([])
         expect(json_response['import_sources']).to match_array(%w(github bitbucket))
         expect(json_response['wiki_page_max_content_bytes']).to eq(12345)
@@ -460,13 +463,32 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do
 
     context "missing spam_check_endpoint_url value when spam_check_endpoint_enabled is true" do
       it "returns a blank parameter error message" do
-        put api("/application/settings", admin), params: { spam_check_endpoint_enabled: true }
+        put api("/application/settings", admin), params: { spam_check_endpoint_enabled: true, spam_check_api_key: "SPAM_CHECK_API_KEY" }
 
         expect(response).to have_gitlab_http_status(:bad_request)
         expect(json_response['error']).to eq('spam_check_endpoint_url is missing')
       end
     end
 
+    context "missing spam_check_api_key value when spam_check_endpoint_enabled is true" do
+      it "returns a blank parameter error message" do
+        put api("/application/settings", admin), params: { spam_check_endpoint_enabled: true, spam_check_endpoint_url: "https://example.com/spam_check" }
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+        expect(json_response['error']).to eq('spam_check_api_key is missing')
+      end
+    end
+
+    context "overly long spam_check_api_key" do
+      it "fails to update the settings with too long spam_check_api_key" do
+        put api("/application/settings", admin), params: { spam_check_api_key: "0123456789" * 500 }
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+        message = json_response["message"]
+        expect(message["spam_check_api_key"]).to include(a_string_matching("is too long"))
+      end
+    end
+
     context "personal access token prefix settings" do
       context "handles validation errors" do
         it "fails to update the settings with too long prefix" do