From d86b57c05764f771b1568409efd31c9e7b302919 Mon Sep 17 00:00:00 2001 From: Stanislav Levin Date: Sat, 15 Jun 2019 17:25:51 +0300 Subject: [PATCH] Make use of the single configuration point for the default shells For now all the default shells of users and admin are hardcoded in different parts of the project. This makes it impossible to run the test suite against the setup, which has the default shell differed from '/bin/sh'. The single configuration point for the shell of users and admin is added to overcome this limitation. Fixes: https://pagure.io/freeipa/issue/7978 Signed-off-by: Stanislav Levin Reviewed-By: Christian Heimes --- install/share/bootstrap-template.ldif | 4 +- ipaplatform/base/constants.py | 2 + ipaplatform/base/paths.py | 2 - ipaserver/install/dsinstance.py | 2 + ipaserver/plugins/migration.py | 4 +- ipaserver/plugins/stageuser.py | 7 ++- ipaserver/plugins/user.py | 4 +- ipatests/test_install/test_updates.py | 5 ++- .../test_integration/test_legacy_clients.py | 22 ++++++--- ipatests/test_integration/test_trust.py | 45 +++++++++++++------ .../test_integration/test_winsyncmigrate.py | 18 +++++--- ipatests/test_xmlrpc/test_stageuser_plugin.py | 3 +- ipatests/test_xmlrpc/test_user_plugin.py | 19 ++++---- .../test_xmlrpc/tracker/stageuser_plugin.py | 3 +- ipatests/test_xmlrpc/tracker/user_plugin.py | 3 +- 15 files changed, 95 insertions(+), 48 deletions(-) diff --git a/install/share/bootstrap-template.ldif b/install/share/bootstrap-template.ldif index b4b2331f4..fc0892835 100644 --- a/install/share/bootstrap-template.ldif +++ b/install/share/bootstrap-template.ldif @@ -237,7 +237,7 @@ sn: Administrator uidNumber: $IDSTART gidNumber: $IDSTART homeDirectory: /home/admin -loginShell: /bin/bash +loginShell: $DEFAULT_ADMIN_SHELL gecos: Administrator nsAccountLock: FALSE ipaUniqueID: autogenerate @@ -402,7 +402,7 @@ ipaGroupSearchFields: cn,description ipaSearchTimeLimit: 2 ipaSearchRecordsLimit: 100 ipaHomesRootDir: /home -ipaDefaultLoginShell: /bin/sh +ipaDefaultLoginShell: $DEFAULT_SHELL ipaDefaultPrimaryGroup: ipausers ipaMaxUsernameLength: 32 ipaMaxHostnameLength: 64 diff --git a/ipaplatform/base/constants.py b/ipaplatform/base/constants.py index 5bbed240e..b24d428ce 100644 --- a/ipaplatform/base/constants.py +++ b/ipaplatform/base/constants.py @@ -10,6 +10,8 @@ import sys class BaseConstantsNamespace: IS_64BITS = sys.maxsize > 2 ** 32 + DEFAULT_ADMIN_SHELL = '/bin/bash' + DEFAULT_SHELL = '/bin/sh' DS_USER = 'dirsrv' DS_GROUP = 'dirsrv' HTTPD_USER = "apache" diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py index 373209b54..9328524f2 100644 --- a/ipaplatform/base/paths.py +++ b/ipaplatform/base/paths.py @@ -25,12 +25,10 @@ import os class BasePathNamespace: - BASH = "/bin/bash" BIN_HOSTNAMECTL = "/bin/hostnamectl" ECHO = "/bin/echo" GZIP = "/bin/gzip" LS = "/bin/ls" - SH = "/bin/sh" SYSTEMCTL = "/bin/systemctl" SYSTEMD_DETECT_VIRT = "/usr/bin/systemd-detect-virt" TAR = "/bin/tar" diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py index 77e176899..ab4af6a7a 100644 --- a/ipaserver/install/dsinstance.py +++ b/ipaserver/install/dsinstance.py @@ -519,6 +519,8 @@ class DsInstance(service.Service): ' '.join(replication.EXCLUDES), TOTAL_EXCLUDES='(objectclass=*) $ EXCLUDE ' + ' '.join(replication.TOTAL_EXCLUDES), + DEFAULT_SHELL=platformconstants.DEFAULT_SHELL, + DEFAULT_ADMIN_SHELL=platformconstants.DEFAULT_ADMIN_SHELL, ) def __create_instance(self): diff --git a/ipaserver/plugins/migration.py b/ipaserver/plugins/migration.py index fffcb30ff..d0ca8369a 100644 --- a/ipaserver/plugins/migration.py +++ b/ipaserver/plugins/migration.py @@ -38,6 +38,7 @@ from ipapython.ipautil import write_tmp_file from ipapython.kerberos import Principal import datetime from ipaplatform.paths import paths +from ipaplatform.constants import constants as platformconstants if six.PY3: unicode = str @@ -233,7 +234,8 @@ def _pre_migrate_user(ldap, pkey, dn, entry_attrs, failed, config, ctx, **kwargs entry_attrs['homedirectory'] = home_dir if 'loginshell' not in entry_attrs: - default_shell = config.get('ipadefaultloginshell', [paths.SH])[0] + default_shell = config.get('ipadefaultloginshell', + [platformconstants.DEFAULT_SHELL])[0] entry_attrs.setdefault('loginshell', default_shell) # do not migrate all attributes diff --git a/ipaserver/plugins/stageuser.py b/ipaserver/plugins/stageuser.py index b8c36efc8..f417df57b 100644 --- a/ipaserver/plugins/stageuser.py +++ b/ipaserver/plugins/stageuser.py @@ -55,6 +55,7 @@ from ipalib.util import set_krbcanonicalname from ipalib import _, ngettext from ipalib import output from ipaplatform.paths import paths +from ipaplatform.constants import constants as platformconstants from ipapython.ipautil import ipa_generate_password, TMP_PWD_ENTROPY_BITS from ipalib.capabilities import client_has_capability @@ -329,7 +330,8 @@ class stageuser_add(baseuser_add): len = int(config.get('ipamaxusernamelength')[0]) ) ) - default_shell = config.get('ipadefaultloginshell', [paths.SH])[0] + default_shell = config.get('ipadefaultloginshell', + [platformconstants.DEFAULT_SHELL])[0] entry_attrs.setdefault('loginshell', default_shell) # hack so we can request separate first and last name in CLI full_name = '%s %s' % (entry_attrs['givenname'], entry_attrs['sn']) @@ -532,7 +534,8 @@ class stageuser_activate(LDAPQuery): ) ) if 'loginshell' not in entry_from: - default_shell = config.get('ipadefaultloginshell', [paths.SH])[0] + default_shell = config.get('ipadefaultloginshell', + [platformconstants.DEFAULT_SHELL])[0] if default_shell: entry_to.setdefault('loginshell', default_shell) diff --git a/ipaserver/plugins/user.py b/ipaserver/plugins/user.py index 980385dc8..50c00a5bc 100644 --- a/ipaserver/plugins/user.py +++ b/ipaserver/plugins/user.py @@ -65,6 +65,7 @@ from ipalib.request import context from ipalib import _, ngettext from ipalib import output from ipaplatform.paths import paths +from ipaplatform.constants import constants as platformconstants from ipapython.dn import DN from ipapython.ipaldap import LDAPClient from ipapython.ipautil import ipa_generate_password, TMP_PWD_ENTROPY_BITS @@ -516,7 +517,8 @@ class user_add(baseuser_add): len = int(config.get('ipamaxusernamelength')[0]) ) ) - default_shell = config.get('ipadefaultloginshell', [paths.SH])[0] + default_shell = config.get('ipadefaultloginshell', + [platformconstants.DEFAULT_SHELL])[0] entry_attrs.setdefault('loginshell', default_shell) # hack so we can request separate first and last name in CLI full_name = '%s %s' % (entry_attrs['givenname'], entry_attrs['sn']) diff --git a/ipatests/test_install/test_updates.py b/ipatests/test_install/test_updates.py index 5e9b2cac8..0fd54fe52 100644 --- a/ipatests/test_install/test_updates.py +++ b/ipatests/test_install/test_updates.py @@ -32,7 +32,7 @@ from ipalib import errors from ipaserver.install.ldapupdate import LDAPUpdate, BadSyntax from ipaserver.install import installutils from ipapython import ipaldap -from ipaplatform.paths import paths +from ipaplatform.constants import constants as platformconstants from ipapython.dn import DN """ @@ -130,7 +130,8 @@ class test_update(unittest.TestCase): for item in ('top', 'person', 'posixaccount', 'krbprincipalaux', 'inetuser'): self.assertTrue(item in objectclasses) - self.assertEqual(entry.single_value['loginshell'], paths.BASH) + self.assertEqual(entry.single_value['loginshell'], + platformconstants.DEFAULT_ADMIN_SHELL) self.assertEqual(entry.single_value['sn'], 'User') self.assertEqual(entry.single_value['uid'], 'tuser') self.assertEqual(entry.single_value['cn'], 'Test User') diff --git a/ipatests/test_integration/test_legacy_clients.py b/ipatests/test_integration/test_legacy_clients.py index 38932f1f6..916d51abb 100644 --- a/ipatests/test_integration/test_legacy_clients.py +++ b/ipatests/test_integration/test_legacy_clients.py @@ -26,6 +26,7 @@ import os import re import unittest +from ipaplatform.constants import constants as platformconstants from ipaplatform.paths import paths from ipatests.pytest_ipa.integration import tasks @@ -49,6 +50,7 @@ class BaseTestLegacyClient: paths.SSSD_CONF] homedir_template = "/home/{domain}/{username}" + default_shell = platformconstants.DEFAULT_SHELL required_extra_roles = () optional_extra_roles = () @@ -91,7 +93,9 @@ class BaseTestLegacyClient: result = self.legacy_client.run_command(['getent', 'passwd', 'admin']) admin_regex = r"admin:\*:(\d+):(\d+):"\ - r"Administrator:/home/admin:/bin/bash" + r"Administrator:/home/admin:{}".format( + platformconstants.DEFAULT_ADMIN_SHELL, + ) assert re.search(admin_regex, result.stdout_text) @@ -121,13 +125,14 @@ class BaseTestLegacyClient: result = self.legacy_client.run_command(['getent', 'passwd', testuser]) testuser_regex = r"testuser@%s:\*:%s:%s:"\ - r"Test User:%s:/bin/sh"\ + r"Test User:%s:%s"\ % (re.escape(self.ad.domain.name), self.testuser_uid_regex, self.testuser_gid_regex, self.homedir_template.format( username='testuser', - domain=re.escape(self.ad.domain.name)) + domain=re.escape(self.ad.domain.name)), + self.default_shell, ) assert re.search(testuser_regex, result.stdout_text) @@ -241,13 +246,14 @@ class BaseTestLegacyClient: testuser_regex = r"subdomaintestuser@%s:\*:%s:%s:"\ r"Subdomaintest User:%s:"\ - r"/bin/sh"\ + r"%s"\ % (re.escape(self.ad_subdomain), self.subdomain_testuser_uid_regex, self.subdomain_testuser_gid_regex, self.homedir_template.format( username='subdomaintestuser', - domain=re.escape(self.ad_subdomain)) + domain=re.escape(self.ad_subdomain)), + self.default_shell, ) assert re.search(testuser_regex, result.stdout_text) @@ -339,10 +345,12 @@ class BaseTestLegacyClient: result = self.legacy_client.run_command(['getent', 'passwd', testuser]) testuser_regex = (r"treetestuser@{0}:\*:{1}:{2}:TreeTest User:" - r"/home/{0}/treetestuser:/bin/sh".format( + r"/home/{0}/treetestuser:{3}".format( re.escape(self.ad_treedomain), self.treedomain_testuser_uid_regex, - self.treedomain_testuser_gid_regex)) + self.treedomain_testuser_gid_regex, + self.default_shell, + )) assert re.search(testuser_regex, result.stdout_text) diff --git a/ipatests/test_integration/test_trust.py b/ipatests/test_integration/test_trust.py index 0e620fefb..cd13cfa4c 100644 --- a/ipatests/test_integration/test_trust.py +++ b/ipatests/test_integration/test_trust.py @@ -5,6 +5,8 @@ from __future__ import absolute_import import re import unittest +from ipaplatform.constants import constants as platformconstants + from ipatests.test_integration.base import IntegrationTest from ipatests.pytest_ipa.integration import tasks @@ -22,6 +24,7 @@ class BaseTestTrust(IntegrationTest): upn_password = 'Secret123456' shared_secret = 'qwertyuiopQq!1' + default_shell = platformconstants.DEFAULT_SHELL @classmethod def install(cls, mh): @@ -115,9 +118,11 @@ class TestTrust(BaseTestTrust): # This regex checks that Test User does not have UID 10042 nor belongs # to the group with GID 10047 testuser_regex = r"^testuser@%s:\*:(?!10042)(\d+):(?!10047)(\d+):"\ - r"Test User:/home/%s/testuser:/bin/sh$"\ + r"Test User:/home/%s/testuser:%s$"\ % (re.escape(self.ad_domain), - re.escape(self.ad_domain)) + re.escape(self.ad_domain), + self.default_shell, + ) assert re.search( testuser_regex, result.stdout_text), result.stdout_text @@ -158,9 +163,11 @@ class TestTrust(BaseTestTrust): # result will contain AD domain, not UPN upnuser_regex = ( - r"^{}@{}:\*:(\d+):(\d+):{}:/home/{}/{}:/bin/sh$".format( + r"^{}@{}:\*:(\d+):(\d+):{}:/home/{}/{}:{}$".format( self.upn_username, self.ad_domain, self.upn_name, - self.ad_domain, self.upn_username) + self.ad_domain, self.upn_username, + self.default_shell, + ) ) assert re.search(upnuser_regex, result.stdout_text), result.stdout_text @@ -198,8 +205,10 @@ class TestTrust(BaseTestTrust): result = self.master.run_command(['getent', 'passwd', testuser]) testuser_stdout = "testuser@%s:*:10042:10047:"\ - "Test User:/home/%s/testuser:/bin/sh"\ - % (self.ad_domain, self.ad_domain) + "Test User:/home/%s/testuser:%s"\ + % (self.ad_domain, self.ad_domain, + self.default_shell, + ) assert testuser_stdout in result.stdout_text @@ -264,9 +273,11 @@ class TestTrust(BaseTestTrust): testuser_regex = (r"^subdomaintestuser@{0}:\*:(?!10142)(\d+):" r"(?!10147)(\d+):Subdomaintest User:" - r"/home/{1}/subdomaintestuser:/bin/sh$".format( + r"/home/{1}/subdomaintestuser:{2}$".format( re.escape(self.ad_subdomain), - re.escape(self.ad_subdomain))) + re.escape(self.ad_subdomain), + self.default_shell, + )) assert re.search(testuser_regex, result.stdout_text) @@ -312,9 +323,11 @@ class TestTrust(BaseTestTrust): testuser_regex = (r"^treetestuser@{0}:\*:(?!10242)(\d+):" r"(?!10247)(\d+):TreeTest User:" - r"/home/{1}/treetestuser:/bin/sh$".format( + r"/home/{1}/treetestuser:{2}$".format( re.escape(self.ad_treedomain), - re.escape(self.ad_treedomain))) + re.escape(self.ad_treedomain), + self.default_shell, + )) assert re.search( testuser_regex, result.stdout_text), result.stdout_text @@ -399,9 +412,11 @@ class TestTrust(BaseTestTrust): # This regex checks that Test User does not have UID 10042 nor belongs # to the group with GID 10047 testuser_regex = r"^testuser@%s:\*:(?!10042)(\d+):(?!10047)(\d+):"\ - r"Test User:/home/%s/testuser:/bin/sh$"\ + r"Test User:/home/%s/testuser:%s$"\ % (re.escape(self.ad_domain), - re.escape(self.ad_domain)) + re.escape(self.ad_domain), + self.default_shell, + ) assert re.search( testuser_regex, result.stdout_text), result.stdout_text @@ -459,9 +474,11 @@ class TestTrust(BaseTestTrust): # This regex checks that Test User does not have UID 10042 nor belongs # to the group with GID 10047 testuser_regex = r"^testuser@%s:\*:(?!10042)(\d+):(?!10047)(\d+):"\ - r"Test User:/home/%s/testuser:/bin/sh$"\ + r"Test User:/home/%s/testuser:%s$"\ % (re.escape(self.ad_domain), - re.escape(self.ad_domain)) + re.escape(self.ad_domain), + self.default_shell, + ) assert re.search( testuser_regex, result.stdout_text), result.stdout_text diff --git a/ipatests/test_integration/test_winsyncmigrate.py b/ipatests/test_integration/test_winsyncmigrate.py index e4ff03e6d..d0069b53f 100644 --- a/ipatests/test_integration/test_winsyncmigrate.py +++ b/ipatests/test_integration/test_winsyncmigrate.py @@ -10,6 +10,8 @@ import re import pytest +from ipaplatform.constants import constants as platformconstants + from ipatests.pytest_ipa.integration import tasks from ipatests.test_integration.base import IntegrationTest @@ -56,6 +58,7 @@ class TestWinsyncMigrate(IntegrationTest): ipa_group = 'ipa_group' ad_user = 'testuser' + default_shell = platformconstants.DEFAULT_SHELL test_role = 'test_role' test_hbac_rule = 'test_hbac_rule' test_selinux_map = 'test_selinux_map' @@ -81,8 +84,10 @@ class TestWinsyncMigrate(IntegrationTest): # store user uid and gid result = cls.master.run_command(['getent', 'passwd', cls.ad_user]) testuser_regex = ( - r"^{0}:\*:(\d+):(\d+):{0}:/home/{0}:/bin/sh$".format( - cls.ad_user)) + r"^{0}:\*:(\d+):(\d+):{0}:/home/{0}:{1}$".format( + cls.ad_user, cls.default_shell, + ) + ) m = re.match(testuser_regex, result.stdout_text) cls.test_user_uid, cls.test_user_gid = m.groups() @@ -150,10 +155,12 @@ class TestWinsyncMigrate(IntegrationTest): result = self.master.run_command(['getent', 'passwd', self.trust_test_user]) passwd_template = ( - '{trust_user}:*:{uid}:{gid}:{user}:/home/{domain}/{user}:/bin/sh') + '{trust_user}:*:{uid}:{gid}:{user}:/home/{domain}/{user}:{shell}') expected_result = passwd_template.format( user=self.ad_user, uid=self.test_user_uid, gid=self.test_user_gid, - domain=self.ad.domain.name, trust_user=self.trust_test_user) + domain=self.ad.domain.name, trust_user=self.trust_test_user, + shell=self.default_shell, + ) assert result.stdout_text.strip() == expected_result def test_idoverride(self): @@ -168,8 +175,7 @@ class TestWinsyncMigrate(IntegrationTest): 'uidnumber: %s' % self.test_user_uid, 'gidnumber: %s' % self.test_user_gid, 'gecos: %s' % self.ad_user, - 'loginshell: /bin/sh' - + 'loginshell: %s' % self.default_shell, ] assert sorted(idoverride_fields) == sorted(expected_fields) diff --git a/ipatests/test_xmlrpc/test_stageuser_plugin.py b/ipatests/test_xmlrpc/test_stageuser_plugin.py index a2f0650b4..81c6a4c6a 100644 --- a/ipatests/test_xmlrpc/test_stageuser_plugin.py +++ b/ipatests/test_xmlrpc/test_stageuser_plugin.py @@ -12,6 +12,7 @@ import six from collections import OrderedDict from ipalib import api, errors +from ipaplatform.constants import constants as platformconstants from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test, raises_exact @@ -49,7 +50,7 @@ options_def = OrderedDict([ ('display name', {u'displayname': u'display'}), ('home directory', {u'homedirectory': u'/home/homedir'}), ('GECOS', {u'gecos': u'gecos'}), - ('shell', {u'loginshell': u'/bin/shell'}), + ('shell', {u'loginshell': platformconstants.DEFAULT_SHELL}), ('email address', {u'mail': u'email@email.email'}), ('job title', {u'title': u'newbie'}), ('kerberos principal', { diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py index e6f2bc2ef..329947f68 100644 --- a/ipatests/test_xmlrpc/test_user_plugin.py +++ b/ipatests/test_xmlrpc/test_user_plugin.py @@ -31,6 +31,7 @@ import ldap import re from ipalib import api, errors +from ipaplatform.constants import constants as platformconstants from ipatests.test_xmlrpc import objectclasses from ipatests.util import ( assert_deepequal, assert_equal, assert_not_equal, raises) @@ -612,7 +613,8 @@ class TestCreate(XMLRPC_test): result = command() user.check_create(result) user.run_command( - 'config_mod', **{u'ipadefaultloginshell': u'/bin/sh'} + 'config_mod', + **{u'ipadefaultloginshell': platformconstants.DEFAULT_SHELL} ) user.delete() @@ -1067,7 +1069,7 @@ def get_user_result(uid, givenname, sn, operation='show', omit=[], cn[0] = cn[0].strip() result = add_sid(dict( homedirectory=[u'/home/%s' % uid], - loginshell=[u'/bin/sh'], + loginshell=[platformconstants.DEFAULT_SHELL], uid=[uid], uidnumber=[fuzzy_digits], gidnumber=[fuzzy_digits], @@ -1116,12 +1118,13 @@ def get_admin_result(operation='show', **overrides): Any additional or non-default values can be given in ``overrides``. """ - result = get_user_result(u'admin', None, u'Administrator', operation, - omit=['mail'], - has_keytab=True, - has_password=True, - loginshell=[u'/bin/bash'], - **overrides) + result = get_user_result( + u'admin', None, u'Administrator', operation, + omit=['mail'], + has_keytab=True, + has_password=True, + loginshell=[platformconstants.DEFAULT_ADMIN_SHELL], + **overrides) return result diff --git a/ipatests/test_xmlrpc/tracker/stageuser_plugin.py b/ipatests/test_xmlrpc/tracker/stageuser_plugin.py index 633ecce28..4ed3a1cb7 100644 --- a/ipatests/test_xmlrpc/tracker/stageuser_plugin.py +++ b/ipatests/test_xmlrpc/tracker/stageuser_plugin.py @@ -5,6 +5,7 @@ import six from ipalib import api, errors +from ipaplatform.constants import constants as platformconstants from ipatests.test_xmlrpc.tracker.base import Tracker from ipatests.test_xmlrpc.tracker.kerberos_aliases import KerberosAliasMixin @@ -143,7 +144,7 @@ class StageUserTracker(KerberosAliasMixin, Tracker): krbcanonicalname=[u'%s@%s' % (self.uid, self.api.env.realm)], mail=[u'%s@%s' % (self.uid, self.api.env.domain)], gecos=[u'%s %s' % (self.givenname, self.sn)], - loginshell=[u'/bin/sh'], + loginshell=[platformconstants.DEFAULT_SHELL], has_keytab=False, has_password=False, nsaccountlock=[u'true'], diff --git a/ipatests/test_xmlrpc/tracker/user_plugin.py b/ipatests/test_xmlrpc/tracker/user_plugin.py index 006c94aeb..fa3957e05 100644 --- a/ipatests/test_xmlrpc/tracker/user_plugin.py +++ b/ipatests/test_xmlrpc/tracker/user_plugin.py @@ -3,6 +3,7 @@ # from ipalib import api, errors +from ipaplatform.constants import constants as platformconstants from ipapython.dn import DN import six @@ -181,7 +182,7 @@ class UserTracker(CertmapdataMixin, KerberosAliasMixin, Tracker): krbcanonicalname=[u'%s@%s' % (self.uid, self.api.env.realm)], mail=[u'%s@%s' % (self.uid, self.api.env.domain)], gecos=[u'%s %s' % (self.givenname, self.sn)], - loginshell=[u'/bin/sh'], + loginshell=[platformconstants.DEFAULT_SHELL], has_keytab=False, has_password=False, mepmanagedentry=[get_group_dn(self.uid)],