Commit aca6be50d3dc1963491c9dcff61dac3b3ec937ca
1 parent
6ba4cb1d
Exists in
spb-stable
and in
2 other branches
Refactor project transfer service. Add security check when create or transfer project into group
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Showing
3 changed files
with
63 additions
and
60 deletions
Show diff stats
app/services/project_transfer_service.rb
| @@ -1,46 +0,0 @@ | @@ -1,46 +0,0 @@ | ||
| 1 | -# ProjectTransferService class | ||
| 2 | -# | ||
| 3 | -# Used for transfer project to another namespace | ||
| 4 | -# | ||
| 5 | -class ProjectTransferService | ||
| 6 | - include Gitlab::ShellAdapter | ||
| 7 | - | ||
| 8 | - class TransferError < StandardError; end | ||
| 9 | - | ||
| 10 | - attr_accessor :project | ||
| 11 | - | ||
| 12 | - def transfer(project, new_namespace) | ||
| 13 | - Project.transaction do | ||
| 14 | - old_path = project.path_with_namespace | ||
| 15 | - new_path = File.join(new_namespace.try(:path) || '', project.path) | ||
| 16 | - | ||
| 17 | - if Project.where(path: project.path, namespace_id: new_namespace.try(:id)).present? | ||
| 18 | - raise TransferError.new("Project with same path in target namespace already exists") | ||
| 19 | - end | ||
| 20 | - | ||
| 21 | - # Remove old satellite | ||
| 22 | - project.satellite.destroy | ||
| 23 | - | ||
| 24 | - # Apply new namespace id | ||
| 25 | - project.namespace = new_namespace | ||
| 26 | - project.save! | ||
| 27 | - | ||
| 28 | - # Move main repository | ||
| 29 | - unless gitlab_shell.mv_repository(old_path, new_path) | ||
| 30 | - raise TransferError.new('Cannot move project') | ||
| 31 | - end | ||
| 32 | - | ||
| 33 | - # Move wiki repo also if present | ||
| 34 | - gitlab_shell.mv_repository("#{old_path}.wiki", "#{new_path}.wiki") | ||
| 35 | - | ||
| 36 | - # Create a new satellite (reload project from DB) | ||
| 37 | - Project.find(project.id).ensure_satellite_exists | ||
| 38 | - | ||
| 39 | - # clear project cached events | ||
| 40 | - project.reset_events_cache | ||
| 41 | - | ||
| 42 | - true | ||
| 43 | - end | ||
| 44 | - end | ||
| 45 | -end | ||
| 46 | - |
app/services/projects/create_service.rb
| @@ -97,7 +97,7 @@ module Projects | @@ -97,7 +97,7 @@ module Projects | ||
| 97 | 97 | ||
| 98 | def allowed_namespace?(user, namespace_id) | 98 | def allowed_namespace?(user, namespace_id) |
| 99 | namespace = Namespace.find_by(id: namespace_id) | 99 | namespace = Namespace.find_by(id: namespace_id) |
| 100 | - current_user.can?(:manage_namespace, namespace) | 100 | + current_user.can?(:create_projects, namespace) |
| 101 | end | 101 | end |
| 102 | end | 102 | end |
| 103 | end | 103 | end |
app/services/projects/transfer_service.rb
| 1 | +# Projects::TransferService class | ||
| 2 | +# | ||
| 3 | +# Used for transfer project to another namespace | ||
| 4 | +# | ||
| 5 | +# Ex. | ||
| 6 | +# # Move projects to namespace with ID 17 by user | ||
| 7 | +# Projects::TransferService.new(project, user, namespace_id: 17).execute | ||
| 8 | +# | ||
| 1 | module Projects | 9 | module Projects |
| 2 | class TransferService < BaseService | 10 | class TransferService < BaseService |
| 3 | - def execute(role = :default) | ||
| 4 | - namespace_id = params[:project].delete(:namespace_id) | ||
| 5 | - allowed_transfer = can?(current_user, :change_namespace, project) || role == :admin | ||
| 6 | - | ||
| 7 | - if allowed_transfer && namespace_id.present? | ||
| 8 | - if namespace_id.to_i != project.namespace_id | ||
| 9 | - # Transfer to someone namespace | ||
| 10 | - namespace = Namespace.find(namespace_id) | ||
| 11 | - project.transfer(namespace) | ||
| 12 | - end | ||
| 13 | - end | 11 | + include Gitlab::ShellAdapter |
| 12 | + class TransferError < StandardError; end | ||
| 13 | + | ||
| 14 | + def execute | ||
| 15 | + namespace_id = params.delete(:namespace_id) | ||
| 16 | + namespace = Namespace.find_by(id: namespace_id) | ||
| 14 | 17 | ||
| 15 | - rescue ProjectTransferService::TransferError => ex | 18 | + if allowed_transfer?(current_user, project, namespace) |
| 19 | + transfer(project, namespace) | ||
| 20 | + else | ||
| 21 | + project.errors.add(:namespace, 'is invalid') | ||
| 22 | + false | ||
| 23 | + end | ||
| 24 | + rescue Projects::TransferService::TransferError => ex | ||
| 16 | project.reload | 25 | project.reload |
| 17 | project.errors.add(:namespace_id, ex.message) | 26 | project.errors.add(:namespace_id, ex.message) |
| 18 | false | 27 | false |
| 19 | end | 28 | end |
| 29 | + | ||
| 30 | + def transfer(project, new_namespace) | ||
| 31 | + Project.transaction do | ||
| 32 | + old_path = project.path_with_namespace | ||
| 33 | + new_path = File.join(new_namespace.try(:path) || '', project.path) | ||
| 34 | + | ||
| 35 | + if Project.where(path: project.path, namespace_id: new_namespace.try(:id)).present? | ||
| 36 | + raise TransferError.new("Project with same path in target namespace already exists") | ||
| 37 | + end | ||
| 38 | + | ||
| 39 | + # Remove old satellite | ||
| 40 | + project.satellite.destroy | ||
| 41 | + | ||
| 42 | + # Apply new namespace id | ||
| 43 | + project.namespace = new_namespace | ||
| 44 | + project.save! | ||
| 45 | + | ||
| 46 | + # Move main repository | ||
| 47 | + unless gitlab_shell.mv_repository(old_path, new_path) | ||
| 48 | + raise TransferError.new('Cannot move project') | ||
| 49 | + end | ||
| 50 | + | ||
| 51 | + # Move wiki repo also if present | ||
| 52 | + gitlab_shell.mv_repository("#{old_path}.wiki", "#{new_path}.wiki") | ||
| 53 | + | ||
| 54 | + # Create a new satellite (reload project from DB) | ||
| 55 | + Project.find(project.id).ensure_satellite_exists | ||
| 56 | + | ||
| 57 | + # clear project cached events | ||
| 58 | + project.reset_events_cache | ||
| 59 | + | ||
| 60 | + true | ||
| 61 | + end | ||
| 62 | + end | ||
| 63 | + | ||
| 64 | + def allowed_transfer?(current_user, project, namespace) | ||
| 65 | + namespace && | ||
| 66 | + can?(current_user, :change_namespace, project) && | ||
| 67 | + namespace.id != project.namespace_id && | ||
| 68 | + current_user.can?(:create_projects, namespace) | ||
| 69 | + end | ||
| 20 | end | 70 | end |
| 21 | end | 71 | end |
| 22 | - |