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

15 files changed, 454 insertions, 123 deletions

diff --git a/app/graphql/types/permission_types/base_permission_type.rb b/app/graphql/types/permission_types/base_permission_type.rb
index 3c0e68bdaf2..3a8519e8196 100644
--- a/app/graphql/types/permission_types/base_permission_type.rb
+++ b/app/graphql/types/permission_types/base_permission_type.rb
@@ -30,7 +30,7 @@ module Types
       def self.define_field_resolver_method(ability)
         unless respond_to?(ability)
           define_method ability.to_sym do |*args|
-            Ability.allowed?(context[:current_user], ability, object, args.to_h)
+            Ability.allowed?(context[:current_user], ability, object, **args.to_h)
           end
         end
       end
diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb
index f48157cb65a..6cabdf21483 100644
--- a/app/helpers/groups_helper.rb
+++ b/app/helpers/groups_helper.rb
@@ -207,6 +207,17 @@ module GroupsHelper
     new_group_custom_emoji_path(group)
   end
 
+  def access_level_roles_user_can_assign(group)
+    return {} unless current_user
+    return group.access_level_roles if current_user.can_admin_all_resources?
+
+    max_access_level = group.highest_group_member(current_user)&.access_level
+
+    return {} unless max_access_level
+
+    GroupMember.access_level_roles.select { |_k, v| v <= max_access_level }
+  end
+
   private
 
   def group_title_link(group, hidable: false, show_avatar: false, for_dropdown: false)
diff --git a/app/models/ability.rb b/app/models/ability.rb
index b8433191d84..9ae96c35d4f 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -70,13 +70,13 @@ class Ability
       end
     end
 
-    def allowed?(user, ability, subject = :global, opts = {})
+    def allowed?(user, ability, subject = :global, **opts)
       if subject.is_a?(Hash)
         opts = subject
         subject = :global
       end
 
-      policy = policy_for(user, subject)
+      policy = policy_for(user, subject, **opts.slice(:cache))
 
       before_check(policy, ability.to_sym, user, subject, opts)
 
@@ -100,8 +100,14 @@ class Ability
       # See Support::AbilityCheck and Support::PermissionsCheck.
     end
 
-    def policy_for(user, subject = :global)
-      DeclarativePolicy.policy_for(user, subject, cache: ::Gitlab::SafeRequestStore.storage)
+    # We cache in the request store by default. This can lead to unexpected
+    # results if abilities are re-checked after objects are modified and the
+    # check depends on the modified attributes. In such cases, you should pass
+    # `cache: false` for the second check to ensure all rules get re-evaluated.
+    def policy_for(user, subject = :global, cache: true)
+      policy_cache = cache ? ::Gitlab::SafeRequestStore.storage : {}
+
+      DeclarativePolicy.policy_for(user, subject, cache: policy_cache)
     end
 
     # This method is something of a band-aid over the problem. The problem is
diff --git a/app/models/user.rb b/app/models/user.rb
index 25f22563136..4a66192e9d8 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1307,9 +1307,11 @@ class User < MainClusterwide::ApplicationRecord
     several_namespaces? || admin
   end
 
-  def can?(action, subject = :global)
-    Ability.allowed?(self, action, subject)
+  # rubocop: disable Style/ArgumentsForwarding
+  def can?(action, subject = :global, **opts)
+    Ability.allowed?(self, action, subject, **opts)
   end
+  # rubocop: enable Style/ArgumentsForwarding
 
   def confirm_deletion_with_password?
     !password_automatically_set? && allow_password_authentication?
diff --git a/app/policies/ci/pipeline_schedule_policy.rb b/app/policies/ci/pipeline_schedule_policy.rb
index cbc60c4a30a..9e558cd91c1 100644
--- a/app/policies/ci/pipeline_schedule_policy.rb
+++ b/app/policies/ci/pipeline_schedule_policy.rb
@@ -25,7 +25,7 @@ module Ci
 
     rule { can?(:create_pipeline) }.enable :play_pipeline_schedule
 
-    rule { can?(:admin_pipeline) | (can?(:update_build) & owner_of_schedule) }.policy do
+    rule { can?(:admin_pipeline) | (owner_of_schedule & can?(:update_build)) }.policy do
       enable :admin_pipeline_schedule
       enable :read_pipeline_schedule_variables
     end
@@ -45,6 +45,7 @@ module Ci
     rule { protected_ref }.policy do
       prevent :play_pipeline_schedule
       prevent :create_pipeline_schedule
+      prevent :update_pipeline_schedule
     end
 
     private
diff --git a/app/services/ci/pipeline_schedules/base_save_service.rb b/app/services/ci/pipeline_schedules/base_save_service.rb
index 45d70e5a65d..e6f633498e9 100644
--- a/app/services/ci/pipeline_schedules/base_save_service.rb
+++ b/app/services/ci/pipeline_schedules/base_save_service.rb
@@ -23,7 +23,11 @@ module Ci
       attr_reader :project, :user, :params, :schedule
 
       def allowed_to_save?
-        user.can?(self.class::AUTHORIZE, schedule)
+        # Disable cache because the same ability may already have been checked
+        # for the same records with different attributes. For example, we do not
+        # want an unauthorized user to change an unprotected ref to a protected
+        # ref.
+        user.can?(self.class::AUTHORIZE, schedule, cache: false)
       end
 
       def forbidden_to_save
diff --git a/app/services/ci/pipeline_schedules/update_service.rb b/app/services/ci/pipeline_schedules/update_service.rb
index 2fd1173ecce..76b2121c4e1 100644
--- a/app/services/ci/pipeline_schedules/update_service.rb
+++ b/app/services/ci/pipeline_schedules/update_service.rb
@@ -12,6 +12,12 @@ module Ci
         @params = params
       end
 
+      def execute
+        return forbidden_to_save unless allowed_to_save?
+
+        super
+      end
+
       private
 
       def authorize_message
diff --git a/app/services/members/creator_service.rb b/app/services/members/creator_service.rb
index 22d8b30db18..d7bf073d8e9 100644
--- a/app/services/members/creator_service.rb
+++ b/app/services/members/creator_service.rb
@@ -156,12 +156,13 @@ module Members
     end
 
     def commit_member
-      if can_commit_member?
-        assign_member_attributes
-        commit_changes
-      else
-        add_commit_error
-      end
+      return add_commit_error unless can_commit_member?
+
+      assign_member_attributes
+
+      return add_member_role_error if member_role_too_high?
+
+      commit_changes
     end
 
     def can_commit_member?
@@ -175,6 +176,11 @@ module Members
       end
     end
 
+    # overridden in Members::Groups::CreatorService
+    def member_role_too_high?
+      false
+    end
+
     def can_create_new_member?
       raise NotImplementedError
     end
@@ -240,6 +246,12 @@ module Members
       member.errors.add(:base, msg)
     end
 
+    def add_member_role_error
+      msg = _("the member access level can't be higher than the current user's one")
+
+      member.errors.add(:base, msg)
+    end
+
     def find_or_build_member
       @member = builder.new(source, invitee, existing_members).execute
     end
