Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

(#11046) improve freebsd user and group providers #338

Merged
merged 4 commits into from about 2 years ago

3 participants

Patrick Carlisle Daniel Pittman Tim Bishop
Patrick Carlisle
Owner

Add password support to FreeBSD and enhance the pw user and group providers.

added some commits November 18, 2011
Tim Bishop (#11318) Add password management on FreeBSD
This adds the manages_passwords feature to the pw user provider. It is based
on the patch by Andrew Hust that was integrated into FreeBSD puppet port. It
adds tests covering the create, delete and modify processes of the provider.

This integrates a fix for #7500 that was introduced by the original patch.
The existing code takes the first character of each property and uses it as a
flag. However, with pw, the -p flag is for setting the password expiration.
The result is that the password isn't set at create time and that the password
is set to expire. The next run of puppet correctly sets the password but the
expiry is still set. The new code avoids using -p for passwords, and also sets
the password correctly when an account is created.

Reviewed-by: Patrick Carlisle <patrick@puppetlabs.com>
884381f
Tim Bishop (#10962) Make sure managehome is respected on FreeBSD
When modifying the home directory of a user and managehome is set
the -m flag should be used with pw. This ensures that the new home
directory is created if it doesn't exist.

Also add test to verify this behaviour.
fb111ef
Tim Bishop (#11046) Improve pw group provider on FreeBSD
Make the pw group provider on FreeBSD support managing group members.
Also readd the allowdupe feature since in testing on FreeBSD 7, 8
and 9 the -o flag to pw works as documented.

Add tests for the provider.

Reviewed-by: Patrick Carlisle <patrick@puppetlabs.com>
9b8829d
Tim Bishop (#11046) Add support for user expiry in pw user provider
Add support for setting an expiry date for a user in the pw user
provider. FreeBSD uses the format DD-MM-YYYY rather than Puppet's
YYYY-MM-DD. Tests added to confirm the value is correctly swapped
around.

Also added custom accessor method to take the unix timestamp given
by the operating system to a Puppet-style YYYY-MM-DD. This stops
Puppet from repeatedly trying to set the expiry date if it's already
correct.
032043e
Daniel Pittman daniel-pittman merged commit 75d7cad into from January 17, 2012
Daniel Pittman daniel-pittman closed this January 17, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 4 unique commits by 2 authors.

Jan 17, 2012
Tim Bishop (#11318) Add password management on FreeBSD
This adds the manages_passwords feature to the pw user provider. It is based
on the patch by Andrew Hust that was integrated into FreeBSD puppet port. It
adds tests covering the create, delete and modify processes of the provider.

This integrates a fix for #7500 that was introduced by the original patch.
The existing code takes the first character of each property and uses it as a
flag. However, with pw, the -p flag is for setting the password expiration.
The result is that the password isn't set at create time and that the password
is set to expire. The next run of puppet correctly sets the password but the
expiry is still set. The new code avoids using -p for passwords, and also sets
the password correctly when an account is created.

Reviewed-by: Patrick Carlisle <patrick@puppetlabs.com>
884381f
Tim Bishop (#10962) Make sure managehome is respected on FreeBSD
When modifying the home directory of a user and managehome is set
the -m flag should be used with pw. This ensures that the new home
directory is created if it doesn't exist.

Also add test to verify this behaviour.
fb111ef
Tim Bishop (#11046) Improve pw group provider on FreeBSD
Make the pw group provider on FreeBSD support managing group members.
Also readd the allowdupe feature since in testing on FreeBSD 7, 8
and 9 the -o flag to pw works as documented.

Add tests for the provider.

Reviewed-by: Patrick Carlisle <patrick@puppetlabs.com>
9b8829d
Tim Bishop (#11046) Add support for user expiry in pw user provider
Add support for setting an expiry date for a user in the pw user
provider. FreeBSD uses the format DD-MM-YYYY rather than Puppet's
YYYY-MM-DD. Tests added to confirm the value is correctly swapped
around.

Also added custom accessor method to take the unix timestamp given
by the operating system to a Puppet-style YYYY-MM-DD. This stops
Puppet from repeatedly trying to set the expiry date if it's already
correct.
032043e
This page is out of date. Refresh to see the latest.
34  lib/puppet/provider/group/pw.rb
... ...
@@ -1,34 +1,48 @@
1 1
 require 'puppet/provider/nameservice/pw'
2 2
 
3 3
 Puppet::Type.type(:group).provide :pw, :parent => Puppet::Provider::NameService::PW do
4  
-  desc "Group management via `pw`.
  4
+  desc "Group management via `pw` on FreeBSD."
5 5
 
6  
-  Only works on FreeBSD.
  6
+  commands :pw => "pw"
  7
+  has_features :manages_members
7 8
 
8  
-  "
9  
-
10  
-  commands :pw => "/usr/sbin/pw"
11 9
   defaultfor :operatingsystem => :freebsd
12 10
 
  11
+  options :members, :flag => "-M", :method => :mem
  12
+
13 13
   verify :gid, "GID must be an integer" do |value|
14 14
     value.is_a? Integer
15 15
   end
16 16
 
17 17
   def addcmd
18 18
     cmd = [command(:pw), "groupadd", @resource[:name]]
  19
+
19 20
     if gid = @resource.should(:gid)
20 21
       unless gid == :absent
21 22
         cmd << flag(:gid) << gid
22 23
       end
23 24
     end
24 25
 
25  
-    # Apparently, contrary to the man page, groupadd does
26  
-    # not accept -o.
27  
-    #if @parent[:allowdupe] == :true
28  
-    #    cmd << "-o"
29  
-    #end
  26
+    if members = @resource.should(:members)
  27
+      unless members == :absent
  28
+        if members.is_a?(Array)
  29
+          members = members.join(",")
  30
+        end
  31
+        cmd << "-M" << members
  32
+      end
  33
+    end
  34
+
  35
+    cmd << "-o" if @resource.allowdupe?
30 36
 
31 37
     cmd
32 38
   end
  39
+
  40
+  def modifycmd(param, value)
  41
+    # members may be an array, need a comma separated list
  42
+    if param == :members and value.is_a?(Array)
  43
+      value = value.join(",")
  44
+    end
  45
+    super(param, value)
  46
+  end
33 47
 end
34 48
 
58  lib/puppet/provider/user/pw.rb
... ...
@@ -1,16 +1,18 @@
1 1
 require 'puppet/provider/nameservice/pw'
  2
+require 'open3'
2 3
 
3 4
 Puppet::Type.type(:user).provide :pw, :parent => Puppet::Provider::NameService::PW do
4 5
   desc "User management via `pw` on FreeBSD."
5 6
 
6 7
   commands :pw => "pw"
7  
-  has_features :manages_homedir, :allows_duplicates
  8
+  has_features :manages_homedir, :allows_duplicates, :manages_passwords, :manages_expiry
8 9
 
9 10
   defaultfor :operatingsystem => :freebsd
10 11
 
11 12
   options :home, :flag => "-d", :method => :dir
12 13
   options :comment, :method => :gecos
13 14
   options :groups, :flag => "-G"
  15
+  options :expiry, :method => :expire
14 16
 
15 17
   verify :gid, "GID must be an integer" do |value|
16 18
     value.is_a? Integer
@@ -23,10 +25,14 @@
23 25
   def addcmd
24 26
     cmd = [command(:pw), "useradd", @resource[:name]]
25 27
     @resource.class.validproperties.each do |property|
26  
-      next if property == :ensure
  28
+      next if property == :ensure or property == :password
27 29
       # the value needs to be quoted, mostly because -c might
28 30
       # have spaces in it
29 31
       if value = @resource.should(property) and value != ""
  32
+        if property == :expiry
  33
+          # FreeBSD uses DD-MM-YYYY rather than YYYY-MM-DD
  34
+          value = value.split("-").reverse.join("-")
  35
+        end
30 36
         cmd << flag(property) << value
31 37
       end
32 38
     end
@@ -37,5 +43,53 @@ def addcmd
37 43
 
38 44
     cmd
39 45
   end
  46
+
  47
+  def modifycmd(param, value)
  48
+    if param == :expiry
  49
+      # FreeBSD uses DD-MM-YYYY rather than YYYY-MM-DD
  50
+      value = value.split("-").reverse.join("-")
  51
+    end
  52
+    cmd = super(param, value)
  53
+    cmd << "-m" if @resource.managehome?
  54
+    cmd
  55
+  end
  56
+
  57
+  def create
  58
+    super
  59
+
  60
+    # Set the password after create if given
  61
+    self.password = @resource[:password] if @resource[:password]
  62
+  end
  63
+
  64
+  # use pw to update password hash
  65
+  def password=(cryptopw)
  66
+    Puppet.debug "change password for user '#{@resource[:name]}' method called with hash '#{cryptopw}'"
  67
+    stdin, stdout, stderr = Open3.popen3("pw user mod #{@resource[:name]} -H 0")
  68
+    stdin.puts(cryptopw)
  69
+    stdin.close
  70
+    Puppet.debug "finished password for user '#{@resource[:name]}' method called with hash '#{cryptopw}'"
  71
+  end
  72
+
  73
+  # get password from /etc/master.passwd
  74
+  def password
  75
+    Puppet.debug "checking password for user '#{@resource[:name]}' method called"
  76
+    current_passline = `getent passwd #{@resource[:name]}`
  77
+    current_password = current_passline.chomp.split(':')[1] if current_passline
  78
+    Puppet.debug "finished password for user '#{@resource[:name]}' method called : '#{current_password}'"
  79
+    current_password
  80
+  end
  81
+
  82
+  # Get expiry from system and convert to Puppet-style date
  83
+  def expiry
  84
+    expiry = self.get(:expiry)
  85
+    expiry = :absent if expiry == 0
  86
+
  87
+    if expiry != :absent
  88
+      t = Time.at(expiry)
  89
+      expiry = "%4d-%02d-%02d" % [t.year, t.month, t.mday]
  90
+    end
  91
+
  92
+    expiry
  93
+  end
40 94
 end
41 95
 
81  spec/unit/provider/group/pw_spec.rb
... ...
@@ -0,0 +1,81 @@
  1
+#!/usr/bin/env rspec
  2
+require 'spec_helper'
  3
+
  4
+provider_class = Puppet::Type.type(:group).provider(:pw)
  5
+
  6
+describe provider_class do
  7
+  let :resource do
  8
+    Puppet::Type.type(:group).new(:name => "testgroup", :provider => :pw)
  9
+  end
  10
+
  11
+  let :provider do
  12
+    resource.provider
  13
+  end
  14
+
  15
+  describe "when creating groups" do
  16
+    let :provider do
  17
+      prov = resource.provider
  18
+      prov.expects(:exists?).returns nil
  19
+      prov
  20
+    end
  21
+
  22
+    it "should run pw with no additional flags when no properties are given" do
  23
+      provider.addcmd.must == [provider_class.command(:pw), "groupadd", "testgroup"]
  24
+      provider.expects(:execute).with([provider_class.command(:pw), "groupadd", "testgroup"])
  25
+      provider.create
  26
+    end
  27
+
  28
+    it "should use -o when allowdupe is enabled" do
  29
+      resource[:allowdupe] = true
  30
+      provider.expects(:execute).with(includes("-o"))
  31
+      provider.create
  32
+    end
  33
+
  34
+    it "should use -g with the correct argument when the gid property is set" do
  35
+      resource[:gid] = 12345
  36
+      provider.expects(:execute).with(all_of(includes("-g"), includes(12345)))
  37
+      provider.create
  38
+    end
  39
+
  40
+    it "should use -M with the correct argument when the members property is set" do
  41
+      resource[:members] = "user1"
  42
+      provider.expects(:execute).with(all_of(includes("-M"), includes("user1")))
  43
+      provider.create
  44
+    end
  45
+
  46
+    it "should use -M with all the given users when the members property is set to an array" do
  47
+      resource[:members] = ["user1", "user2"]
  48
+      provider.expects(:execute).with(all_of(includes("-M"), includes("user1,user2")))
  49
+      provider.create
  50
+    end
  51
+  end
  52
+
  53
+  describe "when deleting groups" do
  54
+    it "should run pw with no additional flags" do
  55
+      provider.expects(:exists?).returns true
  56
+      provider.deletecmd.must == [provider_class.command(:pw), "groupdel", "testgroup"]
  57
+      provider.expects(:execute).with([provider_class.command(:pw), "groupdel", "testgroup"])
  58
+      provider.delete
  59
+    end
  60
+  end
  61
+
  62
+  describe "when modifying groups" do
  63
+    it "should run pw with the correct arguments" do
  64
+      provider.modifycmd("gid", 12345).must == [provider_class.command(:pw), "groupmod", "testgroup", "-g", 12345]
  65
+      provider.expects(:execute).with([provider_class.command(:pw), "groupmod", "testgroup", "-g", 12345])
  66
+      provider.gid = 12345
  67
+    end
  68
+
  69
+    it "should use -M with the correct argument when the members property is changed" do
  70
+      resource[:members] = "user1"
  71
+      provider.expects(:execute).with(all_of(includes("-M"), includes("user2")))
  72
+      provider.members = "user2"
  73
+    end
  74
+
  75
+    it "should use -M with all the given users when the members property is changed with an array" do
  76
+      resource[:members] = ["user1", "user2"]
  77
+      provider.expects(:execute).with(all_of(includes("-M"), includes("user3,user4")))
  78
+      provider.members = ["user3", "user4"]
  79
+    end
  80
+  end
  81
+end
183  spec/unit/provider/user/pw_spec.rb
... ...
@@ -0,0 +1,183 @@
  1
+#!/usr/bin/env rspec
  2
+require 'spec_helper'
  3
+
  4
+provider_class = Puppet::Type.type(:user).provider(:pw)
  5
+
  6
+describe provider_class do
  7
+  let :resource do
  8
+    Puppet::Type.type(:user).new(:name => "testuser", :provider => :pw)
  9
+  end
  10
+
  11
+  describe "when creating users" do
  12
+    let :provider do
  13
+      prov = resource.provider
  14
+      prov.expects(:exists?).returns nil
  15
+      prov
  16
+    end
  17
+
  18
+    it "should run pw with no additional flags when no properties are given" do
  19
+      provider.addcmd.must == [provider_class.command(:pw), "useradd", "testuser"]
  20
+      provider.expects(:execute).with([provider_class.command(:pw), "useradd", "testuser"])
  21
+      provider.create
  22
+    end
  23
+
  24
+    it "should use -o when allowdupe is enabled" do
  25
+      resource[:allowdupe] = true
  26
+      provider.expects(:execute).with(includes("-o"))
  27
+      provider.create
  28
+    end
  29
+
  30
+    it "should use -c with the correct argument when the comment property is set" do
  31
+      resource[:comment] = "Testuser Name"
  32
+      provider.expects(:execute).with(all_of(includes("-c"), includes("Testuser Name")))
  33
+      provider.create
  34
+    end
  35
+
  36
+    it "should use -e with the correct argument when the expiry property is set" do
  37
+      resource[:expiry] = "2010-02-19"
  38
+      provider.expects(:execute).with(all_of(includes("-e"), includes("19-02-2010")))
  39
+      provider.create
  40
+    end
  41
+
  42
+    it "should use -g with the correct argument when the gid property is set" do
  43
+      resource[:gid] = 12345
  44
+      provider.expects(:execute).with(all_of(includes("-g"), includes(12345)))
  45
+      provider.create
  46
+    end
  47
+
  48
+    it "should use -G with the correct argument when the groups property is set" do
  49
+      resource[:groups] = "group1"
  50
+      provider.expects(:execute).with(all_of(includes("-G"), includes("group1")))
  51
+      provider.create
  52
+    end
  53
+
  54
+    it "should use -G with all the given groups when the groups property is set to an array" do
  55
+      resource[:groups] = ["group1", "group2"]
  56
+      provider.expects(:execute).with(all_of(includes("-G"), includes("group1,group2")))
  57
+      provider.create
  58
+    end
  59
+
  60
+    it "should use -d with the correct argument when the home property is set" do
  61
+      resource[:home] = "/home/testuser"
  62
+      provider.expects(:execute).with(all_of(includes("-d"), includes("/home/testuser")))
  63
+      provider.create
  64
+    end
  65
+
  66
+    it "should use -m when the managehome property is enabled" do
  67
+      resource[:managehome] = true
  68
+      provider.expects(:execute).with(includes("-m"))
  69
+      provider.create
  70
+    end
  71
+
  72
+    it "should call the password set function with the correct argument when the password property is set" do
  73
+      resource[:password] = "*"
  74
+      provider.expects(:execute)
  75
+      provider.expects(:password=).with("*")
  76
+      provider.create
  77
+    end
  78
+
  79
+    it "should use -s with the correct argument when the shell property is set" do
  80
+      resource[:shell] = "/bin/sh"
  81
+      provider.expects(:execute).with(all_of(includes("-s"), includes("/bin/sh")))
  82
+      provider.create
  83
+    end
  84
+
  85
+    it "should use -u with the correct argument when the uid property is set" do
  86
+      resource[:uid] = 12345
  87
+      provider.expects(:execute).with(all_of(includes("-u"), includes(12345)))
  88
+      provider.create
  89
+    end
  90
+
  91
+    # (#7500) -p should not be used to set a password (it means something else)
  92
+    it "should not use -p when a password is given" do
  93
+      resource[:password] = "*"
  94
+      provider.addcmd.should_not include("-p")
  95
+      provider.expects(:password=)
  96
+      provider.expects(:execute).with(Not(includes("-p")))
  97
+      provider.create
  98
+    end
  99
+  end
  100
+
  101
+  describe "when deleting users" do
  102
+    it "should run pw with no additional flags" do
  103
+      provider = resource.provider
  104
+      provider.expects(:exists?).returns true
  105
+      provider.deletecmd.must == [provider_class.command(:pw), "userdel", "testuser"]
  106
+      provider.expects(:execute).with([provider_class.command(:pw), "userdel", "testuser"])
  107
+      provider.delete
  108
+    end
  109
+  end
  110
+
  111
+  describe "when modifying users" do
  112
+    let :provider do
  113
+      resource.provider
  114
+    end
  115
+
  116
+    it "should run pw with the correct arguments" do
  117
+      provider.modifycmd("uid", 12345).must == [provider_class.command(:pw), "usermod", "testuser", "-u", 12345]
  118
+      provider.expects(:execute).with([provider_class.command(:pw), "usermod", "testuser", "-u", 12345])
  119
+      provider.uid = 12345
  120
+    end
  121
+
  122
+    it "should use -c with the correct argument when the comment property is changed" do
  123
+      resource[:comment] = "Testuser Name"
  124
+      provider.expects(:execute).with(all_of(includes("-c"), includes("Testuser New Name")))
  125
+      provider.comment = "Testuser New Name"
  126
+    end
  127
+
  128
+    it "should use -e with the correct argument when the expiry property is changed" do
  129
+      resource[:expiry] = "2010-02-19"
  130
+      provider.expects(:execute).with(all_of(includes("-e"), includes("19-02-2011")))
  131
+      provider.expiry = "2011-02-19"
  132
+    end
  133
+
  134
+    it "should use -g with the correct argument when the gid property is changed" do
  135
+      resource[:gid] = 12345
  136
+      provider.expects(:execute).with(all_of(includes("-g"), includes(54321)))
  137
+      provider.gid = 54321
  138
+    end
  139
+
  140
+    it "should use -G with the correct argument when the groups property is changed" do
  141
+      resource[:groups] = "group1"
  142
+      provider.expects(:execute).with(all_of(includes("-G"), includes("group2")))
  143
+      provider.groups = "group2"
  144
+    end
  145
+
  146
+    it "should use -G with all the given groups when the groups property is changed with an array" do
  147
+      resource[:groups] = ["group1", "group2"]
  148
+      provider.expects(:execute).with(all_of(includes("-G"), includes("group3,group4")))
  149
+      provider.groups = "group3,group4"
  150
+    end
  151
+
  152
+    it "should use -d with the correct argument when the home property is changed" do
  153
+      resource[:home] = "/home/testuser"
  154
+      provider.expects(:execute).with(all_of(includes("-d"), includes("/newhome/testuser")))
  155
+      provider.home = "/newhome/testuser"
  156
+    end
  157
+
  158
+    it "should use -m and -d with the correct argument when the home property is changed and managehome is enabled" do
  159
+      resource[:home] = "/home/testuser"
  160
+      resource[:managehome] = true
  161
+      provider.expects(:execute).with(all_of(includes("-d"), includes("/newhome/testuser"), includes("-m")))
  162
+      provider.home = "/newhome/testuser"
  163
+    end
  164
+
  165
+    it "should call the password set function with the correct argument when the password property is changed" do
  166
+      resource[:password] = "*"
  167
+      provider.expects(:password=).with("!")
  168
+      provider.password = "!"
  169
+    end
  170
+
  171
+    it "should use -s with the correct argument when the shell property is changed" do
  172
+      resource[:shell] = "/bin/sh"
  173
+      provider.expects(:execute).with(all_of(includes("-s"), includes("/bin/tcsh")))
  174
+      provider.shell = "/bin/tcsh"
  175
+    end
  176
+
  177
+    it "should use -u with the correct argument when the uid property is changed" do
  178
+      resource[:uid] = 12345
  179
+      provider.expects(:execute).with(all_of(includes("-u"), includes(54321)))
  180
+      provider.uid = 54321
  181
+    end
  182
+  end
  183
+end
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.