[opensuse-buildservice] [PATCH] small fix for the acl+remoteproject handling
Hi, I just wrote a patch to fix some small issues with acl+remoteprojects: - app/controllers/build_controller.rb * def index: added support for remoteprojects (also added acl code for hidden remoteprojects) - app/controllers/source_controller.rb * def index_project: added acl code for hidden remoteprojects - app/models/db_project.rb * def is_remote_project?, def find_remote_project: added "skip_access=false" parameter to skip acl checks (by default they're enabled) - added new testcases Btw sometimes it's possible to "detect" hidden projects. For instance compare the output of GET /source/<hiddenproject> and GET /source/<non-existent> (I'm not quite sure if this is intended) Marcus --- src/api/app/controllers/build_controller.rb | 6 +++ src/api/app/controllers/source_controller.rb | 4 ++ src/api/app/models/db_project.rb | 16 +++++--- src/api/script/start_test_backend | 1 + src/api/test/fixtures/db_projects.yml | 9 +++++ src/api/test/fixtures/flags.yml | 6 +++ .../fixtures/project_user_role_relationships.yml | 5 +++ src/api/test/functional/interconnect_test.rb | 39 ++++++++++++++++++++ 8 files changed, 80 insertions(+), 6 deletions(-) diff --git a/src/api/app/controllers/build_controller.rb b/src/api/app/controllers/build_controller.rb index 7c4d3dd..67ff364 100644 --- a/src/api/app/controllers/build_controller.rb +++ b/src/api/app/controllers/build_controller.rb @@ -4,6 +4,12 @@ class BuildController < ApplicationController valid_http_methods :get, :post, :put prj = DbProject.find_by_name params[:project] + if prj.nil? and DbProject.is_remote_project?(params[:project], skip_access=true) + lpro, rpro = DbProject.find_remote_project(params[:project], skip_access=true) + prj = lpro if DbProject.check_access? lpro + # package is irrelevant for a remoteproject + params.delete :package + end pkg = DbPackage.find_by_project_and_name( params[:project], params[:package] ) if prj and params[:package] # todo: check if prj.nil?/pkg.nil? is sufficient diff --git a/src/api/app/controllers/source_controller.rb b/src/api/app/controllers/source_controller.rb index 06bf0ea..0b3e84c 100644 --- a/src/api/app/controllers/source_controller.rb +++ b/src/api/app/controllers/source_controller.rb @@ -79,6 +79,10 @@ class SourceController < ApplicationController deleted = params.has_key? :deleted pro = DbProject.find_by_name project_name hidden = DbProject.is_hidden?(project_name) + if pro.nil? and DbProject.is_remote_project?(project_name, skip_access=true) + lpro, rpro = DbProject.find_remote_project(project_name, skip_access=true) + hidden = DbProject.is_hidden?(lpro.name) && !DbProject.check_access?(lpro) + end # access checks #-------------- diff --git a/src/api/app/models/db_project.rb b/src/api/app/models/db_project.rb index ec72a62..b687869 100644 --- a/src/api/app/models/db_project.rb +++ b/src/api/app/models/db_project.rb @@ -36,10 +36,9 @@ class DbProject < ActiveRecord::Base class << self - def is_remote_project?(name) - lpro, rpro = find_remote_project(name) - return true unless lpro.nil? or lpro.remoteurl.nil? - return false + def is_remote_project?(name, skip_access=false) + lpro, rpro = find_remote_project(name, skip_access) + !lpro.nil? and !lpro.remoteurl.nil? end def is_hidden?(name) @@ -191,7 +190,7 @@ class DbProject < ActiveRecord::Base return dbp end - def find_remote_project(name) + def find_remote_project(name, skip_access=false) return nil unless name fragments = name.split(/:/) local_project = String.new @@ -201,7 +200,12 @@ class DbProject < ActiveRecord::Base remote_project = [fragments.pop, remote_project].compact.join ":" local_project = fragments.join ":" logger.debug "checking local project #{local_project}, remote_project #{remote_project}" - lpro = DbProject.find_by_name local_project + if skip_access + # hmm calling a private class method is not the best idea.. + lpro = find_initial :conditions => ["name = BINARY ?", local_project] + else + lpro = DbProject.find_by_name local_project + end return lpro, remote_project unless lpro.nil? or lpro.remoteurl.nil? end return nil diff --git a/src/api/script/start_test_backend b/src/api/script/start_test_backend index 9878c6c..349bbe9 100755 --- a/src/api/script/start_test_backend +++ b/src/api/script/start_test_backend @@ -154,6 +154,7 @@ Suse::Backend.put( '/source/LocalProject/_meta', DbProject.find_by_name('LocalPr Suse::Backend.put( '/source/LocalProject/remotepackage/_meta', DbPackage.find_by_project_and_name("LocalProject", "remotepackage").to_axml) Suse::Backend.put( '/source/LocalProject/remotepackage/_link', "") Suse::Backend.put( '/source/RemoteInstance/_meta', DbProject.find_by_name('RemoteInstance').to_axml) +Suse::Backend.put( '/source/HiddenRemoteInstance/_meta', DbProject.find_by_name('HiddenRemoteInstance').to_axml) Suse::Backend.put( '/source/UseRemoteInstance/_meta', DbProject.find_by_name('UseRemoteInstance').to_axml) Suse::Backend.put( '/source/home:adrian:BaseDistro/_meta', DbProject.find_by_name('home:adrian:BaseDistro').to_axml) Suse::Backend.put( '/source/home:adrian:ProtectionTest/_meta', DbProject.find_by_name('home:adrian:ProtectionTest').to_axml) diff --git a/src/api/test/fixtures/db_projects.yml b/src/api/test/fixtures/db_projects.yml index 45adf8b..cad7774 100644 --- a/src/api/test/fixtures/db_projects.yml +++ b/src/api/test/fixtures/db_projects.yml @@ -147,6 +147,15 @@ homeadrianProtectionTest: description: "" created_at: 2005-01-01 00:00:01 +HiddenRemoteInstance: + name: HiddenRemoteInstance + updated_at: 2005-01-01 00:00:01 + title: Remoteurl project which is hidden (access disabled) + id: 3006 + remoteurl: http://localhost:3200 + description: "for testing hidden remoteurl projects (access disabled)" + created_at: 2005-01-01 00:00:01 + # For testing remote project handling (we are our own remote instance) RemoteInstance: name: RemoteInstance diff --git a/src/api/test/fixtures/flags.yml b/src/api/test/fixtures/flags.yml index c8ecf2a..66059fc 100644 --- a/src/api/test/fixtures/flags.yml +++ b/src/api/test/fixtures/flags.yml @@ -77,6 +77,12 @@ hide_project: flag: access position: 1 +hide_remoteurlproject: + status: "disable" + db_project_id: 3006 + flag: access + position: 1 + source_protected_project: status: "disable" db_project_id: 3001 diff --git a/src/api/test/fixtures/project_user_role_relationships.yml b/src/api/test/fixtures/project_user_role_relationships.yml index 97c57ca..564f8dd 100644 --- a/src/api/test/fixtures/project_user_role_relationships.yml +++ b/src/api/test/fixtures/project_user_role_relationships.yml @@ -72,6 +72,11 @@ hiddenproject_maintainer_homer: bs_user_id: 46 role_id: 3 +hiddenremoteurlproject_maintainer_homer: + db_project_id: 3006 + bs_user_id: 46 + role_id: 3 + sourceaccess_project_maintainer_homer: db_project_id: 3001 bs_user_id: 49 diff --git a/src/api/test/functional/interconnect_test.rb b/src/api/test/functional/interconnect_test.rb index fe3c7d4..bffb122 100644 --- a/src/api/test/functional/interconnect_test.rb +++ b/src/api/test/functional/interconnect_test.rb @@ -240,4 +240,43 @@ end assert_response :success end + def test_get_packagelist_with_hidden_remoteurlproject + prepare_request_with_user "tom", "thunder" + get "/source/HiddenRemoteInstance" + assert_response 404 + get "/source/HiddenRemoteInstance:BaseDistro" + assert_response 404 + ActionController::IntegrationTest::reset_auth + prepare_request_with_user "hidden_homer", "homer" + get "/source/HiddenRemoteInstance" + assert_response :success + get "/source/HiddenRemoteInstance:BaseDistro" + assert_response :success + end + + def test_read_access_hidden_remoteurlproject_index + prepare_request_with_user "tom", "thunder" + get "/build/HiddenRemoteInstance:BaseDistro/BaseDistro_repo/i586/_repository" + assert_response 404 + get "/build/HiddenRemoteInstance:BaseDistro/BaseDistro_repo/i586/_repository?view=cache" + assert_response 404 + get "/build/HiddenRemoteInstance:BaseDistro/BaseDistro_repo/i586/_repository?view=binaryversions" + assert_response 404 + get "/build/HiddenRemoteInstance:BaseDistro/BaseDistro_repo/i586/_repository?view=cpio" + assert_response 404 + get "/build/HiddenRemoteInstance:BaseDistro/BaseDistro_repo/i586/pack1" + assert_response 404 + ActionController::IntegrationTest::reset_auth + prepare_request_with_user "hidden_homer", "homer" + get "/build/HiddenRemoteInstance:BaseDistro/BaseDistro_repo/i586/_repository" + assert_response :success + get "/build/HiddenRemoteInstance:BaseDistro/BaseDistro_repo/i586/_repository?view=cache" + assert_response :success + get "/build/HiddenRemoteInstance:BaseDistro/BaseDistro_repo/i586/_repository?view=binaryversions" + assert_response :success + get "/build/HiddenRemoteInstance:BaseDistro/BaseDistro_repo/i586/_repository?view=cpio" + assert_response :success + get "/build/HiddenRemoteInstance:BaseDistro/BaseDistro_repo/i586/pack1" + assert_response :success + end end -- 1.6.0.2 -- To unsubscribe, e-mail: opensuse-buildservice+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-buildservice+help@opensuse.org
Hi Marcus, I am also testing the upstream version of OBS with LDAP feature and ACLs. Can you point me to the test cases you were talking about? Thanks, Senthil M -----Original Message----- From: ext Marcus Huewe [mailto:suse-tux@gmx.de] Sent: 27 December, 2010 20:36 To: opensuse-buildservice@opensuse.org Subject: [opensuse-buildservice] [PATCH] small fix for the acl+remoteproject handling Hi, I just wrote a patch to fix some small issues with acl+remoteprojects: - app/controllers/build_controller.rb * def index: added support for remoteprojects (also added acl code for hidden remoteprojects) - app/controllers/source_controller.rb * def index_project: added acl code for hidden remoteprojects - app/models/db_project.rb * def is_remote_project?, def find_remote_project: added "skip_access=false" parameter to skip acl checks (by default they're enabled) - added new testcases Btw sometimes it's possible to "detect" hidden projects. For instance compare the output of GET /source/<hiddenproject> and GET /source/<non-existent> (I'm not quite sure if this is intended) Marcus --- src/api/app/controllers/build_controller.rb | 6 +++ src/api/app/controllers/source_controller.rb | 4 ++ src/api/app/models/db_project.rb | 16 +++++--- src/api/script/start_test_backend | 1 + src/api/test/fixtures/db_projects.yml | 9 +++++ src/api/test/fixtures/flags.yml | 6 +++ .../fixtures/project_user_role_relationships.yml | 5 +++ src/api/test/functional/interconnect_test.rb | 39 ++++++++++++++++++++ 8 files changed, 80 insertions(+), 6 deletions(-) diff --git a/src/api/app/controllers/build_controller.rb b/src/api/app/controllers/build_controller.rb index 7c4d3dd..67ff364 100644 --- a/src/api/app/controllers/build_controller.rb +++ b/src/api/app/controllers/build_controller.rb @@ -4,6 +4,12 @@ class BuildController < ApplicationController valid_http_methods :get, :post, :put prj = DbProject.find_by_name params[:project] + if prj.nil? and DbProject.is_remote_project?(params[:project], skip_access=true) + lpro, rpro = DbProject.find_remote_project(params[:project], skip_access=true) + prj = lpro if DbProject.check_access? lpro + # package is irrelevant for a remoteproject + params.delete :package + end pkg = DbPackage.find_by_project_and_name( params[:project], params[:package] ) if prj and params[:package] # todo: check if prj.nil?/pkg.nil? is sufficient diff --git a/src/api/app/controllers/source_controller.rb b/src/api/app/controllers/source_controller.rb index 06bf0ea..0b3e84c 100644 --- a/src/api/app/controllers/source_controller.rb +++ b/src/api/app/controllers/source_controller.rb @@ -79,6 +79,10 @@ class SourceController < ApplicationController deleted = params.has_key? :deleted pro = DbProject.find_by_name project_name hidden = DbProject.is_hidden?(project_name) + if pro.nil? and DbProject.is_remote_project?(project_name, skip_access=true) + lpro, rpro = DbProject.find_remote_project(project_name, skip_access=true) + hidden = DbProject.is_hidden?(lpro.name) && !DbProject.check_access?(lpro) + end # access checks #-------------- diff --git a/src/api/app/models/db_project.rb b/src/api/app/models/db_project.rb index ec72a62..b687869 100644 --- a/src/api/app/models/db_project.rb +++ b/src/api/app/models/db_project.rb @@ -36,10 +36,9 @@ class DbProject < ActiveRecord::Base class << self - def is_remote_project?(name) - lpro, rpro = find_remote_project(name) - return true unless lpro.nil? or lpro.remoteurl.nil? - return false + def is_remote_project?(name, skip_access=false) + lpro, rpro = find_remote_project(name, skip_access) + !lpro.nil? and !lpro.remoteurl.nil? end def is_hidden?(name) @@ -191,7 +190,7 @@ class DbProject < ActiveRecord::Base return dbp end - def find_remote_project(name) + def find_remote_project(name, skip_access=false) return nil unless name fragments = name.split(/:/) local_project = String.new @@ -201,7 +200,12 @@ class DbProject < ActiveRecord::Base remote_project = [fragments.pop, remote_project].compact.join ":" local_project = fragments.join ":" logger.debug "checking local project #{local_project}, remote_project #{remote_project}" - lpro = DbProject.find_by_name local_project + if skip_access + # hmm calling a private class method is not the best idea.. + lpro = find_initial :conditions => ["name = BINARY ?", local_project] + else + lpro = DbProject.find_by_name local_project + end return lpro, remote_project unless lpro.nil? or lpro.remoteurl.nil? end return nil diff --git a/src/api/script/start_test_backend b/src/api/script/start_test_backend index 9878c6c..349bbe9 100755 --- a/src/api/script/start_test_backend +++ b/src/api/script/start_test_backend @@ -154,6 +154,7 @@ Suse::Backend.put( '/source/LocalProject/_meta', DbProject.find_by_name('LocalPr Suse::Backend.put( '/source/LocalProject/remotepackage/_meta', DbPackage.find_by_project_and_name("LocalProject", "remotepackage").to_axml) Suse::Backend.put( '/source/LocalProject/remotepackage/_link', "") Suse::Backend.put( '/source/RemoteInstance/_meta', DbProject.find_by_name('RemoteInstance').to_axml) +Suse::Backend.put( '/source/HiddenRemoteInstance/_meta', DbProject.find_by_name('HiddenRemoteInstance').to_axml) Suse::Backend.put( '/source/UseRemoteInstance/_meta', DbProject.find_by_name('UseRemoteInstance').to_axml) Suse::Backend.put( '/source/home:adrian:BaseDistro/_meta', DbProject.find_by_name('home:adrian:BaseDistro').to_axml) Suse::Backend.put( '/source/home:adrian:ProtectionTest/_meta', DbProject.find_by_name('home:adrian:ProtectionTest').to_axml) diff --git a/src/api/test/fixtures/db_projects.yml b/src/api/test/fixtures/db_projects.yml index 45adf8b..cad7774 100644 --- a/src/api/test/fixtures/db_projects.yml +++ b/src/api/test/fixtures/db_projects.yml @@ -147,6 +147,15 @@ homeadrianProtectionTest: description: "" created_at: 2005-01-01 00:00:01 +HiddenRemoteInstance: + name: HiddenRemoteInstance + updated_at: 2005-01-01 00:00:01 + title: Remoteurl project which is hidden (access disabled) + id: 3006 + remoteurl: http://localhost:3200 + description: "for testing hidden remoteurl projects (access disabled)" + created_at: 2005-01-01 00:00:01 + # For testing remote project handling (we are our own remote instance) RemoteInstance: name: RemoteInstance diff --git a/src/api/test/fixtures/flags.yml b/src/api/test/fixtures/flags.yml index c8ecf2a..66059fc 100644 --- a/src/api/test/fixtures/flags.yml +++ b/src/api/test/fixtures/flags.yml @@ -77,6 +77,12 @@ hide_project: flag: access position: 1 +hide_remoteurlproject: + status: "disable" + db_project_id: 3006 + flag: access + position: 1 + source_protected_project: status: "disable" db_project_id: 3001 diff --git a/src/api/test/fixtures/project_user_role_relationships.yml b/src/api/test/fixtures/project_user_role_relationships.yml index 97c57ca..564f8dd 100644 --- a/src/api/test/fixtures/project_user_role_relationships.yml +++ b/src/api/test/fixtures/project_user_role_relationships.yml @@ -72,6 +72,11 @@ hiddenproject_maintainer_homer: bs_user_id: 46 role_id: 3 +hiddenremoteurlproject_maintainer_homer: + db_project_id: 3006 + bs_user_id: 46 + role_id: 3 + sourceaccess_project_maintainer_homer: db_project_id: 3001 bs_user_id: 49 diff --git a/src/api/test/functional/interconnect_test.rb b/src/api/test/functional/interconnect_test.rb index fe3c7d4..bffb122 100644 --- a/src/api/test/functional/interconnect_test.rb +++ b/src/api/test/functional/interconnect_test.rb @@ -240,4 +240,43 @@ end assert_response :success end + def test_get_packagelist_with_hidden_remoteurlproject + prepare_request_with_user "tom", "thunder" + get "/source/HiddenRemoteInstance" + assert_response 404 + get "/source/HiddenRemoteInstance:BaseDistro" + assert_response 404 + ActionController::IntegrationTest::reset_auth + prepare_request_with_user "hidden_homer", "homer" + get "/source/HiddenRemoteInstance" + assert_response :success + get "/source/HiddenRemoteInstance:BaseDistro" + assert_response :success + end + + def test_read_access_hidden_remoteurlproject_index + prepare_request_with_user "tom", "thunder" + get "/build/HiddenRemoteInstance:BaseDistro/BaseDistro_repo/i586/_repository" + assert_response 404 + get "/build/HiddenRemoteInstance:BaseDistro/BaseDistro_repo/i586/_repository?view=cache" + assert_response 404 + get "/build/HiddenRemoteInstance:BaseDistro/BaseDistro_repo/i586/_repository?view=binaryversions" + assert_response 404 + get "/build/HiddenRemoteInstance:BaseDistro/BaseDistro_repo/i586/_repository?view=cpio" + assert_response 404 + get "/build/HiddenRemoteInstance:BaseDistro/BaseDistro_repo/i586/pack1" + assert_response 404 + ActionController::IntegrationTest::reset_auth + prepare_request_with_user "hidden_homer", "homer" + get "/build/HiddenRemoteInstance:BaseDistro/BaseDistro_repo/i586/_repository" + assert_response :success + get "/build/HiddenRemoteInstance:BaseDistro/BaseDistro_repo/i586/_repository?view=cache" + assert_response :success + get "/build/HiddenRemoteInstance:BaseDistro/BaseDistro_repo/i586/_repository?view=binaryversions" + assert_response :success + get "/build/HiddenRemoteInstance:BaseDistro/BaseDistro_repo/i586/_repository?view=cpio" + assert_response :success + get "/build/HiddenRemoteInstance:BaseDistro/BaseDistro_repo/i586/pack1" + assert_response :success + end end -- 1.6.0.2 -- To unsubscribe, e-mail: opensuse-buildservice+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-buildservice+help@opensuse.org -- To unsubscribe, e-mail: opensuse-buildservice+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-buildservice+help@opensuse.org
Hi, On 2010-12-28 08:54:26 +0000, ext-senthil.muthukalai@nokia.com wrote:
I am also testing the upstream version of OBS with LDAP feature and ACLs. Can you point me to the test cases you were talking about?
The testcases for this issue are part of the patch (see api/test/functional/interconnect_test.rb and api/test/fixtures/*). All other testcases+fixtures are available in the api/test directory. Marcus -- To unsubscribe, e-mail: opensuse-buildservice+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-buildservice+help@opensuse.org
Am Montag, 27. Dezember 2010, 19:35:56 schrieb Marcus Huewe:
Hi,
I just wrote a patch to fix some small issues with acl+remoteprojects:
Ok, please apply.
- app/controllers/build_controller.rb * def index: added support for remoteprojects (also added acl code for hidden remoteprojects) - app/controllers/source_controller.rb * def index_project: added acl code for hidden remoteprojects - app/models/db_project.rb * def is_remote_project?, def find_remote_project: added "skip_access=false" parameter to skip acl checks (by default they're enabled)
I'm a bit unhappy with the easy override. Perhaps use an own function name ? But up to you.
- added new testcases
Btw sometimes it's possible to "detect" hidden projects. For instance compare the output of
GET /source/<hiddenproject> and GET /source/<non-existent>
(I'm not quite sure if this is intended)
No, not intended. Error messages should and will be synchronized to rule out such a case. Best, Jan-Simon -- To unsubscribe, e-mail: opensuse-buildservice+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-buildservice+help@opensuse.org
On 2010-12-29 08:57:01 +0100, Jan-Simon Möller wrote:
Am Montag, 27. Dezember 2010, 19:35:56 schrieb Marcus Huewe:
Hi,
I just wrote a patch to fix some small issues with acl+remoteprojects:
Ok, please apply.
Ok - done.
- app/controllers/build_controller.rb * def index: added support for remoteprojects (also added acl code for hidden remoteprojects) - app/controllers/source_controller.rb * def index_project: added acl code for hidden remoteprojects - app/models/db_project.rb * def is_remote_project?, def find_remote_project: added "skip_access=false" parameter to skip acl checks (by default they're enabled)
I'm a bit unhappy with the easy override. Perhaps use an own function name ? But up to you.
Hmm I wanted to avoid code duplication that's why I simply added the new parameter. Marcus -- To unsubscribe, e-mail: opensuse-buildservice+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-buildservice+help@opensuse.org
participants (4)
-
ext-senthil.muthukalai＠nokia.com -
Jan-Simon Möller -
Marcus Huewe -
Marcus Hüwe