Merge branch 'allowlist_user_owns_runner_callsite' into 'master'

Allow user owns_runner? callsite

See merge request gitlab-org/gitlab!69527

app/models/user.rb

@@ -1613,6 +1613,8 @@ class User < ApplicationRecord
     true
   end
 
+  # TODO Please check all callers and remove allow_cross_joins_across_databases,
+  # when https://gitlab.com/gitlab-org/gitlab/-/issues/336436 is done.
   def ci_owned_runners
     @ci_owned_runners ||= begin
       project_runners = Ci::RunnerProject
@@ -1629,6 +1631,12 @@ class User < ApplicationRecord
     end
   end
 
+  def owns_runner?(runner)
+    ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336436') do
+      ci_owned_runners.exists?(runner.id)
+    end
+  end
+
   def notification_email_for(notification_group)
     # Return group-specific email address if present, otherwise return global notification email address
     notification_group&.notification_email_for(self) || notification_email

app/policies/ci/runner_policy.rb

@@ -5,9 +5,9 @@ module Ci
     with_options scope: :subject, score: 0
     condition(:locked, scope: :subject) { @subject.locked? }
 
-    # rubocop: disable CodeReuse/ActiveRecord
-    condition(:owned_runner) { @user.ci_owned_runners.exists?(@subject.id) }
-    # rubocop: enable CodeReuse/ActiveRecord
+    condition(:owned_runner) do
+      @user.owns_runner?(@subject)
+    end
 
     rule { anonymous }.prevent_all
 

spec/models/user_spec.rb

@@ -3635,6 +3635,11 @@ RSpec.describe User do
         it 'loads all the runners in the tree of groups' do
           expect(user.ci_owned_runners).to contain_exactly(runner, group_runner)
         end
+
+        it 'returns true for owns_runner?' do
+          expect(user.owns_runner?(runner)).to eq(true)
+          expect(user.owns_runner?(group_runner)).to eq(true)
+        end
       end
     end
 
@@ -3647,6 +3652,10 @@ RSpec.describe User do
         it 'loads the runners in the group' do
           expect(user.ci_owned_runners).to contain_exactly(group_runner)
         end
+
+        it 'returns true for owns_runner?' do
+          expect(user.owns_runner?(group_runner)).to eq(true)
+        end
       end
     end
 
@@ -3655,6 +3664,10 @@ RSpec.describe User do
         it 'loads the runner belonging to the project' do
           expect(user.ci_owned_runners).to contain_exactly(runner)
         end
+
+        it 'returns true for owns_runner?' do
+          expect(user.owns_runner?(runner)).to eq(true)
+        end
       end
     end
 
@@ -3667,6 +3680,10 @@ RSpec.describe User do
         it 'loads the runners of the project' do
           expect(user.ci_owned_runners).to contain_exactly(project_runner)
         end
+
+        it 'returns true for owns_runner?' do
+          expect(user.owns_runner?(project_runner)).to eq(true)
+        end
       end
 
       context 'when the user is a developer' do
@@ -3677,6 +3694,10 @@ RSpec.describe User do
         it 'does not load any runner' do
           expect(user.ci_owned_runners).to be_empty
         end
+
+        it 'returns false for owns_runner?' do
+          expect(user.owns_runner?(project_runner)).to eq(false)
+        end
       end
 
       context 'when the user is a reporter' do
@@ -3687,6 +3708,10 @@ RSpec.describe User do
         it 'does not load any runner' do
           expect(user.ci_owned_runners).to be_empty
         end
+
+        it 'returns false for owns_runner?' do
+          expect(user.owns_runner?(project_runner)).to eq(false)
+        end
       end
 
       context 'when the user is a guest' do
@@ -3697,6 +3722,10 @@ RSpec.describe User do
         it 'does not load any runner' do
           expect(user.ci_owned_runners).to be_empty
         end
+
+        it 'returns false for owns_runner?' do
+          expect(user.owns_runner?(project_runner)).to eq(false)
+        end
       end
     end
 
@@ -3709,6 +3738,10 @@ RSpec.describe User do
         it 'does not load the runners of the group' do
           expect(user.ci_owned_runners).to be_empty
         end
+
+        it 'returns false for owns_runner?' do
+          expect(user.owns_runner?(runner)).to eq(false)
+        end
       end
 
       context 'when the user is a developer' do
@@ -3719,6 +3752,10 @@ RSpec.describe User do
         it 'does not load any runner' do
           expect(user.ci_owned_runners).to be_empty
         end
+
+        it 'returns false for owns_runner?' do
+          expect(user.owns_runner?(runner)).to eq(false)
+        end
       end
 
       context 'when the user is a reporter' do
@@ -3729,6 +3766,10 @@ RSpec.describe User do
         it 'does not load any runner' do
           expect(user.ci_owned_runners).to be_empty
         end
+
+        it 'returns false for owns_runner?' do
+          expect(user.owns_runner?(runner)).to eq(false)
+        end
       end
 
       context 'when the user is a guest' do
@@ -3739,6 +3780,10 @@ RSpec.describe User do
         it 'does not load any runner' do
           expect(user.ci_owned_runners).to be_empty
         end
+
+        it 'returns false for owns_runner?' do
+          expect(user.owns_runner?(runner)).to eq(false)
+        end
       end
     end
 
@@ -3746,6 +3791,10 @@ RSpec.describe User do
       it 'does not load any runner' do
         expect(user.ci_owned_runners).to be_empty
       end
+
+      it 'returns false for owns_runner?' do
+        expect(user.owns_runner?(create(:ci_runner))).to eq(false)
+      end
     end
 
     context 'with runner in a personal project' do