Enhance authconfig format to support uri paths and regex

This patch introduces a new set of directive to the authconfig
parser/file format:

path /uripath or patch ~ <regex>
This directive declares a new kind of ACL based on the uri path.

method save, find
This directive which is to be used under path directive restricts a
path ACL to only some REST verbs.

The ACL path system matches on path prefix possible, or
on regex matches (first match wins).
If no path are matching, then the authorization is not allowed.
The same if no ACL matches for the given REST verb.

The old namespace right matching still works as usual.

Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com>

2 files changed, 453 insertions, 21 deletions

diff --git a/spec/unit/network/authconfig.rb b/spec/unit/network/authconfig.rb
index 9d5f6154d..d891fe45a 100755
--- a/spec/unit/network/authconfig.rb
+++ b/spec/unit/network/authconfig.rb
@@ -28,7 +28,7 @@ describe Puppet::Network::AuthConfig do
             Puppet::Network::AuthConfig.new
         end
 
-        it "should raise an error if no file is defined in fine" do
+        it "should raise an error if no file is defined finally" do
             Puppet.stubs(:[]).with(:authconfig).returns(nil)
 
             lambda { Puppet::Network::AuthConfig.new }.should raise_error(Puppet::DevError)
@@ -111,6 +111,14 @@ describe Puppet::Network::AuthConfig do
             @authconfig.read
         end
 
+        it "should increment line number even on commented lines" do
+            @fd.stubs(:each).multiple_yields('  # comment','[puppetca]')
+
+            @rights.expects(:newright).with('[puppetca]', 2)
+
+            @authconfig.read
+        end
+
         it "should skip blank lines" do
             @fd.stubs(:each).yields('  ')
 
@@ -119,7 +127,15 @@ describe Puppet::Network::AuthConfig do
             @authconfig.read
         end
 
-        it "should throw an error if read rights already exist" do
+        it "should increment line number even on blank lines" do
+            @fd.stubs(:each).multiple_yields('  ','[puppetca]')
+
+            @rights.expects(:newright).with('[puppetca]', 2)
+
+            @authconfig.read
+        end
+
+        it "should throw an error if the current namespace right already exist" do
             @fd.stubs(:each).yields('[puppetca]')
 
             @rights.stubs(:include?).with("puppetca").returns(true)
@@ -127,10 +143,19 @@ describe Puppet::Network::AuthConfig do
             lambda { @authconfig.read }.should raise_error
         end
 
+        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(:include?).with("/hello").returns(true)
+
+            lambda { @authconfig.read }.should_not raise_error
+        end
+
         it "should create a new right for found namespaces" do
             @fd.stubs(:each).yields('[puppetca]')
 
-            @rights.expects(:newright).with("puppetca")
+            @rights.expects(:newright).with("[puppetca]", 1)
 
             @authconfig.read
         end
@@ -138,8 +163,24 @@ 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")
-            @rights.expects(:newright).with("fileserver")
+            @rights.expects(:newright).with("[puppetca]", 1)
+            @rights.expects(:newright).with("[fileserver]", 2)
+
+            @authconfig.read
+        end
+
+        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)
+
+            @authconfig.read
+        end
+
+        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)
 
             @authconfig.read
         end
@@ -148,26 +189,47 @@ 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")
-            @rights.stubs(:[]).returns(acl)
+            @rights.stubs(:newright).with("[puppetca]", 1).returns(acl)
 
             acl.expects(:allow).with('127.0.0.1')
 
             @authconfig.read
         end
 
-        it "should create a deny ACE on each subsequent allow" do
+        it "should create a deny ACE on each subsequent deny" do
             acl = stub 'acl', :info
 
             @fd.stubs(:each).multiple_yields('[puppetca]', 'deny 127.0.0.1')
-            @rights.stubs(:newright).with("puppetca")
-            @rights.stubs(:[]).returns(acl)
+            @rights.stubs(:newright).with("[puppetca]", 1).returns(acl)
 
             acl.expects(:deny).with('127.0.0.1')
 
             @authconfig.read
         end
 
