Merge pull request #355 from joseivanlopez/default-home-dir

Always create home

src/include/users/dialogs.rb

@@ -1418,6 +1418,12 @@ def EditUserDialog(what)
               focus_tab.call(current, :home)
               next
             end
+            if new_home.empty?
+              Report.Error(_("Home cannot be empty."))
+              UI.ChangeWidget(Id(:home), :Value, home)
+              focus_tab.call(current, :home)
+              next
+            end
             failed = false
             begin
               error_map = Users.CheckHomeUI(new_i_uid, new_home, ui_map)

src/lib/users/dialogs/inst_user_first.rb

@@ -27,6 +27,7 @@
 require "y2users/username"
 require "users/users_database"
 require "tmpdir"
+require "pathname"
 
 module Yast
   # Dialog for the creation of local users during the installation
@@ -425,10 +426,10 @@ def process_new_user_form
       @full_name = UI.QueryWidget(Id(:full_name), :Value)
 
       user.name = @username
-      # In case of editing the user, the home should be updated to the default value (just in case
-      # the user name has changed) in order to keep the home path as /home/new_username.
-      user.home = user.default_home
       user.gecos = [@full_name].compact
+      # In case of editing an existing user, the home should be updated in order to keep the home
+      # path as /home/new_username.
+      update_home
 
       return false unless valid_user?
 
@@ -439,6 +440,17 @@ def process_new_user_form
       true
     end
 
+    # Updates the user home path if needed
+    def update_home
+      return if !user.home&.path || user.home.path.empty?
+
+      home_path = Pathname.new(user.home.path)
+
+      return if user.name == home_path.basename.to_s
+
+      user.home.path = home_path.dirname.join(user.name).to_s
+    end
+
     # Processes the password for the user
     #
     # Errors could be reported.

src/lib/y2users/config.rb

@@ -48,6 +48,11 @@ class Config
     # @return [LoginConfig, nil] nil if not defined (unknown)
     attr_accessor :login
 
+    # Useradd configuration to be applied to the system before creating the users
+    #
+    # @return [UseraddConfig, nil] nil if the configuration is unknown
+    attr_accessor :useradd
+
     # Constructor
     #
     # @see UsersCollection
@@ -144,11 +149,6 @@ def merge!(other)
       self
     end
 
-    # Useradd configuration to be applied to the system before creating the users
-    #
-    # @return [UseraddConfig, nil] nil if the configuration is unknown
-    attr_accessor :useradd
-
   private
 
     # Collection of users

src/lib/y2users/linux/create_user_action.rb

@@ -111,22 +111,32 @@ def user_options
 
       # Generates options for `useradd` about how to deal with home
       #
-      # The home is not created if explicitly requested with `:skip_hope` param or the user has no
-      # home path.
+      # The home is created in the given home path. If the home path is unknown, then the home is
+      # created in the default path indicated by the useradd defaults configuration.
       #
-      # Note that `useradd` will not try to create the home if the path already exists, it does not
-      # matter whether --create-home is passed.
+      # The home is not created if explicitly requested with `:skip_hope` param.
+      #
+      # Note that `useradd` will not try to create the home if the given path already exists, it
+      # does not matter whether --create-home is passed.
       #
       # @param skip_home [Boolean]
       # @return [Array<String>]
       def home_options(skip_home: false)
         return [] if user.system?
 
-        return ["--no-create-home"] if skip_home || !user.home&.path
+        return ["--no-create-home"] if skip_home
+
+        create_home_options
+      end
 
-        opts = ["--create-home", "--home-dir", user.home.path]
-        opts << "--btrfs-subvolume-home" if user.home.btrfs_subvol?
-        opts << "--key" << "HOME_MODE=#{user.home.permissions}" if user.home.permissions
+      # Options when the home has to be created
+      #
+      # @return [Array<String>]
+      def create_home_options
+        opts = ["--create-home"]
+        opts += ["--home-dir", user.home.path] if user.home&.path && !user.home.path.empty?
+        opts += ["--btrfs-subvolume-home"] if user.home&.btrfs_subvol?
+        opts += ["--key", "HOME_MODE=#{user.home.permissions}"] if user.home&.permissions
         opts
       end
     end

src/lib/y2users/user.rb

@@ -73,6 +73,9 @@ class User < ConfigElement
 
     # User home
     #
+    # @note If home is unknown, then the {Linux::Writer} will create the default home according to
+    #   the useradd defaults configuration.
+    #
     # @return [Home, nil] nil if unknown
     attr_accessor :home
 
@@ -131,16 +134,14 @@ def self.create_system(name)
 
     # Constructor
     #
-    # This creates a local user with its default home. See also {User.create_system} for
-    # specifically creating a system user.
+    # This creates a local user. See also {User.create_system} for creating a system user.
     #
     # @param name [String]
     def initialize(name)
       super()
 
       @name = name
-      @home = default_home
-      # TODO: GECOS
+      @home = Home.new
       @gecos = []
       @authorized_keys = []
       @source = :unknown
@@ -149,16 +150,6 @@ def initialize(name)
       @system = false
     end
 
-    # Default home for the user
-    #
-    # Generates a new {Home} object with the default user home path. The attributes of the current
-    # home object associated to the user are not copied.
-    #
-    # @return [Home]
-    def default_home
-      Home.new("/home/#{name}")
-    end
-
     # Primary group for the user
     #
     # The user must be attached to a config in order to find its primary group.

