diff options
author | Felipe Artur <felipefac@gmail.com> | 2016-12-14 19:39:53 -0200 |
---|---|---|
committer | Felipe Artur <felipefac@gmail.com> | 2016-12-27 19:25:17 -0200 |
commit | 1b082a4c338d7575e15d7450906801db59873441 (patch) | |
tree | 1b5081ce55b63436082b038f499a8c54172fc75a /app | |
parent | 77deeb12f74b857f9356168ccdf92612fc85fe84 (diff) | |
download | gitlab-ce-issue_22664.tar.gz |
Check if user can read issue before being assignedissue_22664
Diffstat (limited to 'app')
-rw-r--r-- | app/models/concerns/issuable.rb | 6 | ||||
-rw-r--r-- | app/services/issuable_base_service.rb | 41 | ||||
-rw-r--r-- | app/services/issues/base_service.rb | 4 | ||||
-rw-r--r-- | app/services/merge_requests/base_service.rb | 4 |
4 files changed, 32 insertions, 23 deletions
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index ecbfb625c5e..5e63825bf99 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -93,9 +93,9 @@ module Issuable def update_assignee_cache_counts # make sure we flush the cache for both the old *and* new assignees(if they exist) - previous_assignee = User.find_by_id(assignee_id_was) - previous_assignee.try(:update_cache_counts) - assignee.try(:update_cache_counts) + previous_assignee = User.find_by_id(assignee_id_was) if assignee_id_was + previous_assignee.update_cache_counts if previous_assignee + assignee.update_cache_counts if assignee end # We want to use optimistic lock for cases when only title or description are involved diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index ab3d2a9a0cd..4ce5fd993d9 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -36,14 +36,10 @@ class IssuableBaseService < BaseService end end - def filter_params(issuable_ability_name = :issue) - filter_assignee - filter_milestone - filter_labels + def filter_params(issuable) + ability_name = :"admin_#{issuable.to_ability_name}" - ability = :"admin_#{issuable_ability_name}" - - unless can?(current_user, ability, project) + unless can?(current_user, ability_name, project) params.delete(:milestone_id) params.delete(:labels) params.delete(:add_label_ids) @@ -52,14 +48,35 @@ class IssuableBaseService < BaseService params.delete(:assignee_id) params.delete(:due_date) end + + filter_assignee(issuable) + filter_milestone + filter_labels end - def filter_assignee - if params[:assignee_id] == IssuableFinder::NONE - params[:assignee_id] = '' + def filter_assignee(issuable) + return unless params[:assignee_id].present? + + assignee_id = params[:assignee_id] + + if assignee_id.to_s == IssuableFinder::NONE + params[:assignee_id] = "" + else + params.delete(:assignee_id) unless assignee_can_read?(issuable, assignee_id) end end + def assignee_can_read?(issuable, assignee_id) + new_assignee = User.find_by_id(assignee_id) + + return false unless new_assignee.present? + + ability_name = :"read_#{issuable.to_ability_name}" + resource = issuable.persisted? ? issuable : project + + can?(new_assignee, ability_name, resource) + end + def filter_milestone milestone_id = params[:milestone_id] return unless milestone_id @@ -138,7 +155,7 @@ class IssuableBaseService < BaseService def create(issuable) merge_slash_commands_into_params!(issuable) - filter_params + filter_params(issuable) params.delete(:state_event) params[:author] ||= current_user @@ -180,7 +197,7 @@ class IssuableBaseService < BaseService change_state(issuable) change_subscription(issuable) change_todo(issuable) - filter_params + filter_params(issuable) old_labels = issuable.labels.to_a old_mentioned_users = issuable.mentioned_users.to_a diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index 742e834df97..35af867a098 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -17,10 +17,6 @@ module Issues private - def filter_params - super(:issue) - end - def execute_hooks(issue, action = 'open') issue_data = hook_data(issue, action) hooks_scope = issue.confidential? ? :confidential_issue_hooks : :issue_hooks diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 800fd39c424..70e25956dc7 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -38,10 +38,6 @@ module MergeRequests private - def filter_params - super(:merge_request) - end - def merge_requests_for(branch) origin_merge_requests = @project.origin_merge_requests .opened.where(source_branch: branch).to_a |