[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, upstream, updated. 0.25.4-89-gcbbd363

[James Turnbull]

The following commit has been merged in the upstream branch:
commit ce233aa2a511bf6818f28c226144ec5b05a468ee
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).

diff --git a/lib/puppet/provider/ssh_authorized_key/parsed.rb b/lib/puppet/provider/ssh_authorized_key/parsed.rb
index b435c51..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
-            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 d5c66d7..fc3f550 100755
--- a/spec/unit/provider/ssh_authorized_key/parsed.rb
+++ b/spec/unit/provider/ssh_authorized_key/parsed.rb
@@ -15,6 +15,9 @@ describe provider_class do
     before :each do
         @sshauthkey_class = Puppet::Type.type(:ssh_authorized_key)
         @provider = @sshauthkey_class.provider(:parsed)
+        @keyfile = File.join(tmpdir, 'authorized_keys')
+        @user = 'random_bob'
+        Puppet::Util.stubs(:uid).with(@user).returns 12345
     end
 
     after :each do
@@ -23,22 +26,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|
@@ -147,20 +151,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"))
@@ -179,17 +198,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
-                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