Add latest changes from gitlab-org/security/gitlab@16-2-stable-ee

4 files changed, 79 insertions, 20 deletions

diff --git a/app/controllers/groups/labels_controller.rb b/app/controllers/groups/labels_controller.rb
index 2d821676677..61f9333bf95 100644
--- a/app/controllers/groups/labels_controller.rb
+++ b/app/controllers/groups/labels_controller.rb
@@ -4,7 +4,8 @@ class Groups::LabelsController < Groups::ApplicationController
   include ToggleSubscriptionAction
 
   before_action :label, only: [:edit, :update, :destroy]
-  before_action :authorize_admin_labels!, only: [:new, :create, :edit, :update, :destroy]
+  before_action :authorize_group_for_admin_labels!, only: [:new, :create, :edit, :update, :destroy]
+  before_action :authorize_label_for_admin_label!, only: [:edit, :update, :destroy]
   before_action :save_previous_label_path, only: [:edit]
 
   respond_to :html
@@ -70,10 +71,14 @@ class Groups::LabelsController < Groups::ApplicationController
 
   protected
 
-  def authorize_admin_labels!
+  def authorize_group_for_admin_labels!
     return render_404 unless can?(current_user, :admin_label, @group)
   end
 
+  def authorize_label_for_admin_label!
+    return render_404 unless can?(current_user, :admin_label, @label)
+  end
+
   def authorize_read_labels!
     return render_404 unless can?(current_user, :read_label, @group)
   end
diff --git a/app/services/error_tracking/list_projects_service.rb b/app/services/error_tracking/list_projects_service.rb
index 35a8179d54d..1539e24df9d 100644
--- a/app/services/error_tracking/list_projects_service.rb
+++ b/app/services/error_tracking/list_projects_service.rb
@@ -20,22 +20,20 @@ module ErrorTracking
 
     def project_error_tracking_setting
       (super || project.build_error_tracking_setting).tap do |setting|
-        url_changed = !setting.api_url&.start_with?(params[:api_host])
-
         setting.api_url = ErrorTracking::ProjectErrorTrackingSetting.build_api_url_from(
           api_host: params[:api_host],
           organization_slug: 'org',
           project_slug: 'proj'
         )
 
-        setting.token = token(setting, url_changed)
+        setting.token = token(setting)
         setting.enabled = true
       end
     end
     strong_memoize_attr :project_error_tracking_setting
 
-    def token(setting, url_changed)
-      return if url_changed && masked_token?
+    def token(setting)
+      return if setting.api_url_changed? && masked_token?
 
       # Use param token if not masked, otherwise use database token
       return params[:token] unless masked_token?
diff --git a/spec/controllers/groups/labels_controller_spec.rb b/spec/controllers/groups/labels_controller_spec.rb
index 916b2cf10dd..3ade85eee9d 100644
--- a/spec/controllers/groups/labels_controller_spec.rb
+++ b/spec/controllers/groups/labels_controller_spec.rb
@@ -5,6 +5,7 @@ require 'spec_helper'
 RSpec.describe Groups::LabelsController, feature_category: :team_planning do
   let_it_be(:group) { create(:group) }
   let_it_be(:user)  { create(:user) }
+  let_it_be(:another_user) { create(:user) }
   let_it_be(:project) { create(:project, namespace: group) }
 
   before do
@@ -66,6 +67,46 @@ RSpec.describe Groups::LabelsController, feature_category: :team_planning do
     end
   end
 
