Merge branch 'refresh-permissions-when-moving-projects' into 'master'
Refresh authorizations when transferring projects See merge request !9029
This commit is contained in:
commit
a658654c69
|
@ -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
|
||||||
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
|
||||||
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
|
||||||
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
|
||||||
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
|
||||||
|
|
|
@ -0,0 +1,4 @@
|
||||||
|
---
|
||||||
|
title: Refresh authorizations when transferring projects
|
||||||
|
merge_request:
|
||||||
|
author:
|
|
@ -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
|
||||||
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
|
||||||
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
|
||||||
|
|
Loading…
Reference in New Issue