Add latest changes from gitlab-org/security/gitlab@15-1-stable-ee

4 files changed, 40 insertions, 84 deletions

diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb
index 6d1ffc1f2e8..32d1ddf920e 100644
--- a/app/controllers/autocomplete_controller.rb
+++ b/app/controllers/autocomplete_controller.rb
@@ -5,7 +5,6 @@ class AutocompleteController < ApplicationController
 
   skip_before_action :authenticate_user!, only: [:users, :award_emojis, :merge_request_target_branches]
   before_action :check_search_rate_limit!, only: [:users, :projects]
-  before_action :authorize_admin_project, only: :deploy_keys_with_owners
 
   feature_category :users, [:users, :user]
   feature_category :projects, [:projects]
@@ -70,10 +69,6 @@ class AutocompleteController < ApplicationController
 
   private
 
-  def authorize_admin_project
-    render_403 unless Ability.allowed?(current_user, :admin_project, project)
-  end
-
   def project
     @project ||= Autocomplete::ProjectFinder
       .new(current_user, params)
diff --git a/app/models/hooks/web_hook_log.rb b/app/models/hooks/web_hook_log.rb
index 24e5f193a32..2f03b3591cf 100644
--- a/app/models/hooks/web_hook_log.rb
+++ b/app/models/hooks/web_hook_log.rb
@@ -22,7 +22,6 @@ class WebHookLog < ApplicationRecord
   validates :web_hook, presence: true
 
   before_save :obfuscate_basic_auth
-  before_save :redact_author_email
 
   def self.recent
     where('created_at >= ?', 2.days.ago.beginning_of_day)
@@ -53,10 +52,4 @@ class WebHookLog < ApplicationRecord
   def obfuscate_basic_auth
     self.url = safe_url
   end
-
-  def redact_author_email
-    return unless self.request_data.dig('commit', 'author', 'email').present?
-
-    self.request_data['commit']['author']['email'] = _('[REDACTED]')
-  end
 end
diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb
index 70e58124d21..e874df62cd7 100644
--- a/spec/controllers/autocomplete_controller_spec.rb
+++ b/spec/controllers/autocomplete_controller_spec.rb
@@ -378,74 +378,63 @@ RSpec.describe AutocompleteController do
   end
 
   context 'GET deploy_keys_with_owners' do
-    let_it_be(:public_project) { create(:project, :public) }
-    let_it_be(:user) { create(:user) }
-    let_it_be(:deploy_key) { create(:deploy_key, user: user) }
-    let_it_be(:deploy_keys_project) do
-      create(:deploy_keys_project, :write_access, project: public_project, deploy_key: deploy_key)
-    end
+    let!(:deploy_key) { create(:deploy_key, user: user) }
+    let!(:deploy_keys_project) { create(:deploy_keys_project, :write_access, project: project, deploy_key: deploy_key) }
 
     context 'unauthorized user' do
       it 'returns a not found response' do
-        get(:deploy_keys_with_owners, params: { project_id: public_project.id })
+        get(:deploy_keys_with_owners, params: { project_id: project.id })
 
         expect(response).to have_gitlab_http_status(:redirect)
       end
     end
 
-    context 'when the user is logged in' do
+    context 'when the user who can read the project is logged in' do
       before do
         sign_in(user)
       end
 
-      context 'with a non-existing project' do
+      context 'and they cannot read the project' do
         it 'returns a not found response' do
-          get(:deploy_keys_with_owners, params: { project_id: 9999 })
+          allow(Ability).to receive(:allowed?).and_call_original
+          allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(false)
+
+          get(:deploy_keys_with_owners, params: { project_id: project.id })
 
           expect(response).to have_gitlab_http_status(:not_found)
         end
       end
 
-      context 'with an existing project' do
-        context 'when user cannot admin project' do
-          it 'returns a forbidden response' do
-            get(:deploy_keys_with_owners, params: { project_id: public_project.id })
+      it 'renders the deploy key in a json payload, with its owner' do
+        get(:deploy_keys_with_owners, params: { project_id: project.id })
 