diff --git a/app/views/groups/_invite_members_modal.html.haml b/app/views/groups/_invite_members_modal.html.haml
index d53190948fd..c60e7a78b1d 100644
--- a/app/views/groups/_invite_members_modal.html.haml
+++ b/app/views/groups/_invite_members_modal.html.haml
@@ -1,7 +1,7 @@
 - return unless can_admin_group_member?(group)
 
 .js-invite-members-modal{ data: { is_project: 'false',
-  access_levels: group.access_level_roles.to_json,
+  access_levels: access_level_roles_user_can_assign(group).to_json,
   reload_page_on_submit: current_path?('group_members#index').to_s,
   help_link: help_page_url('user/permissions'),
   is_signup_enabled: signup_enabled?.to_s,
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index f95890d9ccf..234a9601a4c 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -58301,6 +58301,9 @@ msgstr ""
 msgid "the following issues"
 msgstr ""
 
+msgid "the member access level can't be higher than the current user's one"
+msgstr ""
+
 msgid "the wiki"
 msgstr ""
 
diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
index cd828c956a0..7cd4f43d4da 100644
--- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb
+++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
@@ -65,6 +65,10 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu
           create(:protected_branch, *branch_access_levels, name: ref_name, project: project)
         end
 
+        after do
+          ProtectedBranches::CacheService.new(project).refresh
+        end
+
         it { expect { go }.to try(maintainer_accessible, :maintainer).of(project) }
         it { expect { go }.to try(developer_accessible, :developer).of(project) }
       end
diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb
index f5cadfc2761..93191cc956b 100644
--- a/spec/helpers/groups_helper_spec.rb
+++ b/spec/helpers/groups_helper_spec.rb
@@ -604,4 +604,67 @@ RSpec.describe GroupsHelper, feature_category: :groups_and_projects do
       end
     end
   end
+
+  describe '#access_level_roles_user_can_assign' do
+    subject { helper.access_level_roles_user_can_assign(group) }
+
+    let_it_be(:group) { create(:group) }
+    let_it_be_with_reload(:user) { create(:user) }
+
+    context 'when user is provided' do
+      before do
+        allow(helper).to receive(:current_user).and_return(user)
+      end
+
+      context 'when a user is a group member' do
+        before do
+          group.add_developer(user)
+        end
+
+        it 'returns only the roles the provided user can assign' do
+          expect(subject).to eq(
+            {
+              'Guest' => 10,
+              'Reporter' => 20,
+              'Developer' => 30
+            }
+          )
+        end
+      end
+
+      context 'when a user is an admin', :enable_admin_mode do
+        before do
+          user.update!(admin: true)
+        end
+
+        it 'returns all roles' do
+          expect(subject).to eq(
+            {
+              'Guest' => 10,
+              'Reporter' => 20,
+              'Developer' => 30,
+              'Maintainer' => 40,
+              'Owner' => 50
+            }
+          )
+        end
+      end
+
+      context 'when a user is not a group member' do
+        it 'returns the empty array' do
+          expect(subject).to be_empty
+        end
+      end
+
+      context 'when user is not provided' do
+        before do
+          allow(helper).to receive(:current_user).and_return(nil)
+        end
+
+        it 'returns the empty array' do
+          expect(subject).to be_empty
+        end
+      end
+    end
+  end
 end
diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb
index a808cb1c823..1f0e074d90b 100644
--- a/spec/models/ability_spec.rb
+++ b/spec/models/ability_spec.rb
@@ -3,9 +3,42 @@
 require 'spec_helper'
 
 RSpec.describe Ability do
-  context 'using a nil subject' do
-    it 'has no permissions' do
-      expect(described_class.policy_for(nil, nil)).to be_banned
+  describe '#policy_for' do
+    subject(:policy) { described_class.policy_for(user, subject, **options) }
+
+    let(:user) { User.new }
+    let(:subject) { :global }
+    let(:options) { {} }
+
+    context 'using a nil subject' do
+      let(:user) { nil }
+      let(:subject) { nil }
+
+      it 'has no permissions' do
+        expect(policy).to be_banned
+      end
+    end
+
+    context 'with request store', :request_store do
+      before do
+        ::Gitlab::SafeRequestStore.write(:example, :value) # make request store different from {}
+      end
+
+      it 'caches in the request store' do
+        expect(DeclarativePolicy).to receive(:policy_for).with(user, subject, cache: ::Gitlab::SafeRequestStore.storage)
+
+        policy
+      end
+
+      context 'when cache: false' do
+        let(:options) { { cache: false } }
+
+        it 'uses a fresh cache each time' do
+          expect(DeclarativePolicy).to receive(:policy_for).with(user, subject, cache: {})
+
+          policy
+        end
+      end
     end
   end
 
diff --git a/spec/policies/ci/pipeline_schedule_policy_spec.rb b/spec/policies/ci/pipeline_schedule_policy_spec.rb
index 8fc5c6ca296..1d353b9a35e 100644
--- a/spec/policies/ci/pipeline_schedule_policy_spec.rb
+++ b/spec/policies/ci/pipeline_schedule_policy_spec.rb
@@ -6,6 +6,7 @@ RSpec.describe Ci::PipelineSchedulePolicy, :models, :clean_gitlab_redis_cache, f
   using RSpec::Parameterized::TableSyntax
 
   let_it_be(:user) { create(:user) }
+  let_it_be(:other_user) { create(:user) }
   let_it_be_with_reload(:project) { create(:project, :repository, create_tag: tag_ref_name) }
   let_it_be_with_reload(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project) }
   let_it_be(:tag_ref_name) { "v1.0.0" }
@@ -17,89 +18,180 @@ RSpec.describe Ci::PipelineSchedulePolicy, :models, :clean_gitlab_redis_cache, f
   describe 'rules' do
     describe 'rules for protected ref' do
       context 'for branch' do
+        subject(:policy) { described_class.new(user, pipeline_schedule) }
+
         %w[refs/heads/master master].each do |branch_ref|
           context "with #{branch_ref}" do
             let_it_be(:branch_ref_name) { "master" }
-            let_it_be(:branch_pipeline_schedule) do
+            let_it_be(:pipeline_schedule) do
               create(:ci_pipeline_schedule, :nightly, project: project, ref: branch_ref)
             end
 
-            where(:push_access_level, :merge_access_level, :project_role, :accessible) do
-              :no_one_can_push | :no_one_can_merge | :owner      | :be_disallowed
-              :no_one_can_push | :no_one_can_merge | :maintainer | :be_disallowed
-              :no_one_can_push | :no_one_can_merge | :developer  | :be_disallowed
-              :no_one_can_push | :no_one_can_merge | :reporter   | :be_disallowed
-              :no_one_can_push | :no_one_can_merge | :guest      | :be_disallowed
-
-              :maintainers_can_push | :no_one_can_merge | :owner      | :be_allowed
-              :maintainers_can_push | :no_one_can_merge | :maintainer | :be_allowed
-              :maintainers_can_push | :no_one_can_merge | :developer  | :be_disallowed
-              :maintainers_can_push | :no_one_can_merge | :reporter   | :be_disallowed
-              :maintainers_can_push | :no_one_can_merge | :guest      | :be_disallowed
-
-              :developers_can_push | :no_one_can_merge |  :owner      | :be_allowed
-              :developers_can_push | :no_one_can_merge |  :maintainer | :be_allowed
-              :developers_can_push | :no_one_can_merge |  :developer  | :be_allowed
-              :developers_can_push | :no_one_can_merge |  :reporter   | :be_disallowed
-              :developers_can_push | :no_one_can_merge |  :guest      | :be_disallowed
-
-              :no_one_can_push | :maintainers_can_merge | :owner      | :be_allowed
-              :no_one_can_push | :maintainers_can_merge | :maintainer | :be_allowed
-              :no_one_can_push | :maintainers_can_merge | :developer  | :be_disallowed
-              :no_one_can_push | :maintainers_can_merge | :reporter   | :be_disallowed
-              :no_one_can_push | :maintainers_can_merge | :guest      | :be_disallowed
-
-              :maintainers_can_push | :maintainers_can_merge | :owner      | :be_allowed
-              :maintainers_can_push | :maintainers_can_merge | :maintainer | :be_allowed
-              :maintainers_can_push | :maintainers_can_merge | :developer  | :be_disallowed
-              :maintainers_can_push | :maintainers_can_merge | :reporter   | :be_disallowed
-              :maintainers_can_push | :maintainers_can_merge | :guest      | :be_disallowed
-
-              :developers_can_push | :maintainers_can_merge |  :owner      | :be_allowed
-              :developers_can_push | :maintainers_can_merge |  :maintainer | :be_allowed
-              :developers_can_push | :maintainers_can_merge |  :developer  | :be_allowed
-              :developers_can_push | :maintainers_can_merge |  :reporter   | :be_disallowed
-              :developers_can_push | :maintainers_can_merge |  :guest      | :be_disallowed
-
-              :no_one_can_push | :developers_can_merge | :owner      | :be_allowed
-              :no_one_can_push | :developers_can_merge | :maintainer | :be_allowed
-              :no_one_can_push | :developers_can_merge | :developer  | :be_allowed
-              :no_one_can_push | :developers_can_merge | :reporter   | :be_disallowed
-              :no_one_can_push | :developers_can_merge | :guest      | :be_disallowed
-
-              :maintainers_can_push | :developers_can_merge | :owner      | :be_allowed
-              :maintainers_can_push | :developers_can_merge | :maintainer | :be_allowed
-              :maintainers_can_push | :developers_can_merge | :developer  | :be_allowed
-              :maintainers_can_push | :developers_can_merge | :reporter   | :be_disallowed
-              :maintainers_can_push | :developers_can_merge | :guest      | :be_disallowed
-
-              :developers_can_push | :developers_can_merge |  :owner      | :be_allowed
-              :developers_can_push | :developers_can_merge |  :maintainer | :be_allowed
-              :developers_can_push | :developers_can_merge |  :developer  | :be_allowed
-              :developers_can_push | :developers_can_merge |  :reporter   | :be_disallowed
-              :developers_can_push | :developers_can_merge |  :guest      | :be_disallowed
+            shared_examples_for 'allowed by those who can update the branch' do
+              where(:push_access_level, :merge_access_level, :project_role, :accessible) do
+                :no_one_can_push | :no_one_can_merge | :owner      | :be_disallowed
+                :no_one_can_push | :no_one_can_merge | :maintainer | :be_disallowed
+                :no_one_can_push | :no_one_can_merge | :developer  | :be_disallowed
+                :no_one_can_push | :no_one_can_merge | :reporter   | :be_disallowed
+                :no_one_can_push | :no_one_can_merge | :guest      | :be_disallowed
+
+                :maintainers_can_push | :no_one_can_merge | :owner      | :be_allowed
+                :maintainers_can_push | :no_one_can_merge | :maintainer | :be_allowed
+                :maintainers_can_push | :no_one_can_merge | :developer  | :be_disallowed
+                :maintainers_can_push | :no_one_can_merge | :reporter   | :be_disallowed
+                :maintainers_can_push | :no_one_can_merge | :guest      | :be_disallowed
+
+                :developers_can_push | :no_one_can_merge |  :owner      | :be_allowed
+                :developers_can_push | :no_one_can_merge |  :maintainer | :be_allowed
+                :developers_can_push | :no_one_can_merge |  :developer  | :be_allowed
+                :developers_can_push | :no_one_can_merge |  :reporter   | :be_disallowed
+                :developers_can_push | :no_one_can_merge |  :guest      | :be_disallowed
+
+                :no_one_can_push | :maintainers_can_merge | :owner      | :be_allowed
+                :no_one_can_push | :maintainers_can_merge | :maintainer | :be_allowed
+                :no_one_can_push | :maintainers_can_merge | :developer  | :be_disallowed
+                :no_one_can_push | :maintainers_can_merge | :reporter   | :be_disallowed
+                :no_one_can_push | :maintainers_can_merge | :guest      | :be_disallowed
+
+                :maintainers_can_push | :maintainers_can_merge | :owner      | :be_allowed
+                :maintainers_can_push | :maintainers_can_merge | :maintainer | :be_allowed
+                :maintainers_can_push | :maintainers_can_merge | :developer  | :be_disallowed
+                :maintainers_can_push | :maintainers_can_merge | :reporter   | :be_disallowed
+                :maintainers_can_push | :maintainers_can_merge | :guest      | :be_disallowed
+
+                :developers_can_push | :maintainers_can_merge |  :owner      | :be_allowed
+                :developers_can_push | :maintainers_can_merge |  :maintainer | :be_allowed
+                :developers_can_push | :maintainers_can_merge |  :developer  | :be_allowed
+                :developers_can_push | :maintainers_can_merge |  :reporter   | :be_disallowed
+                :developers_can_push | :maintainers_can_merge |  :guest      | :be_disallowed
+
+                :no_one_can_push | :developers_can_merge | :owner      | :be_allowed
+                :no_one_can_push | :developers_can_merge | :maintainer | :be_allowed
+                :no_one_can_push | :developers_can_merge | :developer  | :be_allowed
+                :no_one_can_push | :developers_can_merge | :reporter   | :be_disallowed
+                :no_one_can_push | :developers_can_merge | :guest      | :be_disallowed
+
+                :maintainers_can_push | :developers_can_merge | :owner      | :be_allowed
+                :maintainers_can_push | :developers_can_merge | :maintainer | :be_allowed
+                :maintainers_can_push | :developers_can_merge | :developer  | :be_allowed
+                :maintainers_can_push | :developers_can_merge | :reporter   | :be_disallowed
+                :maintainers_can_push | :developers_can_merge | :guest      | :be_disallowed
+
+                :developers_can_push | :developers_can_merge |  :owner      | :be_allowed
+                :developers_can_push | :developers_can_merge |  :maintainer | :be_allowed
+                :developers_can_push | :developers_can_merge |  :developer  | :be_allowed
+                :developers_can_push | :developers_can_merge |  :reporter   | :be_disallowed
+                :developers_can_push | :developers_can_merge |  :guest      | :be_disallowed
+              end
+
+              with_them do
+                before do
+                  create(:protected_branch, push_access_level, merge_access_level, name: branch_ref_name,
+                    project: project)
+                  project.add_role(user, project_role)
+                end
+
+                it { expect(policy).to try(accessible, :create_pipeline_schedule) }
+              end
             end
 
-            with_them do
-              before do
-                create(:protected_branch, push_access_level, merge_access_level, name: branch_ref_name,
-                  project: project)
-                project.add_role(user, project_role)
+            shared_examples_for 'only allowed by schedule owners who can update the branch' do
+              where(:push_access_level, :merge_access_level, :schedule_owner, :project_role, :accessible) do
+                :no_one_can_push | :no_one_can_merge  | :other_user | :owner      | :be_disallowed
+                :no_one_can_push | :no_one_can_merge  | :user       | :owner      | :be_disallowed
+                :no_one_can_push | :no_one_can_merge  | :user       | :maintainer | :be_disallowed
+                :no_one_can_push | :no_one_can_merge  | :user       | :developer  | :be_disallowed
+                :no_one_can_push | :no_one_can_merge  | :user       | :reporter   | :be_disallowed
+                :no_one_can_push | :no_one_can_merge  | :user       | :guest      | :be_disallowed
+
+                :maintainers_can_push | :no_one_can_merge | :other_user | :owner      | :be_disallowed
+                :maintainers_can_push | :no_one_can_merge | :user       | :owner      | :be_allowed
+                :maintainers_can_push | :no_one_can_merge | :user       | :maintainer | :be_allowed
+                :maintainers_can_push | :no_one_can_merge | :user       | :developer  | :be_disallowed
+                :maintainers_can_push | :no_one_can_merge | :user       | :reporter   | :be_disallowed
+                :maintainers_can_push | :no_one_can_merge | :user       | :guest      | :be_disallowed
+
+                :developers_can_push | :no_one_can_merge | :other_user | :owner      | :be_disallowed
+                :developers_can_push | :no_one_can_merge | :user       | :owner      | :be_allowed
+                :developers_can_push | :no_one_can_merge | :user       | :maintainer | :be_allowed
+                :developers_can_push | :no_one_can_merge | :user       | :developer  | :be_allowed
+                :developers_can_push | :no_one_can_merge | :user       | :reporter   | :be_disallowed
+                :developers_can_push | :no_one_can_merge | :user       | :guest      | :be_disallowed
+
+                :no_one_can_push | :maintainers_can_merge | :other_user | :owner      | :be_disallowed
+                :no_one_can_push | :maintainers_can_merge | :user       | :owner      | :be_allowed
+                :no_one_can_push | :maintainers_can_merge | :user       | :maintainer | :be_allowed
+                :no_one_can_push | :maintainers_can_merge | :user       | :developer  | :be_disallowed
+                :no_one_can_push | :maintainers_can_merge | :user       | :reporter   | :be_disallowed
+                :no_one_can_push | :maintainers_can_merge | :user       | :guest      | :be_disallowed
+
+                :maintainers_can_push | :maintainers_can_merge | :other_user | :owner      | :be_disallowed
+                :maintainers_can_push | :maintainers_can_merge | :user       | :owner      | :be_allowed
+                :maintainers_can_push | :maintainers_can_merge | :user       | :maintainer | :be_allowed
+                :maintainers_can_push | :maintainers_can_merge | :user       | :developer  | :be_disallowed
+                :maintainers_can_push | :maintainers_can_merge | :user       | :reporter   | :be_disallowed
+                :maintainers_can_push | :maintainers_can_merge | :user       | :guest      | :be_disallowed
+
+                :developers_can_push | :maintainers_can_merge | :other_user | :owner      | :be_disallowed
+                :developers_can_push | :maintainers_can_merge | :user       | :owner      | :be_allowed
+                :developers_can_push | :maintainers_can_merge | :user       | :maintainer | :be_allowed
+                :developers_can_push | :maintainers_can_merge | :user       | :developer  | :be_allowed
+                :developers_can_push | :maintainers_can_merge | :user       | :reporter   | :be_disallowed
+                :developers_can_push | :maintainers_can_merge | :user       | :guest      | :be_disallowed
+
+                :no_one_can_push | :developers_can_merge | :other_user | :owner      | :be_disallowed
+                :no_one_can_push | :developers_can_merge | :user       | :owner      | :be_allowed
+                :no_one_can_push | :developers_can_merge | :user       | :maintainer | :be_allowed
+                :no_one_can_push | :developers_can_merge | :user       | :developer  | :be_allowed
+                :no_one_can_push | :developers_can_merge | :user       | :reporter   | :be_disallowed
+                :no_one_can_push | :developers_can_merge | :user       | :guest      | :be_disallowed
+
+                :maintainers_can_push | :developers_can_merge | :other_user | :owner      | :be_disallowed
+                :maintainers_can_push | :developers_can_merge | :user       | :owner      | :be_allowed
+                :maintainers_can_push | :developers_can_merge | :user       | :maintainer | :be_allowed
+                :maintainers_can_push | :developers_can_merge | :user       | :developer  | :be_allowed
+                :maintainers_can_push | :developers_can_merge | :user       | :reporter   | :be_disallowed
+                :maintainers_can_push | :developers_can_merge | :user       | :guest      | :be_disallowed
+
+                :developers_can_push | :developers_can_merge | :other_user | :owner      | :be_disallowed
+                :developers_can_push | :developers_can_merge | :user       | :owner      | :be_allowed
+                :developers_can_push | :developers_can_merge | :user       | :maintainer | :be_allowed
+                :developers_can_push | :developers_can_merge | :user       | :developer  | :be_allowed
+                :developers_can_push | :developers_can_merge | :user       | :reporter   | :be_disallowed
+                :developers_can_push | :developers_can_merge | :user       | :guest      | :be_disallowed
               end
 
-              context 'for create_pipeline_schedule' do
-                subject(:policy) { described_class.new(user, new_branch_pipeline_schedule) }
+              with_them do
+                before do
+                  create(:protected_branch, push_access_level, merge_access_level, name: branch_ref_name,
+                    project: project)
+                  project.add_role(user, project_role)
+                  project.add_role(other_user, project_role)
 
-                let(:new_branch_pipeline_schedule) { project.pipeline_schedules.new(ref: branch_ref) }
+                  pipeline_schedule.owner = schedule_owner == :user ? user : other_user
+                end
 
-                it { expect(policy).to try(accessible, :create_pipeline_schedule) }
+                it { expect(policy).to try(accessible, ability_name) }
               end
+            end
 
-              context 'for play_pipeline_schedule' do
-                subject(:policy) { described_class.new(user, branch_pipeline_schedule) }
+            describe 'create_pipeline_schedule' do
+              let(:ability_name) { :create_pipeline_schedule }
+              let(:pipeline_schedule) { project.pipeline_schedules.new(ref: branch_ref) }
 
-                it { expect(policy).to try(accessible, :play_pipeline_schedule) }
-              end
+              it_behaves_like 'allowed by those who can update the branch'
+            end
+
+            describe 'play_pipeline_schedule' do
+              let(:ability_name) { :play_pipeline_schedule }
+
+              it_behaves_like 'allowed by those who can update the branch'
+            end
+
+            describe 'update_pipeline_schedule' do
+              let(:ability_name) { :update_pipeline_schedule }
+
+              it_behaves_like 'only allowed by schedule owners who can update the branch'
             end
           end
         end
@@ -108,49 +200,97 @@ RSpec.describe Ci::PipelineSchedulePolicy, :models, :clean_gitlab_redis_cache, f
       context 'for tag' do
         %w[refs/tags/v1.0.0 v1.0.0].each do |tag_ref|
           context "with #{tag_ref}" do
-            let_it_be(:tag_pipeline_schedule) do
+            let_it_be(:pipeline_schedule) do
               create(:ci_pipeline_schedule, :nightly, project: project, ref: tag_ref)
             end
 
-            where(:access_level, :project_role, :accessible) do
-              :no_one_can_create | :owner      | :be_disallowed
-              :no_one_can_create | :maintainer | :be_disallowed
-              :no_one_can_create | :developer  | :be_disallowed
-              :no_one_can_create | :reporter   | :be_disallowed
-              :no_one_can_create | :guest      | :be_disallowed
-
-              :maintainers_can_create | :owner      | :be_allowed
-              :maintainers_can_create | :maintainer | :be_allowed
-              :maintainers_can_create | :developer  | :be_disallowed
-              :maintainers_can_create | :reporter   | :be_disallowed
-              :maintainers_can_create | :guest      | :be_disallowed
-
-              :developers_can_create | :owner      | :be_allowed
-              :developers_can_create | :maintainer | :be_allowed
-              :developers_can_create | :developer  | :be_allowed
-              :developers_can_create | :reporter   | :be_disallowed
-              :developers_can_create | :guest      | :be_disallowed
+            subject(:policy) { described_class.new(user, pipeline_schedule) }
+
+            shared_examples_for 'allowed by those who can update the tag' do
+              where(:access_level, :project_role, :accessible) do
+                :no_one_can_create | :owner      | :be_disallowed
+                :no_one_can_create | :maintainer | :be_disallowed
+                :no_one_can_create | :developer  | :be_disallowed
+                :no_one_can_create | :reporter   | :be_disallowed
+                :no_one_can_create | :guest      | :be_disallowed
+
+                :maintainers_can_create | :owner      | :be_allowed
+                :maintainers_can_create | :maintainer | :be_allowed
+                :maintainers_can_create | :developer  | :be_disallowed
+                :maintainers_can_create | :reporter   | :be_disallowed
+                :maintainers_can_create | :guest      | :be_disallowed
+
+                :developers_can_create | :owner      | :be_allowed
+                :developers_can_create | :maintainer | :be_allowed
+                :developers_can_create | :developer  | :be_allowed
+                :developers_can_create | :reporter   | :be_disallowed
+                :developers_can_create | :guest      | :be_disallowed
+              end
+
+              with_them do
+                before do
+                  create(:protected_tag, access_level, name: tag_ref_name, project: project)
+                  project.add_role(user, project_role)
+                end
+
+                it { expect(policy).to try(accessible, ability_name) }
+              end
             end
 
-            with_them do
-              before do
-                create(:protected_tag, access_level, name: tag_ref_name, project: project)
-                project.add_role(user, project_role)
+            shared_examples_for 'only allowed by schedule owners who can update the tag' do
+              where(:access_level, :schedule_owner, :project_role, :accessible) do
+                :no_one_can_create | :other_user | :owner      | :be_disallowed
+                :no_one_can_create | :user       | :owner      | :be_disallowed
+                :no_one_can_create | :user       | :maintainer | :be_disallowed
+                :no_one_can_create | :user       | :developer  | :be_disallowed
+                :no_one_can_create | :user       | :reporter   | :be_disallowed
+                :no_one_can_create | :user       | :guest      | :be_disallowed
+
+                :maintainers_can_create | :other_user | :owner      | :be_disallowed
+                :maintainers_can_create | :user       | :owner      | :be_allowed
+                :maintainers_can_create | :user       | :maintainer | :be_allowed
+                :maintainers_can_create | :user       | :developer  | :be_disallowed
+                :maintainers_can_create | :user       | :reporter   | :be_disallowed
+                :maintainers_can_create | :user       | :guest      | :be_disallowed
+
+                :developers_can_create | :other_user | :owner      | :be_disallowed
+                :developers_can_create | :user       | :owner      | :be_allowed
+                :developers_can_create | :user       | :maintainer | :be_allowed
+                :developers_can_create | :user       | :developer  | :be_allowed
+                :developers_can_create | :user       | :reporter   | :be_disallowed
+                :developers_can_create | :user       | :guest      | :be_disallowed
               end
 
-              context 'for create_pipeline_schedule' do
-                subject(:policy) { described_class.new(user, new_tag_pipeline_schedule) }
+              with_them do
+                before do
+                  create(:protected_tag, access_level, name: tag_ref_name, project: project)
+                  project.add_role(user, project_role)
+                  project.add_role(other_user, project_role)
 
-                let(:new_tag_pipeline_schedule) { project.pipeline_schedules.new(ref: tag_ref) }
+                  pipeline_schedule.owner = schedule_owner == :user ? user : other_user
+                end
 
-                it { expect(policy).to try(accessible, :create_pipeline_schedule) }
+                it { expect(policy).to try(accessible, ability_name) }
               end
+            end
 
-              context 'for play_pipeline_schedule' do
-                subject(:policy) { described_class.new(user, tag_pipeline_schedule) }
+            describe 'create_pipeline_schedule' do
+              let(:ability_name) { :create_pipeline_schedule }
+              let(:pipeline_schedule) { project.pipeline_schedules.new(ref: tag_ref) }
 
-                it { expect(policy).to try(accessible, :play_pipeline_schedule) }
-              end
+              it_behaves_like 'allowed by those who can update the tag'
+            end
+
+            describe 'play_pipeline_schedule' do
+              let(:ability_name) { :play_pipeline_schedule }
+
+              it_behaves_like 'allowed by those who can update the tag'
+            end
+
+            describe 'update_pipeline_schedule' do
+              let(:ability_name) { :update_pipeline_schedule }
+
+              it_behaves_like 'only allowed by schedule owners who can update the tag'
             end
           end
         end
diff --git a/spec/services/ci/pipeline_schedules/update_service_spec.rb b/spec/services/ci/pipeline_schedules/update_service_spec.rb
index 834bbcfcfeb..b84afacdcff 100644
--- a/spec/services/ci/pipeline_schedules/update_service_spec.rb
+++ b/spec/services/ci/pipeline_schedules/update_service_spec.rb
@@ -7,16 +7,16 @@ RSpec.describe Ci::PipelineSchedules::UpdateService, feature_category: :continuo
   let_it_be_with_reload(:project) { create(:project, :public, :repository) }
   let_it_be_with_reload(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: user) }
   let_it_be(:reporter) { create(:user) }
