When fixing #2424, we were adding a global allow (ie allow(*)) to the plugins/modules mount. Unfortunately global allow always win against any other rules that can be defined in fileserver.conf.
This patch makes sure we add those global allow entries only if we didn't get any rules from fileserver.conf Signed-off-by: Brice Figureau <[email protected]> --- lib/puppet/file_serving/configuration.rb | 4 ++-- lib/puppet/network/authstore.rb | 5 +++++ spec/unit/file_serving/configuration.rb | 18 ++++++++++++++++-- spec/unit/network/authstore.rb | 30 ++++++++++++++++++++++++++++++ 4 files changed, 53 insertions(+), 4 deletions(-) diff --git a/lib/puppet/file_serving/configuration.rb b/lib/puppet/file_serving/configuration.rb index ac54a7a..9034cae 100644 --- a/lib/puppet/file_serving/configuration.rb +++ b/lib/puppet/file_serving/configuration.rb @@ -96,9 +96,9 @@ class Puppet::FileServing::Configuration def mk_default_mounts @mounts["modules"] ||= Mount::Modules.new("modules") - @mounts["modules"].allow('*') + @mounts["modules"].allow('*') if @mounts["modules"].empty? @mounts["plugins"] ||= Mount::Plugins.new("plugins") - @mounts["plugins"].allow('*') + @mounts["plugins"].allow('*') if @mounts["plugins"].empty? end # Read the configuration file. diff --git a/lib/puppet/network/authstore.rb b/lib/puppet/network/authstore.rb index ab31fae..fb3d014 100755 --- a/lib/puppet/network/authstore.rb +++ b/lib/puppet/network/authstore.rb @@ -63,6 +63,11 @@ module Puppet @globalallow end + # does this auth store has any rules? + def empty? + @globalallow.nil? && @declarations.size == 0 + end + def initialize @globalallow = nil @declarations = [] diff --git a/spec/unit/file_serving/configuration.rb b/spec/unit/file_serving/configuration.rb index f6acfad..4621a0c 100755 --- a/spec/unit/file_serving/configuration.rb +++ b/spec/unit/file_serving/configuration.rb @@ -104,17 +104,31 @@ describe Puppet::FileServing::Configuration do it "should allow all access to modules and plugins if no fileserver.conf exists" do FileTest.expects(:exists?).returns false # the file doesn't exist - modules = stub 'modules' + modules = stub 'modules', :empty? => true Puppet::FileServing::Mount::Modules.stubs(:new).returns(modules) modules.expects(:allow).with('*') - plugins = stub 'plugins' + plugins = stub 'plugins', :empty? => true Puppet::FileServing::Mount::Plugins.stubs(:new).returns(plugins) plugins.expects(:allow).with('*') Puppet::FileServing::Configuration.create end + it "should not allow access from all to modules and plugins if the fileserver.conf provided some rules" do + FileTest.expects(:exists?).returns false # the file doesn't exist + + modules = stub 'modules', :empty? => false + Puppet::FileServing::Mount::Modules.stubs(:new).returns(modules) + modules.expects(:allow).with('*').never + + plugins = stub 'plugins', :empty? => false + Puppet::FileServing::Mount::Plugins.stubs(:new).returns(plugins) + plugins.expects(:allow).with('*').never + + Puppet::FileServing::Configuration.create + end + it "should add modules and plugins mounts even if they are not returned by the parser" do @parser.expects(:parse).returns("one" => mock("mount")) FileTest.expects(:exists?).returns true # the file doesn't exist diff --git a/spec/unit/network/authstore.rb b/spec/unit/network/authstore.rb index 55b2c7b..4087b28 100644 --- a/spec/unit/network/authstore.rb +++ b/spec/unit/network/authstore.rb @@ -4,6 +4,36 @@ require File.dirname(__FILE__) + '/../../spec_helper' require 'puppet/network/authconfig' +describe Puppet::Network::AuthStore do + describe "when checking if the acl has some entries" do + before :each do + @authstore = Puppet::Network::AuthStore.new + end + + it "should be empty if no ACE have been entered" do + @authstore.should be_empty + end + + it "should not be empty if it is a global allow" do + @authstore.allow('*') + + @authstore.should_not be_empty + end + + it "should not be empty if at least one allow has been entered" do + @authstore.allow('1.1.1.*') + + @authstore.should_not be_empty + end + + it "should not be empty if at least one deny has been entered" do + @authstore.deny('1.1.1.*') + + @authstore.should_not be_empty + end + end +end + describe Puppet::Network::AuthStore::Declaration do ['100.101.99.98','100.100.100.100','1.2.3.4','11.22.33.44'].each { |ip| -- 1.6.4 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---
