diff --git a/lib/puppet/type/ssh_authorized_key.rb b/lib/puppet/type/ssh_authorized_key.rb index 4d768f1a2..968fcafa8 100644 --- a/lib/puppet/type/ssh_authorized_key.rb +++ b/lib/puppet/type/ssh_authorized_key.rb @@ -1,116 +1,113 @@ module Puppet newtype(:ssh_authorized_key) do @doc = "Manages SSH authorized keys. Currently only type 2 keys are supported. **Autorequires:** If Puppet is managing the user account in which this SSH key should be installed, the `ssh_authorized_key` resource will autorequire that user." ensurable newparam(:name) do desc "The SSH key comment. This attribute is currently used as a system-wide primary key and therefore has to be unique." isnamevar - validate do |value| - raise Puppet::Error, "Resourcename must not contain whitespace: #{value}" if value =~ /\s/ - end end newproperty(:type) do desc "The encryption type used: ssh-dss or ssh-rsa." newvalue("ssh-dss") newvalue("ssh-rsa") aliasvalue(:dsa, "ssh-dss") aliasvalue(:rsa, "ssh-rsa") end newproperty(:key) do desc "The key itself; generally a long string of hex digits." validate do |value| raise Puppet::Error, "Key must not contain whitespace: #{value}" if value =~ /\s/ end end newproperty(:user) do desc "The user account in which the SSH key should be installed. The resource will automatically depend on this user." end newproperty(:target) do desc "The absolute filename in which to store the SSH key. This property is optional and should only be used in cases where keys are stored in a non-standard location (i.e.` not in `~user/.ssh/authorized_keys`)." defaultto :absent def should return super if defined?(@should) and @should[0] != :absent return nil unless user = resource[:user] begin return File.expand_path("~#{user}/.ssh/authorized_keys") rescue Puppet.debug "The required user is not yet present on the system" return nil end end def insync?(is) is == should end end newproperty(:options, :array_matching => :all) do desc "Key options, see sshd(8) for possible values. Multiple values should be specified as an array." defaultto do :absent end def is_to_s(value) if value == :absent or value.include?(:absent) super else value.join(",") end end def should_to_s(value) if value == :absent or value.include?(:absent) super else value.join(",") end end validate do |value| unless value == :absent or value =~ /^[-a-z0-9A-Z_]+(?:=\".*?\")?$/ raise Puppet::Error, "Option #{value} is not valid. A single option must either be of the form 'option' or 'option=\"value\". Multiple options must be provided as an array" end end end autorequire(:user) do should(:user) if should(:user) end validate do # Go ahead if target attribute is defined return if @parameters[:target].shouldorig[0] != :absent # Go ahead if user attribute is defined return if @parameters.include?(:user) # If neither target nor user is defined, this is an error raise Puppet::Error, "Attribute 'user' or 'target' is mandatory" end end end diff --git a/spec/unit/provider/ssh_authorized_key/parsed_spec.rb b/spec/unit/provider/ssh_authorized_key/parsed_spec.rb index 3839c6e32..91e1d9dd7 100755 --- a/spec/unit/provider/ssh_authorized_key/parsed_spec.rb +++ b/spec/unit/provider/ssh_authorized_key/parsed_spec.rb @@ -1,200 +1,205 @@ #!/usr/bin/env rspec require 'spec_helper' require 'shared_behaviours/all_parsedfile_providers' require 'puppet_spec/files' provider_class = Puppet::Type.type(:ssh_authorized_key).provider(:parsed) describe provider_class, :unless => Puppet.features.microsoft_windows? do include PuppetSpec::Files before :each do @keyfile = tmpfile('authorized_keys') @provider_class = provider_class @provider_class.initvars @provider_class.any_instance.stubs(:target).returns @keyfile @user = 'random_bob' Puppet::Util.stubs(:uid).with(@user).returns 12345 end def mkkey(args) args[:target] = @keyfile args[:user] = @user resource = Puppet::Type.type(:ssh_authorized_key).new(args) key = @provider_class.new(resource) args.each do |p,v| key.send(p.to_s + "=", v) end key end def genkey(key) @provider_class.stubs(:filetype).returns(Puppet::Util::FileType::FileTypeRam) File.stubs(:chown) File.stubs(:chmod) Puppet::Util::SUIDManager.stubs(:asuser).yields key.flush @provider_class.target_object(@keyfile).read end it_should_behave_like "all parsedfile providers", provider_class it "should be able to generate a basic authorized_keys file" do key = mkkey(:name => "Just_Testing", :key => "AAAAfsfddsjldjgksdflgkjsfdlgkj", :type => "ssh-dss", :ensure => :present, :options => [:absent] ) genkey(key).should == "ssh-dss AAAAfsfddsjldjgksdflgkjsfdlgkj Just_Testing\n" end it "should be able to generate a authorized_keys file with options" do key = mkkey(:name => "root@localhost", :key => "AAAAfsfddsjldjgksdflgkjsfdlgkj", :type => "ssh-rsa", :ensure => :present, :options => ['from="192.168.1.1"', "no-pty", "no-X11-forwarding"] ) genkey(key).should == "from=\"192.168.1.1\",no-pty,no-X11-forwarding ssh-rsa AAAAfsfddsjldjgksdflgkjsfdlgkj root@localhost\n" end + it "should be able to parse name if it includes whitespace" do + @provider_class.parse_line('ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQC7pHZ1XRj3tXbFpPFhMGU1bVwz7jr13zt/wuE+pVIJA8GlmHYuYtIxHPfDHlkixdwLachCpSQUL9NbYkkRFRn9m6PZ7125ohE4E4m96QS6SGSQowTiRn4Lzd9LV38g93EMHjPmEkdSq7MY4uJEd6DUYsLvaDYdIgBiLBIWPA3OrQ== fancy user')[:name].should == 'fancy user' + @provider_class.parse_line('from="host1.reductlivelabs.com,host.reductivelabs.com",command="/usr/local/bin/run",ssh-pty ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQC7pHZ1XRj3tXbFpPFhMGU1bVwz7jr13zt/wuE+pVIJA8GlmHYuYtIxHPfDHlkixdwLachCpSQUL9NbYkkRFRn9m6PZ7125ohE4E4m96QS6SGSQowTiRn4Lzd9LV38g93EMHjPmEkdSq7MY4uJEd6DUYsLvaDYdIgBiLBIWPA3OrQ== fancy user')[:name].should == 'fancy user' + end + it "should be able to parse options containing commas via its parse_options method" do options = %w{from="host1.reductlivelabs.com,host.reductivelabs.com" command="/usr/local/bin/run" ssh-pty} optionstr = options.join(", ") @provider_class.parse_options(optionstr).should == options end it "should use '' as name for entries that lack a comment" do line = "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAut8aOSxenjOqF527dlsdHWV4MNoAsX14l9M297+SQXaQ5Z3BedIxZaoQthkDALlV/25A1COELrg9J2MqJNQc8Xe9XQOIkBQWWinUlD/BXwoOTWEy8C8zSZPHZ3getMMNhGTBO+q/O+qiJx3y5cA4MTbw2zSxukfWC87qWwcZ64UUlegIM056vPsdZWFclS9hsROVEa57YUMrehQ1EGxT4Z5j6zIopufGFiAPjZigq/vqgcAqhAKP6yu4/gwO6S9tatBeEjZ8fafvj1pmvvIplZeMr96gHE7xS3pEEQqnB3nd4RY7AF6j9kFixnsytAUO7STPh/M3pLiVQBN89TvWPQ==" @provider_class.parse(line)[0][:name].should == "" end end describe provider_class, :unless => Puppet.features.microsoft_windows? do before :each do @resource = Puppet::Type.type(:ssh_authorized_key).new(:name => "foo", :user => "random_bob") @provider = provider_class.new(@resource) provider_class.stubs(:filetype).returns(Puppet::Util::FileType::FileTypeRam) Puppet::Util::SUIDManager.stubs(:asuser).yields provider_class.initvars end describe "when flushing" do before :each do # Stub file and directory operations Dir.stubs(:mkdir) File.stubs(:chmod) File.stubs(:chown) end describe "and both a user and a target have been specified" do before :each do Puppet::Util.stubs(:uid).with("random_bob").returns 12345 @resource[:user] = "random_bob" target = "/tmp/.ssh_dir/place_to_put_authorized_keys" @resource[:target] = target end it "should create the directory" do File.stubs(:exist?).with("/tmp/.ssh_dir").returns false Dir.expects(:mkdir).with("/tmp/.ssh_dir", 0700) @provider.flush end it "should absolutely not chown the directory to the user" do uid = Puppet::Util.uid("random_bob") File.expects(:chown).never @provider.flush end it "should absolutely not chown the key file to the user" do uid = Puppet::Util.uid("random_bob") File.expects(:chown).never @provider.flush end it "should chmod the key file to 0600" do File.expects(:chmod).with(0600, "/tmp/.ssh_dir/place_to_put_authorized_keys") @provider.flush end end describe "and a user has been specified with no target" do before :each do @resource[:user] = "nobody" # # I'd like to use random_bob here and something like # # File.stubs(:expand_path).with("~random_bob/.ssh").returns "/users/r/random_bob/.ssh" # # but mocha objects strenuously to stubbing File.expand_path # so I'm left with using nobody. @dir = File.expand_path("~nobody/.ssh") end 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 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 absolutely not 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).never @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 absolutely not chown the key file to the user" do uid = Puppet::Util.uid("nobody") File.expects(:chown).never @provider.flush end it "should chmod the key file to 0600" do File.expects(:chmod).with(0600, File.expand_path("~nobody/.ssh/authorized_keys")) @provider.flush end end describe "and a target has been specified with no user" do it "should raise an error" do @resource = Puppet::Type.type(:ssh_authorized_key).new(:name => "foo", :target => "/tmp/.ssh_dir/place_to_put_authorized_keys") @provider = provider_class.new(@resource) proc { @provider.flush }.should raise_error end end describe "and a invalid user has been specified with no target" do it "should catch an exception and raise a Puppet error" do @resource[:user] = "thisusershouldnotexist" lambda { @provider.flush }.should raise_error(Puppet::Error) end end end end diff --git a/spec/unit/type/ssh_authorized_key_spec.rb b/spec/unit/type/ssh_authorized_key_spec.rb index 06c2e3604..092deb7a2 100755 --- a/spec/unit/type/ssh_authorized_key_spec.rb +++ b/spec/unit/type/ssh_authorized_key_spec.rb @@ -1,267 +1,266 @@ #!/usr/bin/env rspec require 'spec_helper' ssh_authorized_key = Puppet::Type.type(:ssh_authorized_key) describe ssh_authorized_key, :unless => Puppet.features.microsoft_windows? do include PuppetSpec::Files before do @class = Puppet::Type.type(:ssh_authorized_key) @provider_class = stub 'provider_class', :name => "fake", :suitable? => true, :supports_parameter? => true @class.stubs(:defaultprovider).returns(@provider_class) @class.stubs(:provider).returns(@provider_class) @provider = stub 'provider', :class => @provider_class, :file_path => make_absolute("/tmp/whatever"), :clear => nil @provider_class.stubs(:new).returns(@provider) @catalog = Puppet::Resource::Catalog.new end it "should have :name be its namevar" do @class.key_attributes.should == [:name] end describe "when validating attributes" do [:name, :provider].each do |param| it "should have a #{param} parameter" do @class.attrtype(param).should == :param end end [:type, :key, :user, :target, :options, :ensure].each do |property| it "should have a #{property} property" do @class.attrtype(property).should == :property end end end describe "when validating values" do describe "for name" do it "should support valid names" do proc { @class.new(:name => "username", :ensure => :present, :user => "nobody") }.should_not raise_error proc { @class.new(:name => "username@hostname", :ensure => :present, :user => "nobody") }.should_not raise_error end - it "should not support whitespaces" do - proc { @class.new(:name => "my test", :ensure => :present, :user => "nobody") }.should raise_error(Puppet::Error,/Resourcename must not contain whitespace/) - proc { @class.new(:name => "my\ttest", :ensure => :present, :user => "nobody") }.should raise_error(Puppet::Error,/Resourcename must not contain whitespace/) + it "should support whitespace" do + proc { @class.new(:name => "my test", :ensure => :present, :user => "nobody") }.should_not raise_error end end describe "for ensure" do it "should support :present" do proc { @class.new(:name => "whev", :ensure => :present, :user => "nobody") }.should_not raise_error end it "should support :absent" do proc { @class.new(:name => "whev", :ensure => :absent, :user => "nobody") }.should_not raise_error end it "should not support other values" do proc { @class.new(:name => "whev", :ensure => :foo, :user => "nobody") }.should raise_error(Puppet::Error, /Invalid value/) end end describe "for type" do it "should support ssh-dss" do proc { @class.new(:name => "whev", :type => "ssh-dss", :user => "nobody") }.should_not raise_error end it "should support ssh-rsa" do proc { @class.new(:name => "whev", :type => "ssh-rsa", :user => "nobody") }.should_not raise_error end it "should support :dsa" do proc { @class.new(:name => "whev", :type => :dsa, :user => "nobody") }.should_not raise_error end it "should support :rsa" do proc { @class.new(:name => "whev", :type => :rsa, :user => "nobody") }.should_not raise_error end it "should alias :rsa to :ssh-rsa" do key = @class.new(:name => "whev", :type => :rsa, :user => "nobody") key.should(:type).should == :'ssh-rsa' end it "should alias :dsa to :ssh-dss" do key = @class.new(:name => "whev", :type => :dsa, :user => "nobody") key.should(:type).should == :'ssh-dss' end it "should not support values other than ssh-dss, ssh-rsa, dsa, rsa" do proc { @class.new(:name => "whev", :type => :something) }.should raise_error(Puppet::Error,/Invalid value/) end end describe "for key" do it "should support a valid key like a 1024 bit rsa key" do proc { @class.new(:name => "whev", :type => :rsa, :user => "nobody", :key => 'AAAAB3NzaC1yc2EAAAADAQABAAAAgQDCPfzW2ry7XvMc6E5Kj2e5fF/YofhKEvsNMUogR3PGL/HCIcBlsEjKisrY0aYgD8Ikp7ZidpXLbz5dBsmPy8hJiBWs5px9ZQrB/EOQAwXljvj69EyhEoGawmxQMtYw+OAIKHLJYRuk1QiHAMHLp5piqem8ZCV2mLb9AsJ6f7zUVw==')}.should_not raise_error end it "should support a valid key like a 4096 bit rsa key" do proc { @class.new(:name => "whev", :type => :rsa, :user => "nobody", :key => 'AAAAB3NzaC1yc2EAAAADAQABAAACAQDEY4pZFyzSfRc9wVWI3DfkgT/EL033UZm/7x1M+d+lBD00qcpkZ6CPT7lD3Z+vylQlJ5S8Wcw6C5Smt6okZWY2WXA9RCjNJMIHQbJAzwuQwgnwU/1VMy9YPp0tNVslg0sUUgpXb13WW4mYhwxyGmIVLJnUrjrQmIFhtfHsJAH8ZVqCWaxKgzUoC/YIu1u1ScH93lEdoBPLlwm6J0aiM7KWXRb7Oq1nEDZtug1zpX5lhgkQWrs0BwceqpUbY+n9sqeHU5e7DCyX/yEIzoPRW2fe2Gx1Iq6JKM/5NNlFfaW8rGxh3Z3S1NpzPHTRjw8js3IeGiV+OPFoaTtM1LsWgPDSBlzIdyTbSQR7gKh0qWYCNV/7qILEfa0yIFB5wIo4667iSPZw2pNgESVtenm8uXyoJdk8iWQ4mecdoposV/znknNb2GPgH+n/2vme4btZ0Sl1A6rev22GQjVgbWOn8zaDglJ2vgCN1UAwmq41RXprPxENGeLnWQppTnibhsngu0VFllZR5kvSIMlekLRSOFLFt92vfd+tk9hZIiKm9exxcbVCGGQPsf6dZ27rTOmg0xM2Sm4J6RRKuz79HQgA4Eg18+bqRP7j/itb89DmtXEtoZFAsEJw8IgIfeGGDtHTkfAlAC92mtK8byeaxGq57XCTKbO/r5gcOMElZHy1AcB8kw==')}.should_not raise_error end it "should support a valid key like a 1024 bit dsa key" do proc { @class.new(:name => "whev", :type => :dsa, :user => "nobody", :key => 'AAAAB3NzaC1kc3MAAACBAI80iR78QCgpO4WabVqHHdEDigOjUEHwIjYHIubR/7u7DYrXY+e+TUmZ0CVGkiwB/0yLHK5dix3Y/bpj8ZiWCIhFeunnXccOdE4rq5sT2V3l1p6WP33RpyVYbLmeuHHl5VQ1CecMlca24nHhKpfh6TO/FIwkMjghHBfJIhXK+0w/AAAAFQDYzLupuMY5uz+GVrcP+Kgd8YqMmwAAAIB3SVN71whLWjFPNTqGyyIlMy50624UfNOaH4REwO+Of3wm/cE6eP8n75vzTwQGBpJX3BPaBGW1S1Zp/DpTOxhCSAwZzAwyf4WgW7YyAOdxN3EwTDJZeyiyjWMAOjW9/AOWt9gtKg0kqaylbMHD4kfiIhBzo31ZY81twUzAfN7angAAAIBfva8sTSDUGKsWWIXkdbVdvM4X14K4gFdy0ZJVzaVOtZ6alysW6UQypnsl6jfnbKvsZ0tFgvcX/CPyqNY/gMR9lyh/TCZ4XQcbqeqYPuceGehz+jL5vArfqsW2fJYFzgCcklmr/VxtP5h6J/T0c9YcDgc/xIfWdZAlznOnphI/FA==')}.should_not raise_error end it "should not support whitespaces" do proc { @class.new(:name => "whev", :type => :rsa, :user => "nobody", :key => 'AAA FA==')}.should raise_error(Puppet::Error,/Key must not contain whitespace/) end end describe "for options" do it "should support flags as options" do proc { @class.new(:name => "whev", :type => :rsa, :user => "nobody", :options => 'cert-authority')}.should_not raise_error proc { @class.new(:name => "whev", :type => :rsa, :user => "nobody", :options => 'no-port-forwarding')}.should_not raise_error end it "should support key-value pairs as options" do proc { @class.new(:name => "whev", :type => :rsa, :user => "nobody", :options => 'command="command"')}.should_not raise_error end it "should support key-value pairs where value consist of multiple items" do proc { @class.new(:name => "whev", :type => :rsa, :user => "nobody", :options => 'from="*.domain1,host1.domain2"')}.should_not raise_error end it "should support environments as options" do proc { @class.new(:name => "whev", :type => :rsa, :user => "nobody", :options => 'environment="NAME=value"')}.should_not raise_error end it "should support multiple options as an array" do proc { @class.new(:name => "whev", :type => :rsa, :user => "nobody", :options => ['cert-authority','environment="NAME=value"'])}.should_not raise_error end it "should not support a comma separated list" do proc { @class.new(:name => "whev", :type => :rsa, :user => "nobody", :options => 'cert-authority,no-port-forwarding')}.should raise_error(Puppet::Error, /must be provided as an array/) end it "should use :absent as a default value" do @class.new(:name => "whev", :type => :rsa, :user => "nobody").should(:options).should == [:absent] end it "property should return well formed string of arrays from is_to_s" do resource = @class.new(:name => "whev", :type => :rsa, :user => "nobody", :options => ["a","b","c"]) resource.property(:options).is_to_s(["a","b","c"]).should == "a,b,c" end it "property should return well formed string of arrays from should_to_s" do resource = @class.new(:name => "whev", :type => :rsa, :user => "nobody", :options => ["a","b","c"]) resource.property(:options).should_to_s(["a","b","c"]).should == "a,b,c" end end describe "for user" do it "should support present users" do proc { @class.new(:name => "whev", :type => :rsa, :user => "root") }.should_not raise_error end it "should support absent users" do proc { @class.new(:name => "whev", :type => :rsa, :user => "ihopeimabsent") }.should_not raise_error end end describe "for target" do it "should support absolute paths" do proc { @class.new(:name => "whev", :type => :rsa, :target => "/tmp/here") }.should_not raise_error end it "should use the user's path if not explicitly specified" do @class.new(:name => "whev", :user => 'root').should(:target).should == File.expand_path("~root/.ssh/authorized_keys") end it "should not consider the user's path if explicitly specified" do @class.new(:name => "whev", :user => 'root', :target => '/tmp/here').should(:target).should == '/tmp/here' end it "should inform about an absent user" do Puppet::Log.level = :debug @class.new(:name => "whev", :user => 'idontexist').should(:target) @logs.map(&:message).should include("The required user is not yet present on the system") end end end describe "when neither user nor target is specified" do it "should raise an error" do proc do @class.new( :name => "Test", :key => "AAA", :type => "ssh-rsa", :ensure => :present) end.should raise_error(Puppet::Error,/user.*or.*target.*mandatory/) end end describe "when both target and user are specified" do it "should use target" do resource = @class.new( :name => "Test", :user => "root", :target => "/tmp/blah" ) resource.should(:target).should == "/tmp/blah" end end describe "when user is specified" do it "should determine target" do resource = @class.create( :name => "Test", :user => "root" ) target = File.expand_path("~root/.ssh/authorized_keys") resource.should(:target).should == target end # Bug #2124 - ssh_authorized_key always changes target if target is not defined it "should not raise spurious change events" do resource = @class.new(:name => "Test", :user => "root") target = File.expand_path("~root/.ssh/authorized_keys") resource.property(:target).safe_insync?(target).should == true end end describe "when calling validate" do it "should not crash on a non-existant user" do resource = @class.create( :name => "Test", :user => "ihopesuchuserdoesnotexist" ) proc { resource.validate }.should_not raise_error end end end