+  let_it_be(:project_owner) { create(:user) }
 
   let_it_be(:pipeline_schedule_variable) do
     create(:ci_pipeline_schedule_variable,
       key: 'foo', value: 'foovalue', pipeline_schedule: pipeline_schedule)
   end
 
-  subject(:service) { described_class.new(pipeline_schedule, user, params) }
-
   before_all do
     project.add_maintainer(user)
+    project.add_owner(project_owner)
     project.add_reporter(reporter)
 
     pipeline_schedule.reload
@@ -54,8 +54,10 @@ RSpec.describe Ci::PipelineSchedules::UpdateService, feature_category: :continuo
       subject(:service) { described_class.new(pipeline_schedule, user, params) }
 
       it 'updates database values with passed params' do
-        expect { service.execute }
-          .to change { pipeline_schedule.description }.from('pipeline schedule').to('updated_desc')
+        expect do
+          service.execute
+          pipeline_schedule.reload
+        end.to change { pipeline_schedule.description }.from('pipeline schedule').to('updated_desc')
           .and change { pipeline_schedule.ref }.from('master').to('patch-x')
           .and change { pipeline_schedule.active }.from(true).to(false)
           .and change { pipeline_schedule.cron }.from('0 1 * * *').to('*/1 * * * *')
@@ -63,6 +65,48 @@ RSpec.describe Ci::PipelineSchedules::UpdateService, feature_category: :continuo
           .and change { pipeline_schedule.variables.last.value }.from('foovalue').to('barvalue')
       end
 
