Merge pull request #290 from yast/y2users_modify_users

Modify user attributes (home, gecos, shell and groups)

src/lib/y2users/linux/writer.rb

@@ -99,6 +99,7 @@ def initialize(config, initial_config)
       def write
         issues = Y2Issues::List.new
 
+        add_groups(issues)
         add_users(issues)
         edit_users(issues)
 
@@ -152,6 +153,10 @@ def write
       USERADD = "/usr/sbin/useradd".freeze
       private_constant :USERADD
 
+      # Command for modifying users
+      USERMOD = "/usr/sbin/usermod".freeze
+      private_constant :USERMOD
+
       # Command for setting a user password
       #
       # This command is "preferred" over
@@ -169,6 +174,20 @@ def write
       CHAGE = "/usr/bin/chage".freeze
       private_constant :CHAGE
 
+      # Command for creating new users
+      GROUPADD = "/usr/sbin/groupadd".freeze
+      private_constant :GROUPADD
+
+      # Creates the new groups
+      #
+      # @param issues [Y2Issues::List]
+      def add_groups(issues)
+        # handle groups first as it does not depend on users uid, but it is vice versa
+        new_groups = (config.groups.map(&:id) - initial_config.groups.map(&:id))
+        new_groups.map! { |id| config.groups.find { |g| g.id == id } }
+        new_groups.each { |g| add_group(g, issues) }
+      end
+
       # Creates the new users
       #
       # @param issues [Y2Issues::List]
@@ -186,12 +205,26 @@ def edit_users(issues)
 
         edited_users.each do |user|
           initial_user = initial_config.users.by_id(user.id)
-
-          change_password(user, issues) if initial_user.password != user.password
-          write_auth_keys(user, issues) if initial_user.authorized_keys != user.authorized_keys
+          edit_user(user, initial_user, issues)
         end
       end
 
+      # Executes the command for creating the group
+      #
+      # @param group [Y2User::Group] the group to be created on the system
+      # @param issues [Y2Issues::List] a collection for adding an issue if something goes wrong
+      def add_group(group, issues)
+        args = []
+        args << "--gid" << group.gid if group.gid
+        # TODO: system groups?
+        Yast::Execute.on_target!(GROUPADD, *args)
+      rescue Cheetah::ExecutionFailed => e
+        issues << Y2Issues::Issue.new(
+          format(_("The group '%{groupname}' could not be created"), groupname: group.name)
+        )
+        log.error("Error creating group '#{group.name}' - #{e.message}")
+      end
+
       # Executes the command for creating the user
       #
       # @param user [Y2User::User] the user to be created on the system
@@ -271,6 +304,31 @@ def set_password_attributes(user, issues)
         log.error("Error setting password attributes for '#{user.name}' - #{e.message}")
       end
 
+      # Attributes to modify using `usermod`
+      USERMOD_ATTRS = [:gid, :home, :shell, :gecos].freeze
+
+      # Edits the user
+      #
+      # @param new_user [Y2Users::User] User containing the updated information
+      # @param old_user [Y2Users::User] Original user
+      # @param issues [Y2Issues::List] a collection for adding an issue if something goes wrong
+      def edit_user(new_user, old_user, issues)
+        usermod_changes = USERMOD_ATTRS.any? do |attr|
+          !new_user.public_send(attr).nil? &&
+            (new_user.public_send(attr) != old_user.public_send(attr))
+        end
+        usermod_changes ||= different_groups?(new_user, old_user)
+
+        Yast::Execute.on_target!(USERMOD, *usermod_options(new_user, old_user)) if usermod_changes
+        change_password(new_user, issues) if old_user.password != new_user.password
+        write_auth_keys(new_user, issues) if old_user.authorized_keys != new_user.authorized_keys
+      rescue Cheetah::ExecutionFailed => e
+        issues << Y2Issues::Issue.new(
+          format(_("The user '%{username}' could not be modified"), username: new_user.name)
+        )
+        log.error("Error modifying user '#{new_user.name}' - #{e.message}")
+      end
+
       # Generates and returns the options expected by `useradd` for given user
       #
       # @param user [Y2Users::User]
@@ -281,7 +339,8 @@ def useradd_options(user)
           "--gid"      => user.gid,
           "--shell"    => user.shell,
           "--home-dir" => user.home,
