Fix users import during installation

* Fix users import
* Refactor UsersFinishClient as a FinishClient
* Add unit tests for UsersFinishClient

.gitignore

@@ -1,5 +1,4 @@
 Makefile
-Makefile.am
 Makefile.in
 aclocal.m4
 autom4te.cache

.travis.yml

@@ -5,7 +5,7 @@ before_install:
     # disable rvm, use system Ruby
     - rvm reset
     - wget https://raw.githubusercontent.com/yast/yast-devtools/master/travis-tools/travis_setup.sh
-    - sh ./travis_setup.sh -p "rake yast2-devtools yast2-testsuite yast2 yast2-perl-bindings yast2-core-dev yast2-ldap yast2-perl-bindings yast2-security libcrack2-dev doxygen libdigest-sha1-perl" -g "yast-rake gettext"
+    - sh ./travis_setup.sh -p "rake yast2-devtools yast2-testsuite yast2 yast2-perl-bindings yast2-core-dev yast2-ldap yast2-perl-bindings yast2-security libcrack2-dev doxygen libdigest-sha1-perl" -g "rspec:3.3.0 yast-rake gettext"
 script:
     - rake check:syntax
     - rake check:pot

package/yast2-users.changes

@@ -1,3 +1,12 @@
+-------------------------------------------------------------------
+Thu Apr  7 08:49:13 UTC 2016 - igonzalezsosa@suse.com
+
+- Does not set empty passwords fields in /etc/shadow during
+  installation (bnc#973639, bnc#974220)
+- Set root password correctly when using a minimal profile
+  (bnc#971804)
+- 3.1.41.3
+
 -------------------------------------------------------------------
 Sat Mar  5 11:04:17 UTC 2016 - igonzalezsosa@suse.com
 

package/yast2-users.spec

@@ -17,7 +17,7 @@
 
 
 Name:           yast2-users
-Version:        3.1.41.2
+Version:        3.1.41.3
 Release:        0
 
 BuildRoot:      %{_tmppath}/%{name}-%{version}-build
@@ -37,6 +37,7 @@ BuildRequires:  yast2-ldap >= 3.1.2
 BuildRequires:  yast2-perl-bindings
 BuildRequires:  yast2-security
 BuildRequires:  yast2-testsuite
+BuildRequires:  rubygem(%rb_default_ruby_abi:rspec)
 
 Requires:       cracklib
 Requires:       perl-Digest-SHA1
@@ -102,6 +103,10 @@ provided by yast2-users package.
 %dir %{yast_yncludedir}/users
 %dir %{yast_moduledir}/YaPI
 %{yast_clientdir}/*.rb
+%dir %{yast_libdir}/users
+%dir %{yast_libdir}/users/clients
+%{yast_libdir}/users/*
+%{yast_libdir}/users/clients/*
 %{yast_desktopdir}/*.desktop
 %{yast_moduledir}/*.pm
 %{yast_moduledir}/UsersUI.rb

src/Makefile.am

@@ -36,6 +36,10 @@ client_DATA = \
   clients/inst_root.rb \
   clients/inst_user_first.rb
 
+ylibclientdir = @ylibdir@/users/clients
+ylibclient_DATA = \
+  lib/users/clients/users_finish.rb
+
 yncludedir = @yncludedir@/users
 ynclude_DATA = \
   include/users/widgets.rb \
@@ -63,6 +67,6 @@ schemafiles_DATA = \
 desktop_DATA = \
   desktop/users.desktop
 
-EXTRA_DIST = $(module_DATA) $(module1_DATA) $(client_DATA) $(ynclude_DATA) $(scrconf_DATA) $(agent_SCRIPTS) $(schemafiles_DATA) $(desktop_DATA)
+EXTRA_DIST = $(module_DATA) $(module1_DATA) $(client_DATA) $(ynclude_DATA) $(scrconf_DATA) $(agent_SCRIPTS) $(schemafiles_DATA) $(desktop_DATA) $(ylibclient_DATA)
 
 include $(top_srcdir)/Makefile.am.common

src/clients/users_finish.rb

@@ -27,75 +27,5 @@
 #
 # $Id$
 
-module Yast
-  class UsersFinishClient < Client
-    def main
-      textdomain "users"
-
-      Yast.import "Autologin"
-      Yast.import "Users"
-      Yast.import "UsersSimple"
-
-      # create_users()
-      Yast.include self, "users/routines.rb"
-
-      @func = ""
-      @param = {}
-
-      # Check arguments
-      if Ops.greater_than(Builtins.size(WFM.Args), 0) &&
-          Ops.is_string?(WFM.Args(0))
-        @func = Convert.to_string(WFM.Args(0))
-        if Ops.greater_than(Builtins.size(WFM.Args), 1) &&
-            Ops.is_map?(WFM.Args(1))
-          @param = Convert.to_map(WFM.Args(1))
-        end
-      end
-
-      Builtins.y2milestone("starting users_finish")
-      Builtins.y2debug("func=%1", @func)
-      Builtins.y2debug("param=%1", @param)
-
-      if @func == "Info"
-        return {
-          "steps" => 1,
-          # progress step title
-          "title" => _("Writing Users Configuration..."),
-          "when"  => [:installation, :live_installation, :autoinst]
-        }
-      elsif @func == "Write"
-        # Creating all users and their environment
-
-        if Mode.autoinst
-          # Write imported users (during autoupgrade no changes are done)
-
-          # During installation, some package could add a new user, so we
-          # need to read them again before writing.
-          Users.SetExportAll(true)
-          saved = Users.Export
-          Users.ReadLocal
-          Users.Import(saved)
-
-          # Write users
-          Users.SetWriteOnly(true)
-          @progress_orig = Progress.set(false)
-          @ret = Users.Write == ""
-          Progress.set(@progress_orig)
-        else
-          # write the root password
-          UsersSimple.Write
-
-          other_users = setup_all_users
-          Users.Write if other_users
-        end
-      else
-        Builtins.y2error("unknown function: %1", @func)
-      end
-
-      Builtins.y2milestone("users_finish finished")
-      nil
-    end
-  end
-end
-
-Yast::UsersFinishClient.new.main
+require "users/clients/users_finish"
+Yast::UsersFinishClient.new.run

src/include/users/routines.rb

@@ -30,6 +30,7 @@ module Yast
   module UsersRoutinesInclude
     def initialize_users_routines(include_target)
       Yast.import "Mode"
+      Yast.import "Autologin"
 
       textdomain "users"
     end

src/lib/users/clients/users_finish.rb

@@ -0,0 +1,94 @@
+# encoding: utf-8
+
+# ------------------------------------------------------------------------------
+# Copyright (c) 2016 SUSE LLC
+#
+# This program is free software; you can redistribute it and/or modify it under
+# the terms of version 2 of the GNU General Public License as published by the
+# Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
+#
+# ------------------------------------------------------------------------------
+
+require "installation/finish_client"
+
+module Yast
+  class UsersFinishClient < ::Installation::FinishClient
+    include Logger
+
+    def initialize
+      textdomain "users"
+
+      Yast.import "Users"
+      Yast.import "UsersSimple"
+      # setup_all_users()
+      Yast.include self, "users/routines.rb"
+    end
+
+    # Write users
+    #
+    # It relies in different methods depending if it's running
+    # during autoinst or in a regular installation.
+    #
+    # @see write_autoinst
+    # @see write_install
+    def write
+      if Mode.autoinst
+        write_autoinst
+      else
+        write_install
+      end
+    end
+
+  protected
+
+    # @see Implements ::Installation::FinishClient#modes
+    def modes
+      [:installation, :live_installation, :autoinst]
+    end
+
+    # @see Implements ::Installation::FinishClient#title
+    def title
+      _("Writing Users Configuration...")
+    end
+
+    # Write imported users during autoinstallation
+    #
+    # During installation, some package could add a new user, so we
+    # need to read them again before writing.
+    #
+    # On the other hand, during autoupgrade no changes are performed.
+    def write_autoinst
+      # 1. Export users imported in inst_autosetup (and store them)
+      Users.SetExportAll(false)
+      saved = Users.Export
+      log.info("Users to import: #{saved}")
+
+      # 2. Read users and settings from the installed system
+      # (bsc#965852, bsc#973639, bsc#974220 and bsc#971804)
+      Users.Read
+
+      # 3. Merge users from the system with new users from
+      #    AutoYaST profile (from step 1)
+      Users.Import(saved)
+
+      # 4. Write users
+      Users.SetWriteOnly(true)
+      @progress_orig = Progress.set(false)
+      error = Users.Write
+      log.error(error) unless error.empty?
+      Progress.set(@progress_orig)
+    end
+
+    # Write root password a new users during regular installation
+    def write_install
+      # write the root password
+      UsersSimple.Write
+      # write new users (if any)
+      Users.Write if setup_all_users
+    end
+  end
+end

src/modules/Users.pm

@@ -6156,8 +6156,7 @@ sub Import {
     # remove cache entries (#50265)
     UsersCache->ResetCache ();
 
-    # Avoid to read local users during 1st stage (bnc#965852)
-    my $error_msg = (Mode->test() || Stage->initial()) ? "" : $self->ReadLocal ();
+    my $error_msg = Mode->test() ? "" : $self->ReadLocal ();
     if ($error_msg) {
 	return 0;
     }

test/Makefile.am

@@ -0,0 +1,8 @@
+# Tests for users
+TESTS = \
+  users_finish_test.rb
+
+TEST_EXTENSIONS = .rb
+RB_LOG_COMPILER = rspec
+VERBOSE = 1
+EXTRA_DIST = $(TESTS)

test/fixtures/root/etc/default/useradd

@@ -0,0 +1,8 @@
+# useradd defaults file
+GROUP=100
+HOME=/home
+INACTIVE=-1
+EXPIRE=
+SHELL=/bin/bash
+SKEL=/etc/skel
+CREATE_MAIL_SPOOL=yes

test/fixtures/root/etc/group

@@ -0,0 +1,37 @@
+root:x:0:
+bin:x:1:daemon
+daemon:x:2:
+sys:x:3:
+tty:x:5:
+disk:x:6:
+lp:x:7:
+www:x:8:
+kmem:x:9:
+wheel:x:10:
+mail:x:12:
+news:x:13:
+uucp:x:14:
+shadow:x:15:
+dialout:x:16:
+audio:x:17:
+floppy:x:19:
+cdrom:x:20:
+console:x:21:
+utmp:x:22:
+public:x:32:
+video:x:33:
+games:x:40:
+xok:x:41:
+trusted:x:42:
+modem:x:43:
+ftp:x:49:
+lock:x:54:
+man:x:62:
+users:x:100:
+nobody:x:65533:
+nogroup:x:65534:nobody
+messagebus:x:499:
+sshd:x:498:
+tape:x:497:
+polkitd:x:496:
+nscd:x:495: