+1 On Oct 26, 2009, at 12:07 PM, Brice Figureau wrote:
> > 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 > > > > -- A computer lets you make more mistakes faster than any invention in human history--with the possible exceptions of handguns and tequila. -- Mitch Ratcliffe --------------------------------------------------------------------- Luke Kanies | http://reductivelabs.com | http://madstop.com --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