+  shared_examples 'when current_user does not have ability to modify the label' do
+    before do
+      sign_in(another_user)
+    end
+
+    it 'responds with status 404' do
+      group_request
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+
+    # No matter what permissions you have in a sub-group, you need the proper
+    # permissions in the group in order to modify a group label
+    # See https://gitlab.com/gitlab-org/gitlab/-/issues/387531
+    context 'when trying to edit a parent group label from inside a subgroup' do
+      it 'responds with status 404' do
+        sub_group.add_owner(another_user)
+        sub_group_request
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+  end
+
+  describe 'GET #edit' do
+    let_it_be(:label) { create(:group_label, group: group) }
+
+    it 'shows the edit page' do
+      get :edit, params: { group_id: group.to_param, id: label.to_param }
+
+      expect(response).to have_gitlab_http_status(:ok)
+    end
+
+    it_behaves_like 'when current_user does not have ability to modify the label' do
+      let_it_be(:sub_group) { create(:group, parent: group) }
+      let(:group_request) { get :edit, params: { group_id: group.to_param, id: label.to_param } }
+      let(:sub_group_request) { get :edit, params: { group_id: sub_group.to_param, id: label.to_param } }
+    end
+  end
+
   describe 'POST #toggle_subscription' do
     it 'allows user to toggle subscription on group labels' do
       label = create(:group_label, group: group)
@@ -99,19 +140,20 @@ RSpec.describe Groups::LabelsController, feature_category: :team_planning do
       end
     end
 
-    context 'when current_user does not have ability to destroy the label' do
-      let(:another_user) { create(:user) }
-
-      before do
-        sign_in(another_user)
-      end
-
-      it 'responds with status 404' do
-        label = create(:group_label, group: group)
-        delete :destroy, params: { group_id: group.to_param, id: label.to_param }
+    it_behaves_like 'when current_user does not have ability to modify the label' do
+      let_it_be(:label) { create(:group_label, group: group) }
+      let_it_be(:sub_group) { create(:group, parent: group) }
+      let(:group_request) { delete :destroy, params: { group_id: group.to_param, id: label.to_param } }
+      let(:sub_group_request) { delete :destroy, params: { group_id: sub_group.to_param, id: label.to_param } }
+    end
+  end
 
-        expect(response).to have_gitlab_http_status(:not_found)
-      end
+  describe 'PUT #update' do
+    it_behaves_like 'when current_user does not have ability to modify the label' do
+      let_it_be(:label) { create(:group_label, group: group) }
+      let_it_be(:sub_group) { create(:group, parent: group) }
+      let(:group_request) { put :update, params: { group_id: group.to_param, id: label.to_param, label: { title: 'Test' } } }
+      let(:sub_group_request) { put :update, params: { group_id: sub_group.to_param, id: label.to_param, label: { title: 'Test' } } }
     end
   end
 end
diff --git a/spec/services/error_tracking/list_projects_service_spec.rb b/spec/services/error_tracking/list_projects_service_spec.rb
index 8408adcc21d..d91808edc8d 100644
--- a/spec/services/error_tracking/list_projects_service_spec.rb
+++ b/spec/services/error_tracking/list_projects_service_spec.rb
@@ -6,7 +6,7 @@ RSpec.describe ErrorTracking::ListProjectsService, feature_category: :integratio
   let_it_be(:user) { create(:user) }
   let_it_be(:project, reload: true) { create(:project) }
 
-  let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' }
+  let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/org/proj/' }
   let(:token) { 'test-token' }
   let(:new_api_host) { 'https://gitlab.com/' }
   let(:new_token) { 'new-token' }
@@ -66,6 +66,20 @@ RSpec.describe ErrorTracking::ListProjectsService, feature_category: :integratio
           end
         end
 
+        context 'with the similar api host' do
+          let(:api_host) { 'https://sentrytest.gitlab.co' }
+
+          it 'returns an error' do
+            expect(result[:message]).to start_with('Token is a required field')
+            expect(error_tracking_setting).not_to be_valid
+            expect(error_tracking_setting).not_to receive(:list_sentry_projects)
+          end
+
+          it 'resets the token' do
+            expect { subject.execute }.to change { error_tracking_setting.token }.from(token).to(nil)
+          end
+        end
+
         context 'with a new api host' do
           let(:api_host) { new_api_host }