Merge branch 'mc_rocha-move-arkose-settings-to-database' into 'master'

Move arkose settings into database

See merge request gitlab-org/gitlab!84415

app/models/application_setting.rb

@@ -621,6 +621,8 @@ class ApplicationSetting < ApplicationRecord
   attr_encrypted :external_pipeline_validation_service_token, encryption_options_base_32_aes_256_gcm
   attr_encrypted :mailgun_signing_key, encryption_options_base_32_aes_256_gcm.merge(encode: false)
   attr_encrypted :database_grafana_api_key, encryption_options_base_32_aes_256_gcm.merge(encode: false, encode_iv: false)
+  attr_encrypted :arkose_labs_public_api_key, encryption_options_base_32_aes_256_gcm.merge(encode: false, encode_iv: false)
+  attr_encrypted :arkose_labs_private_api_key, encryption_options_base_32_aes_256_gcm.merge(encode: false, encode_iv: false)
 
   validates :disable_feed_token,
             inclusion: { in: [true, false], message: _('must be a boolean value') }

config/initializers/1_settings.rb

@@ -1001,14 +1001,6 @@ Settings['prometheus'] ||= Settingslogic.new({})
 Settings.prometheus['enabled'] ||= false
 Settings.prometheus['server_address'] ||= nil
 
-#
-# Arkose settings
-#
-Settings['arkose'] ||= Settingslogic.new({})
-Settings.arkose['public_key'] ||= ENV['ARKOSE_LABS_PUBLIC_KEY']
-Settings.arkose['private_key'] ||= ENV['ARKOSE_LABS_PRIVATE_KEY']
-Settings.arkose['verify_url'] ||= ENV['ARKOSE_LABS_VERIFY_URL']
-
 #
 # Shutdown settings
 #

db/migrate/20220405181814_add_arkose_settings_to_application_settings.rb

+# frozen_string_literal: true
+
+class AddArkoseSettingsToApplicationSettings < Gitlab::Database::Migration[1.0]
+  # rubocop:disable Migration/AddLimitToTextColumns
+  # limit is added in 20220405203843_add_text_limit_to_arkose_verify_url_application_settings.rb
+  def up
+    add_column :application_settings, :encrypted_arkose_labs_public_api_key, :binary
+    add_column :application_settings, :encrypted_arkose_labs_public_api_key_iv, :binary
+
+    add_column :application_settings, :encrypted_arkose_labs_private_api_key, :binary
+    add_column :application_settings, :encrypted_arkose_labs_private_api_key_iv, :binary
+
+    add_column :application_settings, :arkose_labs_verify_api_url, :text
+  end
+  # rubocop:enable Migration/AddLimitToTextColumns
+
+  def down
+    remove_column :application_settings, :encrypted_arkose_labs_public_api_key
+    remove_column :application_settings, :encrypted_arkose_labs_public_api_key_iv
+
+    remove_column :application_settings, :encrypted_arkose_labs_private_api_key
+    remove_column :application_settings, :encrypted_arkose_labs_private_api_key_iv
+
+    remove_column :application_settings, :arkose_labs_verify_api_url
+  end
+end

db/migrate/20220405203843_add_text_limit_to_arkose_verify_url_application_settings.rb

+# frozen_string_literal: true
+
+class AddTextLimitToArkoseVerifyUrlApplicationSettings < Gitlab::Database::Migration[1.0]
+  disable_ddl_transaction!
+
+  def up
+    add_text_limit :application_settings, :arkose_labs_verify_api_url, 255
+  end
+
+  def down
+    remove_text_limit :application_settings, :arkose_labs_verify_api_url
+  end
+end

db/schema_migrations/20220405181814

+0835eaaf3e355f98783a11098a37b894b581176d98c39cdfd3be44e2447fe232
\ No newline at end of file

db/schema_migrations/20220405203843

+ac1892c5f2131e41774cadc8799cb5fb2c7d36fe567850fc1251a23c2d454695
\ No newline at end of file

db/structure.sql

