mirror of
https://github.com/valitydev/salt.git
synced 2024-11-07 17:09:03 +00:00
Merge pull request #46502 from terminalmage/issue43208
user.present: don't change uid/gid unless explicitly told to
This commit is contained in:
Nicole Thomas
GitHub
commit
3e073c7e8a
@ -33,6 +33,7 @@ import salt.utils.dateutils
|
||||
import salt.utils.platform
|
||||
import salt.utils.user
|
||||
from salt.utils.locales import sdecode, sdecode_if_string
|
||||
from salt.exceptions import CommandExecutionError
|
||||
|
||||
# Import 3rd-party libs
|
||||
from salt.ext.six import string_types, iteritems
|
||||
@ -77,7 +78,9 @@ def _changes(name,
|
||||
win_homedrive=None,
|
||||
win_profile=None,
|
||||
win_logonscript=None,
|
||||
win_description=None):
|
||||
win_description=None,
|
||||
allow_uid_change=False,
|
||||
allow_gid_change=False):
|
||||
'''
|
||||
Return a dict of the changes required for a user if the user is present,
|
||||
otherwise return False.
|
||||
@ -191,6 +194,25 @@ def _changes(name,
|
||||
if __salt__['user.get_loginclass'](name) != loginclass:
|
||||
change['loginclass'] = loginclass
|
||||
|
||||
errors = []
|
||||
if not allow_uid_change and 'uid' in change:
|
||||
errors.append(
|
||||
'Changing uid ({0} -> {1}) 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 '
|
||||
'True to force this change. Note that this will not change file '
|
||||
'ownership.'.format(lusr['gid'], gid)
|
||||
)
|
||||
if errors:
|
||||
raise CommandExecutionError(
|
||||
'Encountered error checking for needed changes',
|
||||
info=errors
|
||||
)
|
||||
|
||||
return change
|
||||
|
||||
|
||||
@ -225,7 +247,9 @@ def present(name,
|
||||
win_profile=None,
|
||||
win_logonscript=None,
|
||||
win_description=None,
|
||||
nologinit=False):
|
||||
nologinit=False,
|
||||
allow_uid_change=False,
|
||||
allow_gid_change=False):
|
||||
'''
|
||||
Ensure that the named user is present with the specified properties
|
||||
|
||||
@ -233,16 +257,28 @@ def present(name,
|
||||
The name of the user to manage
|
||||
|
||||
uid
|
||||
The user id to assign, if left empty then the next available user id
|
||||
will be assigned
|
||||
The user id to assign. If not specified, and the user does not exist,
|
||||
then the next available uid will be assigned.
|
||||
|
||||
gid
|
||||
The default group id. Also accepts group name.
|
||||
The id of the default group to assign to the user. Either a group name
|
||||
or gid can be used. If not specified, and the user does not exist, then
|
||||
he next available gid will be assigned.
|
||||
|
||||
gid_from_name
|
||||
If True, the default group id will be set to the id of the group with
|
||||
the same name as the user. If the group does not exist the state will
|
||||
fail. Default is ``False``.
|
||||
gid_from_name : False
|
||||
If ``True``, the default group id will be set to the id of the group
|
||||
with the same name as the user. If the group does not exist the state
|
||||
will fail.
|
||||
|
||||
allow_uid_change : False
|
||||
Set to ``True`` to allow the state to update the uid.
|
||||
|
||||
.. versionadded:: 2018.3.1
|
||||
|
||||
allow_gid_change : False
|
||||
Set to ``True`` to allow the state to update the gid.
|
||||
|
||||
.. versionadded:: 2018.3.1
|
||||
|
||||
groups
|
||||
A list of groups to assign the user to, pass a list object. If a group
|
||||
@ -466,33 +502,40 @@ def present(name,
|
||||
ret['result'] = False
|
||||
return ret
|
||||
|
||||
changes = _changes(name,
|
||||
uid,
|
||||
gid,
|
||||
groups,
|
||||
present_optgroups,
|
||||
remove_groups,
|
||||
home,
|
||||
createhome,
|
||||
password,
|
||||
enforce_password,
|
||||
empty_password,
|
||||
shell,
|
||||
fullname,
|
||||
roomnumber,
|
||||
workphone,
|
||||
homephone,
|
||||
loginclass,
|
||||
date,
|
||||
mindays,
|
||||
maxdays,
|
||||
inactdays,
|
||||
warndays,
|
||||
expire,
|
||||
win_homedrive,
|
||||
win_profile,
|
||||
win_logonscript,
|
||||
win_description)
|
||||
try:
|
||||
changes = _changes(name,
|
||||
uid,
|
||||
gid,
|
||||
groups,
|
||||
present_optgroups,
|
||||
remove_groups,
|
||||
home,
|
||||
createhome,
|
||||
password,
|
||||
enforce_password,
|
||||
empty_password,
|
||||
shell,
|
||||
fullname,
|
||||
roomnumber,
|
||||
workphone,
|
||||
homephone,
|
||||
loginclass,
|
||||
date,
|
||||
mindays,
|
||||
maxdays,
|
||||
inactdays,
|
||||
warndays,
|
||||
expire,
|
||||
win_homedrive,
|
||||
win_profile,
|
||||
win_logonscript,
|
||||
win_description,
|
||||
allow_uid_change,
|
||||
allow_gid_change)
|
||||
except CommandExecutionError as exc:
|
||||
ret['result'] = False
|
||||
ret['comment'] = exc.strerror
|
||||
return ret
|
||||
|
||||
if changes:
|
||||
if __opts__['test']:
|
||||
@ -621,7 +664,13 @@ def present(name,
|
||||
win_homedrive,
|
||||
win_profile,
|
||||
win_logonscript,
|
||||
win_description)
|
||||
win_description,
|
||||
allow_uid_change=True,
|
||||
allow_gid_change=True)
|
||||
# allow_uid_change and allow_gid_change passed as True to avoid race
|
||||
# conditions where a uid/gid is modified outside of Salt. If an
|
||||
# unauthorized change was requested, it would have been caught the
|
||||
# first time we ran _changes().
|
||||
|
||||
if changes:
|
||||
ret['comment'] = 'These values could not be changed: {0}'.format(
|
||||
|
||||
@ -10,6 +10,7 @@ from __future__ import absolute_import, print_function, unicode_literals
|
||||
from tests.support.mixins import LoaderModuleMockMixin
|
||||
from tests.support.unit import TestCase, skipIf
|
||||
from tests.support.mock import (
|
||||
Mock,
|
||||
MagicMock,
|
||||
patch,
|
||||
NO_MOCK,
|
||||
@ -79,6 +80,110 @@ class UserTestCase(TestCase, LoaderModuleMockMixin):
|
||||
' user salt', 'result': False})
|
||||
self.assertDictEqual(user.present('salt'), ret)
|
||||
|
||||
def test_present_invalid_uid_change(self):
|
||||
mock_info = MagicMock(side_effect=[
|
||||
{'uid': 5000,
|
||||
'gid': 5000,
|
||||
'groups': ['foo'],
|
||||
'home': '/home/foo',
|
||||
'fullname': 'Foo Bar'}
|
||||
])
|
||||
dunder_salt = {'user.info': mock_info,
|
||||
'file.group_to_gid': MagicMock(side_effect=['foo']),
|
||||
'file.gid_to_group': MagicMock(side_effect=[5000])}
|
||||
# side_effect used because these mocks should only be called once
|
||||
with patch.dict(user.__grains__, {'kernel': 'Linux'}), \
|
||||
patch.dict(user.__salt__, dunder_salt):
|
||||
ret = user.present('foo', uid=5001)
|
||||
# State should have failed
|
||||
self.assertFalse(ret['result'])
|
||||
# Only one of uid/gid should have been flagged in the comment
|
||||
self.assertEqual(ret['comment'].count('not permitted'), 1)
|
||||
|
||||
def test_present_invalid_gid_change(self):
|
||||
mock_info = MagicMock(side_effect=[
|
||||
{'uid': 5000,
|
||||
'gid': 5000,
|
||||
'groups': ['foo'],
|
||||
'home': '/home/foo',
|
||||
'fullname': 'Foo Bar'}
|
||||
])
|
||||
dunder_salt = {'user.info': mock_info,
|
||||
'file.group_to_gid': MagicMock(side_effect=['foo']),
|
||||
'file.gid_to_group': MagicMock(side_effect=[5000])}
|
||||
# side_effect used because these mocks should only be called once
|
||||
with patch.dict(user.__grains__, {'kernel': 'Linux'}), \
|
||||
patch.dict(user.__salt__, dunder_salt):
|
||||
ret = user.present('foo', gid=5001)
|
||||
# State should have failed
|
||||
self.assertFalse(ret['result'])
|
||||
# Only one of uid/gid should have been flagged in the comment
|
||||
self.assertEqual(ret['comment'].count('not permitted'), 1)
|
||||
|
||||
def test_present_invalid_uid_gid_change(self):
|
||||
mock_info = MagicMock(side_effect=[
|
||||
{'uid': 5000,
|
||||
'gid': 5000,
|
||||
'groups': ['foo'],
|
||||
'home': '/home/foo',
|
||||
'fullname': 'Foo Bar'}
|
||||
])
|
||||
dunder_salt = {'user.info': mock_info,
|
||||
'file.group_to_gid': MagicMock(side_effect=['foo']),
|
||||
'file.gid_to_group': MagicMock(side_effect=[5000])}
|
||||
# side_effect used because these mocks should only be called once
|
||||
with patch.dict(user.__grains__, {'kernel': 'Linux'}), \
|
||||
patch.dict(user.__salt__, dunder_salt):
|
||||
ret = user.present('foo', uid=5001, gid=5001)
|
||||
# State should have failed
|
||||
self.assertFalse(ret['result'])
|
||||
# Both the uid and gid should have been flagged in the comment
|
||||
self.assertEqual(ret['comment'].count('not permitted'), 2)
|
||||
|
||||
def test_present_uid_gid_change(self):
|
||||
before = {'uid': 5000,
|
||||
'gid': 5000,
|
||||
'groups': ['foo'],
|
||||
'home': '/home/foo',
|
||||
'fullname': 'Foo Bar'}
|
||||
after = {'uid': 5001,
|
||||
'gid': 5001,
|
||||
'groups': ['othergroup'],
|
||||
'home': '/home/foo',
|
||||
'fullname': 'Foo Bar'}
|
||||
# user.info should be called 4 times. Once the first time that
|
||||
# _changes() is called, once before and after changes are applied (to
|
||||
# get the before/after for the changes dict, and one last time to
|
||||
# confirm that no changes still need to be made.
|
||||
mock_info = MagicMock(side_effect=[before, before, after, after])
|
||||
mock_group_to_gid = MagicMock(side_effect=['foo', 'othergroup'])
|
||||
mock_gid_to_group = MagicMock(side_effect=[5000, 5001])
|
||||
dunder_salt = {'user.info': mock_info,
|
||||
'user.chuid': Mock(),
|
||||
'user.chgid': Mock(),
|
||||
'file.group_to_gid': mock_group_to_gid,
|
||||
'file.gid_to_group': mock_gid_to_group}
|
||||
# side_effect used because these mocks should only be called once
|
||||
with patch.dict(user.__grains__, {'kernel': 'Linux'}), \
|
||||
patch.dict(user.__salt__, dunder_salt), \
|
||||
patch.dict(user.__opts__, {'test': False}), \
|
||||
patch('os.path.isdir', MagicMock(return_value=True)):
|
||||
ret = user.present(
|
||||
'foo',
|
||||
uid=5001,
|
||||
gid=5001,
|
||||
allow_uid_change=True,
|
||||
allow_gid_change=True)
|
||||
self.assertEqual(
|
||||
ret,
|
||||
{'comment': 'Updated user foo',
|
||||
'changes': {'gid': 5001,
|
||||
'uid': 5001,
|
||||
'groups': ['othergroup']},
|
||||
'name': 'foo',
|
||||
'result': True}
|
||||
)
|
||||
|
||||
def test_absent(self):
|
||||
'''
|
||||
Test to ensure that the named user is absent
|
||||
|
||||
Loading…
Reference in New Issue
Block a user