Merge remote-tracking branch 'dev/13-3-stable' into 13-3-stable

10 files changed, 62 insertions, 29 deletions

diff --git a/app/controllers/clusters/clusters_controller.rb b/app/controllers/clusters/clusters_controller.rb
index 2e8b3d764ca..cae9d098799 100644
--- a/app/controllers/clusters/clusters_controller.rb
+++ b/app/controllers/clusters/clusters_controller.rb
@@ -38,8 +38,7 @@ class Clusters::ClustersController < Clusters::BaseController
 
   def new
     if params[:provider] == 'aws'
-      @aws_role = current_user.aws_role || Aws::Role.new
-      @aws_role.ensure_role_external_id!
+      @aws_role = Aws::Role.create_or_find_by!(user: current_user)
       @instance_types = load_instance_types.to_json
 
     elsif params[:provider] == 'gcp'
@@ -273,7 +272,7 @@ class Clusters::ClustersController < Clusters::BaseController
   end
 
   def aws_role_params
-    params.require(:cluster).permit(:role_arn, :role_external_id)
+    params.require(:cluster).permit(:role_arn)
   end
 
   def generate_gcp_authorize_url
diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb
index 4b4bcc8d37e..2cc51c65c26 100644
--- a/app/controllers/concerns/authenticates_with_two_factor.rb
+++ b/app/controllers/concerns/authenticates_with_two_factor.rb
@@ -22,6 +22,8 @@ module AuthenticatesWithTwoFactor
     return handle_locked_user(user) unless user.can?(:log_in)
 
     session[:otp_user_id] = user.id
+    session[:user_updated_at] = user.updated_at
+
     setup_u2f_authentication(user)
     render 'devise/sessions/two_factor'
   end
@@ -39,6 +41,7 @@ module AuthenticatesWithTwoFactor
   def authenticate_with_two_factor
     user = self.resource = find_user
     return handle_locked_user(user) unless user.can?(:log_in)
+    return handle_changed_user(user) if user_changed?(user)
 
     if user_params[:otp_attempt].present? && session[:otp_user_id]
       authenticate_with_two_factor_via_otp(user)
@@ -63,12 +66,14 @@ module AuthenticatesWithTwoFactor
 
   def clear_two_factor_attempt!
     session.delete(:otp_user_id)
+    session.delete(:user_updated_at)
+    session.delete(:challenge)
   end
 
   def authenticate_with_two_factor_via_otp(user)
     if valid_otp_attempt?(user)
       # Remove any lingering user data from login
-      session.delete(:otp_user_id)
+      clear_two_factor_attempt!
 
       remember_me(user) if user_params[:remember_me] == '1'
       user.save!
@@ -85,8 +90,7 @@ module AuthenticatesWithTwoFactor
   def authenticate_with_two_factor_via_u2f(user)
     if U2fRegistration.authenticate(user, u2f_app_id, user_params[:device_response], session[:challenge])
       # Remove any lingering user data from login
-      session.delete(:otp_user_id)
-      session.delete(:challenge)
+      clear_two_factor_attempt!
 
       remember_me(user) if user_params[:remember_me] == '1'
       sign_in(user, message: :two_factor_authenticated, event: :authentication)
@@ -113,4 +117,18 @@ module AuthenticatesWithTwoFactor
     end
   end
   # rubocop: enable CodeReuse/ActiveRecord
+
+  def handle_changed_user(user)
+    clear_two_factor_attempt!
+
+    redirect_to new_user_session_path, alert: _('An error occurred. Please sign in again.')
+  end
+
+  # If user has been updated since we validated the password,
+  # the password might have changed.
+  def user_changed?(user)
+    return false unless session[:user_updated_at]
+
+    user.updated_at != session[:user_updated_at]
+  end
 end
