Commit ebae3839 authored by Yorick Peterse's avatar Yorick Peterse

Refresh authorizations when transferring 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.
parent 99fceff4
...@@ -197,7 +197,12 @@ class Group < Namespace ...@@ -197,7 +197,12 @@ class Group < Namespace
end end
def refresh_members_authorized_projects 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 end
def members_with_parents def members_with_parents
......
...@@ -213,6 +213,10 @@ class Namespace < ActiveRecord::Base ...@@ -213,6 +213,10 @@ class Namespace < ActiveRecord::Base
self.class.joins(:route).where('routes.path LIKE ?', "#{route.path}/%").reorder('routes.path ASC') self.class.joins(:route).where('routes.path LIKE ?', "#{route.path}/%").reorder('routes.path ASC')
end end
def user_ids_for_project_authorizations
[owner_id]
end
private private
def repository_storage_paths def repository_storage_paths
......
...@@ -25,9 +25,10 @@ module Projects ...@@ -25,9 +25,10 @@ module Projects
end end
def transfer(project, new_namespace) def transfer(project, new_namespace)
old_namespace = project.namespace
Project.transaction do Project.transaction do
old_path = project.path_with_namespace old_path = project.path_with_namespace
old_namespace = project.namespace
old_group = project.group old_group = project.group
new_path = File.join(new_namespace.try(:path) || '', project.path) new_path = File.join(new_namespace.try(:path) || '', project.path)
...@@ -70,8 +71,11 @@ module Projects ...@@ -70,8 +71,11 @@ module Projects
project.old_path_with_namespace = old_path project.old_path_with_namespace = old_path
SystemHooksService.new.execute_hooks_for(project, :transfer) SystemHooksService.new.execute_hooks_for(project, :transfer)
true
end end
refresh_permissions(old_namespace, new_namespace)
true
end end
def allowed_transfer?(current_user, project, namespace) def allowed_transfer?(current_user, project, namespace)
...@@ -80,5 +84,14 @@ module Projects ...@@ -80,5 +84,14 @@ module Projects
namespace.id != project.namespace_id && namespace.id != project.namespace_id &&
current_user.can?(:create_projects, namespace) current_user.can?(:create_projects, namespace)
end 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
end end
---
title: Refresh authorizations when transferring projects
merge_request:
author:
...@@ -300,4 +300,17 @@ describe Group, models: true do ...@@ -300,4 +300,17 @@ describe Group, models: true do
expect(group.members_with_parents).to include(master) expect(group.members_with_parents).to include(master)
end end
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 end
...@@ -218,4 +218,11 @@ describe Namespace, models: true do ...@@ -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]) expect(group.descendants.to_a).to eq([nested_group, deep_nested_group, very_deep_nested_group])
end end
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 end
...@@ -83,4 +83,30 @@ describe Projects::TransferService, services: true do ...@@ -83,4 +83,30 @@ describe Projects::TransferService, services: true do
transfer_project(project, user, group) transfer_project(project, user, group)
end end
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 end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment