Merge pull request #369 from yast/user_nohome_master

User nohome master

package/yast2-users.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Fri Aug 19 08:37:53 UTC 2022 - Josef Reidinger <jreidinger@suse.com>
+
+- AY: Fix writing ssh keys for user without specified home
+  (bsc#1201185)
+- 4.5.2
+
 -------------------------------------------------------------------
 Fri Apr  8 11:27:57 UTC 2022 - José Iván López González <jlopez@suse.com>
 

package/yast2-users.spec

@@ -17,7 +17,7 @@
 
 
 Name:           yast2-users
-Version:        4.5.1
+Version:        4.5.2
 Release:        0
 Summary:        YaST2 - User and Group Configuration
 License:        GPL-2.0-only

src/lib/y2users/linux/users_writer.rb

@@ -30,6 +30,7 @@
 require "y2users/linux/set_home_ownership_action"
 require "y2users/linux/set_auth_keys_action"
 require "y2users/linux/delete_user_action"
+require "y2users/linux/reader"
 
 Yast.import "MailAliases"
 
@@ -54,6 +55,7 @@ def initialize(target_config, initial_config, commit_configs)
         @initial_config = initial_config
         @target_config = target_config
         @commit_configs = commit_configs
+        @users_to_write_ssh_keys = {}
       end
 
     private
@@ -89,6 +91,7 @@ def actions
         edit_users
         add_users
         write_root_aliases
+        write_ssh_auth_keys
       end
 
       # Deletes users
@@ -104,6 +107,27 @@ def add_users
         new_users.each { |u| add_user(u) }
       end
 
+      def write_ssh_auth_keys
+        # we need to re-read system users as for some newly created users
+        # the default home can be used and it depends on useradd and login
+        # defaults. So instead of mimic useradd behavior just read what
+        # useradd creates. (bsc#1201185)
+        system_users = Reader.new.read.users
+        @users_to_write_ssh_keys.each_pair do |user, old_keys|
+          system_user = system_users.by_name(user.name)
+          if !system_user
+            issues << Y2Issues::Issue.new(
+              format(_("Failed to find user with name '%s'"), user.name)
+            )
+            log.error("Failed to find user with name #{user.name}")
+            next
+          end
+
+          system_user.authorized_keys = user.authorized_keys
+          write_user_auth_keys(system_user, old_keys)
+        end
+      end
+
       # Performs all needed actions in order to create and configure a new user (create user, set
       # password, etc).
       #
@@ -119,7 +143,7 @@ def add_user(user)
         remove_home_content(user) if !reusing_home && commit_config.home_without_skel?
         adapt_home_ownership(user) if commit_config.adapt_home_ownership?
         write_password(user) if user.password
-        write_auth_keys(user)
+        @users_to_write_ssh_keys[user] = []
       end
 
       # Edits users
@@ -147,7 +171,9 @@ def edit_user(initial_user, target_user)
         edit_password(target_user) if initial_user.password != target_user.password
 
         previous_keys = initial_user.authorized_keys || []
-        write_auth_keys(target_user, previous_keys) if previous_keys != target_user.authorized_keys
+        return if previous_keys == target_user.authorized_keys
+
+        @users_to_write_ssh_keys[target_user] = previous_keys
       end
 
       # Updates root aliases
@@ -293,9 +319,7 @@ def adapt_home_ownership(user)
       # @param user [User]
       # @param previous_keys [Array<String>] previous auth keys for given user, if any
       # @return [Boolean] true on success
-      def write_auth_keys(user, previous_keys = [])
-        return true unless exist_user_home?(user)
-
+      def write_user_auth_keys(user, previous_keys = [])
         action = SetAuthKeysAction.new(user, commit_config(user), previous_keys)
 
         perform_action(action)

test/lib/y2users/linux/users_writer_test.rb

@@ -53,6 +53,13 @@
 
   let(:commit_config) { Y2Users::CommitConfig.new }
 
+  let(:system_config) { initial_config }
+
+  before do
+    allow(Y2Users::Linux::Reader).to receive(:new)
+      .and_return(instance_double(Y2Users::Linux::Reader, read: system_config))
+  end
+
   describe "#write" do
     let(:create_user_action) { Y2Users::Linux::CreateUserAction }
 
@@ -331,56 +338,45 @@ def issues(messages)
         end
 
         context "and the authorized keys has changed" do
+          let(:system_user) { target_user.copy }
+
           before do
             target_user.authorized_keys = ["new-key"]
+            # cannot overwrite here system_config as we need to have there recent
+            # target user which is modified in `before` code
+            system_config = Y2Users::Config.new.tap { |c| c.attach(system_user) }
+            allow(Y2Users::Linux::Reader).to receive(:new)
+              .and_return(instance_double(Y2Users::Linux::Reader, read: system_config))
+            allow(Yast::FileUtils).to receive(:IsDirectory).with(target_user.home.path)
+              .and_return(true)
           end
 
-          context "and the user home exists" do
-            before do
-              allow(Yast::FileUtils).to receive(:IsDirectory).with(target_user.home.path)
-                .and_return(true)
-            end
-
-            it "performs the action for setting the authorized keys" do
-              action = mock_action(set_auth_keys_action, success, target_user)
-
-              expect(action).to receive(:perform)
-
-              subject.write
-            end
-
-            it "provides previous keys to the action for setting authorized keys" do
-              action = instance_double(set_auth_keys_action, perform: success)
+          it "performs the action for setting the authorized keys" do
+            action = mock_action(set_auth_keys_action, success, system_user)
 
-              expect(set_auth_keys_action)
-                .to receive(:new).with(target_user, any_args) do |*args|
-                  previous_keys = args.last
-                  expect(previous_keys).to eq(initial_user.authorized_keys)
-                end.and_return(action)
+            expect(action).to receive(:perform)
 
-              subject.write
-            end
+            subject.write
+          end
 
-            it "returns the generated issues" do
-              mock_action(set_auth_keys_action, success("issue auth keys"), target_user)
+          it "provides previous keys to the action for setting authorized keys" do
+            action = instance_double(set_auth_keys_action, perform: success)
 
-              issues = subject.write
+            expect(set_auth_keys_action)
+              .to receive(:new).with(target_user, any_args) do |*args|
+                previous_keys = args.last
+                expect(previous_keys).to eq(initial_user.authorized_keys)
+              end.and_return(action)
 
-              expect(issues.map(&:message)).to include(/issue auth keys/)
-            end
+            subject.write
           end
 
-          context "and the user home does not exist" do
-            before do
-              allow(Yast::FileUtils).to receive(:IsDirectory).with(target_user.home.path)
-                .and_return(false)
-            end
+          it "returns the generated issues" do
+            mock_action(set_auth_keys_action, success("issue auth keys"), target_user)
 
-            it "does not perform the action for setting the authorized keys" do
-              expect_any_instance_of(set_auth_keys_action).to_not receive(:perform)
+            issues = subject.write
 
-              subject.write
-            end
+            expect(issues.map(&:message)).to include(/issue auth keys/)
           end
         end
 
@@ -690,6 +686,9 @@ def issues(messages)
         end
 
         context "and the user home exists" do
+          let(:system_user) { test3.copy }
+          let(:system_config) { Y2Users::Config.new.tap { |c| c.attach([system_user]) } }
+
           before do
             allow(Yast::FileUtils).to receive(:IsDirectory).with(test3.home.path).and_return(true)
           end