diff --git a/app/controllers/concerns/enforces_two_factor_authentication.rb b/app/controllers/concerns/enforces_two_factor_authentication.rb
index f1dd46648f1..02082f81598 100644
--- a/app/controllers/concerns/enforces_two_factor_authentication.rb
+++ b/app/controllers/concerns/enforces_two_factor_authentication.rb
@@ -29,12 +29,11 @@ module EnforcesTwoFactorAuthentication
   end
 
   def two_factor_authentication_required?
-    Gitlab::CurrentSettings.require_two_factor_authentication? ||
-      current_user.try(:require_two_factor_authentication_from_group?)
+    two_factor_verifier.two_factor_authentication_required?
   end
 
   def current_user_requires_two_factor?
-    current_user && !current_user.temp_oauth_email? && !current_user.two_factor_enabled? && !skip_two_factor?
+    two_factor_verifier.current_user_needs_to_setup_two_factor? && !skip_two_factor?
   end
 
   # rubocop: disable CodeReuse/ActiveRecord
@@ -43,7 +42,7 @@ module EnforcesTwoFactorAuthentication
       if Gitlab::CurrentSettings.require_two_factor_authentication?
         global.call
       else
-        groups = current_user.expanded_groups_requiring_two_factor_authentication.reorder(name: :asc)
+        groups = current_user.source_groups_of_two_factor_authentication_requirement.reorder(name: :asc)
         group.call(groups)
       end
     end
@@ -51,14 +50,11 @@ module EnforcesTwoFactorAuthentication
   # rubocop: enable CodeReuse/ActiveRecord
 
   def two_factor_grace_period
-    periods = [Gitlab::CurrentSettings.two_factor_grace_period]
-    periods << current_user.two_factor_grace_period if current_user.try(:require_two_factor_authentication_from_group?)
-    periods.min
+    two_factor_verifier.two_factor_grace_period
   end
 
   def two_factor_grace_period_expired?
-    date = current_user.otp_grace_period_started_at
-    date && (date + two_factor_grace_period.hours) < Time.current
+    two_factor_verifier.two_factor_grace_period_expired?
   end
 
   def two_factor_skippable?
@@ -70,6 +66,10 @@ module EnforcesTwoFactorAuthentication
   def skip_two_factor?
     session[:skip_two_factor] && session[:skip_two_factor] > Time.current
   end
+
+  def two_factor_verifier
+    @two_factor_verifier ||= Gitlab::Auth::TwoFactorAuthVerifier.new(current_user) # rubocop:disable Gitlab/ModuleWithInstanceVariables
+  end
 end
 
 EnforcesTwoFactorAuthentication.prepend_if_ee('EE::EnforcesTwoFactorAuthentication')
diff --git a/app/controllers/concerns/hooks_execution.rb b/app/controllers/concerns/hooks_execution.rb
index e8add1f4055..ad1f8341109 100644
--- a/app/controllers/concerns/hooks_execution.rb
+++ b/app/controllers/concerns/hooks_execution.rb
@@ -17,4 +17,16 @@ module HooksExecution
       flash[:alert] = "Hook execution failed: #{message}"
     end
   end
+
+  def create_rate_limit(key, scope)
+    if rate_limiter.throttled?(key, scope: [scope, current_user])
+      rate_limiter.log_request(request, "#{key}_request_limit".to_sym, current_user)
+
+      render plain: _('This endpoint has been requested too many times. Try again later.'), status: :too_many_requests
+    end
+  end
+
+  def rate_limiter
+    ::Gitlab::ApplicationRateLimiter
+  end
 end
diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index 6a393405e4d..a558b01f0c6 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -50,12 +50,6 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     redirect_unverified_saml_initiation
   end
 
-  def omniauth_error
-    @provider = params[:provider]
-    @error = params[:error]
-    render 'errors/omniauth_error', layout: "oauth_error", status: :unprocessable_entity
-  end
-
   def cas3
     ticket = params['ticket']
     if ticket
@@ -205,9 +199,10 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
   def fail_login(user)
     log_failed_login(user.username, oauth['provider'])
 
-    error_message = user.errors.full_messages.to_sentence
+    @provider = Gitlab::Auth::OAuth::Provider.label_for(params[:action])
+    @error = user.errors.full_messages.to_sentence
 
-    redirect_to omniauth_error_path(oauth['provider'], error: error_message)
+    render 'errors/omniauth_error', layout: "oauth_error", status: :unprocessable_entity
   end
 
   def fail_auth0_login
diff --git a/app/controllers/profiles/active_sessions_controller.rb b/app/controllers/profiles/active_sessions_controller.rb
index f409193aefc..d9ec3195fd1 100644
--- a/app/controllers/profiles/active_sessions_controller.rb
+++ b/app/controllers/profiles/active_sessions_controller.rb
@@ -7,6 +7,7 @@ class Profiles::ActiveSessionsController < Profiles::ApplicationController
 
   def destroy
     ActiveSession.destroy_with_public_id(current_user, params[:id])
+    current_user.forget_me!
 
     respond_to do |format|
       format.html { redirect_to profile_active_sessions_url, status: :found }
diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb
index 95b9344c551..50fbf8146e5 100644
--- a/app/controllers/profiles/two_factor_auths_controller.rb
+++ b/app/controllers/profiles/two_factor_auths_controller.rb
@@ -4,7 +4,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
   skip_before_action :check_two_factor_requirement
 
   def show
-    unless current_user.otp_secret
+    unless current_user.two_factor_enabled?
       current_user.otp_secret = User.generate_otp_secret(32)
     end
 
@@ -38,6 +38,8 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
 
   def create
     if current_user.validate_and_consume_otp!(params[:pin_code])
+      ActiveSession.destroy_all_but_current(current_user, session)
+
       Users::UpdateService.new(current_user, user: current_user, otp_required_for_login: true).execute! do |user|
         @codes = user.generate_otp_backup_codes!
       end
diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb
index 097a357889f..2f4dc1ddc3a 100644
--- a/app/controllers/projects/hooks_controller.rb
+++ b/app/controllers/projects/hooks_controller.rb
@@ -6,6 +6,7 @@ class Projects::HooksController < Projects::ApplicationController
   # Authorize
   before_action :authorize_admin_project!
   before_action :hook_logs, only: :edit
+  before_action -> { create_rate_limit(:project_testing_hook, @project) }, only: :test
 
   respond_to :html
 
diff --git a/app/controllers/projects/tags_controller.rb b/app/controllers/projects/tags_controller.rb
index df20daa8f7e..475ca8894e4 100644
--- a/app/controllers/projects/tags_controller.rb
+++ b/app/controllers/projects/tags_controller.rb
@@ -92,7 +92,7 @@ class Projects::TagsController < Projects::ApplicationController
         end
 
         format.js do
-          render status: :unprocessable_entity
+          render status: :ok
         end
       end
     end
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index f82212591b6..9435b9887e9 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -8,6 +8,7 @@ class SessionsController < Devise::SessionsController
   include Recaptcha::Verify
   include RendersLdapServers
   include KnownSignIn
+  include Gitlab::Utils::StrongMemoize
 
   skip_before_action :check_two_factor_requirement, only: [:destroy]
   # replaced with :require_no_authentication_without_flash
@@ -197,10 +198,14 @@ class SessionsController < Devise::SessionsController
   end
 
   def find_user
-    if session[:otp_user_id]
-      User.find(session[:otp_user_id])
-    elsif user_params[:login]
-      User.by_login(user_params[:login])
+    strong_memoize(:find_user) do
+      if session[:otp_user_id] && user_params[:login]
+        User.by_id_and_login(session[:otp_user_id], user_params[:login]).first
+      elsif session[:otp_user_id]
+        User.find(session[:otp_user_id])
+      elsif user_params[:login]
+        User.by_login(user_params[:login])
+      end
     end
   end