-          "--comment"  => user.gecos.join(",")
+          "--comment"  => user.gecos.join(","),
+          "--groups"   => user.secondary_groups_name.join(",")
         }
         opts = opts.reject { |_, v| v.to_s.empty? }.flatten
 
@@ -295,6 +354,36 @@ def useradd_options(user)
         opts
       end
 
+      # Command to modity the user
+      #
+      # @param new_user [Y2Users::User] User containing the updated information
+      # @param old_user [Y2Users::User] Original user
+      # @return [Array<String>] usermod options
+      # rubocop:disable Metrics/CyclomaticComplexity
+      # rubocop:disable Metrics/AbcSize
+      # rubocop:disable Metrics/PerceivedComplexity
+      def usermod_options(new_user, old_user)
+        args = []
+        args << "--gid" << new_user.gid if new_user.gid != old_user.gid && new_user.gid
+        args << "--comment" << new_user.gecos.join(",") if new_user.gecos != old_user.gecos
+        if new_user.home != old_user.home && new_user.home
+          args << "--home" << new_user.home << "--move-home"
+        end
+        args << "--shell" << new_user.shell if new_user.shell != old_user.shell && new_user.shell
+        if different_groups?(new_user, old_user)
+          args << "--groups" << new_user.secondary_groups_name.join(",")
+        end
+        args << new_user.name
+        args
+      end
+      # rubocop:enable Metrics/CyclomaticComplexity
+      # rubocop:enable Metrics/AbcSize
+      # rubocop:enable Metrics/PerceivedComplexity
+
+      def different_groups?(user1, user2)
+        user1.groups(with_primary: false).sort != user2.groups(with_primary: false).sort
+      end
+
       # Options for `useradd` to create the home directory
       #
       # @param _user [Y2Users::User]

src/lib/y2users/user.rb

@@ -96,7 +96,8 @@ class User < ConfigElement
 
     # Only relevant attributes are compared. For example, the config in which the user is attached
     # and the internal user id are not considered.
-    eql_attr :name, :uid, :gid, :shell, :home, :gecos, :source, :password, :authorized_keys
+    eql_attr :name, :uid, :gid, :shell, :home, :gecos, :source, :password, :authorized_keys,
+      :secondary_groups_name
 
     # Constructor
     #
@@ -135,12 +136,19 @@ def primary_group
     def groups(with_primary: true)
       return [] unless attached?
 
-      groups = config.groups.select { |g| g.users.include?(self) }
+      groups = config.groups.select { |g| g.users.map(&:name).include?(name) }
       groups.reject! { |g| g.gid == gid } if gid && !with_primary
 
       groups
     end
 
+    # Secondary group names where the user is included
+    #
+    # @return [Array<String>]
+    def secondary_groups_name
+      groups(with_primary: false).map(&:name).sort
+    end
+
     # @return [Date, nil] date when the account expires or nil if never
     def expire_date
       password&.account_expiration&.date

test/lib/y2users/config_merger_test.rb

@@ -198,6 +198,7 @@ def lhs_group(name)
           subject.merge
 
           expect(lhs_group("test1")).to eq(group1)
+          expect(lhs_group("test1").users_name).to eq(["test1", "test2"])
         end
       end
     end

test/lib/y2users/linux/writer_test.rb

@@ -24,6 +24,7 @@
 require "date"
 require "y2users/config"
 require "y2users/user"
+require "y2users/group"
 require "y2users/password"
 require "y2users/linux/writer"
 
@@ -34,6 +35,7 @@
     let(:initial_config) do
       config = Y2Users::Config.new
       config.attach(users)
+      config.attach(groups)
       config
     end
 
@@ -47,6 +49,15 @@
       user
     end
 
+    let(:groups) { [] }
+
+    let(:group) do
+      Y2Users::Group.new("users").tap do |group|
+        group.gid = "100"
+        group.users_name = [user.name]
+      end
+    end
+
     let(:password) { Y2Users::Password.new(pwd_value) }
 
     let(:username) { "testuser" }
@@ -238,6 +249,119 @@
         writer.write
       end
 