@@ -11265,6 +11265,11 @@ CREATE TABLE application_settings (
     database_grafana_api_url text,
     database_grafana_tag text,
     public_runner_releases_url text DEFAULT 'https://gitlab.com/api/v4/projects/gitlab-org%2Fgitlab-runner/releases'::text NOT NULL,
+    encrypted_arkose_labs_public_api_key bytea,
+    encrypted_arkose_labs_public_api_key_iv bytea,
+    encrypted_arkose_labs_private_api_key bytea,
+    encrypted_arkose_labs_private_api_key_iv bytea,
+    arkose_labs_verify_api_url text,
     delete_inactive_projects boolean DEFAULT false NOT NULL,
     inactive_projects_delete_after_months integer DEFAULT 2 NOT NULL,
     inactive_projects_min_size_mb integer DEFAULT 0 NOT NULL,
@@ -11299,7 +11304,8 @@ CREATE TABLE application_settings (
     CONSTRAINT check_d820146492 CHECK ((char_length(spam_check_endpoint_url) <= 255)),
     CONSTRAINT check_e5024c8801 CHECK ((char_length(elasticsearch_username) <= 255)),
     CONSTRAINT check_e5aba18f02 CHECK ((char_length(container_registry_version) <= 255)),
-    CONSTRAINT check_ef6176834f CHECK ((char_length(encrypted_cloud_license_auth_token_iv) <= 255))
+    CONSTRAINT check_ef6176834f CHECK ((char_length(encrypted_cloud_license_auth_token_iv) <= 255)),
+    CONSTRAINT check_f6563bc000 CHECK ((char_length(arkose_labs_verify_api_url) <= 255))
 );
 
 COMMENT ON COLUMN application_settings.content_validation_endpoint_url IS 'JiHu-specific column';

ee/app/controllers/ee/sessions_controller.rb

@@ -24,7 +24,7 @@ module EE
         redirect_to oauth_geo_auth_url(host: current_node_uri.host, port: current_node_uri.port, state: state)
       else
         if ::Feature.enabled?(:arkose_labs_login_challenge)
-          @arkose_labs_public_key ||= Settings.arkose['public_key'] # rubocop:disable Gitlab/ModuleWithInstanceVariables
+          @arkose_labs_public_key ||= arkose_public_api_key # rubocop:disable Gitlab/ModuleWithInstanceVariables
         end
 
         super
@@ -125,5 +125,9 @@ module EE
 
       respond_with_navigational(resource) { render :new }
     end
+
+    def arkose_public_api_key
+      ::Gitlab::CurrentSettings.arkose_labs_public_api_key || ENV['ARKOSE_LABS_PUBLIC_KEY']
+    end
   end
 end

ee/app/services/arkose/user_verification_service.rb

@@ -12,7 +12,7 @@ module Arkose
     end
 
     def execute
-      response = Gitlab::HTTP.perform_request(Net::HTTP::Post, VERIFY_URL, body: body).parsed_response
+      response = Gitlab::HTTP.perform_request(Net::HTTP::Post, arkose_verify_url, body: body).parsed_response
       logger.info(build_message(response))
 
       return false if invalid_token(response)
@@ -80,7 +80,7 @@ module Arkose
 
     def body
       {
-        private_key: Settings.arkose['private_key'],
+        private_key: arkose_private_api_key,
         session_token: session_token,
         log_data: user.id
       }
@@ -132,5 +132,13 @@ module Arkose
       telltale_list = response&.dig('session_details', 'telltale_list') || []
       telltale_list.include?(ALLOWLIST_TELLTALE)
     end
+
+    def arkose_private_api_key
+      Gitlab::CurrentSettings.arkose_labs_private_api_key || ENV['ARKOSE_LABS_PRIVATE_KEY']
+    end
+
+    def arkose_verify_url
+      Gitlab::CurrentSettings.arkose_labs_verify_api_url || VERIFY_URL
+    end
   end
 end

ee/spec/controllers/ee/sessions_controller_spec.rb

@@ -6,8 +6,11 @@ RSpec.describe SessionsController, :geo do
   include DeviseHelpers
   include EE::GeoHelpers
 
+  let(:arkose_labs_public_api_key) { 'foo' }
+
   before do
     set_devise_mapping(context: @request)
+    stub_application_setting(arkose_labs_public_api_key: arkose_labs_public_api_key)
   end
 
   describe '#new' do

ee/spec/services/arkose/user_verification_service_spec.rb

@@ -8,108 +8,130 @@ RSpec.describe Arkose::UserVerificationService do
 
   let(:service) { Arkose::UserVerificationService.new(session_token: session_token, user: user) }
   let(:response) { instance_double(HTTParty::Response, success?: true, code: 200, parsed_response: arkose_ec_response) }
+  let(:arkose_labs_private_api_key) { 'foo' }
+  let(:arkose_labs_verify_api_url) { 'https://bar' }
 
   subject { service.execute }
 
   describe '#execute' do
-    context 'when the user did not solve the challenge' do
-      let(:arkose_ec_response) { Gitlab::Json.parse(File.read(Rails.root.join('ee/spec/fixtures/arkose/failed_ec_response.json'))) }
+    shared_examples_for 'interacting with Arkose verify API' do
+      context 'when the user did not solve the challenge' do
+        let(:arkose_ec_response) { Gitlab::Json.parse(File.read(Rails.root.join('ee/spec/fixtures/arkose/failed_ec_response.json'))) }
 
-      it 'returns false' do
-        allow(Gitlab::HTTP).to receive(:perform_request).and_return(response)
-        expect(subject).to be_falsey
+        it 'returns false' do
+          allow(Gitlab::HTTP).to receive(:perform_request).and_return(response)
+          expect(subject).to be_falsey
+        end
       end
-    end
-
-    context 'when feature arkose_labs_prevent_login is enabled' do
-      context 'when the user solved the challenge' do
-        context 'when the risk score is not high' do
-          let(:arkose_ec_response) { Gitlab::Json.parse(File.read(Rails.root.join('ee/spec/fixtures/arkose/successfully_solved_ec_response.json'))) }
-
-          it 'returns true' do
-            allow(Gitlab::HTTP).to receive(:perform_request).and_return(response)
-            expect(subject).to be_truthy
-          end
 
-          it 'adds arkose data to custom attributes' do
-            allow(Gitlab::HTTP).to receive(:perform_request).and_return(response)
-            subject
-            expect(user.custom_attributes.count).to eq(4)
+      context 'when feature arkose_labs_prevent_login is enabled' do
+        context 'when the user solved the challenge' do
+          context 'when the risk score is not high' do
+            let(:arkose_ec_response) { Gitlab::Json.parse(File.read(Rails.root.join('ee/spec/fixtures/arkose/successfully_solved_ec_response.json'))) }
 
-            expect(user.custom_attributes.find_by(key: 'arkose_session').value).to eq('22612c147bb418c8.2570749403')
-            expect(user.custom_attributes.find_by(key: 'arkose_risk_band').value).to eq('Low')
-            expect(user.custom_attributes.find_by(key: 'arkose_global_score').value).to eq('0')
-            expect(user.custom_attributes.find_by(key: 'arkose_custom_score').value).to eq('0')
-          end
+            it 'returns true' do
+              allow(Gitlab::HTTP).to receive(:perform_request).and_return(response)
+              expect(subject).to be_truthy
+            end
 
-          it 'logs Arkose verify response' do
-            allow(Gitlab::HTTP).to receive(:perform_request).and_return(response)
-            allow(Gitlab::AppLogger).to receive(:info)
-            allow(Gitlab::ApplicationContext).to receive(:current).and_return({ 'correlation_id': 'be025cf83013ac4f52ffd2bf712b11a2' })
-
-            subject
-
-            expect(Gitlab::AppLogger).to have_received(:info).with(correlation_id: 'be025cf83013ac4f52ffd2bf712b11a2',
-                                                                   message: 'Arkose verify response',
-                                                                   response: arkose_ec_response,
-                                                                   username: user.username,
-                                                                   'arkose.session_id': '22612c147bb418c8.2570749403',
-                                                                   'arkose.global_score': '0',
-                                                                   'arkose.global_telltale_list': [],
-                                                                   'arkose.custom_score': '0',
-                                                                   'arkose.custom_telltale_list': [],
-                                                                   'arkose.risk_band': 'Low',
-                                                                   'arkose.risk_category': 'NO-THREAT')
-          end
+            it 'adds arkose data to custom attributes' do
+              allow(Gitlab::HTTP).to receive(:perform_request).and_return(response)
+              subject
+              expect(user.custom_attributes.count).to eq(4)
 
-          context 'when the risk score is high' do
-            let(:arkose_ec_response) { Gitlab::Json.parse(File.read(Rails.root.join('ee/spec/fixtures/arkose/successfully_solved_ec_response_high_risk.json'))) }
+              expect(user.custom_attributes.find_by(key: 'arkose_session').value).to eq('22612c147bb418c8.2570749403')
+              expect(user.custom_attributes.find_by(key: 'arkose_risk_band').value).to eq('Low')
+              expect(user.custom_attributes.find_by(key: 'arkose_global_score').value).to eq('0')
+              expect(user.custom_attributes.find_by(key: 'arkose_custom_score').value).to eq('0')
+            end
 
-            it 'returns false' do
+            it 'logs Arkose verify response' do
               allow(Gitlab::HTTP).to receive(:perform_request).and_return(response)
-              expect(subject).to be_falsey
+              allow(Gitlab::AppLogger).to receive(:info)
+              allow(Gitlab::ApplicationContext).to receive(:current).and_return({ 'correlation_id': 'be025cf83013ac4f52ffd2bf712b11a2' })
+
+              subject
+
+              expect(Gitlab::AppLogger).to have_received(:info).with(correlation_id: 'be025cf83013ac4f52ffd2bf712b11a2',
+                                                                     message: 'Arkose verify response',
+                                                                     response: arkose_ec_response,
+                                                                     username: user.username,
+                                                                     'arkose.session_id': '22612c147bb418c8.2570749403',
+                                                                     'arkose.global_score': '0',
+                                                                     'arkose.global_telltale_list': [],
+                                                                     'arkose.custom_score': '0',
+                                                                     'arkose.custom_telltale_list': [],
+                                                                     'arkose.risk_band': 'Low',
+                                                                     'arkose.risk_category': 'NO-THREAT')
             end
 
-            context 'when the session is allowlisted' do
-              before do
-                arkose_ec_response['session_details']['telltale_list'].push(Arkose::UserVerificationService::ALLOWLIST_TELLTALE)
-              end
+            context 'when the risk score is high' do
+              let(:arkose_ec_response) { Gitlab::Json.parse(File.read(Rails.root.join('ee/spec/fixtures/arkose/successfully_solved_ec_response_high_risk.json'))) }
 
-              it 'returns true' do
+              it 'returns false' do
                 allow(Gitlab::HTTP).to receive(:perform_request).and_return(response)
-                expect(subject).to be_truthy
+                expect(subject).to be_falsey
+              end
+
+              context 'when the session is allowlisted' do
+                before do
+                  arkose_ec_response['session_details']['telltale_list'].push(Arkose::UserVerificationService::ALLOWLIST_TELLTALE)
+                end
+
+                it 'returns true' do
+                  allow(Gitlab::HTTP).to receive(:perform_request).and_return(response)
+                  expect(subject).to be_truthy
+                end
               end
             end
           end
         end
-      end
 
-      context 'when the response does not include the risk session' do
-        context 'when the user solved the challenge' do
-          let(:arkose_ec_response) { Gitlab::Json.parse(File.read(Rails.root.join('ee/spec/fixtures/arkose/successfully_solved_ec_response_without_session_risk.json'))) }
+        context 'when the response does not include the risk session' do
+          context 'when the user solved the challenge' do
+            let(:arkose_ec_response) { Gitlab::Json.parse(File.read(Rails.root.join('ee/spec/fixtures/arkose/successfully_solved_ec_response_without_session_risk.json'))) }
 
-          it 'returns true' do
-            allow(Gitlab::HTTP).to receive(:perform_request).and_return(response)
-            expect(subject).to be_truthy
+            it 'returns true' do
+              allow(Gitlab::HTTP).to receive(:perform_request).and_return(response)
+              expect(subject).to be_truthy
+            end
           end
-        end
 
-        context 'when the user did not solve the challenge' do
-          let(:arkose_ec_response) { Gitlab::Json.parse(File.read(Rails.root.join('ee/spec/fixtures/arkose/failed_ec_response_without_risk_session.json'))) }
+          context 'when the user did not solve the challenge' do
+            let(:arkose_ec_response) { Gitlab::Json.parse(File.read(Rails.root.join('ee/spec/fixtures/arkose/failed_ec_response_without_risk_session.json'))) }
 
-          it 'returns false' do
-            allow(Gitlab::HTTP).to receive(:perform_request).and_return(response)
-            expect(subject).to be_falsey
+            it 'returns false' do
+              allow(Gitlab::HTTP).to receive(:perform_request).and_return(response)
+              expect(subject).to be_falsey
+            end
           end
         end
       end
+
+      context 'when an error occurs during the Arkose request' do
+        it 'returns true' do
+          allow(Gitlab::HTTP).to receive(:perform_request).and_raise(Gitlab::HTTP::BlockedUrlError)
+          expect(subject).to be_truthy
+        end
+      end
+    end
+
+    context 'when Arkose is configured using application settings' do
+      before do
+        stub_application_setting(arkose_labs_private_api_key: arkose_labs_private_api_key)
+        stub_application_setting(arkose_labs_verify_api_url: arkose_labs_verify_api_url)
+      end
+
+      it_behaves_like 'interacting with Arkose verify API'
     end
 
-    context 'when an error occurs during the Arkose request' do
-      it 'returns true' do
-        allow(Gitlab::HTTP).to receive(:perform_request).and_raise(Gitlab::HTTP::BlockedUrlError)
-        expect(subject).to be_truthy
+    context 'when Arkose application settings are not present, fallback to environment variables' do
+      before do
+        stub_env('ARKOSE_LABS_PRIVATE_KEY': arkose_labs_private_api_key)
+        stub_env('ARKOSE_LABS_VERIFY_URL': arkose_labs_verify_api_url)
       end
+
+      it_behaves_like 'interacting with Arkose verify API'
     end
 
     context 'when feature arkose_labs_prevent_login is disabled' do