Merge branch 'ticket/2.6.next/5838' into 2.6.next

* ticket/2.6.next/5838:
  (#5838) Support paths as part of file bucket requests.
  (#5838) Improve the quality of file bucket specs.
  (#5838) Make file bucket dipper efficient when saving a file that already exists
  (#5838) Implemented the "head" method for FileBucketFile::File terminus.
  (#5838) Reworked file dipper spec to perform less stubbing.
  (#5838) Added support for HEAD requests to the indirector.
  (#5838) Refactored error handling logic into find_in_cache.
  (#5838) Refactored Puppet::Network::Rights#fail_on_deny

4 files changed, 81 insertions, 24 deletions

diff --git a/spec/unit/network/http/api/v1_spec.rb b/spec/unit/network/http/api/v1_spec.rb
index c593242c0..23a291cf3 100644
--- a/spec/unit/network/http/api/v1_spec.rb
+++ b/spec/unit/network/http/api/v1_spec.rb
@@ -68,6 +68,10 @@ describe Puppet::Network::HTTP::API::V1 do
       @tester.uri2indirection("GET", "/env/foo/bar", {}).method.should == :find
     end
 
+    it "should choose 'head' as the indirection method if the http method is a HEAD and the indirection name is singular" do
+      @tester.uri2indirection("HEAD", "/env/foo/bar", {}).method.should == :head
+    end
+
     it "should choose 'search' as the indirection method if the http method is a GET and the indirection name is plural" do
       @tester.uri2indirection("GET", "/env/foos/bar", {}).method.should == :search
     end
diff --git a/spec/unit/network/http/handler_spec.rb b/spec/unit/network/http/handler_spec.rb
index cdbce41f7..8464ae68e 100755
--- a/spec/unit/network/http/handler_spec.rb
+++ b/spec/unit/network/http/handler_spec.rb
@@ -256,6 +256,39 @@ describe Puppet::Network::HTTP::Handler do
       end
     end
 
+    describe "when performing head operation" do
+      before do
+        @irequest = stub 'indirection_request', :method => :head, :indirection_name => "my_handler", :to_hash => {}, :key => "my_result", :model => @model_class
+
+        @model_class.stubs(:head).returns true
+      end
+
+      it "should use the indirection request to find the model class" do
+        @irequest.expects(:model).returns @model_class
+
+        @handler.do_head(@irequest, @request, @response)
+      end
+
+      it "should use the escaped request key" do
+        @model_class.expects(:head).with do |key, args|
+          key == "my_result"
+        end.returns true
+        @handler.do_head(@irequest, @request, @response)
+      end
+
+      it "should not generate a response when a model head call succeeds" do
+        @handler.expects(:set_response).never
+        @handler.do_head(@irequest, @request, @response)
+      end
+
+      it "should return a 404 when the model head call returns false" do
+        @model_class.stubs(:name).returns "my name"
+        @handler.expects(:set_response).with { |response, body, status| status == 404 }
+        @model_class.stubs(:head).returns(false)
+        @handler.do_head(@irequest, @request, @response)
+      end
+    end
+
     describe "when searching for model instances" do
       before do
         @irequest = stub 'indirection_request', :method => :find, :indirection_name => "my_handler", :to_hash => {}, :key => "key", :model => @model_class
diff --git a/spec/unit/network/rest_authconfig_spec.rb b/spec/unit/network/rest_authconfig_spec.rb
index 06436e723..d629f8670 100755
--- a/spec/unit/network/rest_authconfig_spec.rb
+++ b/spec/unit/network/rest_authconfig_spec.rb
@@ -47,7 +47,7 @@ describe Puppet::Network::RestAuthConfig do
   end
 
   it "should ask for authorization to the ACL subsystem" do
-    @acl.expects(:fail_on_deny).with("/path/to/resource", :node => "me", :ip => "127.0.0.1", :method => :save, :environment => :env, :authenticated => true)
+    @acl.expects(:is_request_forbidden_and_why?).with(@request).returns(nil)
 
     @authconfig.allowed?(@request)
   end
diff --git a/spec/unit/network/rights_spec.rb b/spec/unit/network/rights_spec.rb
index 969fc189e..3b9e48374 100755
--- a/spec/unit/network/rights_spec.rb
+++ b/spec/unit/network/rights_spec.rb
@@ -9,6 +9,26 @@ describe Puppet::Network::Rights do
     @right = Puppet::Network::Rights.new
   end
 
+  describe "when validating a :head request" do
+    [:find, :save].each do |allowed_method|
+      it "should allow the request if only #{allowed_method} is allowed" do
+        rights = Puppet::Network::Rights.new
+        rights.newright("/")
+        rights.allow("/", "*")
+        rights.restrict_method("/", allowed_method)
+        rights.restrict_authenticated("/", :any)
+        request = Puppet::Indirector::Request.new(:indirection_name, :head, "key")
+        rights.is_request_forbidden_and_why?(request).should == nil
+      end
+    end
+
+    it "should disallow the request if neither :find nor :save is allowed" do
+      rights = Puppet::Network::Rights.new
+      request = Puppet::Indirector::Request.new(:indirection_name, :head, "key")
+      rights.is_request_forbidden_and_why?(request).should be_instance_of(Puppet::Network::AuthorizationError)
+    end
+  end
+
   [:allow, :deny, :restrict_method, :restrict_environment, :restrict_authenticated].each do |m|
     it "should have a #{m} method" do
       @right.should respond_to(m)
@@ -155,19 +175,19 @@ describe Puppet::Network::Rights do
       Puppet::Network::Rights::Right.stubs(:new).returns(@pathacl)
     end
 
-    it "should delegate to fail_on_deny" do
-      @right.expects(:fail_on_deny).with("namespace", :node => "host.domain.com", :ip => "127.0.0.1")
+    it "should delegate to is_forbidden_and_why?" do
+      @right.expects(:is_forbidden_and_why?).with("namespace", :node => "host.domain.com", :ip => "127.0.0.1").returns(nil)
 
       @right.allowed?("namespace", "host.domain.com", "127.0.0.1")
     end
 
-    it "should return true if fail_on_deny doesn't fail" do
-      @right.stubs(:fail_on_deny)
+    it "should return true if is_forbidden_and_why? returns nil" do
+      @right.stubs(:is_forbidden_and_why?).returns(nil)
       @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"))
+    it "should return false if is_forbidden_and_why? returns an AuthorizationError" do
+      @right.stubs(:is_forbidden_and_why?).returns(Puppet::Network::AuthorizationError.new("forbidden"))
       @right.allowed?("namespace", :args1, :args2).should be_false
     end
 
@@ -179,7 +199,7 @@ describe Puppet::Network::Rights do
       acl.expects(:match?).returns(true)
       acl.expects(:allowed?).with { |node,ip,h| node == "node" and ip == "ip" }.returns(true)
 
-      @right.fail_on_deny("namespace", { :node => "node", :ip => "ip" } )
+      @right.is_forbidden_and_why?("namespace", { :node => "node", :ip => "ip" } ).should == nil
     end
 
     it "should then check for path rights if no namespace match" do
@@ -195,7 +215,7 @@ describe Puppet::Network::Rights do
       acl.expects(:allowed?).never
       @pathacl.expects(:allowed?).returns(true)
 
-      @right.fail_on_deny("/path/to/there", {})
+      @right.is_forbidden_and_why?("/path/to/there", {}).should == nil
     end
 
     it "should pass the match? return to allowed?" do
@@ -204,12 +224,12 @@ describe Puppet::Network::Rights do
       @pathacl.expects(:match?).returns(:match)
       @pathacl.expects(:allowed?).with { |node,ip,h| h[:match] == :match }.returns(true)
 
-      @right.fail_on_deny("/path/to/there", {})
+      @right.is_forbidden_and_why?("/path/to/there", {}).should == nil
     end
 
     describe "with namespace acls" do
-      it "should raise an error if this namespace right doesn't exist" do
-        lambda{ @right.fail_on_deny("namespace") }.should raise_error
+      it "should return an ArgumentError if this namespace right doesn't exist" do
+        lambda { @right.is_forbidden_and_why?("namespace") }.should raise_error(ArgumentError)
       end
     end
 
@@ -235,7 +255,7 @@ describe Puppet::Network::Rights do
         @long_acl.expects(:allowed?).returns(true)
         @short_acl.expects(:allowed?).never
 
-        @right.fail_on_deny("/path/to/there/and/there", {})
+        @right.is_forbidden_and_why?("/path/to/there/and/there", {}).should == nil
       end
 
       it "should select the first match that doesn't return :dunno" do
@@ -248,7 +268,7 @@ describe Puppet::Network::Rights do
         @long_acl.expects(:allowed?).returns(:dunno)
         @short_acl.expects(:allowed?).returns(true)
 
-        @right.fail_on_deny("/path/to/there/and/there", {})
+        @right.is_forbidden_and_why?("/path/to/there/and/there", {}).should == nil
       end
 
       it "should not select an ACL that doesn't match" do
@@ -261,7 +281,7 @@ describe Puppet::Network::Rights do
         @long_acl.expects(:allowed?).never
         @short_acl.expects(:allowed?).returns(true)
 
-        @right.fail_on_deny("/path/to/there/and/there", {})
+        @right.is_forbidden_and_why?("/path/to/there/and/there", {}).should == nil
       end
 
       it "should not raise an AuthorizationError if allowed" do
@@ -270,7 +290,7 @@ describe Puppet::Network::Rights do
         @long_acl.stubs(:match?).returns(true)
         @long_acl.stubs(:allowed?).returns(true)
 
-        lambda { @right.fail_on_deny("/path/to/there/and/there", {}) }.should_not raise_error(Puppet::Network::AuthorizationError)
+        @right.is_forbidden_and_why?("/path/to/there/and/there", {}).should == nil
       end
 
       it "should raise an AuthorizationError if the match is denied" do
@@ -279,11 +299,11 @@ describe Puppet::Network::Rights do
         @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)
+        @right.is_forbidden_and_why?("/path/to/there", {}).should be_instance_of(Puppet::Network::AuthorizationError)
       end
 
       it "should raise an AuthorizationError if no path match" do
-        lambda { @right.fail_on_deny("/nomatch", {}) }.should raise_error(Puppet::Network::AuthorizationError)
+        @right.is_forbidden_and_why?("/nomatch", {}).should be_instance_of(Puppet::Network::AuthorizationError)
       end
     end
 
@@ -309,7 +329,7 @@ describe Puppet::Network::Rights do
         @regex_acl1.expects(:allowed?).returns(true)
         @regex_acl2.expects(:allowed?).never
 
-        @right.fail_on_deny("/files/repository/myfile/other", {})
+        @right.is_forbidden_and_why?("/files/repository/myfile/other", {}).should == nil
       end
 
       it "should select the first match that doesn't return :dunno" do
@@ -322,7 +342,7 @@ describe Puppet::Network::Rights do
         @regex_acl1.expects(:allowed?).returns(:dunno)
         @regex_acl2.expects(:allowed?).returns(true)
 
-        @right.fail_on_deny("/files/repository/myfile/other", {})
+        @right.is_forbidden_and_why?("/files/repository/myfile/other", {}).should == nil
       end
 
       it "should not select an ACL that doesn't match" do
@@ -335,7 +355,7 @@ describe Puppet::Network::Rights do
         @regex_acl1.expects(:allowed?).never
         @regex_acl2.expects(:allowed?).returns(true)
 
-        @right.fail_on_deny("/files/repository/myfile/other", {})
+        @right.is_forbidden_and_why?("/files/repository/myfile/other", {}).should == nil
       end
 
       it "should not raise an AuthorizationError if allowed" do
@@ -344,15 +364,15 @@ describe Puppet::Network::Rights do
         @regex_acl1.stubs(:match?).returns(true)
         @regex_acl1.stubs(:allowed?).returns(true)
 
-        lambda { @right.fail_on_deny("/files/repository/myfile/other", {}) }.should_not raise_error(Puppet::Network::AuthorizationError)
+        @right.is_forbidden_and_why?("/files/repository/myfile/other", {}).should == nil
       end
 
       it "should raise an error if no regex acl match" do
-        lambda{ @right.fail_on_deny("/path", {}) }.should raise_error(Puppet::Network::AuthorizationError)
+        @right.is_forbidden_and_why?("/path", {}).should be_instance_of(Puppet::Network::AuthorizationError)
       end
 
       it "should raise an AuthorizedError on deny" do
-        lambda { @right.fail_on_deny("/path", {}) }.should raise_error(Puppet::Network::AuthorizationError)
+        @right.is_forbidden_and_why?("/path", {}).should be_instance_of(Puppet::Network::AuthorizationError)
       end
 
     end