From 0f77200763e7044c0d0f6ac559bcf694be2e314a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrian=20Schr=C3=B6ter?= Date: Mon, 16 Mar 2015 13:15:03 +0100 Subject: [PATCH] [api] support maintenance incident handling with "noaccess" submissions --- .../app/mixins/submit_request_source_diff.rb | 15 ++------- src/api/app/models/bs_request.rb | 8 ++--- src/api/app/models/bs_request_action.rb | 31 ++++++++++++++++++- .../app/models/bs_request_permission_check.rb | 11 +++---- src/api/test/functional/maintenance_test.rb | 31 ++++++++++--------- 5 files changed, 56 insertions(+), 40 deletions(-) diff --git a/src/api/app/mixins/submit_request_source_diff.rb b/src/api/app/mixins/submit_request_source_diff.rb index 6252822daea..2bca2307f05 100644 --- a/src/api/app/mixins/submit_request_source_diff.rb +++ b/src/api/app/mixins/submit_request_source_diff.rb @@ -22,19 +22,8 @@ def gather_source_packages return [action.source_package] else if action.source_package - sp = Package.get_by_project_and_name(action.source_project, action.source_package) - if sp.class == String - # a remote package - return [action.source_package] - end - if sp - sp.check_source_access! - return [sp.name] - end - if Project.exists_by_name(action.source_project) - # it is a remote project - return [action.source_package] - end + action.source_access_check! + return [action.source_package] else prj = Project.find_by_name(action.source_project) prj.packages.each do |p| diff --git a/src/api/app/models/bs_request.rb b/src/api/app/models/bs_request.rb index db8931d369a..ba21fb5d0fd 100644 --- a/src/api/app/models/bs_request.rb +++ b/src/api/app/models/bs_request.rb @@ -376,8 +376,8 @@ def changestate_accepted(opts) # all maintenance_incident actions go into the same incident project incident_project = nil # .where(type: 'maintenance_incident') self.bs_request_actions.each do |action| - if action.source_project - sprj = Project.get_by_name action.source_project + sprj = Project.find_by_name action.source_project + if action.source_project and action.is_maintenance_release? if sprj.kind_of? Project at = AttribType.find_by_namespace_and_name!("OBS", "EmbargoDate") attrib = sprj.attribs.where(attrib_type_id: at.id).first @@ -394,15 +394,13 @@ def changestate_accepted(opts) end end - tprj = Project.get_by_name action.target_project next unless action.is_maintenance_incident? tprj = Project.get_by_name action.target_project - # create a new incident if needed if tprj.is_maintenance? # create incident if it is a maintenance project - incident_project ||= create_new_maintenance_incident(tprj, nil, self).project + incident_project ||= create_new_maintenance_incident(tprj, nil, self, sprj.nil?).project opts[:check_for_patchinfo] = true unless incident_project.name.start_with?(tprj.name) diff --git a/src/api/app/models/bs_request_action.rb b/src/api/app/models/bs_request_action.rb index 03ff8151b7d..3c94a42d153 100644 --- a/src/api/app/models/bs_request_action.rb +++ b/src/api/app/models/bs_request_action.rb @@ -444,7 +444,8 @@ def source_cleanup end def source_cleanup_delete_path - source_project = Project.find_by_name!(self.source_project) + source_project = Project.find_by_name(self.source_project) + return unless source_project if (source_project.packages.count == 1 and ::Configuration.first.cleanup_empty_projects) or !self.source_package # remove source project, if this is the only package and not a user's home project @@ -888,6 +889,34 @@ def expand_targets(ignore_build_state) return nil end + def source_access_check! + sp = Package.find_by_project_and_name(self.source_project, self.source_package) + if sp.nil? + # either not there or read permission problem + if Package.exists_on_backend?(self.source_package, self.source_project) + # user is not allowed to read the source, but when he can write + # the target, the request creator (who must have permissions to read source) + # wanted the target owner to review it + tprj = Project.find_by_name(self.target_project) + if tprj.nil? or not User.current.can_modify_project? tprj + # produce an error for the source + Package.get_by_project_and_name(self.source_project, self.source_package) + end + return + end + if Project.exists_by_name(self.source_project) + # it is a remote project + return + end + raise UnknownPackage.new "Package #{self.source_project} #{self.source_package} not found" + end + if sp.class == String + # a remote package + return + end + sp.check_source_access! + end + def check_for_expand_errors!(add_revision) return unless [:submit, :maintenance_incident, :maintenance_release].include? self.action_type diff --git a/src/api/app/models/bs_request_permission_check.rb b/src/api/app/models/bs_request_permission_check.rb index 1a59167802f..8db3740d2fb 100644 --- a/src/api/app/models/bs_request_permission_check.rb +++ b/src/api/app/models/bs_request_permission_check.rb @@ -145,7 +145,6 @@ def check_action_target(action) end # full read access checks - @source_project = Project.get_by_name(action.source_project) @target_project = Project.get_by_name(action.target_project) # require a local source package @@ -156,12 +155,10 @@ def check_action_target(action) case action.action_type when :change_devel err = "Local source package is missing for request #{action.bs_request.id} (type #{action.action_type})" - when :submit - # accept also a remote source package - unless @source_project.class == String or Package.exists_by_project_and_name(@source_project.name, action.source_package, - follow_project_links: true, allow_remote_packages: true) - err = "Source package is missing for request #{action.bs_request.id} (type #{action.action_type})" - end + when :set_bugowner + when :add_role + else + action.source_access_check! end raise SourceMissing.new err if err end diff --git a/src/api/test/functional/maintenance_test.rb b/src/api/test/functional/maintenance_test.rb index 92bf21382ec..a75d49e1d6d 100644 --- a/src/api/test/functional/maintenance_test.rb +++ b/src/api/test/functional/maintenance_test.rb @@ -332,23 +332,15 @@ def test_mbranch_and_maintenance_entire_project_request assert_xml_tag :tag => 'collection', :children => { count: 3 } # do the real mbranch for default maintained packages + # test it with "noaccess" login_tom - post '/source', :cmd => 'branch', :package => 'pack2', :noaccess => 1 - assert_response :success - get '/source/home:tom:branches:OBS_Maintained:pack2/_meta' - assert_response :success - assert_xml_tag( :parent => { tag: 'access' }, :tag => 'disable', :content => nil ) - delete '/source/home:tom:branches:OBS_Maintained:pack2' - assert_response :success - post '/source', :cmd => 'branch', :package => 'pack2' + post '/source', :cmd => 'branch', :package => 'pack2', :noaccess => "1" assert_response :success # validate result - get '/source/home:tom:branches:OBS_Maintained:pack2' - assert_response :success get '/source/home:tom:branches:OBS_Maintained:pack2/_meta' assert_response :success - assert_no_xml_tag( :parent => { tag: 'access' }, :tag => 'disable', :content => nil ) + assert_xml_tag( :parent => { tag: 'access' }, :tag => 'disable', :content => nil ) get '/source/home:tom:branches:OBS_Maintained:pack2/pack2.BaseDistro2.0_LinkedUpdateProject/_meta' assert_response :success get '/source/home:tom:branches:OBS_Maintained:pack2/pack2.BaseDistro_Update/_meta' @@ -458,8 +450,8 @@ def test_mbranch_and_maintenance_entire_project_request get '/source/home:tom:branches:OBS_Maintained:pack2/patchinfo/_meta' assert_response :success assert_xml_tag :parent => { tag: 'build' }, :tag => 'enable' - assert_xml_tag :parent => { tag: 'publish' }, :tag => 'enable' assert_xml_tag :parent => { tag: 'useforbuild' }, :tag => 'disable' + assert_no_xml_tag :parent => { tag: 'publish' } # due to noaccess # delete kdelibs package again or incident creation will fail since it does not point to a maintained project. delete '/source/home:tom:branches:OBS_Maintained:pack2/kdelibs.kde4' @@ -498,8 +490,18 @@ def test_mbranch_and_maintenance_entire_project_request oprojectmeta = ActiveXML::Node.new(@response.body) assert_response :success - # accept request + get "/source/home:tom:branches:OBS_Maintained:pack2/_meta" + assert_response :success + assert_xml_tag( :parent => { tag: 'access' }, :tag => 'disable', :content => nil ) + + # switch user, still diffable prepare_request_with_user 'maintenance_coord', 'power' + get "/source/home:tom:branches:OBS_Maintained:pack2/_meta" + assert_response 404 # due to noaccess + post "/request/#{id}?cmd=diff&view=xml", nil + assert_response :success + + # accept request post "/request/#{id}?cmd=changestate&newstate=accepted" assert_response :success @@ -517,6 +519,7 @@ def test_mbranch_and_maintenance_entire_project_request get "/source/#{incidentProject}/_meta" assert_response :success assert_xml_tag( :parent => { tag: 'build' }, :tag => 'disable', :content => nil ) + assert_xml_tag( :parent => { tag: 'access' }, :tag => 'disable', :content => nil ) node = ActiveXML::Node.new(@response.body) # repository definition must be the same, except for the maintenance trigger node.each('repository') do |r| @@ -545,7 +548,7 @@ def test_mbranch_and_maintenance_entire_project_request get "/source/#{incidentProject}/patchinfo/_meta" assert_response :success assert_xml_tag( :tag => 'enable', :parent => { tag: 'build' } ) - assert_xml_tag( :tag => 'enable', :parent => { tag: 'publish' } ) + assert_no_xml_tag( tag: 'publish' ) # noaccess get "/source/#{incidentProject}/patchinfo?view=issues" assert_response :success