Refresh authorizations when transferring projectsrefresh-permissions-when-moving-projects

This ensures that project authorizations are refreshed when moving a
project from one namespace to another. When doing so the permissions for
all users of both the old and new namespaces are refreshed.

See #26194 for more information.

7 files changed, 75 insertions, 3 deletions

diff --git a/app/models/group.rb b/app/models/group.rb
index 4cdfd022094..a5b92283daa 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -197,7 +197,12 @@ class Group < Namespace
   end
 
   def refresh_members_authorized_projects
-    UserProjectAccessChangedService.new(users_with_parents.pluck(:id)).execute
+    UserProjectAccessChangedService.new(user_ids_for_project_authorizations).
+      execute
+  end
+
+  def user_ids_for_project_authorizations
+    users_with_parents.pluck(:id)
   end
 
   def members_with_parents
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index 2fb2eb44aaa..c5713fb7818 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -213,6 +213,10 @@ class Namespace < ActiveRecord::Base
     self.class.joins(:route).where('routes.path LIKE ?', "#{route.path}/%").reorder('routes.path ASC')
   end
 
+  def user_ids_for_project_authorizations
+    [owner_id]
+  end
+
   private
 
   def repository_storage_paths
diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb
index 20b049b5973..484700c8c29 100644
--- a/app/services/projects/transfer_service.rb
+++ b/app/services/projects/transfer_service.rb
@@ -25,9 +25,10 @@ module Projects
     end
 
     def transfer(project, new_namespace)
+      old_namespace = project.namespace
+
       Project.transaction do
         old_path = project.path_with_namespace
-        old_namespace = project.namespace
         old_group = project.group
         new_path = File.join(new_namespace.try(:path) || '', project.path)
 
@@ -70,8 +71,11 @@ module Projects
         project.old_path_with_namespace = old_path
 
         SystemHooksService.new.execute_hooks_for(project, :transfer)
-        true
       end
+
+      refresh_permissions(old_namespace, new_namespace)
+
+      true
     end
 
     def allowed_transfer?(current_user, project, namespace)
@@ -80,5 +84,14 @@ module Projects
         namespace.id != project.namespace_id &&
         current_user.can?(:create_projects, namespace)
     end
+
+    def refresh_permissions(old_namespace, new_namespace)
+      # This ensures we only schedule 1 job for every user that has access to
+      # the namespaces.
+      user_ids = old_namespace.user_ids_for_project_authorizations |
+        new_namespace.user_ids_for_project_authorizations
+
+      UserProjectAccessChangedService.new(user_ids).execute
+    end
   end
 end
diff --git a/changelogs/unreleased/refresh-permissions-when-moving-projects.yml b/changelogs/unreleased/refresh-permissions-when-moving-projects.yml
new file mode 100644
index 00000000000..a94bcdaa9a3
--- /dev/null
+++ b/changelogs/unreleased/refresh-permissions-when-moving-projects.yml
@@ -0,0 +1,4 @@
+---
+title: Refresh authorizations when transferring projects
+merge_request:
+author:
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 9ca50555191..a4e6eb4e3a6 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -300,4 +300,17 @@ describe Group, models: true do
       expect(group.members_with_parents).to include(master)
     end
   end
+
+  describe '#user_ids_for_project_authorizations' do
+    it 'returns the user IDs for which to refresh authorizations' do
+      master = create(:user)
+      developer = create(:user)
+
+      group.add_user(master, GroupMember::MASTER)
+      group.add_user(developer, GroupMember::DEVELOPER)
+
+      expect(group.user_ids_for_project_authorizations).
+        to include(master.id, developer.id)
+    end
+  end
 end
diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb
index 4e96f19eb6f..7bb1657bc3a 100644
--- a/spec/models/namespace_spec.rb
+++ b/spec/models/namespace_spec.rb
@@ -218,4 +218,11 @@ describe Namespace, models: true do
       expect(group.descendants.to_a).to eq([nested_group, deep_nested_group, very_deep_nested_group])
     end
   end
+
+  describe '#user_ids_for_project_authorizations' do
+    it 'returns the user IDs for which to refresh authorizations' do
+      expect(namespace.user_ids_for_project_authorizations).
+        to eq([namespace.owner_id])
+    end
+  end
 end
diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb
index 5d5812c2c15..5c6fbea8d0e 100644
--- a/spec/services/projects/transfer_service_spec.rb
+++ b/spec/services/projects/transfer_service_spec.rb
@@ -83,4 +83,30 @@ describe Projects::TransferService, services: true do
       transfer_project(project, user, group)
     end
   end
+
+  describe 'refreshing project authorizations' do
+    let(:group) { create(:group) }
+    let(:owner) { project.namespace.owner }
+    let(:group_member) { create(:user) }
+
+    before do
+      group.add_user(owner, GroupMember::MASTER)
+      group.add_user(group_member, GroupMember::DEVELOPER)
+    end
+
+    it 'refreshes the permissions of the old and new namespace' do
+      transfer_project(project, owner, group)
+
+      expect(group_member.authorized_projects).to include(project)
+      expect(owner.authorized_projects).to include(project)
+    end
+
+    it 'only schedules a single job for every user' do
+      expect(UserProjectAccessChangedService).to receive(:new).
+        with([owner.id, group_member.id]).
+        and_call_original
+
+      transfer_project(project, owner, group)
+    end
+  end
 end