Allow REST auth system to restrict an ACL to authenticated or unauthenticated request

Introduces a new auth.conf directive (auth or authenticated) which
takes an argument (on,yes/off,no/all,any).
This can be used to restrict an ACL to only some state of
authentication of a REST request, or any.
If no auth directive is given, the ACL will only trigger for
authenticated requests.

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

4 files changed, 116 insertions, 12 deletions

diff --git a/lib/puppet/network/authconfig.rb b/lib/puppet/network/authconfig.rb
index 3e40c9d7c..41f8f1c3e 100644
--- a/lib/puppet/network/authconfig.rb
+++ b/lib/puppet/network/authconfig.rb
@@ -111,7 +111,7 @@ module Puppet
                             end
                             name.chomp!
                             right = newrights.newright(name, count, @file)
-                        when /^\s*(allow|deny|method|environment)\s+(.+)$/
+                        when /^\s*(allow|deny|method|environment|auth(?:enticated)?)\s+(.+)$/
                             parse_right_directive(right, $1, $2, count)
                         else
                             raise ConfigurationError, "Invalid line %s: %s" % [count, line]
@@ -155,6 +155,11 @@ module Puppet
                     raise ConfigurationError, "'environment' directive not allowed in namespace ACL at line %s of %s" % [count, @config]
                 end
                 modify_right(right, :restrict_environment, value, "adding environment %s", count)
+            when /auth(?:enticated)?/
+                unless right.acl_type == :regex
+                    raise ConfigurationError, "'authenticated' directive not allowed in namespace ACL at line %s of %s" % [count, @config]
+                end
+                modify_right(right, :restrict_authenticated, value, "adding authentication %s", count)
             else
                 raise ConfigurationError,
                     "Invalid argument '%s' at line %s" % [var, count]
diff --git a/lib/puppet/network/rights.rb b/lib/puppet/network/rights.rb
index c98b84e8d..14b171081 100755
--- a/lib/puppet/network/rights.rb
+++ b/lib/puppet/network/rights.rb
@@ -14,7 +14,7 @@ class Rights
 
     # We basically just proxy directly to our rights.  Each Right stores
     # its own auth abilities.
-    [:allow, :deny, :restrict_method, :restrict_environment].each do |method|
+    [:allow, :deny, :restrict_method, :restrict_environment, :restrict_authenticated].each do |method|
         define_method(method) do |name, *args|
             if obj = self[name]
                 obj.send(method, *args)
@@ -27,7 +27,7 @@ class Rights
     # Check that name is allowed or not
     def allowed?(name, *args)
         begin
-            fail_on_deny(name, *args)
+            fail_on_deny(name, :node => args[0], :ip => args[1])
         rescue AuthorizationError
             return false
         rescue ArgumentError
@@ -59,10 +59,12 @@ class Rights
 
         # 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 =~ /^\//
+        if name =~ /^\// or right
             # 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] ]
 
+            msg += " authenticated " if args[:authenticated]
+
             error = AuthorizationError.new("Forbidden request: " + msg)
             if right
                 error.file = right.file
@@ -123,7 +125,7 @@ class Rights
         include Puppet::FileCollection::Lookup
 
         attr_accessor :name, :key, :acl_type
-        attr_accessor :methods, :environment
+        attr_accessor :methods, :environment, :authentication
 
         ALL = [:save, :destroy, :find, :search]
 
@@ -132,6 +134,7 @@ class Rights
         def initialize(name, line, file)
             @methods = []
             @environment = []
+            @authentication = true # defaults to authenticated
             @name = name
             @line = line || 0
             @file = file
@@ -175,9 +178,10 @@ class 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, args)
+        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])
+            return :dunno if acl_type == :regex and not @authentication.nil? and args[:authenticated] != @authentication
 
             begin
                 # make sure any capture are replaced if needed
@@ -218,6 +222,20 @@ class Rights
             @environment << env
         end
 
+        def restrict_authenticated(authentication)
+            case authentication
+            when "yes", "on", "true", true
+                authentication = true
+            when "no", "off", "false", false
+                authentication = false
+            when "all","any", :all, :any
+                authentication = nil
+            else
+                raise ArgumentError, "'%s' incorrect authenticated value: %s" % [name, authentication]
+            end
+            @authentication = authentication
+        end
+
         def match?(key)
             # if we are a namespace compare directly
             return self.key == namespace_to_key(key) if acl_type == :name
@@ -249,7 +267,7 @@ class Rights
 
         def ==(name)
             return self.key == namespace_to_key(name) if acl_type == :name
-            return self.name == name
+            return self.name == name.gsub(/^~\s+/,'')
         end
 
     end
diff --git a/spec/unit/network/authconfig.rb b/spec/unit/network/authconfig.rb
index 21af89bfd..f6665234b 100755
--- a/spec/unit/network/authconfig.rb
+++ b/spec/unit/network/authconfig.rb
@@ -253,6 +253,40 @@ describe Puppet::Network::AuthConfig do
             lambda { @authconfig.read }.should raise_error
         end
 
