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