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

4 files changed, 42 insertions, 7 deletions

diff --git a/app/models/user.rb b/app/models/user.rb
index 18bf5ceaa0e..ee42a987939 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1327,7 +1327,7 @@ class User < ApplicationRecord
         .select('ci_runners.*')
 
       group_runners = Ci::RunnerNamespace
-        .where(namespace_id: owned_or_maintainers_groups.select(:id))
+        .where(namespace_id: owned_groups.select(:id))
         .joins(:runner)
         .select('ci_runners.*')
 
diff --git a/changelogs/unreleased/security-master-mc-api-runner-owner-permissions.yml b/changelogs/unreleased/security-master-mc-api-runner-owner-permissions.yml
new file mode 100644
index 00000000000..2f23dbf7b9f
--- /dev/null
+++ b/changelogs/unreleased/security-master-mc-api-runner-owner-permissions.yml
@@ -0,0 +1,5 @@
+---
+title: Return only runners from groups where user is owner for user CI owned runners.
+merge_request:
+author:
+type: security
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 749d80ebfc2..58aa945bff0 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -2637,8 +2637,8 @@ describe User, :do_not_mock_admin_mode do
           add_user(:maintainer)
         end
 
-        it 'loads' do
-          expect(user.ci_owned_runners).to contain_exactly(runner)
+        it 'does not load' do
+          expect(user.ci_owned_runners).to be_empty
         end
       end
 
@@ -2653,6 +2653,20 @@ describe User, :do_not_mock_admin_mode do
       end
     end
 
+    shared_examples :group_member do
+      context 'when the user is owner' do
+        before do
+          add_user(:owner)
+        end
+
+        it 'loads' do
+          expect(user.ci_owned_runners).to contain_exactly(runner)
+        end
+      end
+
+      it_behaves_like :member
+    end
+
     context 'with groups projects runners' do
       let(:group) { create(:group) }
       let!(:project) { create(:project, group: group) }
@@ -2661,7 +2675,7 @@ describe User, :do_not_mock_admin_mode do
         group.add_user(user, access)
       end
 
-      it_behaves_like :member
+      it_behaves_like :group_member
     end
 
     context 'with groups runners' do
@@ -2672,14 +2686,14 @@ describe User, :do_not_mock_admin_mode do
         group.add_user(user, access)
       end
 
-      it_behaves_like :member
+      it_behaves_like :group_member
     end
 
     context 'with other projects runners' do
       let!(:project) { create(:project) }
 
       def add_user(access)
-        project.add_role(user, access)
+        project.add_user(user, access)
       end
 
       it_behaves_like :member
@@ -2697,7 +2711,7 @@ describe User, :do_not_mock_admin_mode do
         subgroup.add_user(another_user, :owner)
       end
 
-      it_behaves_like :member
+      it_behaves_like :group_member
     end
   end
 
diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb
index 8daba204d50..7bad30d107d 100644
--- a/spec/requests/api/runners_spec.rb
+++ b/spec/requests/api/runners_spec.rb
@@ -6,6 +6,7 @@ describe API::Runners do
   let(:admin) { create(:user, :admin) }
   let(:user) { create(:user) }
   let(:user2) { create(:user) }
+  let(:group_maintainer) { create(:user) }
 
   let(:project) { create(:project, creator_id: user.id) }
   let(:project2) { create(:project, creator_id: user.id) }
@@ -20,6 +21,7 @@ describe API::Runners do
 
   before do
     # Set project access for users
+    create(:group_member, :maintainer, user: group_maintainer, group: group)
     create(:project_member, :maintainer, user: user, project: project)
     create(:project_member, :maintainer, user: user, project: project2)
     create(:project_member, :reporter, user: user2, project: project)
@@ -525,6 +527,20 @@ describe API::Runners do
           end.to change { Ci::Runner.project_type.count }.by(-1)
         end
 
+        it 'does not delete group runner with maintainer access' do
+          delete api("/runners/#{group_runner.id}", group_maintainer)
+
+          expect(response).to have_http_status(403)
+        end
+
+        it 'deletes group runner with owner access' do
+          expect do
+            delete api("/runners/#{group_runner.id}", user)
+
+            expect(response).to have_http_status(204)
+          end.to change { Ci::Runner.group_type.count }.by(-1)
+        end
+
         it_behaves_like '412 response' do
           let(:request) { api("/runners/#{project_runner.id}", user) }
         end