diff options
| -rw-r--r-- | lib/puppet/network/authconfig.rb | 5 | ||||
| -rw-r--r-- | lib/puppet/network/http/handler.rb | 14 | ||||
| -rw-r--r-- | lib/puppet/network/rest_authconfig.rb | 9 | ||||
| -rw-r--r-- | lib/puppet/network/rest_authorization.rb | 27 | ||||
| -rwxr-xr-x | lib/puppet/network/rights.rb | 95 | ||||
| -rwxr-xr-x | spec/integration/indirector/certificate/rest.rb | 2 | ||||
| -rwxr-xr-x | spec/integration/indirector/certificate_request/rest.rb | 2 | ||||
| -rwxr-xr-x | spec/integration/indirector/certificate_revocation_list/rest.rb | 2 | ||||
| -rw-r--r-- | spec/integration/indirector/report/rest.rb | 2 | ||||
| -rwxr-xr-x | spec/integration/indirector/rest.rb | 4 | ||||
| -rwxr-xr-x | spec/unit/network/authconfig.rb | 28 | ||||
| -rwxr-xr-x | spec/unit/network/http/handler.rb | 8 | ||||
| -rwxr-xr-x | spec/unit/network/rest_authconfig.rb | 2 | ||||
| -rwxr-xr-x | spec/unit/network/rest_authorization.rb | 21 | ||||
| -rwxr-xr-x | spec/unit/network/rights.rb | 129 |
15 files changed, 213 insertions, 137 deletions
diff --git a/lib/puppet/network/authconfig.rb b/lib/puppet/network/authconfig.rb index 3e0807ad1..3e40c9d7c 100644 --- a/lib/puppet/network/authconfig.rb +++ b/lib/puppet/network/authconfig.rb @@ -32,9 +32,8 @@ module Puppet return @rights[name].allowed?(request.name, request.ip) elsif @rights.include?(namespace) return @rights[namespace].allowed?(request.name, request.ip) - else - return false end + false end # Does the file exist? Puppetmasterd does not require it, but @@ -111,7 +110,7 @@ module Puppet name = $3 end name.chomp! - right = newrights.newright(name, count) + right = newrights.newright(name, count, @file) when /^\s*(allow|deny|method|environment)\s+(.+)$/ parse_right_directive(right, $1, $2, count) else diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb index 20234b2da..c6d34fe43 100644 --- a/lib/puppet/network/http/handler.rb +++ b/lib/puppet/network/http/handler.rb @@ -3,6 +3,7 @@ end require 'puppet/network/http/api/v1' require 'puppet/network/rest_authorization' +require 'puppet/network/rights' module Puppet::Network::HTTP::Handler include Puppet::Network::HTTP::API::V1 @@ -40,11 +41,9 @@ module Puppet::Network::HTTP::Handler def process(request, response) indirection_request = uri2indirection(http_method(request), path(request), params(request)) - if authorized?(indirection_request) - send("do_%s" % indirection_request.method, indirection_request, request, response) - else - return do_exception(response, "Request forbidden by configuration %s %s" % [indirection_request.indirection_name, indirection_request.key], 403) - end + check_authorization(indirection_request) + + send("do_%s" % indirection_request.method, indirection_request, request, response) rescue Exception => e return do_exception(response, e) end @@ -60,6 +59,11 @@ module Puppet::Network::HTTP::Handler end def do_exception(response, exception, status=400) + if exception.is_a?(Puppet::Network::AuthorizationError) + # make sure we return the correct status code + # for authorization issues + status = 403 if status == 400 + end if exception.is_a?(Exception) puts exception.backtrace if Puppet[:trace] Puppet.err(exception) diff --git a/lib/puppet/network/rest_authconfig.rb b/lib/puppet/network/rest_authconfig.rb index e3fd51753..22be8b007 100644 --- a/lib/puppet/network/rest_authconfig.rb +++ b/lib/puppet/network/rest_authconfig.rb @@ -23,9 +23,16 @@ module Puppet end # check wether this request is allowed in our ACL + # raise an Puppet::Network::AuthorizedError if the request + # is denied. def allowed?(request) read() - return @rights.allowed?(build_uri(request), request.node, request.ip, request.method, request.environment) + + @rights.fail_on_deny(build_uri(request), + :node => request.node, + :ip => request.ip, + :method => request.method, + :environment => request.environment) end def initialize(file = nil, parsenow = true) diff --git a/lib/puppet/network/rest_authorization.rb b/lib/puppet/network/rest_authorization.rb index 3278640fe..e6f62d914 100644 --- a/lib/puppet/network/rest_authorization.rb +++ b/lib/puppet/network/rest_authorization.rb @@ -2,12 +2,10 @@ require 'puppet/network/client_request' require 'puppet/network/rest_authconfig' module Puppet::Network - # Most of our subclassing is just so that we can get - # access to information from the request object, like - # the client name and IP address. - class InvalidClientRequest < Puppet::Error; end + module RestAuthorization + # Create our config object if necessary. If there's no configuration file # we install our defaults def authconfig @@ -20,28 +18,25 @@ module Puppet::Network # Verify that our client has access. We allow untrusted access to # certificates terminus but no others. - def authorized?(request) - msg = "%s client %s access to %s [%s]" % - [ request.authenticated? ? "authenticated" : "unauthenticated", - (request.node.nil? ? request.ip : "#{request.node}(#{request.ip})"), - request.indirection_name, request.method ] - + def check_authorization(request) if request.authenticated? - res = authenticated_authorized?(request, msg ) + authenticated_authorized?(request) else - res = unauthenticated_authorized?(request, msg) + unless unauthenticated_authorized?(request) + msg = "%s access to %s [%s]" % [ (request.node.nil? ? request.ip : "#{request.node}(#{request.ip})"), request.indirection_name, request.method ] + Puppet.warning("Denying access: " + msg) + raise AuthorizationError.new( "Forbidden request:" + msg ) + end end - Puppet.notice((res ? "Allowing " : "Denying ") + msg) - return res end # delegate to our authorization file - def authenticated_authorized?(request, msg) + def authenticated_authorized?(request) authconfig.allowed?(request) end # allow only certificate requests when not authenticated - def unauthenticated_authorized?(request, msg) + def unauthenticated_authorized?(request) request.indirection_name == :certificate or request.indirection_name == :certificate_request end end diff --git a/lib/puppet/network/rights.rb b/lib/puppet/network/rights.rb index 7f4bed7f6..c98b84e8d 100755 --- a/lib/puppet/network/rights.rb +++ b/lib/puppet/network/rights.rb @@ -1,10 +1,16 @@ require 'puppet/network/authstore' +require 'puppet/error' + +module Puppet::Network + +# this exception is thrown when a request is not authenticated +class AuthorizationError < Puppet::Error; end # Define a set of rights and who has access to them. # There are two types of rights: # * named rights (ie a common string) # * path based rights (which are matched on a longest prefix basis) -class Puppet::Network::Rights +class Rights # We basically just proxy directly to our rights. Each Right stores # its own auth abilities. @@ -18,31 +24,57 @@ class Puppet::Network::Rights end end + # Check that name is allowed or not def allowed?(name, *args) + begin + fail_on_deny(name, *args) + rescue AuthorizationError + return false + rescue ArgumentError + # the namespace contract says we should raise this error + # if we didn't find the right acl + raise + end + return true + end + + def fail_on_deny(name, args = {}) res = :nomatch right = @rights.find do |acl| + found = false # an acl can return :dunno, which means "I'm not qualified to answer your question, # please ask someone else". This is used when for instance an acl matches, but not for the # current rest method, where we might think some other acl might be more specific. if match = acl.match?(name) - args << match - if (res = acl.allowed?(*args)) != :dunno - return res + args[:match] = match + if (res = acl.allowed?(args[:node], args[:ip], args)) != :dunno + # return early if we're allowed + return if res + # we matched, select this acl + found = true end end - false + found end - # if allowed or denied, tell it to the world - return res unless res == :nomatch - - # there were no rights allowing/denying name - # if name is not a path, let's throw - raise ArgumentError, "Unknown namespace right '%s'" % name unless name =~ /^\// + # if we end here, then that means we either didn't match + # or failed, in any case will throw an error to the outside world + if name =~ /^\// + # we're a patch ACL, let's fail + msg = "%s access to %s [%s]" % [ (args[:node].nil? ? args[:ip] : "#{args[:node]}(#{args[:ip]})"), name, args[:method] ] - # but if this was a path, we implement a deny all policy by default - # on unknown rights. - return false + error = AuthorizationError.new("Forbidden request: " + msg) + if right + error.file = right.file + error.line = right.line + end + Puppet.warning("Denying access: " + error.to_s) + else + # there were no rights allowing/denying name + # if name is not a path, let's throw + error = ArgumentError.new "Unknown namespace right '%s'" % name + end + raise error end def initialize() @@ -62,8 +94,8 @@ class Puppet::Network::Rights end # Define a new right to which access can be provided. - def newright(name, line=nil) - add_right( Right.new(name, line) ) + def newright(name, line=nil, file=nil) + add_right( Right.new(name, line, file) ) end private @@ -88,18 +120,21 @@ class Puppet::Network::Rights # A right. class Right < Puppet::Network::AuthStore - attr_accessor :name, :key, :acl_type, :line + include Puppet::FileCollection::Lookup + + attr_accessor :name, :key, :acl_type attr_accessor :methods, :environment ALL = [:save, :destroy, :find, :search] Puppet::Util.logmethods(self, true) - def initialize(name, line) + def initialize(name, line, file) @methods = [] @environment = [] @name = name @line = line || 0 + @file = file case name when Symbol @@ -140,18 +175,16 @@ class Puppet::Network::Rights # if this right is too restrictive (ie we don't match this access method) # then return :dunno so that upper layers have a chance to try another right # tailored to the given method - def allowed?(name, ip, method = nil, environment = nil, match = nil) - return :dunno if acl_type == :regex and not @methods.include?(method) - return :dunno if acl_type == :regex and @environment.size > 0 and not @environment.include?(environment) - - if acl_type == :regex and match # make sure any capture are replaced - interpolate(match) - end - - res = super(name,ip) - - if acl_type == :regex - reset_interpolation + def allowed?(name, ip, args) + return :dunno if acl_type == :regex and not @methods.include?(args[:method]) + return :dunno if acl_type == :regex and @environment.size > 0 and not @environment.include?(args[:environment]) + + begin + # make sure any capture are replaced if needed + interpolate(args[:match]) if acl_type == :regex and args[:match] + res = super(name,ip) + ensure + reset_interpolation if acl_type == :regex end res end @@ -222,4 +255,4 @@ class Puppet::Network::Rights end end - +end diff --git a/spec/integration/indirector/certificate/rest.rb b/spec/integration/indirector/certificate/rest.rb index c42bafbd0..8512fa95a 100755 --- a/spec/integration/indirector/certificate/rest.rb +++ b/spec/integration/indirector/certificate/rest.rb @@ -48,7 +48,7 @@ describe "Certificate REST Terminus" do @mock_model = stub('faked model', :name => "certificate") Puppet::Indirector::Request.any_instance.stubs(:model).returns(@mock_model) - Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:authorized?).returns(true) + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:check_authorization).returns(true) end after do diff --git a/spec/integration/indirector/certificate_request/rest.rb b/spec/integration/indirector/certificate_request/rest.rb index 138187691..a1dc5c017 100755 --- a/spec/integration/indirector/certificate_request/rest.rb +++ b/spec/integration/indirector/certificate_request/rest.rb @@ -53,7 +53,7 @@ describe "Certificate Request REST Terminus" do @mock_model = stub('faked model', :name => "certificate request") Puppet::Indirector::Request.any_instance.stubs(:model).returns(@mock_model) - Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:authorized?).returns(true) + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:check_authorization).returns(true) end after do diff --git a/spec/integration/indirector/certificate_revocation_list/rest.rb b/spec/integration/indirector/certificate_revocation_list/rest.rb index a1ba4f9a0..dce0cf05b 100755 --- a/spec/integration/indirector/certificate_revocation_list/rest.rb +++ b/spec/integration/indirector/certificate_revocation_list/rest.rb @@ -57,7 +57,7 @@ describe "Certificate REST Terminus" do @mock_model = stub('faked model', :name => "certificate") Puppet::Indirector::Request.any_instance.stubs(:model).returns(@mock_model) - Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:authorized?).returns(true) + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:check_authorization).returns(true) end after do diff --git a/spec/integration/indirector/report/rest.rb b/spec/integration/indirector/report/rest.rb index 115007588..7903ac9fb 100644 --- a/spec/integration/indirector/report/rest.rb +++ b/spec/integration/indirector/report/rest.rb @@ -50,7 +50,7 @@ describe "Report REST Terminus" do @mock_model = stub_everything 'faked model', :name => "report", :convert_from => @report Puppet::Indirector::Request.any_instance.stubs(:model).returns(@mock_model) - Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:authorized?).returns(true) + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:check_authorization) end after do diff --git a/spec/integration/indirector/rest.rb b/spec/integration/indirector/rest.rb index 34619c409..ede073d45 100755 --- a/spec/integration/indirector/rest.rb +++ b/spec/integration/indirector/rest.rb @@ -78,7 +78,7 @@ describe Puppet::Indirector::REST do Puppet::Indirector::Request.any_instance.stubs(:model).returns(@mock_model) # do not trigger the authorization layer - Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:authorized?).returns(true) + Puppet::Network::HTTP::WEBrickREST.any_instance.stubs(:check_authorization).returns(true) end describe "when finding a model instance over REST" do @@ -311,7 +311,7 @@ describe Puppet::Indirector::REST do Puppet::Indirector::Request.any_instance.stubs(:model).returns(@mock_model) # do not trigger the authorization layer - Puppet::Network::HTTP::MongrelREST.any_instance.stubs(:authorized?).returns(true) + Puppet::Network::HTTP::MongrelREST.any_instance.stubs(:check_authorization).returns(true) end after do diff --git a/spec/unit/network/authconfig.rb b/spec/unit/network/authconfig.rb index 186d30ce3..21af89bfd 100755 --- a/spec/unit/network/authconfig.rb +++ b/spec/unit/network/authconfig.rb @@ -114,7 +114,7 @@ describe Puppet::Network::AuthConfig do it "should increment line number even on commented lines" do @fd.stubs(:each).multiple_yields(' # comment','[puppetca]') - @rights.expects(:newright).with('[puppetca]', 2) + @rights.expects(:newright).with('[puppetca]', 2, 'dummy') @authconfig.read end @@ -130,7 +130,7 @@ describe Puppet::Network::AuthConfig do it "should increment line number even on blank lines" do @fd.stubs(:each).multiple_yields(' ','[puppetca]') - @rights.expects(:newright).with('[puppetca]', 2) + @rights.expects(:newright).with('[puppetca]', 2, 'dummy') @authconfig.read end @@ -146,7 +146,7 @@ describe Puppet::Network::AuthConfig do it "should not throw an error if the current path right already exist" do @fd.stubs(:each).yields('path /hello') - @rights.stubs(:newright).with("/hello",1) + @rights.stubs(:newright).with("/hello",1, 'dummy') @rights.stubs(:include?).with("/hello").returns(true) lambda { @authconfig.read }.should_not raise_error @@ -155,7 +155,7 @@ describe Puppet::Network::AuthConfig do it "should create a new right for found namespaces" do @fd.stubs(:each).yields('[puppetca]') - @rights.expects(:newright).with("[puppetca]", 1) + @rights.expects(:newright).with("[puppetca]", 1, 'dummy') @authconfig.read end @@ -163,8 +163,8 @@ describe Puppet::Network::AuthConfig do it "should create a new right for each found namespace line" do @fd.stubs(:each).multiple_yields('[puppetca]', '[fileserver]') - @rights.expects(:newright).with("[puppetca]", 1) - @rights.expects(:newright).with("[fileserver]", 2) + @rights.expects(:newright).with("[puppetca]", 1, 'dummy') + @rights.expects(:newright).with("[fileserver]", 2, 'dummy') @authconfig.read end @@ -172,7 +172,7 @@ describe Puppet::Network::AuthConfig do it "should create a new right for each found path line" do @fd.stubs(:each).multiple_yields('path /certificates') - @rights.expects(:newright).with("/certificates", 1) + @rights.expects(:newright).with("/certificates", 1, 'dummy') @authconfig.read end @@ -180,7 +180,7 @@ describe Puppet::Network::AuthConfig do it "should create a new right for each found regex line" do @fd.stubs(:each).multiple_yields('path ~ .rb$') - @rights.expects(:newright).with("~ .rb$", 1) + @rights.expects(:newright).with("~ .rb$", 1, 'dummy') @authconfig.read end @@ -189,7 +189,7 @@ describe Puppet::Network::AuthConfig do acl = stub 'acl', :info @fd.stubs(:each).multiple_yields('[puppetca]', 'allow 127.0.0.1') - @rights.stubs(:newright).with("[puppetca]", 1).returns(acl) + @rights.stubs(:newright).with("[puppetca]", 1, 'dummy').returns(acl) acl.expects(:allow).with('127.0.0.1') @@ -200,7 +200,7 @@ describe Puppet::Network::AuthConfig do acl = stub 'acl', :info @fd.stubs(:each).multiple_yields('[puppetca]', 'deny 127.0.0.1') - @rights.stubs(:newright).with("[puppetca]", 1).returns(acl) + @rights.stubs(:newright).with("[puppetca]", 1, 'dummy').returns(acl) acl.expects(:deny).with('127.0.0.1') @@ -212,7 +212,7 @@ describe Puppet::Network::AuthConfig do acl.stubs(:acl_type).returns(:regex) @fd.stubs(:each).multiple_yields('path /certificates', 'method search,find') - @rights.stubs(:newright).with("/certificates", 1).returns(acl) + @rights.stubs(:newright).with("/certificates", 1, 'dummy').returns(acl) acl.expects(:restrict_method).with('search') acl.expects(:restrict_method).with('find') @@ -225,7 +225,7 @@ describe Puppet::Network::AuthConfig do acl.stubs(:acl_type).returns(:regex) @fd.stubs(:each).multiple_yields('[puppetca]', 'method search,find') - @rights.stubs(:newright).with("puppetca", 1).returns(acl) + @rights.stubs(:newright).with("puppetca", 1, 'dummy').returns(acl) lambda { @authconfig.read }.should raise_error end @@ -235,7 +235,7 @@ describe Puppet::Network::AuthConfig do acl.stubs(:acl_type).returns(:regex) @fd.stubs(:each).multiple_yields('path /certificates', 'environment production,development') - @rights.stubs(:newright).with("/certificates", 1).returns(acl) + @rights.stubs(:newright).with("/certificates", 1, 'dummy').returns(acl) acl.expects(:restrict_environment).with('production') acl.expects(:restrict_environment).with('development') @@ -248,7 +248,7 @@ describe Puppet::Network::AuthConfig do acl.stubs(:acl_type).returns(:regex) @fd.stubs(:each).multiple_yields('[puppetca]', 'environment env') - @rights.stubs(:newright).with("puppetca", 1).returns(acl) + @rights.stubs(:newright).with("puppetca", 1, 'dummy').returns(acl) lambda { @authconfig.read }.should raise_error end diff --git a/spec/unit/network/http/handler.rb b/spec/unit/network/http/handler.rb index 7b7ef4722..0786d37d2 100755 --- a/spec/unit/network/http/handler.rb +++ b/spec/unit/network/http/handler.rb @@ -49,7 +49,7 @@ describe Puppet::Network::HTTP::Handler do @result = stub 'result', :render => "mytext" - @handler.stubs(:authorized?).returns(true) + @handler.stubs(:check_authorization) stub_server_interface end @@ -97,7 +97,7 @@ describe Puppet::Network::HTTP::Handler do @handler.expects(:do_mymethod).with(request, @request, @response) - @handler.expects(:authorized?).with(request).returns(true) + @handler.expects(:check_authorization).with(request) @handler.process(@request, @response) end @@ -108,9 +108,9 @@ describe Puppet::Network::HTTP::Handler do @handler.expects(:do_mymethod).never - @handler.expects(:authorized?).with(request).returns(false) + @handler.expects(:check_authorization).with(request).raises(Puppet::Network::AuthorizationError.new("forbindden")) - @handler.expects(:set_response)#.with { |response, body, status| status == 403 } + @handler.expects(:set_response).with { |response, body, status| status == 403 } @handler.process(@request, @response) end diff --git a/spec/unit/network/rest_authconfig.rb b/spec/unit/network/rest_authconfig.rb index ea5a82cce..7dc97382c 100755 --- a/spec/unit/network/rest_authconfig.rb +++ b/spec/unit/network/rest_authconfig.rb @@ -33,7 +33,7 @@ describe Puppet::Network::RestAuthConfig do end it "should ask for authorization to the ACL subsystem" do - @acl.expects(:allowed?).with("/path/to/resource", "me", "127.0.0.1", :save, :env) + @acl.expects(:fail_on_deny).with("/path/to/resource", :node => "me", :ip => "127.0.0.1", :method => :save, :environment => :env) @authconfig.allowed?(@request) end diff --git a/spec/unit/network/rest_authorization.rb b/spec/unit/network/rest_authorization.rb index 15351b172..3acd23f77 100755 --- a/spec/unit/network/rest_authorization.rb +++ b/spec/unit/network/rest_authorization.rb @@ -18,6 +18,7 @@ describe Puppet::Network::RestAuthorization do @request = stub_everything 'request' @request.stubs(:method).returns(:find) @request.stubs(:node).returns("node") + @request.stubs(:ip).returns("ip") end describe "when testing request authorization" do @@ -29,14 +30,14 @@ describe Puppet::Network::RestAuthorization do [ :certificate, :certificate_request].each do |indirection| it "should allow #{indirection}" do @request.stubs(:indirection_name).returns(indirection) - @auth.authorized?(@request).should be_true + lambda { @auth.check_authorization(@request) }.should_not raise_error Puppet::Network::AuthorizationError end end [ :facts, :file_metadata, :file_content, :catalog, :report, :checksum, :runner ].each do |indirection| it "should not allow #{indirection}" do @request.stubs(:indirection_name).returns(indirection) - @auth.authorized?(@request).should be_false + lambda { @auth.check_authorization(@request) }.should raise_error Puppet::Network::AuthorizationError end end end @@ -47,9 +48,21 @@ describe Puppet::Network::RestAuthorization do end it "should delegate to the current rest authconfig" do - @authconfig.expects(:allowed?).with(@request) + @authconfig.expects(:allowed?).with(@request).returns(true) - @auth.authorized?(@request) + @auth.check_authorization(@request) + end + + it "should raise an AuthorizationError if authconfig raises an AuthorizationError" do + @authconfig.expects(:allowed?).with(@request).raises(Puppet::Network::AuthorizationError.new("forbidden")) + + lambda { @auth.check_authorization(@request) }.should raise_error Puppet::Network::AuthorizationError + end + + it "should not raise an AuthorizationError if request is allowed" do + @authconfig.expects(:allowed?).with(@request).returns(true) + + lambda { @auth.check_authorization(@request) }.should_not raise_error Puppet::Network::AuthorizationError end end end diff --git a/spec/unit/network/rights.rb b/spec/unit/network/rights.rb index 97094f8e5..be1db723d 100755 --- a/spec/unit/network/rights.rb +++ b/spec/unit/network/rights.rb @@ -145,55 +145,75 @@ describe Puppet::Network::Rights do before :each do @right.stubs(:right).returns(nil) - @pathacl = stub 'pathacl', :acl_type => :path + @pathacl = stub 'pathacl', :acl_type => :regex, :"<=>" => 1, :line => 0, :file => 'dummy' Puppet::Network::Rights::Right.stubs(:new).returns(@pathacl) end + it "should delegate to fail_on_deny" do + @right.expects(:fail_on_deny).with("namespace", :args) + + @right.allowed?("namespace", :args) + end + + it "should return true if fail_on_deny doesn't fail" do + @right.stubs(:fail_on_deny) + @right.allowed?("namespace", :args).should be_true + end + + it "should return false if fail_on_deny raises an AuthorizationError" do + @right.stubs(:fail_on_deny).raises(Puppet::Network::AuthorizationError.new("forbidden")) + @right.allowed?("namespace", :args1, :args2).should be_false + end + it "should first check namespace rights" do acl = stub 'acl', :acl_type => :name, :key => :namespace Puppet::Network::Rights::Right.stubs(:new).returns(acl) @right.newright("[namespace]") acl.expects(:match?).returns(true) - acl.expects(:allowed?).with(:args, true).returns(true) + acl.expects(:allowed?).with { |node,ip,h| node == "node" and ip == "ip" }.returns(true) - @right.allowed?("namespace", :args) + @right.fail_on_deny("namespace", { :node => "node", :ip => "ip" } ) end it "should then check for path rights if no namespace match" do - acl = stub 'acl', :acl_type => :name, :match? => false + acl = stub 'nmacl', :acl_type => :name, :key => :namespace, :"<=>" => -1, :line => 0, :file => 'dummy' + acl.stubs(:match?).returns(false) + Puppet::Network::Rights::Right.stubs(:new).with("[namespace]").returns(acl) - acl.expects(:allowed?).with(:args).never - @right.newright("/path/to/there") + @right.newright("[namespace]") + @right.newright("/path/to/there", 0, nil) @pathacl.stubs(:match?).returns(true) - @pathacl.expects(:allowed?) - @right.allowed?("/path/to/there", :args) + acl.expects(:allowed?).never + @pathacl.expects(:allowed?).returns(true) + + @right.fail_on_deny("/path/to/there", {}) end it "should pass the match? return to allowed?" do @right.newright("/path/to/there") @pathacl.expects(:match?).returns(:match) - @pathacl.expects(:allowed?).with(:args, :match) + @pathacl.expects(:allowed?).with { |node,ip,h| h[:match] == :match }.returns(true) - @right.allowed?("/path/to/there", :args) + @right.fail_on_deny("/path/to/there", {}) end describe "with namespace acls" do it "should raise an error if this namespace right doesn't exist" do - lambda{ @right.allowed?("namespace") }.should raise_error + lambda{ @right.fail_on_deny("namespace") }.should raise_error end end describe "with path acls" do before :each do - @long_acl = stub 'longpathacl', :name => "/path/to/there", :acl_type => :regex - Puppet::Network::Rights::Right.stubs(:new).with("/path/to/there", 0).returns(@long_acl) + @long_acl = stub 'longpathacl', :name => "/path/to/there", :acl_type => :regex, :line => 0, :file => 'dummy' + Puppet::Network::Rights::Right.stubs(:new).with("/path/to/there", 0, nil).returns(@long_acl) - @short_acl = stub 'shortpathacl', :name => "/path/to", :acl_type => :regex - Puppet::Network::Rights::Right.stubs(:new).with("/path/to", 0).returns(@short_acl) + @short_acl = stub 'shortpathacl', :name => "/path/to", :acl_type => :regex, :line => 0, :file => 'dummy' + Puppet::Network::Rights::Right.stubs(:new).with("/path/to", 0, nil).returns(@short_acl) @long_acl.stubs(:"<=>").with(@short_acl).returns(0) @short_acl.stubs(:"<=>").with(@long_acl).returns(0) @@ -209,20 +229,20 @@ describe Puppet::Network::Rights do @long_acl.expects(:allowed?).returns(true) @short_acl.expects(:allowed?).never - @right.allowed?("/path/to/there/and/there", :args) + @right.fail_on_deny("/path/to/there/and/there", {}) end it "should select the first match that doesn't return :dunno" do - @right.newright("/path/to/there", 0) - @right.newright("/path/to", 0) + @right.newright("/path/to/there", 0, nil) + @right.newright("/path/to", 0, nil) @long_acl.stubs(:match?).returns(true) @short_acl.stubs(:match?).returns(true) @long_acl.expects(:allowed?).returns(:dunno) - @short_acl.expects(:allowed?) + @short_acl.expects(:allowed?).returns(true) - @right.allowed?("/path/to/there/and/there", :args) + @right.fail_on_deny("/path/to/there/and/there", {}) end it "should not select an ACL that doesn't match" do @@ -233,36 +253,41 @@ describe Puppet::Network::Rights do @short_acl.stubs(:match?).returns(true) @long_acl.expects(:allowed?).never - @short_acl.expects(:allowed?) + @short_acl.expects(:allowed?).returns(true) - @right.allowed?("/path/to/there/and/there", :args) + @right.fail_on_deny("/path/to/there/and/there", {}) end - it "should return the result of the acl" do + it "should not raise an AuthorizationError if allowed" do @right.newright("/path/to/there", 0) @long_acl.stubs(:match?).returns(true) - @long_acl.stubs(:allowed?).returns(:returned) + @long_acl.stubs(:allowed?).returns(true) - @right.allowed?("/path/to/there/and/there", :args).should == :returned + lambda { @right.fail_on_deny("/path/to/there/and/there", {}) }.should_not raise_error(Puppet::Network::AuthorizationError) end - it "should not raise an error if this path acl doesn't exist" do - lambda{ @right.allowed?("/path", :args) }.should_not raise_error + it "should raise an AuthorizationError if the match is denied" do + @right.newright("/path/to/there", 0, nil) + + @long_acl.stubs(:match?).returns(true) + @long_acl.stubs(:allowed?).returns(false) + + lambda{ @right.fail_on_deny("/path/to/there", {}) }.should raise_error(Puppet::Network::AuthorizationError) end - it "should return false if no path match" do - @right.allowed?("/path", :args).should be_false + it "should raise an AuthorizationError if no path match" do + lambda { @right.fail_on_deny("/nomatch", {}) }.should raise_error(Puppet::Network::AuthorizationError) end end describe "with regex acls" do before :each do - @regex_acl1 = stub 'regex_acl1', :name => "/files/(.*)/myfile", :acl_type => :regex - Puppet::Network::Rights::Right.stubs(:new).with("~ /files/(.*)/myfile", 0).returns(@regex_acl1) + @regex_acl1 = stub 'regex_acl1', :name => "/files/(.*)/myfile", :acl_type => :regex, :line => 0, :file => 'dummy' + Puppet::Network::Rights::Right.stubs(:new).with("~ /files/(.*)/myfile", 0, nil).returns(@regex_acl1) - @regex_acl2 = stub 'regex_acl2', :name => "/files/(.*)/myfile/", :acl_type => :regex - Puppet::Network::Rights::Right.stubs(:new).with("~ /files/(.*)/myfile/", 0).returns(@regex_acl2) + @regex_acl2 = stub 'regex_acl2', :name => "/files/(.*)/myfile/", :acl_type => :regex, :line => 0, :file => 'dummy' + Puppet::Network::Rights::Right.stubs(:new).with("~ /files/(.*)/myfile/", 0, nil).returns(@regex_acl2) @regex_acl1.stubs(:"<=>").with(@regex_acl2).returns(0) @regex_acl2.stubs(:"<=>").with(@regex_acl1).returns(0) @@ -278,7 +303,7 @@ describe Puppet::Network::Rights do @regex_acl1.expects(:allowed?).returns(true) @regex_acl2.expects(:allowed?).never - @right.allowed?("/files/repository/myfile/other", :args) + @right.fail_on_deny("/files/repository/myfile/other", {}) end it "should select the first match that doesn't return :dunno" do @@ -289,9 +314,9 @@ describe Puppet::Network::Rights do @regex_acl2.stubs(:match?).returns(true) @regex_acl1.expects(:allowed?).returns(:dunno) - @regex_acl2.expects(:allowed?) + @regex_acl2.expects(:allowed?).returns(true) - @right.allowed?("/files/repository/myfile/other", :args) + @right.fail_on_deny("/files/repository/myfile/other", {}) end it "should not select an ACL that doesn't match" do @@ -302,26 +327,26 @@ describe Puppet::Network::Rights do @regex_acl2.stubs(:match?).returns(true) @regex_acl1.expects(:allowed?).never - @regex_acl2.expects(:allowed?) + @regex_acl2.expects(:allowed?).returns(true) - @right.allowed?("/files/repository/myfile/other", :args) + @right.fail_on_deny("/files/repository/myfile/other", {}) end - it "should return the result of the acl" do + it "should not raise an AuthorizationError if allowed" do @right.newright("~ /files/(.*)/myfile", 0) @regex_acl1.stubs(:match?).returns(true) - @regex_acl1.stubs(:allowed?).returns(:returned) + @regex_acl1.stubs(:allowed?).returns(true) - @right.allowed?("/files/repository/myfile/other", :args).should == :returned + lambda { @right.fail_on_deny("/files/repository/myfile/other", {}) }.should_not raise_error(Puppet::Network::AuthorizationError) end - it "should not raise an error if no regex acl match" do - lambda{ @right.allowed?("/path", :args) }.should_not raise_error + it "should raise an error if no regex acl match" do + lambda{ @right.fail_on_deny("/path", {}) }.should raise_error(Puppet::Network::AuthorizationError) end - it "should return false if no regex match" do - @right.allowed?("/path", :args).should be_false + it "should raise an AuthorizedError on deny" do + lambda { @right.fail_on_deny("/path", {}) }.should raise_error(Puppet::Network::AuthorizationError) end end @@ -329,7 +354,7 @@ describe Puppet::Network::Rights do describe Puppet::Network::Rights::Right do before :each do - @acl = Puppet::Network::Rights::Right.new("/path",0) + @acl = Puppet::Network::Rights::Right.new("/path",0, nil) end describe "with path" do @@ -352,7 +377,7 @@ describe Puppet::Network::Rights do describe "with regex" do before :each do - @acl = Puppet::Network::Rights::Right.new("~ .rb$",0) + @acl = Puppet::Network::Rights::Right.new("~ .rb$",0, nil) end it "should say it's a regex ACL" do @@ -403,14 +428,14 @@ describe Puppet::Network::Rights do it "should return :dunno if this right is not restricted to the given method" do @acl.restrict_method(:destroy) - @acl.allowed?("me","127.0.0.1", :save).should == :dunno + @acl.allowed?("me","127.0.0.1", { :method => :save } ).should == :dunno end it "should return allow/deny if this right is restricted to the given method" do @acl.restrict_method(:save) @acl.allow("127.0.0.1") - @acl.allowed?("me","127.0.0.1", :save).should be_true + @acl.allowed?("me","127.0.0.1", { :method => :save }).should be_true end it "should return :dunno if this right is not restricted to the given environment" do @@ -418,19 +443,19 @@ describe Puppet::Network::Rights do @acl.restrict_environment(:production) - @acl.allowed?("me","127.0.0.1", :save, :development).should == :dunno + @acl.allowed?("me","127.0.0.1", { :method => :save, :environment => :development }).should == :dunno end it "should interpolate allow/deny patterns with the given match" do @acl.expects(:interpolate).with(:match) - @acl.allowed?("me","127.0.0.1", :save, nil, :match) + @acl.allowed?("me","127.0.0.1", :method => :save, :match => :match) end it "should reset interpolation after the match" do @acl.expects(:reset_interpolation) - @acl.allowed?("me","127.0.0.1", :save, nil, :match) + @acl.allowed?("me","127.0.0.1", :method => :save, :match => :match) end # mocha doesn't allow testing super... |