Hi, One small comment,
On Wed, 2011-05-04 at 16:45 -0700, Pieter van de Bruggen wrote: > By default, it is useful to permit an individual node to query > information about itself, and there is no good reason to reject > this by default. > > Paired-With: Nick Lewis > > Signed-off-by: Pieter van de Bruggen <[email protected]> > --- > Local-branch: tickets/2.7.x/7179 > conf/auth.conf | 5 +++++ > lib/puppet/network/rest_authconfig.rb | 1 + > spec/unit/network/rest_authconfig_spec.rb | 13 +------------ > 3 files changed, 7 insertions(+), 12 deletions(-) > > diff --git a/conf/auth.conf b/conf/auth.conf > index 431e4b2..cb202a9 100644 > --- a/conf/auth.conf > +++ b/conf/auth.conf > @@ -53,6 +53,11 @@ path ~ ^/catalog/([^/]+)$ > method find > allow $1 > > +# allow nodes to retrieve their own node definition > +path ~ ^/node/([^/]+)$ > +method find > +allow $1 > + > # allow all nodes to access the certificates services > path /certificate_revocation_list/ca > method find > diff --git a/lib/puppet/network/rest_authconfig.rb > b/lib/puppet/network/rest_authconfig.rb > index cf76978..dfe8f85 100644 > --- a/lib/puppet/network/rest_authconfig.rb > +++ b/lib/puppet/network/rest_authconfig.rb > @@ -8,6 +8,7 @@ module Puppet > > DEFAULT_ACL = [ > { :acl => "~ ^\/catalog\/([^\/]+)$", :method => :find, :allow => '$1', > :authenticated => true }, > + { :acl => "~ ^\/node\/([^\/]+)$", :method => :find, :allow => '$1', > :authenticated => true }, > # this one will allow all file access, and thus delegate > # to fileserver.conf > { :acl => "/file" }, > diff --git a/spec/unit/network/rest_authconfig_spec.rb > b/spec/unit/network/rest_authconfig_spec.rb > index 499a14b..e140399 100755 > --- a/spec/unit/network/rest_authconfig_spec.rb > +++ b/spec/unit/network/rest_authconfig_spec.rb > @@ -5,18 +5,7 @@ require 'puppet/network/rest_authconfig' > > describe Puppet::Network::RestAuthConfig do > > - DEFAULT_ACL = [ > - { :acl => "~ ^\/catalog\/([^\/]+)$", :method => :find, :allow => '$1', > :authenticated => true }, > - # this one will allow all file access, and thus delegate > - # to fileserver.conf > - { :acl => "/file" }, > - { :acl => "/certificate_revocation_list/ca", :method => :find, > :authenticated => true }, > - { :acl => "/report", :method => :save, :authenticated => true }, > - { :acl => "/certificate/ca", :method => :find, :authenticated => false }, > - { :acl => "/certificate/", :method => :find, :authenticated => false }, > - { :acl => "/certificate_request", :method => [:find, :save], > :authenticated => false }, > - { :acl => "/status", :method => [:find], :authenticated => true }, > - ] > + DEFAULT_ACL = Puppet::Network::RestAuthConfig::DEFAULT_ACL While I understand why you're doing this, the problem is that you're losing some test rationales: testing that the above access control entries are still correctly applied. If someone goes to change the original catalog acl with an invalid rule the new test won't fail. -- Brice Figureau Follow the latest Puppet Community evolutions on www.planetpuppet.org! -- 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.