+      context 'when the new branch is protected', :request_store do
+        let(:maintainer_access) { :no_one_can_merge }
+
+        before do
+          create(:protected_branch, :no_one_can_push, maintainer_access, name: 'patch-x', project: project)
+        end
+
+        after do
+          ProtectedBranches::CacheService.new(project).refresh
+        end
+
+        context 'when called by someone other than the schedule owner who can update the ref' do
+          let(:maintainer_access) { :maintainers_can_merge }
+
+          subject(:service) { described_class.new(pipeline_schedule, project_owner, params) }
+
+          it 'does not update the schedule' do
+            expect do
+              service.execute
+              pipeline_schedule.reload
+            end.not_to change { pipeline_schedule.description }
+          end
+        end
+
+        context 'when called by the schedule owner' do
+          it 'does not update the schedule' do
+            expect do
+              service.execute
+              pipeline_schedule.reload
+            end.not_to change { pipeline_schedule.description }
+          end
+
+          context 'when the owner can update the ref' do
+            let(:maintainer_access) { :maintainers_can_merge }
+
+            it 'updates the schedule' do
+              expect { service.execute }.to change { pipeline_schedule.description }
+            end
+          end
+        end
+      end
+
       context 'when creating a variable' do
         let(:params) do
           {
@@ -126,6 +170,8 @@ RSpec.describe Ci::PipelineSchedules::UpdateService, feature_category: :continuo
       end
     end
 
-    it_behaves_like 'pipeline schedules checking variables permission'
+    it_behaves_like 'pipeline schedules checking variables permission' do
+      subject(:service) { described_class.new(pipeline_schedule, user, params) }
+    end
   end
 end