+        it "should inform the current ACL if we get the 'method' directive" do
+            acl = stub 'acl', :info
+            acl.stubs(:acl_type).returns(:regex)
+
+            @fd.stubs(:each).multiple_yields('path /certificates', 'method search,find')
+            @rights.stubs(:newright).with("/certificates", 1).returns(acl)
+
+            acl.expects(:restrict_method).with('search')
+            acl.expects(:restrict_method).with('find')
+
+            @authconfig.read
+        end
+
+        it "should raise an error if the 'method' directive is used in a right different than a path/regex one" do
+            acl = stub 'acl', :info
+            acl.stubs(:acl_type).returns(:regex)
+
+            @fd.stubs(:each).multiple_yields('[puppetca]', 'method search,find')
+            @rights.stubs(:newright).with("puppetca", 1).returns(acl)
+
+            lambda { @authconfig.read }.should raise_error
+        end
+
     end
 
 end
diff --git a/spec/unit/network/rights.rb b/spec/unit/network/rights.rb
index 5fe8e51f4..6e918124f 100755
--- a/spec/unit/network/rights.rb
+++ b/spec/unit/network/rights.rb
@@ -9,7 +9,7 @@ describe Puppet::Network::Rights do
         @right = Puppet::Network::Rights.new
     end
 
-    [:allow, :allowed?, :deny].each do |m|
+    [:allow, :deny].each do |m|
         it "should have a #{m} method" do
             @right.should respond_to(m)
         end
@@ -17,7 +17,7 @@ describe Puppet::Network::Rights do
         describe "when using #{m}" do
             it "should delegate to the correct acl" do
                 acl = stub 'acl'
-                @right.stubs(:right).returns(acl)
+                @right.stubs(:[]).returns(acl)
 
                 acl.expects(m).with("me")
 
@@ -26,29 +26,399 @@ describe Puppet::Network::Rights do
         end
     end
 
-    describe "when creating new ACLs" do
+    it "should throw an error if type can't be determined" do
+        lambda { @right.newright("name") }.should raise_error
+    end
+
+    describe "when creating new namespace ACLs" do
+
         it "should throw an error if the ACL already exists" do
-            @right.newright("name")
+            @right.newright("[name]")
 
-            lambda { @right.newright("name")}.should raise_error
+            lambda { @right.newright("[name]") }.should raise_error
         end
 
         it "should create a new ACL with the correct name" do
-            @right.newright("name")
+            @right.newright("[name]")
 
-            @right["name"].name.should == :name
+            @right["name"].key.should == :name
         end
 
         it "should create an ACL of type Puppet::Network::AuthStore" do
-            @right.newright("name")
+            @right.newright("[name]")
 
             @right["name"].should be_a_kind_of(Puppet::Network::AuthStore)
         end
