Adding authorization hooks to the file_server and

module_files indirection terminus types.  Both hooks
use the fileserver configuration, but the module_files
hook only uses the 'modules' mount.

Also moved all responsibility for knowing whether to
use the 'modules' terminus type to the terminus selector;
it was previously spread between that and the file_server
terminus, which made some things annoyingly complicated.
This normalizes the deprecation notices and the logic about
how we make these decisions.

5 files changed, 51 insertions, 15 deletions

diff --git a/lib/puppet/file_serving/configuration.rb b/lib/puppet/file_serving/configuration.rb
index 03be1b9dd..ccf0957d1 100644
--- a/lib/puppet/file_serving/configuration.rb
+++ b/lib/puppet/file_serving/configuration.rb
@@ -28,6 +28,14 @@ class Puppet::FileServing::Configuration
 
     private_class_method  :new
 
+    # Verify that the client is allowed access to this file.
+    def authorized?(file, options = {})
+        mount, file_path = split_path(file, options[:node])
+        # If we're not serving this mount, then access is denied.
+        return false unless mount
+        return mount.allowed?(options[:node], options[:ipaddress])
+    end
+
     # Search for a file.
     def file_path(key, options = {})
         mount, file_path = split_path(key, options[:node])
@@ -81,6 +89,7 @@ class Puppet::FileServing::Configuration
             return
         end
 
+        # Don't assign the mounts hash until we're sure the parsing succeeded.
         begin
             newmounts = @parser.parse
             @mounts = newmounts
diff --git a/lib/puppet/file_serving/terminus_selector.rb b/lib/puppet/file_serving/terminus_selector.rb
index 5952cfffa..aa08f087e 100644
--- a/lib/puppet/file_serving/terminus_selector.rb
+++ b/lib/puppet/file_serving/terminus_selector.rb
@@ -12,7 +12,7 @@ module Puppet::FileServing::TerminusSelector
     PROTOCOL_MAP = {"puppet" => :rest, "file" => :local, "puppetmounts" => :file_server}
 
     # Pick an appropriate terminus based on the protocol.
-    def select_terminus(full_uri)
+    def select_terminus(full_uri, options = {})
         # Short-circuit to :local if it's a fully-qualified path.
         return PROTOCOL_MAP["file"] if full_uri =~ /^#{::File::SEPARATOR}/
         begin
@@ -29,8 +29,14 @@ module Puppet::FileServing::TerminusSelector
             terminus = :file_server
         end
 
-        if uri.path =~ /^\/modules\b/ and terminus == :file_server
-            terminus = :modules
+        if terminus == :file_server and uri.path =~ %r{^/([^/]+)\b}
+            modname = $1
+            if modname == "modules"
+                terminus = :modules
+            elsif terminus(:modules).find_module(modname, options[:node])
+                Puppet.warning "DEPRECATION NOTICE: Found file '%s' in module without using the 'modules' mount; please prefix path with '/modules'" % uri.path
+                terminus = :modules
+            end
         end
 
         return terminus
diff --git a/lib/puppet/indirector/file_server.rb b/lib/puppet/indirector/file_server.rb
index 1b2e047e8..51e53d8c9 100644
--- a/lib/puppet/indirector/file_server.rb
+++ b/lib/puppet/indirector/file_server.rb
@@ -10,16 +10,19 @@ require 'puppet/indirector/terminus'
 class Puppet::Indirector::FileServer < Puppet::Indirector::Terminus
     include Puppet::Util::URIHelper
 
+    # Is the client authorized to perform this action?
+    def authorized?(method, key, options = {})
+        return false unless [:find, :search].include?(method)
+
+        uri = key2uri(key)
+
+        configuration.authorized?(uri.path, :node => options[:node], :ipaddress => options[:ipaddress])
+    end
+
     # Find our key using the fileserver.
     def find(key, options = {})
         uri = key2uri(key)
 
-        # First try the modules mount, at least for now.
-        if instance = indirection.terminus(:modules).find(key, options)
-            Puppet.warning "DEPRECATION NOTICE: Found file in module without using the 'modules' mount; please fix"
-            return instance
-        end
-
         return nil unless path = configuration.file_path(uri.path, :node => options[:node]) and FileTest.exists?(path)
 
         return model.new(path)
diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb
index 313117b25..f464f846f 100644
--- a/lib/puppet/indirector/indirection.rb
+++ b/lib/puppet/indirector/indirection.rb
@@ -104,7 +104,7 @@ class Puppet::Indirector::Indirection
         # of URI that the indirection can use for routing to the appropriate
         # terminus.
         if respond_to?(:select_terminus)
-            terminus_name = select_terminus(key)
+            terminus_name = select_terminus(key, *args)
         else
             terminus_name = terminus_class
         end
diff --git a/lib/puppet/indirector/module_files.rb b/lib/puppet/indirector/module_files.rb
index e0374d7a4..739d7b7b5 100644
--- a/lib/puppet/indirector/module_files.rb
+++ b/lib/puppet/indirector/module_files.rb
@@ -4,11 +4,24 @@
 
 require 'puppet/util/uri_helper'
 require 'puppet/indirector/terminus'
+require 'puppet/file_serving/configuration'
 
 # Look files up in Puppet modules.
 class Puppet::Indirector::ModuleFiles < Puppet::Indirector::Terminus
     include Puppet::Util::URIHelper
 
+    # Is the client allowed access to this key with this method?
+    def authorized?(method, key, options = {})
+        return false unless [:find, :search].include?(method)
+
+        uri = key2uri(key)
+
+        # Make sure our file path starts with /modules
+        path = uri.path =~ /^\/modules/ ? uri.path : "/modules" + uri.path
+
+        configuration.authorized?(path, :node => options[:node], :ipaddress => options[:ipaddress])
+    end
+
     # Find our key in a module.
     def find(key, options = {})
         uri = key2uri(key)
@@ -27,7 +40,17 @@ class Puppet::Indirector::ModuleFiles < Puppet::Indirector::Terminus
         return model.new(path)
     end
 
+    # Try to find our module.
+    def find_module(module_name, node_name)
+        Puppet::Module::find(module_name, environment(node_name))
+    end
+
     private
+
+    # Our fileserver configuration, if needed.
+    def configuration
+        Puppet::FileServing::Configuration.create
+    end
     
     # Determine the environment to use, if any.
     def environment(node_name)
@@ -39,9 +62,4 @@ class Puppet::Indirector::ModuleFiles < Puppet::Indirector::Terminus
             nil
         end
     end
-
-    # Try to find our module.
-    def find_module(module_name, node_name)
-        Puppet::Module::find(module_name, environment(node_name))
-    end
 end