test/lib/users/dialogs/inst_user_first_test.rb

@@ -184,7 +184,6 @@
           subject.next_handler
 
           expect(config.users.by_name("test")).to_not be_nil
-          expect(config.users.by_name("test").home.path).to eq("/home/test")
         end
 
         it "configures the autologin" do
@@ -194,6 +193,34 @@
           expect(config.login.autologin?).to eq(true)
         end
 
+        context "when the user name has changed" do
+          let(:user) { Y2Users::User.new("oldname") }
+
+          context "and the user has a home path" do
+            before do
+              user.home.path = "/home/oldname"
+            end
+
+            it "updates the home path according to the new user name" do
+              subject.next_handler
+
+              expect(user.home.path).to eq("/home/test")
+            end
+          end
+
+          context "and the user has no home path" do
+            before do
+              user.home.path = nil
+            end
+
+            it "does not set a home path" do
+              subject.next_handler
+
+              expect(user.home.path).to be_nil
+            end
+          end
+        end
+
         context "when the option \"use passwor for root\" is selected" do
           let(:root_pw) { true }
 

test/lib/y2users/linux/create_user_action_test.rb

@@ -36,7 +36,6 @@
   end
 
   describe "#perform" do
-
     it "creates user with useradd" do
       expect(Yast::Execute).to receive(:on_target!) do |cmd, *_args|
         expect(cmd).to eq "/usr/sbin/useradd"
@@ -78,17 +77,6 @@
       action.perform
     end
 
-    it "passes --home-dir parameter" do
-      user.home.path = "/home/test5"
-
-      expect(Yast::Execute).to receive(:on_target!) do |_cmd, *args|
-        expect(args).to include "--home-dir"
-        expect(args).to include "/home/test5"
-      end
-
-      action.perform
-    end
-
     it "passes --comment parameter" do
       user.gecos = ["full name"]
 
@@ -139,10 +127,24 @@
       action.perform
     end
 
-    context "non-system users" do
+    context "for a local user" do
       it "passes --create-home parameter" do
         expect(Yast::Execute).to receive(:on_target!) do |_cmd, *args|
-          expect(args).to include "--create-home"
+          expect(args).to include("--create-home")
+          expect(args).to_not include("--home-dir")
+          expect(args).to_not include("--btrfs-subvolume-home")
+          expect(args).to_not include("--key")
+        end
+
+        action.perform
+      end
+
+      it "passes --home-dir parameter if the home path is known" do
+        user.home.path = "/home/test"
+
+        expect(Yast::Execute).to receive(:on_target!) do |_cmd, *args|
+          expect(args).to include("--home-dir")
+          expect(args).to include("/home/test")
         end
 
         action.perform

test/lib/y2users/linux/set_auth_keys_action_test.rb

@@ -27,7 +27,12 @@
 
 describe Y2Users::Linux::SetAuthKeysAction do
   subject(:action) { described_class.new(user, commit_config) }
-  let(:user) { Y2Users::User.new("test").tap { |u| u.authorized_keys = ["test"] } }
+  let(:user) do
+    Y2Users::User.new("test").tap do |user|
+      user.home.path = "/home/test"
+      user.authorized_keys = ["test"]
+    end
+  end
   let(:commit_config) { nil }
 
   describe "#perform" do

test/lib/y2users/linux/users_writer_test.rb

@@ -47,9 +47,9 @@
 
   let(:users) { [test1, test2] }
 
-  let(:test1) { Y2Users::User.new("test1") }
+  let(:test1) { Y2Users::User.new("test1").tap { |u| u.home.path = "/home/test1" } }
 
-  let(:test2) { Y2Users::User.new("test2") }
+  let(:test2) { Y2Users::User.new("test2").tap { |u| u.home.path = "/home/test2" } }
 
   let(:commit_config) { Y2Users::CommitConfig.new }
 
@@ -507,7 +507,7 @@ def issues(messages)
     end
 
     context "when a user is added to the target config" do
-      let(:test3) { Y2Users::User.new("test3") }
+      let(:test3) { Y2Users::User.new("test3").tap { |u| u.home.path = "/home/test3" } }
 
       before do
         target_config.attach(test3)

test/lib/y2users/user_test.rb

@@ -61,10 +61,10 @@
       expect(user.system?).to eq(false)
     end
 
-    it "creates the user with the default home" do
+    it "creates the user with unknown home path" do
       user = described_class.new("test")
 
-      expect(user.home.path).to eq("/home/test")
+      expect(user.home.path).to be_nil
     end
   end
 
@@ -90,6 +90,8 @@
     end
 
     it "generates a copy with a duplicated home" do
+      subject.home.path = "/home/test"
+
       other = subject.copy
       other.home.path = "/home/other"
 
@@ -122,15 +124,6 @@
     end
   end
 
-  describe "#default_home" do
-    it "returns a home with the default path for the home user" do
-      home = subject.default_home
-
-      expect(home).to be_a(Y2Users::Home)
-      expect(home.path).to eq("/home/test")
-    end
-  end
-
   describe "#primary_group" do
     before do
       group = Y2Users::Group.new("test")