-            expect(response).to have_gitlab_http_status(:forbidden)
-          end
-        end
-
-        context 'when user can admin project' do
-          before do
-            public_project.add_maintainer(user)
-          end
+        expect(json_response.count).to eq(1)
+        expect(json_response.first['title']).to eq(deploy_key.title)
+        expect(json_response.first['owner']['id']).to eq(deploy_key.user.id)
+        expect(json_response.first['deploy_keys_projects']).to be_nil
+      end
 
-          context 'and user can read owner of key' do
-            it 'renders the deploy keys in a json payload, with owner' do
-              get(:deploy_keys_with_owners, params: { project_id: public_project.id })
+      context 'with an unknown project' do
+        it 'returns a not found response' do
+          get(:deploy_keys_with_owners, params: { project_id: 9999 })
 
-              expect(json_response.count).to eq(1)
-              expect(json_response.first['title']).to eq(deploy_key.title)
-              expect(json_response.first['owner']['id']).to eq(deploy_key.user.id)
-              expect(json_response.first['deploy_keys_projects']).to be_nil
-            end
-          end
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
 
-          context 'and user cannot read owner of key' do
-            before do
-              allow(Ability).to receive(:allowed?).and_call_original
-              allow(Ability).to receive(:allowed?).with(user, :read_user, deploy_key.user).and_return(false)
-            end
+      context 'and the user cannot read the owner of the key' do
+        before do
+          allow(Ability).to receive(:allowed?).and_call_original
+          allow(Ability).to receive(:allowed?).with(user, :read_user, deploy_key.user).and_return(false)
+        end
 
-            it 'returns a payload without owner' do
-              get(:deploy_keys_with_owners, params: { project_id: public_project.id })
+        it 'returns a payload without owner' do
+          get(:deploy_keys_with_owners, params: { project_id: project.id })
 
-              expect(json_response.count).to eq(1)
-              expect(json_response.first['title']).to eq(deploy_key.title)
-              expect(json_response.first['owner']).to be_nil
-              expect(json_response.first['deploy_keys_projects']).to be_nil
-            end
-          end
+          expect(json_response.count).to eq(1)
+          expect(json_response.first['title']).to eq(deploy_key.title)
+          expect(json_response.first['owner']).to be_nil
+          expect(json_response.first['deploy_keys_projects']).to be_nil
         end
       end
     end
diff --git a/spec/models/hooks/web_hook_log_spec.rb b/spec/models/hooks/web_hook_log_spec.rb
index 8ff8a1c3865..e1fea3318f6 100644
--- a/spec/models/hooks/web_hook_log_spec.rb
+++ b/spec/models/hooks/web_hook_log_spec.rb
@@ -30,12 +30,15 @@ RSpec.describe WebHookLog do
   end
 
   describe '#save' do
-    context 'with basic auth credentials' do
-      let(:web_hook_log) { build(:web_hook_log, url: 'http://test:123@example.com') }
+    let(:web_hook_log) { build(:web_hook_log, url: url) }
+    let(:url) { 'http://example.com' }
+
+    subject { web_hook_log.save! }
 
-      subject { web_hook_log.save! }
+    it { is_expected.to eq(true) }
 
-      it { is_expected.to eq(true) }
+    context 'with basic auth credentials' do
+      let(:url) { 'http://test:123@example.com'}
 
       it 'obfuscates the basic auth credentials' do
         subject
@@ -43,30 +46,6 @@ RSpec.describe WebHookLog do
         expect(web_hook_log.url).to eq('http://*****:*****@example.com')
       end
     end
-
-    context 'with author email' do
-      let(:author) { create(:user) }
-      let(:web_hook_log) { create(:web_hook_log, request_data: data) }
-      let(:data) do
-        {
-          commit: {
-            author: {
-              name: author.name,
-              email: author.email
-            }
-          }
-        }.deep_stringify_keys
-      end
-
-      it "redacts author's email" do
-        expect(web_hook_log.request_data['commit']).to match a_hash_including(
-          'author' => {
-            'name' => author.name,
-            'email' => _('[REDACTED]')
-          }
-        )
-      end
-    end
   end
 
   describe '.delete_batch_for' do