Use new classes for run_command and Service

User and Group now return unmodified instance when they are called with
an instance of themselves: User(user) is user.

run_command() and Service class accept either names or User object.

Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>

ipaplatform/base/constants.py

@@ -14,6 +14,13 @@ import sys
 class _Entity(str):
     __slots__ = ("_entity", )
 
+    def __new__(cls, name):
+        # if 'name' is already an instance of cls, return identical name
+        if isinstance(name, cls):
+            return name
+        else:
+            return super().__new__(cls, name)
+
     def __init__(self, name):
         super().__init__()
         self._entity = None

ipapython/ipautil.py

@@ -36,8 +36,6 @@ import re
 import datetime
 import netaddr
 import time
-import pwd
-import grp
 from contextlib import contextmanager
 import locale
 import collections
@@ -53,6 +51,8 @@ except ImportError:
 
 from ipapython.dn import DN
 from ipaplatform.paths import paths
+from ipaplatform.constants import User, Group
+
 
 logger = logging.getLogger(__name__)
 
@@ -387,7 +387,7 @@ class CalledProcessError(subprocess.CalledProcessError):
 
 def run(args, stdin=None, raiseonerr=True, nolog=(), env=None,
         capture_output=False, skip_output=False, cwd=None,
-        runas=None, suplementary_groups=[],
+        runas=None, suplementary_groups=(),
         capture_error=False, encoding=None, redirect_output=False,
         umask=None, nolog_output=False, nolog_error=False):
     """
@@ -415,11 +415,12 @@ def run(args, stdin=None, raiseonerr=True, nolog=(), env=None,
     :param capture_output: Capture stdout
     :param skip_output: Redirect the output to /dev/null and do not log it
     :param cwd: Current working directory
-    :param runas: Name of a user that the command should be run as. The spawned
-        process will have both real and effective UID and GID set.
-    :param suplementary_groups: List of group names that will be used as
-        suplementary groups for subporcess.
-        The option runas must be specified together with this option.
+    :param runas: Name or User object of a user that the command should be
+        run as. The spawned process will have both real and effective UID and
+        GID set.
+    :param suplementary_groups: List of group names or Group object that will
+        be used as suplementary groups for subporcess. The option runas must
+        be specified together with this option.
     :param capture_error: Capture stderr
     :param nolog_output: do not log stdout even if it is being captured
     :param nolog_error: do not log stderr even if it is being captured
@@ -450,7 +451,7 @@ def run(args, stdin=None, raiseonerr=True, nolog=(), env=None,
     For backwards compatibility, the return value can also be used as a
     (output, error_output, returncode) triple.
     """
-    assert isinstance(suplementary_groups, list)
+    assert isinstance(suplementary_groups, (tuple, list))
     p_in = None
     p_out = None
     p_err = None
@@ -500,25 +501,26 @@ def run(args, stdin=None, raiseonerr=True, nolog=(), env=None,
     logger.debug('args=%s', arg_string)
 
     if runas is not None:
-        pent = pwd.getpwnam(runas)
+        runas = User(runas)
+        suplementary_groups = [Group(group) for group in suplementary_groups]
+        suplementary_gids = [group.gid for group in suplementary_groups]
 
-        suplementary_gids = [
-            grp.getgrnam(sgroup).gr_gid for sgroup in suplementary_groups
-        ]
-
-        logger.debug('runas=%s (UID %d, GID %s)', runas,
-                     pent.pw_uid, pent.pw_gid)
+        logger.debug(
+            'runas=%s (UID %d, GID %s)', runas, runas.uid, runas.pgid
+        )
         if suplementary_groups:
-            for group, gid in zip(suplementary_groups, suplementary_gids):
-                logger.debug('suplementary_group=%s (GID %d)', group, gid)
+            for group in suplementary_groups:
+                logger.debug(
+                    'supplementary_group=%s (GID %d)', group, group.gid
+                )
 
     if runas is not None or umask is not None:
         # preexec function is not supported in WSGI environment
         def preexec_fn():
             if runas is not None:
                 os.setgroups(suplementary_gids)
-                os.setregid(pent.pw_gid, pent.pw_gid)
-                os.setreuid(pent.pw_uid, pent.pw_uid)
+                os.setregid(runas.pgid, runas.pgid)
+                os.setreuid(runas.uid, runas.uid)
 
             if umask is not None:
                 os.umask(umask)

ipaserver/install/service.py

@@ -36,6 +36,7 @@ from ipapython.dn import DN
 from ipapython import kerberos
 from ipalib import api, errors, x509
 from ipaplatform import services
+from ipaplatform.constants import User
 from ipaplatform.paths import paths
 from ipaserver.masters import (
     CONFIGURED_SERVICE, ENABLED_SERVICE, HIDDEN_SERVICE, SERVICE_LIST
@@ -308,6 +309,8 @@ class Service:
         self.keytab = keytab
         self.cert = None
         self.api = api
+        if service_user is not None:
+            service_user = User(service_user)
         self.service_user = service_user
         self.keytab_user = service_user
         self.dm_password = None  # silence pylint

ipatests/test_ipaplatform/test_constants.py

@@ -10,11 +10,15 @@ def test_user_root():
     user = User("root")
     assert user == "root"
     assert str(user) == "root"
+    assert type(str(user)) is str
     assert repr(user) == '<User "root">'
     assert user.uid == 0
     assert user.pgid == 0
     assert user.entity.pw_uid == 0
 
+    assert User(user) is user
+    assert User(str(user)) is not user
+
 
 def test_user_invalid():
     invalid = User("invalid")
@@ -27,10 +31,14 @@ def test_group():
     group = Group("root")
     assert group == "root"
     assert str(group) == "root"
+    assert type(str(group)) is str
     assert repr(group) == '<Group "root">'
     assert group.gid == 0
     assert group.entity.gr_gid == 0
 
+    assert Group(group) is group
+    assert Group(str(group)) is not group
+
 
 def test_group_invalid():
     invalid = Group("invalid")
@@ -51,3 +59,5 @@ def test_user_group_daemon():
     group = Group("daemon")
     assert group == "daemon"
     assert group.gid == 2
+
+    assert Group(user) is not user