Skip to content

Commit

Permalink
Fix bugs
Browse files Browse the repository at this point in the history
  • Loading branch information
joseivanlopez committed Oct 20, 2021
1 parent 2737129 commit f765f62
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 10 deletions.
9 changes: 8 additions & 1 deletion src/lib/y2users/linux/users_writer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,14 @@ def create_user(user)
#
# @return [Boolean] true on success
def modify_user(initial_user, target_user)
action = EditUserAction.new(initial_user, target_user, commit_config(target_user))
# If a new home was assigned to the user and that home already exists, then the content of
# previous home cannot be moved to the new home. Note that "usermod --move-home" fails in
# that scenario (exit status different to 0). To prevent such errors, the commit config is
# forced to not move the home content in that case.
commit_config = commit_config(target_user).dup
commit_config.move_home = false if exist_user_home?(target_user)

action = EditUserAction.new(initial_user, target_user, commit_config)

perform_action(action)
end
Expand Down
5 changes: 4 additions & 1 deletion src/lib/y2users/users_module/commit_config_reader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,10 @@ def commit_config(user)
# @param user [Hash] a user representation in the format used by Yast::Users
# @return [Boolean]
def move_home?(user)
previous_home = user_attr(user, "org_homeDirectory")
# Surprisingly, when a user is edited, the "org_homeDirectory" attribute does not contain
# the original home value but the current one (i.e., the same value as "homeDirectory"). It
# is more reliable to check the old value from the "org_user" hash.
previous_home = user_attr(user.fetch("org_user", {}), "homeDirectory")
home = user_attr(user, "homeDirectory")

return false if previous_home.nil? || home.nil?
Expand Down
30 changes: 30 additions & 0 deletions test/lib/y2users/linux/users_writer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,36 @@ def issues(messages)
expect(issues.map(&:message)).to include(/issue editing user/)
end

context "and the new home path already exists" do
before do
target_user.home.path = "/home/new_home"

allow(File).to receive(:exist?)
allow(File).to receive(:exist?).with("/home/new_home").and_return(true)
end

let(:commit_config) do
Y2Users::CommitConfig.new.tap do |config|
config.username = target_user.name
config.move_home = true
end
end

it "forces to not move the content of the current home" do
action = instance_double(edit_user_action, perform: success)

expect(edit_user_action)
.to receive(:new).with(initial_user, target_user, anything) do |*args|
commit_config = args.last
expect(commit_config.move_home?).to eq(false)
end.and_return(action)

expect(action).to receive(:perform)

subject.write
end
end

context "and the action for editing the user successes" do
before do
mock_action(edit_user_action, success, initial_user, target_user)
Expand Down
18 changes: 10 additions & 8 deletions test/lib/y2users/users_module/commit_config_reader_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,16 @@
let(:users) do
[
{
"uid" => "test1",
"chown_home" => true,
"create_home" => true,
"homeDirectory" => "/home/test",
"org_homeDirectory" => "/home/test1",
"home_mode" => "755",
"modified" => "edited",
"no_skeleton" => true
"uid" => "test1",
"chown_home" => true,
"create_home" => true,
"homeDirectory" => "/home/test",
"org_user" => {
"homeDirectory" => "/home/test1"
},
"home_mode" => "755",
"modified" => "edited",
"no_skeleton" => true
},
{
"uid" => "test2",
Expand Down

0 comments on commit f765f62

Please sign in to comment.