Fix 20789 - Window's user creation and password policy requirements (#59563)

* surface reason why user could not be created

* init commit before tests

* add changelog file

* bruh

* fix docstring

* better handle on logic

* convert first test to pytest

* pre-commit

* add runs_on to test

* fix lint

* more fixes

* fix nix tests

* fix windows tests

* add extra check to not break other tests

* pre commit

* remove extra check

Co-authored-by: Joe Eacott <jeacott@vmware.com>
Co-authored-by: Sage the Rage <36676171+sagetherage@users.noreply.github.com>

changelog/20789.fixed

@@ -0,0 +1 @@
+Surface strerror to user state instead of returning false

salt/modules/win_useradd.py

@@ -1,6 +1,5 @@
-# -*- coding: utf-8 -*-
 """
-Module for managing Windows Users
+Module for managing Windows Users.
 
 .. important::
     If you feel that Salt should be using this module to manage users on a
@@ -22,14 +21,11 @@ Module for managing Windows Users
 .. note::
     This currently only works with local user accounts, not domain accounts
 """
-# Import Python libs
-from __future__ import absolute_import, print_function, unicode_literals
 
 import logging
 import time
 from datetime import datetime
 
-# Import Salt libs
 import salt.utils.args
 import salt.utils.dateutils
 import salt.utils.platform
@@ -93,10 +89,10 @@ def _to_unicode(instr):
     Returns:
         str: Unicode type string
     """
-    if instr is None or isinstance(instr, six.text_type):
+    if instr is None or isinstance(instr, str):
         return instr
     else:
-        return six.text_type(instr, "utf-8")
+        return str(instr, "utf-8")
 
 
 def add(
@@ -175,7 +171,7 @@ def add(
         log.error("nbr: %s", exc.winerror)
         log.error("ctx: %s", exc.funcname)
         log.error("msg: %s", exc.strerror)
-        return False
+        return exc.strerror
 
     update(name=name, homedrive=homedrive, profile=profile, fullname=fullname)
 
@@ -255,7 +251,7 @@ def update(
     .. code-block:: bash
 
         salt '*' user.update bob password=secret profile=C:\\Users\\Bob
-                 home=\\server\homeshare\bob homedrive=U:
+                 home=\\server\\homeshare\\bob homedrive=U:
     """
     # pylint: enable=anomalous-backslash-in-string
     if six.PY2:
@@ -277,7 +273,7 @@ def update(
         log.error("nbr: %s", exc.winerror)
         log.error("ctx: %s", exc.funcname)
         log.error("msg: %s", exc.strerror)
-        return False
+        return exc.strerror
 
     # Check parameters to update
     # Update the user object with new settings
@@ -302,7 +298,7 @@ def update(
             try:
                 dt_obj = salt.utils.dateutils.date_cast(expiration_date)
             except (ValueError, RuntimeError):
-                return "Invalid Date/Time Format: {0}".format(expiration_date)
+                return "Invalid Date/Time Format: {}".format(expiration_date)
             user_info["acct_expires"] = time.mktime(dt_obj.timetuple())
     if expired is not None:
         if expired:
@@ -336,7 +332,7 @@ def update(
         log.error("nbr: %s", exc.winerror)
         log.error("ctx: %s", exc.funcname)
         log.error("msg: %s", exc.strerror)
-        return False
+        return exc.strerror
 
     return True
 
@@ -376,7 +372,7 @@ def delete(name, purge=False, force=False):
         log.error("nbr: %s", exc.winerror)
         log.error("ctx: %s", exc.funcname)
         log.error("msg: %s", exc.strerror)
-        return False
+        return exc.strerror
 
     # Check if the user is logged in
     # Return a list of logged in users
@@ -414,7 +410,7 @@ def delete(name, purge=False, force=False):
                 log.error("nbr: %s", exc.winerror)
                 log.error("ctx: %s", exc.funcname)
                 log.error("msg: %s", exc.strerror)
-                return False
+                return exc.strerror
         else:
             log.error("User %s is currently logged in.", name)
             return False
@@ -433,7 +429,7 @@ def delete(name, purge=False, force=False):
                 log.error("nbr: %s", exc.winerror)
                 log.error("ctx: %s", exc.funcname)
                 log.error("msg: %s", exc.strerror)
-                return False
+                return exc.strerror
 
     # And finally remove the user account
     try:
@@ -443,7 +439,7 @@ def delete(name, purge=False, force=False):
         log.error("nbr: %s", exc.winerror)
         log.error("ctx: %s", exc.funcname)
         log.error("msg: %s", exc.strerror)
-        return False
+        return exc.strerror
 
     return True
 
@@ -529,7 +525,7 @@ def addgroup(name, group):
     if group in user["groups"]:
         return True
 
-    cmd = 'net localgroup "{0}" {1} /add'.format(group, name)
+    cmd = 'net localgroup "{}" {} /add'.format(group, name)
     ret = __salt__["cmd.run_all"](cmd, python_shell=True)
 
     return ret["retcode"] == 0
@@ -568,7 +564,7 @@ def removegroup(name, group):
     if group not in user["groups"]:
         return True
 
-    cmd = 'net localgroup "{0}" {1} /delete'.format(group, name)
+    cmd = 'net localgroup "{}" {} /delete'.format(group, name)
     ret = __salt__["cmd.run_all"](cmd, python_shell=True)
 
     return ret["retcode"] == 0
@@ -709,14 +705,14 @@ def chgroups(name, groups, append=True):
         for group in ugrps:
             group = _cmd_quote(group).lstrip("'").rstrip("'")
             if group not in groups:
-                cmd = 'net localgroup "{0}" {1} /delete'.format(group, name)
+                cmd = 'net localgroup "{}" {} /delete'.format(group, name)
                 __salt__["cmd.run_all"](cmd, python_shell=True)
 
     for group in groups:
         if group in ugrps:
             continue
         group = _cmd_quote(group).lstrip("'").rstrip("'")
-        cmd = 'net localgroup "{0}" {1} /add'.format(group, name)
+        cmd = 'net localgroup "{}" {} /add'.format(group, name)
         out = __salt__["cmd.run_all"](cmd, python_shell=True)
         if out["retcode"] != 0:
             log.error(out["stdout"])
@@ -853,7 +849,7 @@ def _get_userprofile_from_registry(user, sid):
     """
     profile_dir = __utils__["reg.read_value"](
         "HKEY_LOCAL_MACHINE",
-        "SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\ProfileList\\{0}".format(sid),
+        "SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\ProfileList\\{}".format(sid),
         "ProfileImagePath",
     )["vdata"]
     log.debug('user %s with sid=%s profile is located at "%s"', user, sid, profile_dir)
@@ -987,12 +983,12 @@ def rename(name, new_name):
     # Load information for the current name
     current_info = info(name)
     if not current_info:
-        raise CommandExecutionError("User '{0}' does not exist".format(name))
+        raise CommandExecutionError("User '{}' does not exist".format(name))
 
     # Look for an existing user with the new name
     new_info = info(new_name)
     if new_info:
-        raise CommandExecutionError("User '{0}' already exists".format(new_name))
+        raise CommandExecutionError("User '{}' already exists".format(new_name))
 
     # Rename the user account
     # Connect to WMI
@@ -1003,7 +999,7 @@ def rename(name, new_name):
         try:
             user = c.Win32_UserAccount(Name=name)[0]
         except IndexError:
-            raise CommandExecutionError("User '{0}' does not exist".format(name))
+            raise CommandExecutionError("User '{}' does not exist".format(name))
 
         # Rename the user
         result = user.Rename(new_name)[0]
@@ -1025,7 +1021,7 @@ def rename(name, new_name):
                 10: "Internal error",
             }
             raise CommandExecutionError(
-                "There was an error renaming '{0}' to '{1}'. Error: {2}".format(
+                "There was an error renaming '{}' to '{}'. Error: {}".format(
                     name, new_name, error_dict[result]
                 )
             )

salt/states/user.py

@@ -1,7 +1,6 @@
-# -*- coding: utf-8 -*-
 """
-Management of user accounts
-===========================
+Management of user accounts.
+============================
 
 The user module is used to create and manage user settings, users can be set
 as either absent or present
@@ -23,13 +22,10 @@ as either absent or present
     testuser:
       user.absent
 """
-# Import Python libs
-from __future__ import absolute_import, print_function, unicode_literals
 
 import logging
 import os
 
-# Import Salt libs
 import salt.utils.data
 import salt.utils.dateutils
 import salt.utils.platform
@@ -37,9 +33,6 @@ import salt.utils.user
 import salt.utils.versions
 from salt.exceptions import CommandExecutionError
 
-# Import 3rd-party libs
-from salt.ext import six
-
 log = logging.getLogger(__name__)
 
 
@@ -219,13 +212,13 @@ def _changes(
     errors = []
     if not allow_uid_change and "uid" in change:
         errors.append(
-            "Changing uid ({0} -> {1}) not permitted, set allow_uid_change to "
+            "Changing uid ({} -> {}) not permitted, set allow_uid_change to "
             "True to force this change. Note that this will not change file "
             "ownership.".format(lusr["uid"], uid)
         )
     if not allow_gid_change and "gid" in change:
         errors.append(
-            "Changing gid ({0} -> {1}) not permitted, set allow_gid_change to "
+            "Changing gid ({} -> {}) not permitted, set allow_gid_change to "
             "True to force this change. Note that this will not change file "
             "ownership.".format(lusr["gid"], gid)
         )
@@ -522,21 +515,21 @@ def present(
         "name": name,
         "changes": {},
         "result": True,
-        "comment": "User {0} is present and up to date".format(name),
+        "comment": "User {} is present and up to date".format(name),
     }
 
     # the comma is used to separate field in GECOS, thus resulting into
     # salt adding the end of fullname each time this function is called
     for gecos_field in [fullname, roomnumber, workphone]:
-        if isinstance(gecos_field, six.string_types) and "," in gecos_field:
-            ret["comment"] = "Unsupported char ',' in {0}".format(gecos_field)
+        if isinstance(gecos_field, str) and "," in gecos_field:
+            ret["comment"] = "Unsupported char ',' in {}".format(gecos_field)
             ret["result"] = False
             return ret
 
     if groups:
         missing_groups = [x for x in groups if not __salt__["group.info"](x)]
         if missing_groups:
-            ret["comment"] = "The following group(s) are not present: " "{0}".format(
+            ret["comment"] = "The following group(s) are not present: " "{}".format(
                 ",".join(missing_groups)
             )
             ret["result"] = False
@@ -638,12 +631,12 @@ def present(
         if __opts__["test"]:
             ret["result"] = None
             ret["comment"] = "The following user attributes are set to be " "changed:\n"
-            for key, val in six.iteritems(changes):
+            for key, val in changes.items():
                 if key == "passwd":
                     val = "XXX-REDACTED-XXX"
                 elif key == "group" and not remove_groups:
                     key = "ensure groups"
-                ret["comment"] += "{0}: {1}\n".format(key, val)
+                ret["comment"] += "{}: {}\n".format(key, val)
             return ret
         # The user is present
         if "shadow.info" in __salt__:
@@ -732,9 +725,9 @@ def present(
         for key in [
             x
             for x in list(changes)
-            if x != "groups" and "user.ch{0}".format(x) in __salt__
+            if x != "groups" and "user.ch{}".format(x) in __salt__
         ]:
-            __salt__["user.ch{0}".format(key)](name, changes.pop(key))
+            __salt__["user.ch{}".format(key)](name, changes.pop(key))
 
         # Do group changes last
         if "groups" in changes:
@@ -742,7 +735,7 @@ def present(
 
         if changes:
             ret.get("warnings", []).append(
-                "Unhandled changes: {0}".format(", ".join(changes))
+                "Unhandled changes: {}".format(", ".join(changes))
             )
 
         post = __salt__["user.info"](name)
@@ -765,7 +758,7 @@ def present(
         if __grains__["kernel"] in ("OpenBSD", "FreeBSD") and lcpost != lcpre:
             ret["changes"]["loginclass"] = lcpost
         if ret["changes"]:
-            ret["comment"] = "Updated user {0}".format(name)
+            ret["comment"] = "Updated user {}".format(name)
         changes = _changes(
             name,
             uid,
@@ -804,7 +797,7 @@ def present(
         # first time we ran _changes().
 
         if changes:
-            ret["comment"] = "These values could not be changed: {0}".format(changes)
+            ret["comment"] = "These values could not be changed: {}".format(changes)
             ret["result"] = False
         return ret
 
@@ -812,7 +805,7 @@ def present(
         # The user is not present, make it!
         if __opts__["test"]:
             ret["result"] = None
-            ret["comment"] = "User {0} set to be added".format(name)
+            ret["comment"] = "User {} set to be added".format(name)
             return ret
         if groups and present_optgroups:
             groups.extend(present_optgroups)
@@ -854,8 +847,10 @@ def present(
                 "logonscript": win_logonscript,
             }
 
-        if __salt__["user.add"](**params):
-            ret["comment"] = "New user {0} created".format(name)
+        # user.add returns true, false, or a str in the case of windows failure
+        result = __salt__["user.add"](**params)
+        if result is True:
+            ret["comment"] = "New user {} created".format(name)
             ret["changes"] = __salt__["user.info"](name)
             if not createhome:
                 # pwd incorrectly reports presence of home
@@ -870,9 +865,9 @@ def present(
                     spost = __salt__["shadow.info"](name)
                     if spost["passwd"] != password:
                         ret["comment"] = (
-                            "User {0} created but failed to set"
+                            "User {} created but failed to set"
                             " password to"
-                            " {1}".format(name, "XXX-REDACTED-XXX")
+                            " {}".format(name, "XXX-REDACTED-XXX")
                         )
                         ret["result"] = False
                     ret["changes"]["password"] = "XXX-REDACTED-XXX"
@@ -881,7 +876,7 @@ def present(
                     spost = __salt__["shadow.info"](name)
                     if spost["passwd"] != "":
                         ret["comment"] = (
-                            "User {0} created but failed to "
+                            "User {} created but failed to "
                             "empty password".format(name)
                         )
                         ret["result"] = False
@@ -891,9 +886,9 @@ def present(
                     spost = __salt__["shadow.info"](name)
                     if spost["lstchg"] != date:
                         ret["comment"] = (
-                            "User {0} created but failed to set"
+                            "User {} created but failed to set"
                             " last change date to"
-                            " {1}".format(name, date)
+                            " {}".format(name, date)
                         )
                         ret["result"] = False
                     ret["changes"]["date"] = date
@@ -902,9 +897,9 @@ def present(
                     spost = __salt__["shadow.info"](name)
                     if spost["min"] != mindays:
                         ret["comment"] = (
-                            "User {0} created but failed to set"
+                            "User {} created but failed to set"
                             " minimum days to"
-                            " {1}".format(name, mindays)
+                            " {}".format(name, mindays)
                         )
                         ret["result"] = False
                     ret["changes"]["mindays"] = mindays
@@ -913,9 +908,9 @@ def present(
                     spost = __salt__["shadow.info"](name)
                     if spost["max"] != maxdays:
                         ret["comment"] = (
-                            "User {0} created but failed to set"
+                            "User {} created but failed to set"
                             " maximum days to"
-                            " {1}".format(name, maxdays)
+                            " {}".format(name, maxdays)
                         )
                         ret["result"] = False
                     ret["changes"]["maxdays"] = maxdays
@@ -924,9 +919,9 @@ def present(
                     spost = __salt__["shadow.info"](name)
                     if spost["inact"] != inactdays:
                         ret["comment"] = (
-                            "User {0} created but failed to set"
+                            "User {} created but failed to set"
                             " inactive days to"
-                            " {1}".format(name, inactdays)
+                            " {}".format(name, inactdays)
                         )
                         ret["result"] = False
                     ret["changes"]["inactdays"] = inactdays
@@ -935,9 +930,9 @@ def present(
                     spost = __salt__["shadow.info"](name)
                     if spost["warn"] != warndays:
                         ret["comment"] = (
-                            "User {0} created but failed to set"
+                            "User {} created but failed to set"
                             " warn days to"
-                            " {1}".format(name, warndays)
+                            " {}".format(name, warndays)
                         )
                         ret["result"] = False
                     ret["changes"]["warndays"] = warndays
@@ -946,9 +941,9 @@ def present(
                     spost = __salt__["shadow.info"](name)
                     if spost["expire"] != expire:
                         ret["comment"] = (
-                            "User {0} created but failed to set"
+                            "User {} created but failed to set"
                             " expire days to"
-                            " {1}".format(name, expire)
+                            " {}".format(name, expire)
                         )
                         ret["result"] = False
                     ret["changes"]["expire"] = expire
@@ -956,9 +951,9 @@ def present(
                 if password and not empty_password:
                     if not __salt__["user.setpassword"](name, password):
                         ret["comment"] = (
-                            "User {0} created but failed to set"
+                            "User {} created but failed to set"
                             " password to"
-                            " {1}".format(name, "XXX-REDACTED-XXX")
+                            " {}".format(name, "XXX-REDACTED-XXX")
                         )
                         ret["result"] = False
                     ret["changes"]["passwd"] = "XXX-REDACTED-XXX"
@@ -969,25 +964,29 @@ def present(
                         spost["expire"]
                     ) != salt.utils.dateutils.strftime(expire):
                         ret["comment"] = (
-                            "User {0} created but failed to set"
+                            "User {} created but failed to set"
                             " expire days to"
-                            " {1}".format(name, expire)
+                            " {}".format(name, expire)
                         )
                         ret["result"] = False
                     ret["changes"]["expiration_date"] = spost["expire"]
             elif salt.utils.platform.is_darwin() and password and not empty_password:
                 if not __salt__["shadow.set_password"](name, password):
                     ret["comment"] = (
-                        "User {0} created but failed to set"
+                        "User {} created but failed to set"
                         " password to"
-                        " {1}".format(name, "XXX-REDACTED-XXX")
+                        " {}".format(name, "XXX-REDACTED-XXX")
                     )
                     ret["result"] = False
                 ret["changes"]["passwd"] = "XXX-REDACTED-XXX"
         else:
-            ret["comment"] = "Failed to create new user {0}".format(name)
+            # if we failed to create a user, result is either false or
+            # str in the case of windows so handle both cases here
+            if isinstance(result, str):
+                ret["comment"] = result
+            else:
+                ret["comment"] = "Failed to create new user {}".format(name)
             ret["result"] = False
-
     return ret
 
 
@@ -1014,22 +1013,22 @@ def absent(name, purge=False, force=False):
         # The user is present, make it not present
         if __opts__["test"]:
             ret["result"] = None
-            ret["comment"] = "User {0} set for removal".format(name)
+            ret["comment"] = "User {} set for removal".format(name)
             return ret
         beforegroups = set(salt.utils.user.get_group_list(name))
         ret["result"] = __salt__["user.delete"](name, purge, force)
-        aftergroups = set([g for g in beforegroups if __salt__["group.info"](g)])
+        aftergroups = {g for g in beforegroups if __salt__["group.info"](g)}
         if ret["result"]:
             ret["changes"] = {}
             for g in beforegroups - aftergroups:
-                ret["changes"]["{0} group".format(g)] = "removed"
+                ret["changes"]["{} group".format(g)] = "removed"
             ret["changes"][name] = "removed"
-            ret["comment"] = "Removed user {0}".format(name)
+            ret["comment"] = "Removed user {}".format(name)
         else:
             ret["result"] = False
-            ret["comment"] = "Failed to remove user {0}".format(name)
+            ret["comment"] = "Failed to remove user {}".format(name)
         return ret
 
-    ret["comment"] = "User {0} is not present".format(name)
+    ret["comment"] = "User {} is not present".format(name)
 
     return ret

tests/integration/modules/test_useradd.py

@@ -1,226 +0,0 @@
-import pytest
-from tests.support.case import ModuleCase
-from tests.support.helpers import random_string, requires_system_grains, runs_on
-
-
-@runs_on(kernel="Linux")
-@pytest.mark.destructive_test
-@pytest.mark.skip_if_not_root
-class UseraddModuleTestLinux(ModuleCase):
-    @requires_system_grains
-    @pytest.mark.slow_test
-    def test_groups_includes_primary(self, grains):
-        # Let's create a user, which usually creates the group matching the
-        # name
-        uname = random_string("RS-", lowercase=False)
-        if self.run_function("user.add", [uname]) is not True:
-            # Skip because creating is not what we're testing here
-            self.run_function("user.delete", [uname, True, True])
-            self.skipTest("Failed to create user")
-
-        try:
-            uinfo = self.run_function("user.info", [uname])
-            if grains["os_family"] in ("Suse",):
-                self.assertIn("users", uinfo["groups"])
-            else:
-                self.assertIn(uname, uinfo["groups"])
-
-            # This uid is available, store it
-            uid = uinfo["uid"]
-
-            self.run_function("user.delete", [uname, True, True])
-
-            # Now, a weird group id
-            gname = random_string("RS-", lowercase=False)
-            if self.run_function("group.add", [gname]) is not True:
-                self.run_function("group.delete", [gname, True, True])
-                self.skipTest("Failed to create group")
-
-            ginfo = self.run_function("group.info", [gname])
-
-            # And create the user with that gid
-            if self.run_function("user.add", [uname, uid, ginfo["gid"]]) is False:
-                # Skip because creating is not what we're testing here
-                self.run_function("user.delete", [uname, True, True])
-                self.skipTest("Failed to create user")
-
-            uinfo = self.run_function("user.info", [uname])
-            self.assertIn(gname, uinfo["groups"])
-
-        except AssertionError:
-            self.run_function("user.delete", [uname, True, True])
-            raise
-
-    @pytest.mark.slow_test
-    def test_user_primary_group(self):
-        """
-        Tests the primary_group function
-        """
-        name = "saltyuser"
-
-        # Create a user to test primary group function
-        if self.run_function("user.add", [name]) is not True:
-            self.run_function("user.delete", [name])
-            self.skipTest("Failed to create a user")
-
-        try:
-            # Test useradd.primary_group
-            primary_group = self.run_function("user.primary_group", [name])
-            uid_info = self.run_function("user.info", [name])
-            self.assertIn(primary_group, uid_info["groups"])
-
-        except Exception:  # pylint: disable=broad-except
-            self.run_function("user.delete", [name])
-            raise
-
-
-@runs_on(kernel="Windows")
-@pytest.mark.windows_whitelisted
-@pytest.mark.destructive_test
-@pytest.mark.skip_if_not_root
-class UseraddModuleTestWindows(ModuleCase):
-    def setUp(self):
-        self.user_name = random_string("RS-", lowercase=False)
-        self.group_name = random_string("RS-", lowercase=False)
-
-    def tearDown(self):
-        self.run_function("user.delete", [self.user_name, True, True])
-        self.run_function("group.delete", [self.group_name])
-
-    def _add_user(self):
-        """
-        helper class to add user
-        """
-        if self.run_function("user.add", [self.user_name]) is False:
-            # Skip because creating is not what we're testing here
-            self.skipTest("Failed to create user")
-
-    def _add_group(self):
-        """
-        helper class to add group
-        """
-        if self.run_function("group.add", [self.group_name]) is False:
-            # Skip because creating is not what we're testing here
-            self.skipTest("Failed to create group")
-
-    @pytest.mark.slow_test
-    def test_add_user(self):
-        """
-        Test adding a user
-        """
-        self._add_user()
-        user_list = self.run_function("user.list_users")
-        self.assertIn(self.user_name, user_list)
-
-    @pytest.mark.slow_test
-    def test_add_group(self):
-        """
-        Test adding a user
-        """
-        self._add_group()
-        group_list = self.run_function("group.list_groups")
-        self.assertIn(self.group_name, group_list)
-
-    @pytest.mark.slow_test
-    def test_add_user_to_group(self):
-        """
-        Test adding a user to a group
-        """
-        self._add_group()
-        # And create the user as a member of that group
-        self.run_function("user.add", [self.user_name], groups=self.group_name)
-        user_info = self.run_function("user.info", [self.user_name])
-        self.assertIn(self.group_name, user_info["groups"])
-
-    @pytest.mark.slow_test
-    def test_add_user_addgroup(self):
-        """
-        Test adding a user to a group with groupadd
-        """
-        self._add_group()
-        self._add_user()
-        self.run_function("user.addgroup", [self.user_name, self.group_name])
-        info = self.run_function("user.info", [self.user_name])
-        self.assertEqual(info["groups"], [self.group_name])
-
-    @pytest.mark.slow_test
-    def test_user_chhome(self):
-        """
-        Test changing a users home dir
-        """
-        self._add_user()
-        user_dir = r"c:\salt"
-        self.run_function("user.chhome", [self.user_name, user_dir])
-        info = self.run_function("user.info", [self.user_name])
-        self.assertEqual(info["home"], user_dir)
-
-    @pytest.mark.slow_test
-    def test_user_chprofile(self):
-        """
-        Test changing a users profile
-        """
-        self._add_user()
-        config = r"c:\salt\config"
-        self.run_function("user.chprofile", [self.user_name, config])
-        info = self.run_function("user.info", [self.user_name])
-        self.assertEqual(info["profile"], config)
-
-    @pytest.mark.slow_test
-    def test_user_chfullname(self):
-        """
-        Test changing a users fullname
-        """
-        self._add_user()
-        name = "Salt Test"
-        self.run_function("user.chfullname", [self.user_name, name])
-        info = self.run_function("user.info", [self.user_name])
-        self.assertEqual(info["fullname"], name)
-
-    @pytest.mark.slow_test
-    def test_user_delete(self):
-        """
-        Test deleting a user
-        """
-        self._add_user()
-        self.assertTrue(self.run_function("user.info", [self.user_name])["active"])
-        self.run_function("user.delete", [self.user_name])
-        self.assertEqual({}, self.run_function("user.info", [self.user_name]))
-
-    @pytest.mark.slow_test
-    def test_user_removegroup(self):
-        """
-        Test removing a group
-        """
-        self._add_user()
-        self._add_group()
-        self.run_function("user.addgroup", [self.user_name, self.group_name])
-        self.assertIn(
-            self.group_name, self.run_function("user.list_groups", [self.user_name])
-        )
-        self.run_function("user.removegroup", [self.user_name, self.group_name])
-        self.assertNotIn(
-            self.group_name, self.run_function("user.list_groups", [self.user_name])
-        )
-
-    @pytest.mark.slow_test
-    def test_user_rename(self):
-        """
-        Test changing a users name
-        """
-        self._add_user()
-        name = "newuser"
-        self.run_function("user.rename", [self.user_name, name])
-        info = self.run_function("user.info", [name])
-        self.assertTrue(info["active"])
-
-        # delete new user
-        self.run_function("user.delete", [name, True, True])
-
-    @pytest.mark.slow_test
-    def test_user_setpassword(self):
-        """
-        Test setting a password
-        """
-        self._add_user()
-        passwd = "sup3rs3cr3T!"
-        self.assertTrue(self.run_function("user.setpassword", [self.user_name, passwd]))

tests/pytests/integration/modules/test_useradd.py

@@ -0,0 +1,279 @@
+"""
+Integration tests for modules/useradd.py and modules/win_useradd.py
+"""
+import pytest
+from tests.support.helpers import random_string, requires_system_grains
+
+pytestmark = [pytest.mark.windows_whitelisted, pytest.mark.skip_unless_on_windows]
+
+
+@pytest.fixture(scope="function")
+def setup_teardown_vars(salt_call_cli):
+    user_name = random_string("RS-", lowercase=False)
+    group_name = random_string("RS-", lowercase=False)
+    try:
+        yield user_name, group_name
+    finally:
+        salt_call_cli.run("user.delete", user_name, True, True)
+        salt_call_cli.run("group.delete", group_name)
+        salt_call_cli.run("lgpo.set", "computer_policy={'Minimum Password Length': 0}")
+
+
+@pytest.mark.skip_on_windows(reason="Windows does not do user checks")
+@pytest.mark.destructive_test
+@pytest.mark.skip_if_not_root
+@requires_system_grains
+def test_groups_includes_primary(setup_teardown_vars, grains, salt_call_cli):
+    # Let's create a user, which usually creates the group matching the
+    # name
+    uname = random_string("RS-", lowercase=False)
+    ret = salt_call_cli.run("user.add", uname)
+    if ret.json is False:
+        # Skip because creating is not what we're testing here
+        salt_call_cli.run("user.delete", [uname, True, True])
+        pytest.skip("Failed to create user")
+
+    try:
+        uinfo = salt_call_cli.run("user.info", uname)
+        if grains["os_family"] in ("Suse",):
+            assert "users" in uinfo.json["groups"]
+        else:
+            assert uname in uinfo.json["groups"]
+
+        # This uid is available, store it
+        uid = uinfo.json["uid"]
+
+        salt_call_cli.run("user.delete", uname, True, True)
+
+        # Now, a weird group id
+        gname = random_string("RS-", lowercase=False)
+        ret = salt_call_cli.run("group.add", gname)
+        if ret.json is False:
+            salt_call_cli.run("group.delete", gname, True, True)
+            pytest.skip("Failed to create group")
+
+        ginfo = salt_call_cli.run("group.info", gname)
+        ginfo = ginfo.json
+
+        # And create the user with that gid
+        ret = salt_call_cli.run("user.add", uname, uid, ginfo["gid"])
+        if ret.json is False:
+            # Skip because creating is not what we're testing here
+            salt_call_cli.run("user.delete", [uname, True, True])
+            pytest.skip("Failed to create user")
+
+        uinfo = salt_call_cli.run("user.info", uname)
+        assert gname in uinfo.json["groups"]
+
+    except AssertionError:
+        pytest.raises(salt_call_cli.run("user.delete", [uname, True, True]))
+
+
+@pytest.mark.skip_on_windows(reason="Windows does not do user checks")
+@pytest.mark.destructive_test
+@pytest.mark.skip_if_not_root
+def test_user_primary_group(setup_teardown_vars, salt_call_cli):
+    """
+    Tests the primary_group function
+    """
+    name = "saltyuser"
+
+    # Create a user to test primary group function
+    ret = salt_call_cli.run("user.add", name)
+    if ret.json is False:
+        salt_call_cli.run("user.delete", name)
+        pytest.skip("Failed to create a user")
+
+    # Test useradd.primary_group
+    primary_group = salt_call_cli.run("user.primary_group", name)
+    uid_info = salt_call_cli.run("user.info", name)
+    assert primary_group.json in uid_info.json["groups"]
+
+
+@pytest.mark.skip_unless_on_windows(reason="Test is only applicable to Windows.")
+@pytest.mark.destructive_test
+def test_add_user(setup_teardown_vars, salt_call_cli):
+    """
+    Test adding a user.
+    """
+    user_name = setup_teardown_vars[0]
+    salt_call_cli.run("user.add", user_name)
+    user_add = salt_call_cli.run("user.list_users")
+    assert user_name in user_add.json
+
+
+@pytest.mark.skip_unless_on_windows(reason="Test is only applicable to Windows.")
+@pytest.mark.destructive_test
+def test_add_group(setup_teardown_vars, salt_call_cli):
+    """
+    Test adding a user and check its apart of a group.
+    """
+    group_name = setup_teardown_vars[1]
+    salt_call_cli.run("group.add", group_name)
+    group_list = salt_call_cli.run("group.list_groups")
+    assert group_name in group_list.json
+
+
+@pytest.mark.skip_unless_on_windows(reason="Test is only applicable to Windows.")
+@pytest.mark.destructive_test
+def test_add_user_to_group(setup_teardown_vars, salt_call_cli):
+    """
+    Test adding a user to a group.
+    """
+    user_name = setup_teardown_vars[0]
+    group_name = setup_teardown_vars[1]
+
+    salt_call_cli.run("group.add", group_name)
+    # And create the user as a member of that group
+    salt_call_cli.run("user.add", user_name, groups=group_name)
+
+    user_info = salt_call_cli.run("user.info", user_name)
+    assert group_name in user_info.json["groups"]
+
+
+@pytest.mark.skip_unless_on_windows(reason="Test is only applicable to Windows.")
+@pytest.mark.destructive_test
+def test_add_user_addgroup(setup_teardown_vars, salt_call_cli):
+    """
+    Test adding a user to a group with groupadd.
+    """
+    user_name = setup_teardown_vars[0]
+    group_name = setup_teardown_vars[1]
+
+    salt_call_cli.run("group.add", group_name)
+    salt_call_cli.run("user.add", user_name)
+
+    salt_call_cli.run("user.addgroup", user_name, group_name)
+    info = salt_call_cli.run("user.info", user_name)
+    assert [group_name] == info.json["groups"]
+
+
+@pytest.mark.skip_unless_on_windows(reason="Test is only applicable to Windows.")
+@pytest.mark.destructive_test
+def test_user_chhome(setup_teardown_vars, salt_call_cli):
+    """
+    Test changing a users home dir.
+    """
+    user_dir = r"c:\salt"
+    user_name = setup_teardown_vars[0]
+    salt_call_cli.run("user.add", user_name)
+    salt_call_cli.run("user.chhome", user_name, user_dir)
+
+    info = salt_call_cli.run("user.info", user_name)
+    assert user_dir == info.json["home"]
+
+
+@pytest.mark.skip_unless_on_windows(reason="Test is only applicable to Windows.")
+@pytest.mark.destructive_test
+def test_user_chprofile(setup_teardown_vars, salt_call_cli):
+    """
+    Test changing a users profile.
+    """
+    config = r"c:\salt\config"
+    user_name = setup_teardown_vars[0]
+    salt_call_cli.run("user.add", user_name)
+
+    salt_call_cli.run("user.chprofile", user_name, config)
+    info = salt_call_cli.run("user.info", user_name)
+    assert config == info.json["profile"]
+
+
+@pytest.mark.skip_unless_on_windows(reason="Test is only applicable to Windows.")
+@pytest.mark.destructive_test
+def test_user_chfullname(setup_teardown_vars, salt_call_cli):
+    """
+    Test changing a users fullname.
+    """
+    name = "Salt Test"
+    user_name = setup_teardown_vars[0]
+    salt_call_cli.run("user.add", user_name)
+
+    salt_call_cli.run("user.chfullname", user_name, name)
+    info = salt_call_cli.run("user.info", user_name)
+    assert name == info.json["fullname"]
+
+
+@pytest.mark.skip_unless_on_windows(reason="Test is only applicable to Windows.")
+@pytest.mark.destructive_test
+def test_user_delete(setup_teardown_vars, salt_call_cli):
+    """
+    Test deleting a user.
+    """
+    user_name = setup_teardown_vars[0]
+    salt_call_cli.run("user.add", user_name)
+    salt_call_cli.run("user.delete", user_name)
+    ret = salt_call_cli.run("user.info", user_name)
+    assert {} == ret.json
+
+
+@pytest.mark.skip_unless_on_windows(reason="Test is only applicable to Windows.")
+@pytest.mark.destructive_test
+def test_user_removegroup(setup_teardown_vars, salt_call_cli):
+    """
+    Test removing a group.
+    """
+    user_name = setup_teardown_vars[0]
+    group_name = setup_teardown_vars[1]
+
+    salt_call_cli.run("user.add", user_name)
+    salt_call_cli.run("group.add", group_name)
+
+    salt_call_cli.run("user.addgroup", user_name, group_name)
+    ret = salt_call_cli.run("user.list_groups", user_name)
+    assert [group_name] == ret.json
+
+    salt_call_cli.run("user.removegroup", user_name, group_name)
+    ret = salt_call_cli.run("user.list_groups", user_name)
+    assert [group_name] not in ret.json
+
+
+@pytest.mark.skip_unless_on_windows(reason="Test is only applicable to Windows.")
+@pytest.mark.destructive_test
+def test_user_rename(setup_teardown_vars, salt_call_cli):
+    """
+    Test changing a users name.
+    """
+    name = "newuser"
+    user_name = setup_teardown_vars[0]
+    salt_call_cli.run("user.add", user_name)
+
+    salt_call_cli.run("user.rename", user_name, name)
+    info = salt_call_cli.run("user.info", name)
+
+    assert info.json["active"] is True
+
+
+@pytest.mark.skip_unless_on_windows(reason="Test is only applicable to Windows.")
+@pytest.mark.destructive_test
+def test_user_setpassword(setup_teardown_vars, salt_call_cli):
+    """
+    Test setting a password.
+    """
+    passwd = "sup3rs3cr3T!"
+    user_name = setup_teardown_vars[0]
+
+    salt_call_cli.run("user.add", user_name)
+    ret = salt_call_cli.run("user.setpassword", user_name, passwd)
+    assert ret.json is True
+
+
+@pytest.mark.skip_unless_on_windows(reason="Test is only applicable to Windows.")
+@pytest.mark.destructive_test
+def test_user_setpassword_policy(setup_teardown_vars, salt_call_cli):
+    """
+    Test setting a password with a password policy.
+    """
+    passwd = "test"
+    user_name = setup_teardown_vars[0]
+
+    # attempt to set a password policy that will cause a failure when creating a user
+    salt_call_cli.run("lgpo.set", "computer_policy={'Minimum Password Length': 8}")
+    ret = salt_call_cli.run("user.add", user_name, password=passwd)
+
+    # fix the policy and store the previous strerror in ret to cleanup
+    salt_call_cli.run("lgpo.set", "computer_policy={'Minimum Password Length': 0}")
+    assert ret.json == (
+        "The password does not meet the password policy requirements."
+        " Check the minimum password length, password complexity and"
+        " password history requirements."
+    )