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

10 files changed, 124 insertions, 26 deletions

diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb
index 71794964c99..9f55d81b470 100644
--- a/app/models/hooks/web_hook.rb
+++ b/app/models/hooks/web_hook.rb
@@ -41,6 +41,7 @@ class WebHook < ApplicationRecord
   validate :no_missing_url_variables
 
   after_initialize :initialize_url_variables
+  before_validation :reset_token
 
   scope :executable, -> do
     next all unless Feature.enabled?(:web_hooks_disable_failed)
@@ -200,6 +201,10 @@ class WebHook < ApplicationRecord
 
   private
 
+  def reset_token
+    self.token = nil if url_changed? && !encrypted_token_changed?
+  end
+
   def web_hooks_disable_failed?
     self.class.web_hooks_disable_failed?(self)
   end
diff --git a/app/models/hooks/web_hook_log.rb b/app/models/hooks/web_hook_log.rb
index c32957fbef9..5af01cae0f6 100644
--- a/app/models/hooks/web_hook_log.rb
+++ b/app/models/hooks/web_hook_log.rb
@@ -48,6 +48,13 @@ class WebHookLog < ApplicationRecord
     request_data == OVERSIZE_REQUEST_DATA
   end
 
+  def request_headers
+    super unless web_hook.token?
+    super if self[:request_headers]['X-Gitlab-Token'] == _('[REDACTED]')
+
+    self[:request_headers].merge('X-Gitlab-Token' => _('[REDACTED]'))
+  end
+
   private
 
   def obfuscate_basic_auth
diff --git a/app/services/web_hooks/log_execution_service.rb b/app/services/web_hooks/log_execution_service.rb
index 1a40c877bda..448bb7d4097 100644
--- a/app/services/web_hooks/log_execution_service.rb
+++ b/app/services/web_hooks/log_execution_service.rb
@@ -24,6 +24,8 @@ module WebHooks
     private
 
     def log_execution
+      log_data[:request_headers]['X-Gitlab-Token'] = _('[REDACTED]') if hook.token?
+
       WebHookLog.create!(web_hook: hook, **log_data)
     end
 
diff --git a/app/views/projects/tags/_release_link.html.haml b/app/views/projects/tags/_release_link.html.haml
index c942d122a58..6c79b13f438 100644
--- a/app/views/projects/tags/_release_link.html.haml
+++ b/app/views/projects/tags/_release_link.html.haml
@@ -1,4 +1,5 @@
-.gl-text-secondary
-  = sprite_icon("rocket", size: 12)
-  = _("Release")
-  = link_to release.name, project_release_path(project, release), class: "gl-text-blue-600!"
+- if can?(current_user, :read_release, release)
+  .gl-text-secondary
+    = sprite_icon("rocket", size: 12)
+    = _("Release")
+    = link_to release.name, project_release_path(project, release), class: "gl-text-blue-600!"
diff --git a/app/views/projects/tags/show.html.haml b/app/views/projects/tags/show.html.haml
index cb7751ecf2e..a9c3309e38c 100644
--- a/app/views/projects/tags/show.html.haml
+++ b/app/views/projects/tags/show.html.haml
@@ -57,12 +57,13 @@
     %pre.wrap{ data: { qa_selector: 'tag_message_content' } }
       = strip_signature(@tag.message)
 
-.gl-mb-3.gl-mt-3
-  - if @release&.description.present?
-    .description.md{ data: { qa_selector: 'tag_release_notes_content' } }
-      = markdown_field(@release, :description)
-  - else
-    = s_('TagsPage|This tag has no release notes.')
+- if can?(current_user, :read_release, @release)
+  .gl-mb-3.gl-mt-3
+    - if @release&.description.present?
+      .description.md{ data: { qa_selector: 'tag_release_notes_content' } }
+        = markdown_field(@release, :description)
+    - else
+      = s_('TagsPage|This tag has no release notes.')
 
 - if can?(current_user, :admin_tag, @project)
   .js-delete-tag-modal
diff --git a/spec/features/tags/developer_views_tags_spec.rb b/spec/features/tags/developer_views_tags_spec.rb
index 57e1f7da04e..e2399dd9978 100644
--- a/spec/features/tags/developer_views_tags_spec.rb
+++ b/spec/features/tags/developer_views_tags_spec.rb
@@ -53,6 +53,8 @@ RSpec.describe 'Developer views tags' do
     end
 
     it 'views a specific tag page' do
+      create(:release, project: project, tag: 'v1.0.0', name: 'v1.0.0', description: nil)
+
       click_on 'v1.0.0'
 
       expect(page).to have_current_path(
diff --git a/spec/models/hooks/web_hook_log_spec.rb b/spec/models/hooks/web_hook_log_spec.rb
index 3441dfda7d6..e65330d2533 100644
--- a/spec/models/hooks/web_hook_log_spec.rb
+++ b/spec/models/hooks/web_hook_log_spec.rb
@@ -185,4 +185,22 @@ RSpec.describe WebHookLog do
       it { expect(web_hook_log.internal_error?).to be_truthy }
     end
   end
+
+  describe '#request_headers' do
+    let(:hook) { build(:project_hook, :token) }
+    let(:web_hook_log) { build(:web_hook_log, request_headers: request_headers) }
+    let(:expected_headers) { { 'X-Gitlab-Token' => _('[REDACTED]') } }
+
+    context 'with redacted headers token' do
+      let(:request_headers) { { 'X-Gitlab-Token' => _('[REDACTED]') } }
+
+      it { expect(web_hook_log.request_headers).to eq(expected_headers) }
+    end
+
+    context 'with exposed headers token' do
+      let(:request_headers) { { 'X-Gitlab-Token' => hook.token } }
+
+      it { expect(web_hook_log.request_headers).to eq(expected_headers) }
+    end
+  end
 end
diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb
index da8c10b67a6..5fbde9eafa0 100644
--- a/spec/models/hooks/web_hook_spec.rb
+++ b/spec/models/hooks/web_hook_spec.rb
@@ -131,6 +131,36 @@ RSpec.describe WebHook do
         expect(hook.push_events_branch_filter).to eq('')
       end
     end
+
+    describe 'before_validation :reset_token' do
+      subject(:hook) { build_stubbed(:project_hook, :token, project: project) }
+
+      it 'resets token if url changed' do
+        hook.url = 'https://webhook.example.com/new-hook'
+
+        expect(hook).to be_valid
+        expect(hook.token).to be_nil
+      end
+
+      it 'does not reset token if new url is set together with the same token' do
+        hook.url = 'https://webhook.example.com/new-hook'
+        current_token = hook.token
+        hook.token = current_token
+
+        expect(hook).to be_valid
+        expect(hook.token).to eq(current_token)
+        expect(hook.url).to eq('https://webhook.example.com/new-hook')
+      end
+
+      it 'does not reset token if new url is set together with a new token' do
+        hook.url = 'https://webhook.example.com/new-hook'
+        hook.token = 'token'
+
+        expect(hook).to be_valid
+        expect(hook.token).to eq('token')
+        expect(hook.url).to eq('https://webhook.example.com/new-hook')
+      end
+    end
   end
 
   describe 'encrypted attributes' do
@@ -242,7 +272,7 @@ RSpec.describe WebHook do
   end
 
   describe '#executable?' do
-    let(:web_hook) { create(:project_hook, project: project) }
+    let_it_be(:web_hook) { create(:project_hook, project: project) }
 
     where(:recent_failures, :not_until, :executable) do
       [
diff --git a/spec/services/web_hooks/log_execution_service_spec.rb b/spec/services/web_hooks/log_execution_service_spec.rb
index 1b8ff9f2a05..fd97d01fa9f 100644
--- a/spec/services/web_hooks/log_execution_service_spec.rb
+++ b/spec/services/web_hooks/log_execution_service_spec.rb
@@ -11,14 +11,15 @@ RSpec.describe WebHooks::LogExecutionService do
       travel_to(Time.current) { example.run }
     end
 
-    let_it_be_with_reload(:project_hook) { create(:project_hook) }
+    let_it_be_with_reload(:project_hook) { create(:project_hook, :token) }
 
     let(:response_category) { :ok }
+    let(:request_headers) { { 'Header' => 'header value' } }
     let(:data) do
       {
         trigger: 'trigger_name',
         url: 'https://example.com',
-        request_headers: { 'Header' => 'header value' },
+        request_headers: request_headers,
         request_data: { 'Request Data' => 'request data value' },
         response_body: 'Response body',
         response_status: '200',
@@ -163,5 +164,15 @@ RSpec.describe WebHooks::LogExecutionService do
         service.execute
       end
     end
+
+    context 'with X-Gitlab-Token' do
+      let(:request_headers) { { 'X-Gitlab-Token' => project_hook.token } }
+
+      it 'redacts the token' do
+        service.execute
+
+        expect(WebHookLog.recent.first.request_headers).to include('X-Gitlab-Token' => '[REDACTED]')
+      end
+    end
   end
 end
diff --git a/spec/support/shared_examples/features/user_views_tag_shared_examples.rb b/spec/support/shared_examples/features/user_views_tag_shared_examples.rb
index 989de1dbfbb..702964a2610 100644
--- a/spec/support/shared_examples/features/user_views_tag_shared_examples.rb
+++ b/spec/support/shared_examples/features/user_views_tag_shared_examples.rb
@@ -2,33 +2,54 @@
 
 RSpec.shared_examples 'user views tag' do
   context 'when user views with the tag' do
-    let(:project) { create(:project, :repository) }
+    let(:project) { create(:project, :repository, :public) }
     let(:user) { create(:user) }
     let(:tag_name) { "stable" }
-    let!(:release) { create(:release, project: project, tag: tag_name, name: "ReleaseName") }
+    let(:release_name) { 'ReleaseName' }
+    let(:release_notes) { 'Release notes' }
+    let!(:release) do
+      create(:release, project: project, tag: tag_name, name: release_name, description: release_notes)
+    end
 
     before do
-      project.add_developer(user)
       project.repository.add_tag(user, tag_name, project.default_branch_or_main)
-
       sign_in(user)
     end
 
-    shared_examples 'shows tag' do
-      it do
-        visit tag_page
+    context 'and user is authorized to read release' do
+      before do
+        project.add_developer(user)
+      end
+
+      shared_examples 'shows tag' do
+        it do
+          visit tag_page
+
+          expect(page).to have_content tag_name
+          expect(page).to have_link(release_name, href: project_release_path(project, release))
+        end
+      end
 
-        expect(page).to have_content tag_name
-        expect(page).to have_link("ReleaseName", href: project_release_path(project, release))
+      it_behaves_like 'shows tag'
+
+      context 'when tag name contains a slash' do
+        let(:tag_name) { "stable/v0.1" }
+
+        it_behaves_like 'shows tag'
       end
     end
 
-    it_behaves_like 'shows tag'
+    context 'and user is not authorized to read release' do
+      before do
+        project.project_feature.update!(releases_access_level: Featurable::PRIVATE)
+      end
 
-    context 'when tag name contains a slash' do
-      let(:tag_name) { "stable/v0.1" }
+      it 'hides release link and notes', :aggregate_failures do
+        visit tag_page
 
-      it_behaves_like 'shows tag'
+        expect(page).not_to have_link(release_name, href: project_release_path(project, release))
+        expect(page).not_to have_text(release_notes)
+      end
     end
   end
 end