From 5de51f9c58b70a06f9b5ff68b8dc47da1a2e829b Mon Sep 17 00:00:00 2001 From: Jure Grabnar Date: Wed, 20 Mar 2019 12:58:22 +0100 Subject: [PATCH] Prevent smart annotations accessing other teams when importing protocol Closes SCI-3163 --- app/helpers/application_helper.rb | 6 ++-- .../smart_annotations/permision_eval.rb | 28 +++++++++++-------- app/services/smart_annotations/tag_to_html.rb | 14 ++++++---- app/services/smart_annotations/tag_to_text.rb | 12 ++++---- .../smart_annotations/permission_eval_spec.rb | 8 +++--- .../smart_annotations/tag_to_html_spec.rb | 2 +- 6 files changed, 40 insertions(+), 30 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 7270b7cf7..dce7bd915 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -114,15 +114,15 @@ module ApplicationHelper # sometimes happens that the "team" param gets wrong data: "{nil, []}" # so we have to check if the "team" param is kind of Team object team = nil unless team.is_a? Team - new_text = smart_annotation_filter_resources(text) + new_text = smart_annotation_filter_resources(text, team) new_text = smart_annotation_filter_users(new_text, team) new_text end # Check if text have smart annotations of resources # and outputs a link to resource - def smart_annotation_filter_resources(text) - SmartAnnotations::TagToHtml.new(current_user, text).html + def smart_annotation_filter_resources(text, team) + SmartAnnotations::TagToHtml.new(current_user, team, text).html end # Check if text have smart annotations of users diff --git a/app/services/smart_annotations/permision_eval.rb b/app/services/smart_annotations/permision_eval.rb index 51f563ed9..ccae1fb81 100644 --- a/app/services/smart_annotations/permision_eval.rb +++ b/app/services/smart_annotations/permision_eval.rb @@ -5,31 +5,37 @@ module SmartAnnotations class << self include Canaid::Helpers::PermissionsHelper - def check(user, type, object) - send("validate_#{type}_permissions", user, object) + def check(user, team, type, object) + send("validate_#{type}_permissions", user, team, object) end private - def validate_prj_permissions(user, object) - can_read_project?(user, object) + def validate_prj_permissions(user, team, object) + object.team.id == team.id && can_read_project?(user, object) end - def validate_exp_permissions(user, object) - can_read_experiment?(user, object) + def validate_exp_permissions(user, team, object) + object.project.team.id == team.id && can_read_experiment?(user, object) end - def validate_tsk_permissions(user, object) - can_read_experiment?(user, object.experiment) + def validate_tsk_permissions(user, team, object) + object.experiment.project.team.id == team.id && + can_read_experiment?(user, object.experiment) end - def validate_rep_item_permissions(user, object) - return can_read_team?(user, object.repository.team) if object.repository + def validate_rep_item_permissions(user, team, object) + if object.repository + return object.repository.team.id == team.id && + can_read_team?(user, object.repository.team) + end + # handles discarded repositories repository = Repository.with_discarded.find_by_id(object.repository_id) # evaluate to false if repository not found return false unless repository - can_read_team?(user, repository.team) + + repository.team.id == team && can_read_team?(user, repository.team) end end end diff --git a/app/services/smart_annotations/tag_to_html.rb b/app/services/smart_annotations/tag_to_html.rb index 9d2e1c3c8..236a51a50 100644 --- a/app/services/smart_annotations/tag_to_html.rb +++ b/app/services/smart_annotations/tag_to_html.rb @@ -7,8 +7,8 @@ module SmartAnnotations class TagToHtml attr_reader :html - def initialize(user, text) - parse(user, text) + def initialize(user, team, text) + parse(user, team, text) end private @@ -19,7 +19,7 @@ module SmartAnnotations tsk: MyModule, rep_item: RepositoryRow }.freeze - def parse(user, text) + def parse(user, team, text) @html = text.gsub(REGEX) do |el| value = extract_values(el) type = value[:object_type] @@ -27,9 +27,10 @@ module SmartAnnotations object = fetch_object(type, value[:object_id]) # handle repository_items edge case if type == 'rep_item' - repository_item(value[:name], user, type, object) + repository_item(value[:name], user, team, type, object) else next unless object && SmartAnnotations::PermissionEval.check(user, + team, type, object) SmartAnnotations::HtmlPreview.html(nil, type, object) @@ -40,9 +41,10 @@ module SmartAnnotations end end - def repository_item(name, user, type, object) + def repository_item(name, user, team, type, object) if object - return unless SmartAnnotations::PermissionEval.check(user, type, object) + return unless SmartAnnotations::PermissionEval.check(user, team, type, object) + return SmartAnnotations::HtmlPreview.html(nil, type, object) end SmartAnnotations::HtmlPreview.html(name, type, object) diff --git a/app/services/smart_annotations/tag_to_text.rb b/app/services/smart_annotations/tag_to_text.rb index 1adc54f2b..e3bbfc85d 100644 --- a/app/services/smart_annotations/tag_to_text.rb +++ b/app/services/smart_annotations/tag_to_text.rb @@ -8,7 +8,7 @@ module SmartAnnotations attr_reader :text def initialize(user, team, text) - parse_items_annotations(user, text) + parse_items_annotations(user, team, text) parse_users_annotations(user, team, @text) end @@ -21,7 +21,7 @@ module SmartAnnotations tsk: MyModule, rep_item: RepositoryRow }.freeze - def parse_items_annotations(user, text) + def parse_items_annotations(user, team, text) @text = text.gsub(ITEMS_REGEX) do |el| value = extract_values(el) type = value[:object_type] @@ -29,9 +29,10 @@ module SmartAnnotations object = fetch_object(type, value[:object_id]) # handle repository_items edge case if type == 'rep_item' - repository_item(value[:name], user, type, object) + repository_item(value[:name], user, team, type, object) else next unless object && SmartAnnotations::PermissionEval.check(user, + team, type, object) SmartAnnotations::TextPreview.text(nil, type, object) @@ -52,9 +53,10 @@ module SmartAnnotations end end - def repository_item(name, user, type, object) + def repository_item(name, user, team, type, object) if object - return unless SmartAnnotations::PermissionEval.check(user, type, object) + return unless SmartAnnotations::PermissionEval.check(user, team, type, object) + return SmartAnnotations::TextPreview.text(nil, type, object) end SmartAnnotations::TextPreview.text(name, type, object) diff --git a/spec/services/smart_annotations/permission_eval_spec.rb b/spec/services/smart_annotations/permission_eval_spec.rb index 0add140ba..9436f440a 100644 --- a/spec/services/smart_annotations/permission_eval_spec.rb +++ b/spec/services/smart_annotations/permission_eval_spec.rb @@ -19,28 +19,28 @@ describe SmartAnnotations::PermissionEval do describe '#validate_prj_permissions/2' do it 'returns a boolean' do - value = subject.send(:validate_prj_permissions, user, project) + value = subject.send(:validate_prj_permissions, user, team, project) expect(value).to be_in([true, false]) end end describe '#validate_exp_permissions/2' do it 'returns a boolean' do - value = subject.send(:validate_exp_permissions, user, experiment) + value = subject.send(:validate_exp_permissions, user, team, experiment) expect(value).to be_in([true, false]) end end describe '#validate_tsk_permissions/2' do it 'returns a boolean' do - value = subject.send(:validate_tsk_permissions, user, task) + value = subject.send(:validate_tsk_permissions, user, team, task) expect(value).to be_in([true, false]) end end describe '#validate_rep_item_permissions/2' do it 'returns a boolean' do - value = subject.send(:validate_rep_item_permissions, user, repository_item) + value = subject.send(:validate_rep_item_permissions, user, team, repository_item) expect(value).to be_in([true, false]) end end diff --git a/spec/services/smart_annotations/tag_to_html_spec.rb b/spec/services/smart_annotations/tag_to_html_spec.rb index 6fbc5665e..f1cf713eb 100644 --- a/spec/services/smart_annotations/tag_to_html_spec.rb +++ b/spec/services/smart_annotations/tag_to_html_spec.rb @@ -11,7 +11,7 @@ describe SmartAnnotations::TagToHtml do let(:text) do "My annotation of [#my project~prj~#{project.id.base62_encode}]" end - let(:subject) { described_class.new(user, text) } + let(:subject) { described_class.new(user, team, text) } describe 'Parsed text' do it 'returns a existing string with smart annotation' do expect(subject.html).to eq(