From cf5505d95fc12df7837cf406774fcea8dc7eb2c6 Mon Sep 17 00:00:00 2001 From: Jure Grabnar Date: Wed, 20 Mar 2019 10:52:58 +0100 Subject: [PATCH 1/3] Add missing inventory name in notification Closes SCI-3149 --- app/controllers/repository_rows_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/repository_rows_controller.rb b/app/controllers/repository_rows_controller.rb index 6a5ac04fd..887e00ce4 100644 --- a/app/controllers/repository_rows_controller.rb +++ b/app/controllers/repository_rows_controller.rb @@ -410,7 +410,7 @@ class RepositoryRowsController < ApplicationController user: current_user.full_name, column: cell.repository_column.name, record: record.name, - repository: record.repository), + repository: record.repository.name), message: t('notifications.repository_annotation_message_html', record: link_to(record.name, table_url), column: link_to(cell.repository_column.name, table_url)) From 5de51f9c58b70a06f9b5ff68b8dc47da1a2e829b Mon Sep 17 00:00:00 2001 From: Jure Grabnar Date: Wed, 20 Mar 2019 12:58:22 +0100 Subject: [PATCH 2/3] 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( From f3e6a351210344a44dd078181174182e5ff1dc54 Mon Sep 17 00:00:00 2001 From: Jure Grabnar Date: Thu, 21 Mar 2019 10:45:21 +0100 Subject: [PATCH 3/3] Update PermissionEval tests with team check --- .../smart_annotations/permission_eval_spec.rb | 54 ++++++++++++++++--- 1 file changed, 48 insertions(+), 6 deletions(-) diff --git a/spec/services/smart_annotations/permission_eval_spec.rb b/spec/services/smart_annotations/permission_eval_spec.rb index 9436f440a..a336ed25b 100644 --- a/spec/services/smart_annotations/permission_eval_spec.rb +++ b/spec/services/smart_annotations/permission_eval_spec.rb @@ -5,8 +5,10 @@ describe SmartAnnotations::PermissionEval do let(:subject) { described_class } let(:user) { create :user } let(:team) { create :team } - let(:user_team) { create :user_team, user: user, team: team, role: 2 } - let(:project) { create :project, name: 'my project' } + let(:another_team) { create :team } + let!(:user_team) { create :user_team, user: user, team: team, role: :admin } + let(:project) { create :project, name: 'my project', team: team } + let!(:user_project) { create :user_project, :owner, project: project, user: user } let(:experiment) do create :experiment, name: 'my experiment', project: project, @@ -19,29 +21,69 @@ describe SmartAnnotations::PermissionEval do describe '#validate_prj_permissions/2' do it 'returns a boolean' do - value = subject.send(:validate_prj_permissions, user, team, project) + value = subject.__send__(:validate_prj_permissions, user, team, project) expect(value).to be_in([true, false]) end + + it 'returns false on wrong team' do + value = subject.__send__(:validate_prj_permissions, user, another_team, project) + expect(value).to be false + end + + it 'returns true on the same team' do + value = subject.__send__(:validate_prj_permissions, user, team, project) + expect(value).to be true + end end describe '#validate_exp_permissions/2' do it 'returns a boolean' do - value = subject.send(:validate_exp_permissions, user, team, experiment) + value = subject.__send__(:validate_exp_permissions, user, team, experiment) expect(value).to be_in([true, false]) end + + it 'returns false on wrong team' do + value = subject.__send__(:validate_exp_permissions, user, another_team, experiment) + expect(value).to be false + end + + it 'returns true on the same team' do + value = subject.__send__(:validate_exp_permissions, user, team, experiment) + expect(value).to be true + end end describe '#validate_tsk_permissions/2' do it 'returns a boolean' do - value = subject.send(:validate_tsk_permissions, user, team, task) + value = subject.__send__(:validate_tsk_permissions, user, team, task) expect(value).to be_in([true, false]) end + + it 'returns false on wrong team' do + value = subject.__send__(:validate_tsk_permissions, user, another_team, task) + expect(value).to be false + end + + it 'returns true on the same team' do + value = subject.__send__(:validate_tsk_permissions, user, team, task) + expect(value).to be true + end end describe '#validate_rep_item_permissions/2' do it 'returns a boolean' do - value = subject.send(:validate_rep_item_permissions, user, team, repository_item) + value = subject.__send__(:validate_rep_item_permissions, user, team, repository_item) expect(value).to be_in([true, false]) end + + it 'returns false on wrong team' do + value = subject.__send__(:validate_rep_item_permissions, user, another_team, repository_item) + expect(value).to be false + end + + it 'returns true on the same team' do + value = subject.__send__(:validate_rep_item_permissions, user, team, repository_item) + expect(value).to be true + end end end