fix editUser api for multiple kerberos support
This commit is contained in:
Yu Ming Zhu
Tomas Kopecek
parent
180cf894e2
commit
3e124f685a
3 changed files with 105 additions and 26 deletions
|
|
@ -3732,22 +3732,28 @@ def get_user(userInfo=None, strict=False, krb_princs=False):
|
|||
user['krb_principals'] = list_user_krb_principals(user['id'])
|
||||
return user
|
||||
|
||||
def edit_user(userInfo, name=None, krb_principal=None):
|
||||
def edit_user(userInfo, name=None, krb_principal_mappings=None):
|
||||
"""Edit information for an existing user.
|
||||
|
||||
userInfo specifies the user to edit
|
||||
fields changes are provided as keyword arguments:
|
||||
name: rename the user
|
||||
krb_principal: change user's kerberos principal
|
||||
krb_principal_mappings: change user's kerberos principal, it is a list
|
||||
contains krb_principal pair(s)
|
||||
- old: krb_principal to modify, None and ''
|
||||
indicates adding a new krb_principal
|
||||
- new: new value of krb_principal, None and ''
|
||||
indicates removing the old krb_principal
|
||||
"""
|
||||
|
||||
context.session.assertPerm('admin')
|
||||
_edit_user(userInfo, name=name, krb_principal=krb_principal)
|
||||
_edit_user(userInfo, name=name,
|
||||
krb_principal_mappings=krb_principal_mappings)
|
||||
|
||||
|
||||
def _edit_user(userInfo, name=None, krb_principal=None):
|
||||
def _edit_user(userInfo, name=None, krb_principal_mappings=None):
|
||||
"""Edit information for an existing user."""
|
||||
user = get_user(userInfo, strict=True)
|
||||
user = get_user(userInfo, strict=True, krb_princs=True)
|
||||
if name and user['name'] != name:
|
||||
# attempt to update user name
|
||||
values = {
|
||||
|
|
@ -3759,14 +3765,42 @@ def _edit_user(userInfo, name=None, krb_principal=None):
|
|||
if id is not None:
|
||||
# new name is taken
|
||||
raise koji.GenericError("Name %s already taken by user %s" % (name, id))
|
||||
update = UpdateProcessor('users', values={'userID': user['id']}, clauses=['id = %(userID)i'])
|
||||
update = UpdateProcessor('users',
|
||||
values={'userID': user['id']},
|
||||
clauses=['id = %(userID)i'])
|
||||
update.set(name=name)
|
||||
update.execute()
|
||||
if krb_principal and user['krb_principal'] != krb_principal:
|
||||
if krb_principal_mappings:
|
||||
added = set()
|
||||
removed = set()
|
||||
for pairs in krb_principal_mappings:
|
||||
old = pairs.get('old')
|
||||
new = pairs.get('new')
|
||||
if old:
|
||||
removed.add(old)
|
||||
if new:
|
||||
added.add(new)
|
||||
dups = added & removed
|
||||
if dups:
|
||||
raise koji.GenericError("There are some conflicts between added"
|
||||
" and removed Kerberos principals: %s"
|
||||
% ', '.join(dups))
|
||||
currents = set(user.get('krb_principals'))
|
||||
dups = added & currents
|
||||
if dups:
|
||||
raise koji.GenericError("Cannot add existing Kerberos"
|
||||
" principals: %s" % ', '.join(dups))
|
||||
unable_removed = removed - currents
|
||||
if unable_removed:
|
||||
raise koji.GenericError("Cannot remove non-existent Kerberos"
|
||||
" principals: %s"
|
||||
% ', '.join(unable_removed))
|
||||
|
||||
# attempt to update kerberos principal
|
||||
update = UpdateProcessor('users', values={'userID': user['id']}, clauses=['id = %(userID)i'])
|
||||
update.set(krb_principal=krb_principal)
|
||||
update.execute()
|
||||
for r in removed:
|
||||
context.session.removeKrbPrincipal(user['id'], krb_principal=r)
|
||||
for a in added:
|
||||
context.session.setKrbPrincipal(user['id'], krb_principal=a)
|
||||
|
||||
|
||||
def list_user_krb_principals(user_info=None):
|
||||
|
|
|
|||
18
koji/auth.py
18
koji/auth.py
|
|
@ -689,7 +689,12 @@ class Session(object):
|
|||
def setKrbPrincipal(self, name, krb_principal, krb_princ_check=True):
|
||||
if krb_princ_check:
|
||||
self.checkKrbPrincipal(krb_principal)
|
||||
select = """SELECT id FROM users WHERE name = %(name)s"""
|
||||
select = """SELECT id FROM users WHERE %s"""
|
||||
if isinstance(name, six.integer_types):
|
||||
user_condition = 'id = %(name)i'
|
||||
else:
|
||||
user_condition = 'name = %(name)s'
|
||||
select = select % user_condition
|
||||
cursor = context.cnx.cursor()
|
||||
cursor.execute(select, locals())
|
||||
r = cursor.fetchone()
|
||||
|
|
@ -708,15 +713,20 @@ class Session(object):
|
|||
select = """SELECT id FROM users
|
||||
JOIN user_krb_principals
|
||||
ON users.id = user_krb_principals.user_id
|
||||
WHERE name = %(name)s
|
||||
AND krb_principal = %(krb_principal)s"""
|
||||
WHERE %s
|
||||
AND krb_principal = %%(krb_principal)s"""
|
||||
if isinstance(name, six.integer_types):
|
||||
user_condition = 'id = %(name)i'
|
||||
else:
|
||||
user_condition = 'name = %(name)s'
|
||||
select = select % user_condition
|
||||
cursor = context.cnx.cursor()
|
||||
cursor.execute(select, locals())
|
||||
r = cursor.fetchone()
|
||||
if not r:
|
||||
context.cnx.rollback()
|
||||
raise koji.AuthError(
|
||||
'could not automatically remove Kerberos Principal:'
|
||||
'cannot remove Kerberos Principal:'
|
||||
' %(krb_principal)s with user %(name)s' % locals())
|
||||
else:
|
||||
user_id = r[0]
|
||||
|
|
|
|||
|
|
@ -30,11 +30,11 @@ class TestEditUser(unittest.TestCase):
|
|||
|
||||
def test_edit(self):
|
||||
self.get_user.return_value = {'id': 333,
|
||||
'name': 'user',
|
||||
'krb_principal': 'krb'}
|
||||
'name': 'user',
|
||||
'krb_principals': ['krb']}
|
||||
self._singleValue.return_value = None
|
||||
|
||||
kojihub._edit_user('user', name='newuser', krb_principal='krb')
|
||||
kojihub._edit_user('user', name='newuser')
|
||||
# check the update
|
||||
self.assertEqual(len(self.updates), 1)
|
||||
update = self.updates[0]
|
||||
|
|
@ -43,18 +43,53 @@ class TestEditUser(unittest.TestCase):
|
|||
self.assertEqual(update.values, {'userID': 333})
|
||||
self.assertEqual(update.clauses, ['id = %(userID)i'])
|
||||
|
||||
kojihub._edit_user('user', name='user', krb_principal='newkrb')
|
||||
# check the insert/update
|
||||
self.assertEqual(len(self.updates), 2)
|
||||
update = self.updates[1]
|
||||
self.assertEqual(update.table, 'users')
|
||||
self.assertEqual(update.data, {'krb_principal': 'newkrb'})
|
||||
self.assertEqual(update.values, {'userID': 333})
|
||||
self.assertEqual(update.clauses, ['id = %(userID)i'])
|
||||
kojihub._edit_user('user', krb_principal_mappings=[{'old': 'krb',
|
||||
'new': 'newkrb'}])
|
||||
self.context.session.removeKrbPrincipal. \
|
||||
assert_called_once_with(333, krb_principal='krb')
|
||||
self.context.session.setKrbPrincipal. \
|
||||
assert_called_once_with(333, krb_principal='newkrb')
|
||||
|
||||
self.context.reset_mock()
|
||||
with self.assertRaises(koji.GenericError) as cm:
|
||||
kojihub._edit_user('user',
|
||||
krb_principal_mappings=[{'old': 'krb',
|
||||
'new': 'newkrb'},
|
||||
{'old': 'newkrb',
|
||||
'new': 'newnewkrb'}
|
||||
])
|
||||
self.assertEqual(cm.exception.args[0],
|
||||
'There are some conflicts between added and removed'
|
||||
' Kerberos principals: newkrb')
|
||||
self.context.session.removeKrbPrincipal.assert_not_called()
|
||||
self.context.session.setKrbPrincipal.assert_not_called()
|
||||
|
||||
self.context.reset_mock()
|
||||
with self.assertRaises(koji.GenericError) as cm:
|
||||
kojihub._edit_user('user',
|
||||
krb_principal_mappings=[{'old': 'otherkrb',
|
||||
'new': 'newkrb'}])
|
||||
self.assertEqual(cm.exception.args[0],
|
||||
'Cannot remove non-existent Kerberos principals:'
|
||||
' otherkrb')
|
||||
self.context.session.removeKrbPrincipal.assert_not_called()
|
||||
self.context.session.setKrbPrincipal.assert_not_called()
|
||||
|
||||
self.context.reset_mock()
|
||||
with self.assertRaises(koji.GenericError) as cm:
|
||||
kojihub._edit_user('user',
|
||||
krb_principal_mappings=[{'old': None,
|
||||
'new': 'krb'}])
|
||||
self.assertEqual(cm.exception.args[0],
|
||||
'Cannot add existing Kerberos principals: krb')
|
||||
self.context.session.removeKrbPrincipal.assert_not_called()
|
||||
self.context.session.setKrbPrincipal.assert_not_called()
|
||||
|
||||
|
||||
self._singleValue.reset_mock()
|
||||
self._singleValue.return_value = 2
|
||||
with self.assertRaises(koji.GenericError) as cm:
|
||||
kojihub._edit_user('user', name='newuser')
|
||||
self._singleValue.assert_called_once()
|
||||
self.assertEqual(cm.exception.args[0], 'Name newuser already taken by user 2')
|
||||
self.assertEqual(cm.exception.args[0],
|
||||
'Name newuser already taken by user 2')
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue