Add latest changes from gitlab-org/security/gitlab@14-3-stable-ee

13 files changed, 166 insertions, 29 deletions

diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index dfc1434d909..cdfb3a32f4c 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -45,7 +45,7 @@ class Admin::UsersController < Admin::ApplicationController
   end
 
   def impersonate
-    if can?(user, :log_in)
+    if can?(user, :log_in) && !impersonation_in_progress?
       session[:impersonator_id] = current_user.id
 
       warden.set_user(user, scope: :user)
@@ -58,7 +58,9 @@ class Admin::UsersController < Admin::ApplicationController
       redirect_to root_path
     else
       flash[:alert] =
-        if user.blocked?
+        if impersonation_in_progress?
+          _("You are already impersonating another user")
+        elsif user.blocked?
           _("You cannot impersonate a blocked user")
         elsif user.internal?
           _("You cannot impersonate an internal user")
diff --git a/app/controllers/concerns/impersonation.rb b/app/controllers/concerns/impersonation.rb
index a8788e7f8bd..539dd9ad69d 100644
--- a/app/controllers/concerns/impersonation.rb
+++ b/app/controllers/concerns/impersonation.rb
@@ -20,7 +20,7 @@ module Impersonation
   protected
 
   def check_impersonation_availability
-    return unless session[:impersonator_id]
+    return unless impersonation_in_progress?
 
     unless Gitlab.config.gitlab.impersonation_enabled
       stop_impersonation
@@ -38,6 +38,10 @@ module Impersonation
     current_user
   end
 
+  def impersonation_in_progress?
+    session[:impersonator_id].present?
+  end
+
   def log_impersonation_event
     Gitlab::AppLogger.info("User #{impersonator.username} has stopped impersonating #{current_user.username}")
   end
diff --git a/app/controllers/profiles/passwords_controller.rb b/app/controllers/profiles/passwords_controller.rb
index 85e901eb3eb..c8c2dd1c7d6 100644
--- a/app/controllers/profiles/passwords_controller.rb
+++ b/app/controllers/profiles/passwords_controller.rb
@@ -47,6 +47,8 @@ class Profiles::PasswordsController < Profiles::ApplicationController
     password_attributes[:password_automatically_set] = false
 
     unless @user.password_automatically_set || @user.valid_password?(user_params[:current_password])
+      handle_invalid_current_password_attempt!
+
       redirect_to edit_profile_password_path, alert: _('You must provide a valid current password')
       return
     end
@@ -85,6 +87,12 @@ class Profiles::PasswordsController < Profiles::ApplicationController
     render_404 unless @user.allow_password_authentication?
   end
 
+  def handle_invalid_current_password_attempt!
+    Gitlab::AppLogger.info(message: 'Invalid current password when attempting to update user password', username: @user.username, ip: request.remote_ip)
+
+    @user.increment_failed_attempts!
+  end
+
   def user_params
     params.require(:user).permit(:current_password, :password, :password_confirmation)
   end
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index d7c1d87ae4b..de4e51a3a2f 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -19,6 +19,7 @@ class ProjectsController < Projects::ApplicationController
   before_action :redirect_git_extension, only: [:show]
   before_action :project, except: [:index, :new, :create, :resolve]
   before_action :repository, except: [:index, :new, :create, :resolve]
+  before_action :verify_git_import_enabled, only: [:create]
   before_action :project_export_enabled, only: [:export, :download_export, :remove_export, :generate_new_export]
   before_action :present_project, only: [:edit]
   before_action :authorize_download_code!, only: [:refs]
@@ -495,6 +496,10 @@ class ProjectsController < Projects::ApplicationController
     url_for(safe_params)
   end
 
+  def verify_git_import_enabled
+    render_404 if project_params[:import_url] && !git_import_enabled?
+  end
+
   def project_export_enabled
     render_404 unless Gitlab::CurrentSettings.project_export_enabled?
   end
diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb
index 4077a3d3dac..d040ac7f76c 100644
--- a/app/controllers/uploads_controller.rb
+++ b/app/controllers/uploads_controller.rb
@@ -36,14 +36,10 @@ class UploadsController < ApplicationController
   end
 
   def find_model
-    return unless params[:id]
-
     upload_model_class.find(params[:id])
   end
 
   def authorize_access!
-    return unless model
-
     authorized =
       case model
       when Note
@@ -68,8 +64,6 @@ class UploadsController < ApplicationController
   end
 
   def authorize_create_access!
-    return unless model
-
     authorized =
       case model
       when User
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index a92d904be84..34e0b528ced 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -89,6 +89,10 @@ module API
           Gitlab::AppLogger.info({ message: "File exceeds maximum size", file_bytes: file.size, project_id: user_project.id, project_path: user_project.full_path, upload_allowed: allowed })
         end
       end
+
+      def check_import_by_url_is_enabled
+        forbidden! unless Gitlab::CurrentSettings.import_sources&.include?('git')
+      end
     end
 
     helpers do
@@ -267,6 +271,7 @@ module API
         attrs = declared_params(include_missing: false)
         attrs = translate_params_for_compatibility(attrs)
         filter_attributes_using_license!(attrs)
+        check_import_by_url_is_enabled if params[:import_url].present?
         project = ::Projects::CreateService.new(current_user, attrs).execute
 
         if project.saved?
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 6c0b33911ca..03a26c01cd5 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -38380,6 +38380,9 @@ msgstr ""
 msgid "You are already a member of this %{member_source}."
 msgstr ""
 
+msgid "You are already impersonating another user"
+msgstr ""
+
 msgid "You are an admin, which means granting access to %{client_name} will allow them to interact with GitLab as an admin as well. Proceed with caution."
 msgstr ""
 
diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb
index 4d2c311c9a4..3a2b5dcb99d 100644
--- a/spec/controllers/admin/users_controller_spec.rb
+++ b/spec/controllers/admin/users_controller_spec.rb
@@ -815,5 +815,20 @@ RSpec.describe Admin::UsersController do
         expect(response).to have_gitlab_http_status(:not_found)
       end
     end
+
+    context 'when impersonating an admin and attempting to impersonate again' do
+      let(:admin2) { create(:admin) }
+
+      before do
+        post :impersonate, params: { id: admin2.username }
+      end
+
+      it 'does not allow double impersonation', :aggregate_failures do
+        post :impersonate, params: { id: user.username }
+
+        expect(flash[:alert]).to eq(_('You are already impersonating another user'))
+        expect(warden.user).to eq(admin2)
+      end
+    end
   end
 end
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index 8afb80d9cc5..9d070061850 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -408,6 +408,47 @@ RSpec.describe ProjectsController do
     end
   end
 
+  describe 'POST create' do
+    let!(:params) do
+      {
+        path: 'foo',
+        description: 'bar',
+        import_url: project.http_url_to_repo,
+        namespace_id: user.namespace.id
+      }
+    end
+
+    subject { post :create, params: { project: params } }
+
+    before do
+      sign_in(user)
+    end
+
+    context 'when import by url is disabled' do
+      before do
+        stub_application_setting(import_sources: [])
+      end
+
+      it 'does not create project and reports an error' do
+        expect { subject }.not_to change { Project.count }
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
+    context 'when import by url is enabled' do
+      before do
+        stub_application_setting(import_sources: ['git'])
+      end
+
+      it 'creates project' do
+        expect { subject }.to change { Project.count }
+
+        expect(response).to have_gitlab_http_status(:redirect)
+      end
+    end
+  end
+
   describe 'GET edit' do
     it 'allows an admin user to access the page', :enable_admin_mode do
       sign_in(create(:user, :admin))
diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb
index 043fd97f1ad..2aa9b86b20e 100644
--- a/spec/controllers/uploads_controller_spec.rb
+++ b/spec/controllers/uploads_controller_spec.rb
@@ -666,6 +666,6 @@ RSpec.describe UploadsController do
   def post_authorize(verified: true)
     request.headers.merge!(workhorse_internal_api_request_header) if verified
 
-    post :authorize, params: { model: 'personal_snippet', id: model.id }, format: :json
+    post :authorize, params: params, format: :json
   end
 end
diff --git a/spec/features/profiles/password_spec.rb b/spec/features/profiles/password_spec.rb
index c9059395377..893dd2c76e0 100644
--- a/spec/features/profiles/password_spec.rb
+++ b/spec/features/profiles/password_spec.rb
@@ -78,40 +78,80 @@ RSpec.describe 'Profile > Password' do
     end
   end
 
-  context 'Change passowrd' do
+  context 'Change password' do
+    let(:new_password) { '22233344' }
+
     before do
       sign_in(user)
       visit(edit_profile_password_path)
     end
 
-    it 'does not change user passowrd without old one' do
-      page.within '.update-password' do
-        fill_passwords('22233344', '22233344')
+    shared_examples 'user enters an incorrect current password' do
+      subject do
+        page.within '.update-password' do
+          fill_in 'user_current_password', with: user_current_password
+          fill_passwords(new_password, new_password)
+        end
       end
 
-      page.within '.flash-container' do
-        expect(page).to have_content 'You must provide a valid current password'
-      end
-    end
+      it 'handles the invalid password attempt, and prompts the user to try again', :aggregate_failures do
+        expect(Gitlab::AppLogger).to receive(:info)
+          .with(message: 'Invalid current password when attempting to update user password', username: user.username, ip: user.current_sign_in_ip)
+
+        subject
+
+        user.reload
 
-    it 'does not change password with invalid old password' do
-      page.within '.update-password' do
-        fill_in 'user_current_password', with: 'invalid'
-        fill_passwords('password', 'confirmation')
+        expect(user.failed_attempts).to eq(1)
+        expect(user.valid_password?(new_password)).to eq(false)
+        expect(current_path).to eq(edit_profile_password_path)
+
+        page.within '.flash-container' do
+          expect(page).to have_content('You must provide a valid current password')
+        end
       end
 
-      page.within '.flash-container' do
-        expect(page).to have_content 'You must provide a valid current password'
+      it 'locks the user account when user passes the maximum attempts threshold', :aggregate_failures do
+        user.update!(failed_attempts: User.maximum_attempts.pred)
+
+        subject
+
+        expect(current_path).to eq(new_user_session_path)
+
+        page.within '.flash-container' do
+          expect(page).to have_content('Your account is locked.')
+        end
       end
     end
 
-    it 'changes user password' do
-      page.within '.update-password' do
-        fill_in "user_current_password", with: user.password
-        fill_passwords('22233344', '22233344')
+    context 'when current password is blank' do
+      let(:user_current_password) { nil }
+
+      it_behaves_like 'user enters an incorrect current password'
+    end
+
+    context 'when current password is incorrect' do
+      let(:user_current_password) {'invalid' }
+
+      it_behaves_like 'user enters an incorrect current password'
+    end
+
+    context 'when the password reset is successful' do
+      subject do
+        page.within '.update-password' do
+          fill_in "user_current_password", with: user.password
+          fill_passwords(new_password, new_password)
+        end
       end
 
-      expect(current_path).to eq new_user_session_path
+      it 'changes the password, logs the user out and prompts them to sign in again', :aggregate_failures do
+        expect { subject }.to change { user.reload.valid_password?(new_password) }.to(true)
+        expect(current_path).to eq new_user_session_path
+
+        page.within '.flash-container' do
+          expect(page).to have_content('Password was successfully updated. Please sign in again.')
+        end
+      end
     end
   end
 
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index 80bccdfee0c..be8a6c7bdcf 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -1149,6 +1149,16 @@ RSpec.describe API::Projects do
       expect(response).to have_gitlab_http_status(:bad_request)
     end
 
+    it 'disallows creating a project with an import_url when git import source is disabled' do
+      stub_application_setting(import_sources: nil)
+
+      project_params = { import_url: 'http://example.com', path: 'path-project-Foo', name: 'Foo Project' }
+      expect { post api('/projects', user), params: project_params }
+        .not_to change {  Project.count }
+
+      expect(response).to have_gitlab_http_status(:forbidden)
+    end
+
     it 'sets a project as public' do
       project = attributes_for(:project, visibility: 'public')
 
diff --git a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb
index a20c1d78912..62b35923bcd 100644
--- a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb
+++ b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb
@@ -323,6 +323,16 @@ RSpec.shared_examples 'handle uploads authorize' do
       end
     end
 
+    context 'when id is not passed as a param' do
+      let(:params) { super().without(:id) }
+
+      it 'returns 404 status' do
+        post_authorize
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
     context 'when a user can upload a file' do
       before do
         sign_in(user)