[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, upstream, updated. 0.25.5-639-g8f94f35

[test branch]

The following commit has been merged in the upstream branch:
commit ae520057280c2454bc44c64ac1e6686bf2eb086d
Author: Markus Roberts <Markus at reality.com>
Date:   Wed Apr 28 15:39:39 2010 -0700

    Write ssh_authorized_keys as user
    
    This is a targeted fix to the issue of permissions when writing ssh authorized
    key files by 1) requiring that an existing users be specified on the resource
    and 2) doing the write as that user.  It's based on Michael DeHaan's initial
    implementation of Luke's idea, but with a number of simplifications (mostly by
    testing necessary conditions as early as possible so the code isn't cluttered
    up with a lot of checks).
    
    The tests in this version are modified slightly to remove some additional
    implementation couplings that were added in master.

diff --git a/lib/puppet/provider/ssh_authorized_key/parsed.rb b/lib/puppet/provider/ssh_authorized_key/parsed.rb
index 6265c6b..fb4d095 100644
--- a/lib/puppet/provider/ssh_authorized_key/parsed.rb
+++ b/lib/puppet/provider/ssh_authorized_key/parsed.rb
@@ -62,36 +62,16 @@ Puppet::Type.type(:ssh_authorized_key).provide(:parsed,
     end
 
     def flush
-        # As path expansion had to be moved in the provider, we cannot generate new file
-        # resources and thus have to chown and chmod here. It smells hackish.
-
-        # Create target's parent directory if nonexistant
-        if target
-            dir = File.dirname(target)
-            if not File.exist? dir
-                Puppet.debug("Creating directory %s which did not exist" % dir)
-                Dir.mkdir(dir, dir_perm)
-            end
-        end
-
-        # Generate the file
-        super
-
-        # Ensure correct permissions
-        if target and user
-            uid = Puppet::Util.uid(user)
-
-            if uid
-                File.chown(uid, nil, dir)
-                File.chown(uid, nil, target)
-            else
-                raise Puppet::Error, "Specified user does not exist"
-            end
-        end
-
-        if target and FileTest.exist?(target)
-            File.chmod(file_perm, target)
+        raise Puppet::Error, "Cannot write SSH authorized keys without user" unless user
+        raise Puppet::Error, "User '#{user}' does not exist"                 unless uid = Puppet::Util.uid(user)
+        unless File.exist?(dir = File.dirname(target))
+            Puppet.debug "Creating #{dir}"
+            Dir.mkdir(dir, dir_perm)
+            File.chown(uid, nil, dir)
         end
+        Puppet::Util::SUIDManager.asuser(user) { super }
+        File.chown(uid, nil, target)
+        File.chmod(file_perm, target)
     end
 
     # parse sshv2 option strings, wich is a comma separated list of
diff --git a/spec/unit/provider/ssh_authorized_key/parsed.rb b/spec/unit/provider/ssh_authorized_key/parsed.rb
index 584ac7c..0edf2b0 100755
--- a/spec/unit/provider/ssh_authorized_key/parsed.rb
+++ b/spec/unit/provider/ssh_authorized_key/parsed.rb
@@ -17,11 +17,10 @@ describe provider_class do
     before :each do
         @sshauthkey_class = Puppet::Type.type(:ssh_authorized_key)
         @provider = @sshauthkey_class.provider(:parsed)
-
-        @keyfile = tmpfile("ssh_key")
-        #@provider.stubs(:default_target).returns @keyfile
-        #@provider.stubs(:flush)
+        @keyfile = File.join(tmpdir, 'authorized_keys')
         @provider.any_instance.stubs(:target).returns @keyfile
+        @user = 'random_bob'
+        Puppet::Util.stubs(:uid).with(@user).returns 12345
     end
 
     after :each do
@@ -30,22 +29,23 @@ describe provider_class do
 
     def mkkey(args)
         fakeresource = fakeresource(:ssh_authorized_key, args[:name])
+        fakeresource.stubs(:should).with(:user).returns @user
+        fakeresource.stubs(:should).with(:target).returns @keyfile
 
         key = @provider.new(fakeresource)
         args.each do |p,v|
             key.send(p.to_s + "=", v)
         end
 
-        return key
+        key
     end
 
     def genkey(key)
         @provider.stubs(:filetype).returns(Puppet::Util::FileType::FileTypeRam)
-        file = @provider.default_target
-
+        File.stubs(:chown)
+        File.stubs(:chmod)
         key.flush
-        text = @provider.target_object(file).read
-        return text
+        @provider.target_object(@keyfile).read
     end
 
     PuppetTest.fakedata("data/providers/ssh_authorized_key/parsed").each { |file|
@@ -136,7 +136,6 @@ describe provider_class do
             end
 
             it "should chmod the key file to 0600" do
-                FileTest.expects(:exist?).with("/tmp/.ssh_dir/place_to_put_authorized_keys").returns true
                 File.expects(:chmod).with(0600, "/tmp/.ssh_dir/place_to_put_authorized_keys")
                 @provider.flush
             end
@@ -154,20 +153,35 @@ describe provider_class do
                 # but mocha objects strenuously to stubbing File.expand_path
                 # so I'm left with using nobody.
                 @dir = File.expand_path("~nobody/.ssh")
-           end
+            end
 
-            it "should create the directory" do
+            it "should create the directory if it doesn't exist" do
                 File.stubs(:exist?).with(@dir).returns false
                 Dir.expects(:mkdir).with(@dir,0700)
                 @provider.flush
             end
 
-            it "should chown the directory to the user" do
+            it "should not create or chown the directory if it already exist" do
+                File.stubs(:exist?).with(@dir).returns false
+                Dir.expects(:mkdir).never
+                @provider.flush
+            end
+
+            it "should chown the directory to the user if it creates it" do
+                File.stubs(:exist?).with(@dir).returns false
+                Dir.stubs(:mkdir).with(@dir,0700)
                 uid = Puppet::Util.uid("nobody")
                 File.expects(:chown).with(uid, nil, @dir)
                 @provider.flush
             end
 
+            it "should not create or chown the directory if it already exist" do
+                File.stubs(:exist?).with(@dir).returns false
+                Dir.expects(:mkdir).never
+                File.expects(:chown).never
+                @provider.flush
+            end
+
             it "should chown the key file to the user" do
                 uid = Puppet::Util.uid("nobody")
                 File.expects(:chown).with(uid, nil, File.expand_path("~nobody/.ssh/authorized_keys"))
@@ -175,7 +189,6 @@ describe provider_class do
             end
 
             it "should chmod the key file to 0600" do
-                FileTest.expects(:exist?).with(File.expand_path("~nobody/.ssh/authorized_keys")).returns true
                 File.expects(:chmod).with(0600, File.expand_path("~nobody/.ssh/authorized_keys"))
                 @provider.flush
             end
@@ -187,18 +200,9 @@ describe provider_class do
                 @resource.stubs(:should).with(:target).returns("/tmp/.ssh_dir/place_to_put_authorized_keys")
             end
 
-            it "should make the directory" do
-                File.stubs(:exist?).with("/tmp/.ssh_dir").returns false
-                Dir.expects(:mkdir).with("/tmp/.ssh_dir", 0755)
-                @provider.flush
-            end
-
-            it "should chmod the key file to 0644" do
-                FileTest.expects(:exist?).with("/tmp/.ssh_dir/place_to_put_authorized_keys").returns true
-                File.expects(:chmod).with(0644, "/tmp/.ssh_dir/place_to_put_authorized_keys")
-                @provider.flush
+            it "should raise an error" do
+                proc { @provider.flush }.should raise_error
             end
         end
-
     end
 end

-- 
Puppet packaging for Debian