+    end
+
+    describe "when creating new path ACLs" do
+        it "should not throw an error if the ACL already exists" do
+            @right.newright("/name")
+
+            lambda { @right.newright("/name")}.should_not raise_error
+        end
+
+        it "should throw an error if the acl uri path is not absolute" do
+            lambda { @right.newright("name")}.should raise_error
+        end
+
+        it "should create a new ACL with the correct path" do
+            @right.newright("/name")
+
+            @right["/name"].should_not be_nil
+        end
+
+        it "should create an ACL of type Puppet::Network::AuthStore" do
+            @right.newright("/name")
+
+            @right["/name"].should be_a_kind_of(Puppet::Network::AuthStore)
+        end
+    end
+
+    describe "when creating new regex ACLs" do
+        it "should not throw an error if the ACL already exists" do
+            @right.newright("~ .rb$")
+
+            lambda { @right.newright("~ .rb$")}.should_not raise_error
+        end
+
+        it "should create a new ACL with the correct regex" do
+            @right.newright("~ .rb$")
+
+            @right.include?(".rb$").should_not be_nil
+        end
+
+        it "should be able to lookup the regex" do
+            @right.newright("~ .rb$")
+
+            @right[".rb$"].should_not be_nil
+        end
+
+        it "should create an ACL of type Puppet::Network::AuthStore" do
+            @right.newright("~ .rb$").should be_a_kind_of(Puppet::Network::AuthStore)
+        end
+    end
+
+    describe "when checking ACLs existence" do
+        it "should return false if there are no matching rights" do
+            @right.include?("name").should be_false
+        end
+
+        it "should return true if a namespace rights exist" do
+            @right.newright("[name]")
+
+            @right.include?("name").should be_true
+        end
+
+        it "should return false if no matching namespace rights exist" do
+            @right.newright("[name]")
+
+            @right.include?("notname").should be_false
+        end
+
+        it "should return true if a path right exists" do
+            @right.newright("/name")
+
+            @right.include?("/name").should be_true
+        end
+
+        it "should return false if no matching path rights exist" do
+            @right.newright("/name")
+
+            @right.include?("/differentname").should be_false
+        end
+
+        it "should return true if a regex right exists" do
+            @right.newright("~ .rb$")
+
+            @right.include?(".rb$").should be_true
+        end
+
+        it "should return false if no matching path rights exist" do
+            @right.newright("~ .rb$")
+
+            @right.include?(".pp$").should be_false
+        end
+    end
+
+    describe "when checking if right is allowed" do
+        before :each do
+            @right.stubs(:right).returns(nil)
+
+            @pathacl = stub 'pathacl', :acl_type => :path
+            Puppet::Network::Rights::Right.stubs(:new).returns(@pathacl)
+        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)
+
+            @right.allowed?("namespace", :args)
+        end
+
+        it "should then check for path rights if no namespace match" do
+            acl = stub 'acl', :acl_type => :name, :match? => false
+
+            acl.expects(:allowed?).with(:args).never
+            @right.newright("/path/to/there")
+
+            @pathacl.stubs(:match?).returns(true)
+            @pathacl.expects(:allowed?)
+
+            @right.allowed?("/path/to/there", :args)
+        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)
+
+            @right.allowed?("/path/to/there", :args)
+        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
+            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)
+
+                @short_acl = stub 'shortpathacl', :name => "/path/to", :acl_type => :regex
+                Puppet::Network::Rights::Right.stubs(:new).with("/path/to", 0).returns(@short_acl)
+
+                @long_acl.stubs(:"<=>").with(@short_acl).returns(0)
+                @short_acl.stubs(:"<=>").with(@long_acl).returns(0)
+            end
+
+            it "should select the first match" do
+                @right.newright("/path/to/there", 0)
+                @right.newright("/path/to", 0)
+
+                @long_acl.stubs(:match?).returns(true)
+                @short_acl.stubs(:match?).returns(true)
+
+                @long_acl.expects(:allowed?).returns(true)
+                @short_acl.expects(:allowed?).never
+
+                @right.allowed?("/path/to/there/and/there", :args)
+            end
+
+            it "should select the first match that doesn't return :dunno" do
+                @right.newright("/path/to/there", 0)
+                @right.newright("/path/to", 0)
+
+                @long_acl.stubs(:match?).returns(true)
+                @short_acl.stubs(:match?).returns(true)
+
+                @long_acl.expects(:allowed?).returns(:dunno)
+                @short_acl.expects(:allowed?)
+
+                @right.allowed?("/path/to/there/and/there", :args)
+            end
+
+            it "should not select an ACL that doesn't match" do
+                @right.newright("/path/to/there", 0)
+                @right.newright("/path/to", 0)
+
+                @long_acl.stubs(:match?).returns(false)
+                @short_acl.stubs(:match?).returns(true)
+
+                @long_acl.expects(:allowed?).never
+                @short_acl.expects(:allowed?)
+
+                @right.allowed?("/path/to/there/and/there", :args)
+            end
+
+            it "should return the result of the acl" do
+                @right.newright("/path/to/there", 0)
+
+                @long_acl.stubs(:match?).returns(true)
+                @long_acl.stubs(:allowed?).returns(:returned)
+
+                @right.allowed?("/path/to/there/and/there", :args).should == :returned
+            end
+
+            it "should not raise an error if this path acl doesn't exist" do
+                lambda{ @right.allowed?("/path", :args) }.should_not raise_error
+            end
+
+            it "should return false if no path match" do
+                @right.allowed?("/path", :args).should be_false
+            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)
 
