diff --git a/ee/app/controllers/concerns/arkose/token_verifiable.rb b/ee/app/controllers/concerns/arkose/token_verifiable.rb new file mode 100644 index 0000000000000000000000000000000000000000..ce95e8ece10360558452f3ac23d92b9ab2906084 --- /dev/null +++ b/ee/app/controllers/concerns/arkose/token_verifiable.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +module Arkose + module TokenVerifiable + extend ActiveSupport::Concern + include ::Gitlab::Utils::StrongMemoize + + private + + def verify_arkose_labs_token(user: nil) + return true unless arkose_labs_enabled? + + return arkose_labs_verify_response(user: user).present? if token.present? + + if arkose_down? + log_challenge_skipped + return true + end + + log_token_missing + false + end + + def arkose_labs_verify_response(user: nil) + strong_memoize_with(:arkose_labs_verify_response, user) do + result = Arkose::TokenVerificationService.new(session_token: token, user: user).execute + result.success? ? result.payload[:response] : nil + end + end + + def log_challenge_skipped + ::Gitlab::AppLogger.info( + message: 'Sign-up verification skipped', + reason: 'arkose is experiencing an outage', + username: username + ) + end + + def log_token_missing + ::Gitlab::AppLogger.info( + message: 'Sign-up blocked', + reason: 'arkose token is missing in request', + username: username + ) + end + + def token + @token ||= params[:arkose_labs_token] + end + + def arkose_down? + Arkose::StatusService.new.execute.error? + end + end +end diff --git a/ee/app/controllers/ee/registrations_controller.rb b/ee/app/controllers/ee/registrations_controller.rb index b7bc17d00a281e680ae8511296179eecfbdedd8c..c1eeb58d594c648b4ab523101e0e70bc72459257 100644 --- a/ee/app/controllers/ee/registrations_controller.rb +++ b/ee/app/controllers/ee/registrations_controller.rb @@ -8,6 +8,7 @@ module RegistrationsController prepended do include Arkose::ContentSecurityPolicy + include Arkose::TokenVerifiable include RegistrationsTracking include ::Onboarding::SetRedirect include GoogleAnalyticsCSP @@ -120,31 +121,6 @@ def registration_path_params glm_tracking_params.to_h end - def verify_arkose_labs_token - return true unless arkose_labs_enabled? - - unless params[:arkose_labs_token].present? - # if arkose is down, skip challenge - arkose_status = Arkose::StatusService.new.execute - - if arkose_status.error? - log_arkose_challenge_skipped - return true - end - - log_arkose_token_missing - return false - end - - arkose_labs_verify_response.present? - end - - def arkose_labs_verify_response - result = Arkose::TokenVerificationService.new(session_token: params[:arkose_labs_token]).execute - result.success? ? result.payload[:response] : nil - end - strong_memoize_attr :arkose_labs_verify_response - def record_arkose_data return unless resource&.persisted? return unless arkose_labs_enabled? @@ -156,22 +132,6 @@ def record_arkose_data ).execute end - def log_arkose_token_missing - ::Gitlab::AppLogger.info( - message: 'Sign-up blocked', - reason: 'arkose token is missing in request', - username: sign_up_params[:username] - ) - end - - def log_arkose_challenge_skipped - ::Gitlab::AppLogger.info( - message: 'Sign-up verification skipped', - reason: 'arkose is experiencing an outage', - username: sign_up_params[:username] - ) - end - override :arkose_labs_enabled? def arkose_labs_enabled? ::Feature.enabled?(:arkose_labs_signup_challenge) && @@ -182,5 +142,9 @@ def allow_account_deletion? !License.feature_available?(:disable_deleting_account_for_users) || ::Gitlab::CurrentSettings.allow_account_deletion? end + + def username + sign_up_params[:username] + end end end diff --git a/ee/app/controllers/users/identity_verification_controller.rb b/ee/app/controllers/users/identity_verification_controller.rb index 5ed3000e1fb41b3928febeab2aafdf1a7d7aaac8..87bd44f72f452a55b5c2ac372a2c646cffc4228c 100644 --- a/ee/app/controllers/users/identity_verification_controller.rb +++ b/ee/app/controllers/users/identity_verification_controller.rb @@ -5,6 +5,7 @@ class IdentityVerificationController < ApplicationController include AcceptsPendingInvitations include ActionView::Helpers::DateHelper include Arkose::ContentSecurityPolicy + include Arkose::TokenVerifiable include IdentityVerificationHelper include ::Gitlab::RackLoadBalancingHelpers include Recaptcha::Adapters::ControllerMethods @@ -118,7 +119,7 @@ def verify_phone_verification_code def arkose_labs_challenge; end def verify_arkose_labs_session - unless verify_arkose_labs_token + unless verify_arkose_labs_token(user: @user) flash[:alert] = s_('IdentityVerification|Complete verification to sign up.') return render action: :arkose_labs_challenge end @@ -247,8 +248,7 @@ def redirect_banned_user end def require_arkose_verification! - return unless Feature.enabled?(:arkose_labs_oauth_signup_challenge) - return unless ::Arkose::Settings.enabled?(user: @user, user_agent: request.user_agent) + return unless arkose_labs_enabled? return unless @user.identities.any? return unless @user.arkose_risk_band.blank? @@ -310,15 +310,6 @@ def verify_phone_verification_code_params params.require(:identity_verification).permit(:verification_code) end - def verify_arkose_labs_token(save_user_data: true) - return false unless params[:arkose_labs_token].present? - - user = @user if save_user_data - - result = Arkose::TokenVerificationService.new(session_token: params[:arkose_labs_token], user: user).execute - result.success? - end - def load_captcha show_recaptcha_challenge? && Gitlab::Recaptcha.load_configurations! end @@ -342,11 +333,20 @@ def ensure_challenge_completed(category) if arkose_shown log_event(:phone, :arkose_challenge_shown) - return verify_arkose_labs_token(save_user_data: false) + return verify_arkose_labs_token end end true end + + def arkose_labs_enabled? + ::Feature.enabled?(:arkose_labs_oauth_signup_challenge) && + ::Arkose::Settings.enabled?(user: @user, user_agent: request.user_agent) + end + + def username + @user.username + end end end diff --git a/ee/spec/requests/users/identity_verification_controller_spec.rb b/ee/spec/requests/users/identity_verification_controller_spec.rb index f3f9e8bfbe4e118ff3d8f5d0b2329c590564d0dc..0b88f4ee555301fb27b2236ccf841ed41cb7910c 100644 --- a/ee/spec/requests/users/identity_verification_controller_spec.rb +++ b/ee/spec/requests/users/identity_verification_controller_spec.rb @@ -11,12 +11,26 @@ let_it_be(:confirmed_user) { create(:user, :low_risk) } let_it_be(:invalid_verification_user_id) { non_existing_record_id } + let(:verify_response_json) 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(verify_response_json) } + let(:token_verification_service_success_response) { ServiceResponse.success(payload: { response: verify_response }) } + let(:is_arkose_enabled) { true } + before do stub_application_setting_enum('email_confirmation_setting', 'hard') stub_application_setting(require_admin_approval_after_user_signup: false) allow(::Gitlab::ApplicationRateLimiter).to receive(:peek).and_call_original allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_call_original + + allow(::Arkose::Settings).to receive(:enabled?).and_return(is_arkose_enabled) + + allow_next_instance_of(::Arkose::StatusService) do |instance| + allow(instance).to receive(:execute).and_return(ServiceResponse.success) + end end shared_examples 'it requires a valid verification_user_id' do @@ -132,10 +146,8 @@ shared_examples 'it requires oauth users to go through ArkoseLabs challenge' do let(:user) { create(:omniauth_user, :unconfirmed) } let(:arkose_labs_oauth_signup_challenge) { true } - let(:is_arkose_enabled) { true } before do - allow(::Arkose::Settings).to receive(:enabled?).and_return(is_arkose_enabled) stub_feature_flags(arkose_labs_oauth_signup_challenge: arkose_labs_oauth_signup_challenge) stub_session(verification_user_id: user.id) @@ -279,7 +291,7 @@ end context 'and token verification succeeds' do - let(:service_response) { ServiceResponse.success } + let(:service_response) { token_verification_service_success_response } it 'returns a 200' do do_request @@ -832,7 +844,7 @@ end context 'when token verification succeeds' do - let(:service_response) { ServiceResponse.success } + let(:service_response) { token_verification_service_success_response } it 'redirects to show action' do expect(response).to redirect_to(identity_verification_path) @@ -845,7 +857,7 @@ before do allow_next_instance_of(Arkose::TokenVerificationService) do |instance| - allow(instance).to receive(:execute).and_return(ServiceResponse.success) + allow(instance).to receive(:execute).and_return(token_verification_service_success_response) end end