diff --git a/ee/app/services/arkose/token_verification_service.rb b/ee/app/services/arkose/token_verification_service.rb index e730e714742bf8cfa94560549e2a1c5120e9672b..9ed616280a6013e3c41fb4cc974e543c34be336f 100644 --- a/ee/app/services/arkose/token_verification_service.rb +++ b/ee/app/services/arkose/token_verification_service.rb @@ -2,7 +2,7 @@ module Arkose class TokenVerificationService - attr_reader :session_token, :user + attr_reader :session_token, :user, :response ARKOSE_LABS_DEFAULT_NAMESPACE = 'client' ARKOSE_LABS_DEFAULT_SUBDOMAIN = 'verify-api' @@ -13,10 +13,10 @@ def initialize(session_token:, user: nil) end def execute - response = Gitlab::HTTP.perform_request(Net::HTTP::Post, arkose_verify_url, body: body).parsed_response - response = ::Arkose::VerifyResponse.new(response) + parsed_response = Gitlab::HTTP.perform_request(Net::HTTP::Post, arkose_verify_url, body: body).parsed_response + @response = ::Arkose::VerifyResponse.new(parsed_response) - logger.info(build_message(response)) + logger.log_successful_token_verification return ServiceResponse.error(message: response.error) if response.invalid_token? @@ -41,7 +41,8 @@ def execute }.compact Gitlab::ExceptionLogFormatter.format!(error, payload) Gitlab::ErrorTracking.track_exception(error) - logger.error("Error verifying user on Arkose: #{payload}") + + logger.log_failed_token_verification ServiceResponse.success(payload: payload) end @@ -57,28 +58,11 @@ def body end def logger - Gitlab::AppLogger - end - - def build_message(response) - Gitlab::ApplicationContext.current.symbolize_keys.merge( - { - message: 'Arkose verify response', - response: response.response, - username: user&.username - }.compact).merge(arkose_payload(response)) - end - - def arkose_payload(response) - { - 'arkose.session_id': response.session_id, - 'arkose.global_score': response.global_score, - 'arkose.global_telltale_list': response.global_telltale_list, - 'arkose.custom_score': response.custom_score, - 'arkose.custom_telltale_list': response.custom_telltale_list, - 'arkose.risk_band': response.risk_band, - 'arkose.risk_category': response.risk_category - } + @logger ||= ::Arkose::Logger.new( + session_token: session_token, + user: user, + verify_response: response + ) end def arkose_verify_url diff --git a/ee/lib/arkose/logger.rb b/ee/lib/arkose/logger.rb new file mode 100644 index 0000000000000000000000000000000000000000..0c0482aa1bda4953ab5ebdff575414953fcf7bf2 --- /dev/null +++ b/ee/lib/arkose/logger.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +module Arkose + class Logger + attr_reader :session_token, :user, :response + + def initialize(session_token:, user: nil, verify_response: nil) + @session_token = session_token + @user = user + @response = verify_response + end + + def log_successful_token_verification + return unless response + + logger.info(build_message('Arkose verify response')) + end + + def log_failed_token_verification + payload = { + session_token: session_token, + log_data: user&.id + } + + logger.error("Error verifying user on Arkose: #{payload}") + end + + private + + def logger + Gitlab::AppLogger + end + + def build_message(message) + Gitlab::ApplicationContext.current.symbolize_keys.merge( + { + message: message, + response: response.response, + username: user&.username + }.compact).merge(arkose_payload) + end + + def arkose_payload + { + 'arkose.session_id': response.session_id, + 'arkose.global_score': response.global_score, + 'arkose.global_telltale_list': response.global_telltale_list, + 'arkose.custom_score': response.custom_score, + 'arkose.custom_telltale_list': response.custom_telltale_list, + 'arkose.risk_band': response.risk_band, + 'arkose.risk_category': response.risk_category + } + end + end +end diff --git a/ee/spec/lib/arkose/logger_spec.rb b/ee/spec/lib/arkose/logger_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..ac8423340e3e18d2c1548d3c545cf785e7c413e6 --- /dev/null +++ b/ee/spec/lib/arkose/logger_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Arkose::Logger do + let(:user) { build_stubbed(:user) } + let(:session_token) { '22612c147bb418c8.2570749403' } + + describe '#log_successful_token_verification' do + let(:json_verify_response) do + Gitlab::Json.parse(File.read(Rails.root.join('ee/spec/fixtures/arkose/successfully_solved_ec_response.json'))) + end + + let(:verify_response) { Arkose::VerifyResponse.new(json_verify_response) } + let(:expected_payload) do + { + correlation_id: 'be025cf83013ac4f52ffd2bf712b11a2', + message: 'Arkose verify response', + response: json_verify_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' + }.compact + end + + subject(:logger) { described_class.new(session_token: session_token, user: user, verify_response: verify_response) } + + before do + allow(Gitlab::AppLogger).to receive(:info) + allow(Gitlab::ApplicationContext).to receive(:current).and_return( + { 'correlation_id': 'be025cf83013ac4f52ffd2bf712b11a2' } + ) + end + + it 'logs the event with the correct info' do + expect(expected_payload).to include(:username) + expect(Gitlab::AppLogger).to receive(:info).with(expected_payload) + + logger.log_successful_token_verification + end + + context 'when user is nil' do + let(:user) { nil } + + it 'logs the event without username info' do + expect(expected_payload).not_to include(:username) + expect(Gitlab::AppLogger).to receive(:info).with(expected_payload) + + logger.log_successful_token_verification + end + end + end + + describe '#log_failed_token_verification' do + subject(:logger) { described_class.new(session_token: session_token, user: user, verify_response: nil) } + + it 'logs the event with the correct info' do + message = /Error verifying user on Arkose: {:session_token=>"#{session_token}", :log_data=>#{user.id}}/ + expect(Gitlab::AppLogger).to receive(:error).with(a_string_matching(message)) + + logger.log_failed_token_verification + end + + context 'when user is nil' do + let(:user) { nil } + + it 'logs the event with the correct info' do + message = /Error verifying user on Arkose: {:session_token=>"#{session_token}", :log_data=>nil}/ + expect(Gitlab::AppLogger).to receive(:error).with(a_string_matching(message)) + + logger.log_failed_token_verification + end + end + end +end diff --git a/ee/spec/services/arkose/token_verification_service_spec.rb b/ee/spec/services/arkose/token_verification_service_spec.rb index a889ca88b7789d7e5adcb1a0370f2719bcf2016e..bb40dfaf78864718aeb346b57bcc234e73423f93 100644 --- a/ee/spec/services/arkose/token_verification_service_spec.rb +++ b/ee/spec/services/arkose/token_verification_service_spec.rb @@ -74,6 +74,12 @@ ) end + let(:mock_verify_response) { Arkose::VerifyResponse.new(arkose_ec_response) } + + before do + allow(Arkose::VerifyResponse).to receive(:new).with(arkose_ec_response).and_return(mock_verify_response) + end + it 'makes a request to the Verify API' do subject @@ -82,34 +88,18 @@ it_behaves_like 'returns success response with the correct payload' - it 'logs Arkose verify response' do - allow(Gitlab::AppLogger).to receive(:info) - allow(Gitlab::ApplicationContext).to receive(:current).and_return( - { 'correlation_id': 'be025cf83013ac4f52ffd2bf712b11a2' } - ) + it 'logs the event' do + init_args = { session_token: session_token, user: user, verify_response: mock_verify_response } + expect_next_instance_of(::Arkose::Logger, init_args) do |logger| + expect(logger).to receive(:log_successful_token_verification) + end 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 "records user's Arkose data" do - mock_response = Arkose::VerifyResponse.new(arkose_ec_response) - expect(Arkose::VerifyResponse).to receive(:new).with(arkose_ec_response).and_return(mock_response) - - expect_next_instance_of(Arkose::RecordUserDataService, response: mock_response, user: user) do |service| + init_args = { response: mock_verify_response, user: user } + expect_next_instance_of(Arkose::RecordUserDataService, init_args) do |service| expect(service).to receive(:execute) end @@ -191,7 +181,10 @@ end it 'logs the error' do - expect(Gitlab::AppLogger).to receive(:error).with(a_string_matching(/Error verifying user on Arkose: /)) + init_args = { session_token: session_token, user: user, verify_response: nil } + expect_next_instance_of(::Arkose::Logger, init_args) do |logger| + expect(logger).to receive(:log_failed_token_verification) + end subject end @@ -214,35 +207,6 @@ it_behaves_like 'returns success response with correct payload and logs the error' end - - context 'when user is nil' do - let(:user) { nil } - let(:arkose_ec_response) do - Gitlab::Json.parse(File.read(Rails.root.join('ee/spec/fixtures/arkose/successfully_solved_ec_response.json'))) - end - - it 'logs Arkose verify response without username' do - 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, - '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 - end end context 'when arkose_labs_prevent_login feature flag is enabled' do