-        it "should create an ACL with a shortname" do
-            @right.newright("name")
+                @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_acl1.stubs(:"<=>").with(@regex_acl2).returns(0)
+                @regex_acl2.stubs(:"<=>").with(@regex_acl1).returns(0)
+            end
+
+            it "should select the first match" do
+                @right.newright("~ /files/(.*)/myfile", 0)
+                @right.newright("~ /files/(.*)/myfile/", 0)
+
+                @regex_acl1.stubs(:match?).returns(true)
+                @regex_acl2.stubs(:match?).returns(true)
+
+                @regex_acl1.expects(:allowed?).returns(true)
+                @regex_acl2.expects(:allowed?).never
+
+                @right.allowed?("/files/repository/myfile/other", :args)
+            end
+
+            it "should select the first match that doesn't return :dunno" do
+                @right.newright("~ /files/(.*)/myfile", 0)
+                @right.newright("~ /files/(.*)/myfile/", 0)
+
+                @regex_acl1.stubs(:match?).returns(true)
+                @regex_acl2.stubs(:match?).returns(true)
+
+                @regex_acl1.expects(:allowed?).returns(:dunno)
+                @regex_acl2.expects(:allowed?)
+
+                @right.allowed?("/files/repository/myfile/other", :args)
+            end
+
+            it "should not select an ACL that doesn't match" do
+                @right.newright("~ /files/(.*)/myfile", 0)
+                @right.newright("~ /files/(.*)/myfile/", 0)
+
+                @regex_acl1.stubs(:match?).returns(false)
+                @regex_acl2.stubs(:match?).returns(true)
+
+                @regex_acl1.expects(:allowed?).never
+                @regex_acl2.expects(:allowed?)
+
+                @right.allowed?("/files/repository/myfile/other", :args)
+            end
+
+            it "should return the result of the acl" do
+                @right.newright("~ /files/(.*)/myfile", 0)
+
+                @regex_acl1.stubs(:match?).returns(true)
+                @regex_acl1.stubs(:allowed?).returns(:returned)
+
+                @right.allowed?("/files/repository/myfile/other", :args).should == :returned
+            end
+
+            it "should not raise an error if no regex acl match" do
+                lambda{ @right.allowed?("/path", :args) }.should_not raise_error
+            end
+
+            it "should return false if no regex match" do
+                @right.allowed?("/path", :args).should be_false
+            end
 
-            @right["name"].shortname.should == "n"
         end
     end
+
+    describe Puppet::Network::Rights::Right do
+        before :each do
+            @acl = Puppet::Network::Rights::Right.new("/path",0)
+        end
+
+        describe "with path" do
+            it "should say it's a regex ACL" do
+                @acl.acl_type.should == :regex
+            end
+
+            it "should match up to its path length" do
+                @acl.match?("/path/that/works").should_not be_nil
+            end
+
+            it "should match up to its path length" do
+                @acl.match?("/paththatalsoworks").should_not be_nil
+            end
+
+            it "should return nil if no match" do
+                @acl.match?("/notpath").should be_nil
+            end
+        end
+
+        describe "with regex" do
+            before :each do
+                @acl = Puppet::Network::Rights::Right.new("~ .rb$",0)
+            end
+
+            it "should say it's a regex ACL" do
+                @acl.acl_type.should == :regex
+            end
+
+            it "should match as a regex" do
+                @acl.match?("this shoud work.rb").should_not be_nil
+            end
+
+            it "should return nil if no match" do
+                @acl.match?("do not match").should be_nil
+            end
+        end
+
+        it "should allow all rest methods by default" do
+            @acl.methods.should == Puppet::Network::Rights::Right::ALL
+        end
+
+        it "should allow modification of the methods filters" do
+            @acl.restrict_method(:save)
+
+            @acl.methods.should == [:save]
+        end
+
+        it "should stack methods filters" do
+            @acl.restrict_method(:save)
+            @acl.restrict_method(:destroy)
+
+            @acl.methods.should == [:save, :destroy]
+        end
+
+        it "should raise an error if the method is already filtered" do
+            @acl.restrict_method(:save)
+
+            lambda { @acl.restrict_method(:save) }.should raise_error
+        end
+
+        describe "when checking right authorization" do
+            it "should return :dunno if this right doesn't apply" do
+                @acl.restrict_method(:destroy)
+
+                @acl.allowed?("me","127.0.0.1", :save).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, :match)
+            end
+
+            it "should reset interpolation after the match" do
+                @acl.expects(:reset_interpolation)
+
+                @acl.allowed?("me","127.0.0.1", :save, :match)
+            end
+
+            # mocha doesn't allow testing super...
+            # it "should delegate to the AuthStore for the result" do
+            #     @acl.method(:save)
+            # 
+            #     @acl.expects(:allowed?).with("me","127.0.0.1")
+            # 
+            #     @acl.allowed?("me","127.0.0.1", :save)
+            # end
+        end
+    end
+
 end