+        it "should inform the current ACL if we get the 'auth' directive" do
+            acl = stub 'acl', :info
+            acl.stubs(:acl_type).returns(:regex)
+
+            @fd.stubs(:each).multiple_yields('path /certificates', 'auth yes')
+            @rights.stubs(:newright).with("/certificates", 1, 'dummy').returns(acl)
+
+            acl.expects(:restrict_authenticated).with('yes')
+
+            @authconfig.read
+        end
+
+        it "should also allow the longest 'authenticated' directive" do
+            acl = stub 'acl', :info
+            acl.stubs(:acl_type).returns(:regex)
+
+            @fd.stubs(:each).multiple_yields('path /certificates', 'authenticated yes')
+            @rights.stubs(:newright).with("/certificates", 1, 'dummy').returns(acl)
+
+            acl.expects(:restrict_authenticated).with('yes')
+
+            @authconfig.read
+        end
+
+        it "should raise an error if the 'auth' 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]', 'auth yes')
+            @rights.stubs(:newright).with("puppetca", 1, 'dummy').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 be1db723d..8a86c8c34 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, :deny, :restrict_method, :restrict_environment].each do |m|
+    [:allow, :deny, :restrict_method, :restrict_environment, :restrict_authenticated].each do |m|
         it "should have a #{m} method" do
             @right.should respond_to(m)
         end
@@ -94,6 +94,12 @@ describe Puppet::Network::Rights do
             @right[".rb$"].should_not be_nil
         end
 
+        it "should be able to lookup the regex by its full name" 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
@@ -150,9 +156,9 @@ describe Puppet::Network::Rights do
         end
 
         it "should delegate to fail_on_deny" do
-            @right.expects(:fail_on_deny).with("namespace", :args)
+            @right.expects(:fail_on_deny).with("namespace", :node => "host.domain.com", :ip => "127.0.0.1")
 
-            @right.allowed?("namespace", :args)
+            @right.allowed?("namespace", "host.domain.com", "127.0.0.1")
         end
 
         it "should return true if fail_on_deny doesn't fail" do
@@ -397,6 +403,10 @@ describe Puppet::Network::Rights do
             @acl.methods.should == Puppet::Network::Rights::Right::ALL
         end
 
+        it "should allow only authenticated request by default" do
+            @acl.authentication.should be_true
+        end
+
         it "should allow modification of the methods filters" do
             @acl.restrict_method(:save)
 
@@ -424,6 +434,30 @@ describe Puppet::Network::Rights do
             @acl.environment.should == [:env]
         end
 
+        ["on", "yes", "true", true].each do |auth|
+            it "should allow filtering on authenticated requests with '#{auth}'" do
+                @acl.restrict_authenticated(auth)
+
+                @acl.authentication.should be_true
+            end
+        end
+
+        ["off", "no", "false", false].each do |auth|
+            it "should allow filtering on unauthenticated requests with '#{auth}'" do
+                @acl.restrict_authenticated(auth)
+
+                @acl.authentication.should be_false
+            end
+        end
+
+        ["all", "any", :all, :any].each do |auth|
+            it "should not use request authenticated state filtering with '#{auth}'" do
+                @acl.restrict_authenticated(auth)
+
+                @acl.authentication.should be_nil
+            end
+        end
+
         describe "when checking right authorization" do
             it "should return :dunno if this right is not restricted to the given method" do
                 @acl.restrict_method(:destroy)
@@ -446,16 +480,29 @@ describe Puppet::Network::Rights do
                 @acl.allowed?("me","127.0.0.1", { :method => :save, :environment => :development }).should == :dunno
             end
 
+            it "should return :dunno if this right is not restricted to the given request authentication state" do
+                @acl.restrict_authenticated(true)
+
+                @acl.allowed?("me","127.0.0.1", { :method => :save, :authenticated => false }).should == :dunno
+            end
+
+            it "should return allow/deny if this right is restricted to the given request authentication state" do
+                @acl.restrict_authenticated(false)
+                @acl.allow("127.0.0.1")
+
+                @acl.allowed?("me","127.0.0.1", { :authenticated => false }).should be_true
+            end
+
             it "should interpolate allow/deny patterns with the given match" do
                 @acl.expects(:interpolate).with(:match)
 
-                @acl.allowed?("me","127.0.0.1", :method => :save, :match => :match)
+                @acl.allowed?("me","127.0.0.1", { :method => :save, :match => :match, :authenticated => true })
             end
 
             it "should reset interpolation after the match" do
                 @acl.expects(:reset_interpolation)
 
-                @acl.allowed?("me","127.0.0.1", :method => :save, :match => :match)
+                @acl.allowed?("me","127.0.0.1", { :method => :save, :match => :match, :authenticated => true })
             end
 
             # mocha doesn't allow testing super...