+      context "whose gid was changed" do
+        before do
+          current_user = config.users.by_id(user.id)
+          current_user.gid = "1000"
+        end
+
+        it "executes usermod with the new values" do
+          expect(Yast::Execute).to receive(:on_target!).with(
+            /usermod/, "--gid", "1000", user.name
+          )
+
+          writer.write
+        end
+      end
+
+      context "whose home was changed" do
+        before do
+          current_user = config.users.by_id(user.id)
+          current_user.home = "/home/new"
+        end
+
+        it "executes usermod with the new values" do
+          expect(Yast::Execute).to receive(:on_target!).with(
+            /usermod/, "--home", "/home/new", "--move-home", user.name
+          )
+
+          writer.write
+        end
+      end
+
+      context "whose shell was changed" do
+        before do
+          current_user = config.users.by_id(user.id)
+          current_user.shell = "/usr/bin/fish"
+        end
+
+        it "executes usermod with the new values" do
+          expect(Yast::Execute).to receive(:on_target!).with(
+            /usermod/, "--shell", "/usr/bin/fish", user.name
+          )
+
+          writer.write
+        end
+      end
+
+      context "whose gecos was changed" do
+        let(:gecos) { ["Jane", "Doe"] }
+
+        before do
+          user.gecos = ["Admin"]
+          current_user = config.users.by_id(user.id)
+          current_user.gecos = gecos
+        end
+
+        it "executes usermod with the new values" do
+          expect(Yast::Execute).to receive(:on_target!).with(
+            /usermod/, "--comment", "Jane,Doe", user.name
+          )
+
+          writer.write
+        end
+
+        context "and the new value is empty" do
+          let(:gecos) { [] }
+
+          it "executes usermod with the new values" do
+            expect(Yast::Execute).to receive(:on_target!).with(
+              /usermod/, "--comment", "", user.name
+            )
+
+            writer.write
+          end
+        end
+      end
+
+      context "whose groups were changed" do
+        let(:wheel_group) do
+          Y2Users::Group.new("wheel").tap do |group|
+            group.users_name = user.name
+          end
+        end
+        let(:users) { [user] }
+
+        before do
+          config.attach(wheel_group)
+          allow(Yast::Execute).to receive(:on_target!).with(/groupadd/, any_args)
+        end
+
+        it "executes usermod with the new values" do
+          expect(Yast::Execute).to receive(:on_target!).with(
+            /usermod/, "--groups", "wheel", user.name
+          )
+
+          writer.write
+        end
+      end
+
+      context "when modifying a user attribute fails" do
+        before do
+          current_user = config.users.by_id(user.id)
+          current_user.home = "/home/new"
+          allow(Yast::Execute).to receive(:on_target!)
+            .with(/usermod/, any_args)
+            .and_raise(Cheetah::ExecutionFailed.new("", "", "", ""))
+        end
+
+        it "returns an issue" do
+          expect(writer.log).to receive(:error).with(/Error modifying user '#{user.name}'/)
+          issues = writer.write
+          expect(issues.first.message).to match("The user '#{user.name}' could not be modified")
+        end
+      end
+
       context "whose authorized keys were edited" do
         before do
           current_user = config.users.by_id(user.id)
@@ -302,6 +426,7 @@
         user.gecos = ["First line of", "GECOS"]
 
         config.attach(user)
+        config.attach(group)
       end
 
       include_examples "setting password"
@@ -311,7 +436,9 @@
       it "executes useradd with all the parameters, including creation of home directory" do
         expect(Yast::Execute).to receive(:on_target!).with(/useradd/, any_args) do |*args|
           expect(args.last).to eq username
-          expect(args).to include("--uid", "--gid", "--shell", "--home-dir", "--create-home")
+          expect(args).to include(
+            "--uid", "--gid", "--shell", "--home-dir", "--create-home", "--groups"
+          )
         end
 
         writer.write
@@ -453,5 +580,30 @@
         expect(result.map(&:message)).to include(/Error writing authorized keys/)
       end
     end
+
+    context "for a new group" do
+      before do
+        config.attach(group)
+      end
+
+      it "executes groupadd" do
+        expect(Yast::Execute).to receive(:on_target!).with(/groupadd/, "--gid", "100")
+        writer.write
+      end
+
+      context "when creating the groupadd fails" do
+        before do
+          allow(Yast::Execute).to receive(:on_target!)
+            .with(/groupadd/, any_args)
+            .and_raise(Cheetah::ExecutionFailed.new("", "", "", ""))
+        end
+
+        it "returns an issue" do
+          expect(writer.log).to receive(:error).with(/Error creating group '#{group.name}'/)
+          issues = writer.write
+          expect(issues.first.message).to match("The group '#{group.name}' could not be created")
+        end
+      end
+    end
   end
 end