From a624e8be18927842ac7ee95dce20719d884b972f Mon Sep 17 00:00:00 2001 From: Jana Cupova Date: Nov 07 2022 09:31:40 +0000 Subject: Rewrite Query DB to Processors in auth.py Fixes: https://pagure.io/koji/issue/3559 --- diff --git a/koji/auth.py b/koji/auth.py index d21a6db..d1595a5 100644 --- a/koji/auth.py +++ b/koji/auth.py @@ -28,12 +28,15 @@ import socket import string import six -from six.moves import range, urllib, zip +from six.moves import range, urllib import koji from .context import context from .util import to_list +from koji.db import InsertProcessor, QueryProcessor, UpdateProcessor, nextval + + # 1 - load session if provided # - check uri for session id # - load session info from db @@ -95,41 +98,30 @@ class Session(object): except Exception: callnum = None # lookup the session - c = context.cnx.cursor() - fields = { - 'authtype': 'authtype', - 'callnum': 'callnum', - 'exclusive': 'exclusive', - 'expired': 'expired', - 'master': 'master', - 'start_time': 'start_time', - 'update_time': 'update_time', - "date_part('epoch', start_time)": 'start_ts', - "date_part('epoch', update_time)": 'update_ts', - 'user_id': 'user_id', - } # sort for stability (unittests) - fields, aliases = zip(*sorted(fields.items(), key=lambda x: x[1])) - q = """ - SELECT %s FROM sessions - WHERE id = %%(id)i - AND key = %%(key)s - AND hostip = %%(hostip)s - FOR UPDATE - """ % ",".join(fields) - c.execute(q, locals()) - row = c.fetchone() - if not row: - q = "SELECT key, hostip FROM sessions WHERE id = %(id)i" - c.execute(q, locals()) - row = c.fetchone() + + fields = (('authtype', 'authtype'), ('callnum', 'callnum'), ('exclusive', 'exclusive'), + ('expired', 'expired'), ('master', 'master'), ('start_time', 'start_time'), + ('update_time', 'update_time'), ("date_part('epoch', start_time)", 'start_ts'), + ("date_part('epoch', update_time)", 'update_ts'), ('user_id', 'user_id')) + columns, aliases = zip(*fields) + + query = QueryProcessor(tables=['sessions'], columns=columns, aliases=aliases, + clauses=['id = %(id)i', 'key = %(key)s', 'hostip = %(hostip)s'], + values={'id': id, 'key': key, 'hostip': hostip}, + opts={'rowlock': True}) + session_data = query.executeOne(strict=False) + if not session_data: + query = QueryProcessor(tables='sessions', columns=['key', 'hostip'], + clauses=['id = %(id)i'], values={'id': id}) + row = query.executeOne(strict=False) if row: - if key != row[0]: + if key != row['id']: logger.warning("Session ID %s is not related to session key %s.", id, key) - elif hostip != row[1]: + elif hostip != row['hostip']: logger.warning("Session ID %s is not related to host IP %s.", id, hostip) raise koji.AuthError('Invalid session or bad credentials') - session_data = dict(zip(aliases, row)) + # check for expiration if session_data['expired']: raise koji.AuthExpired('session "%i" has expired' % id) @@ -164,10 +156,10 @@ class Session(object): # we used to get a row lock here as an attempt to maintain sanity of exclusive # sessions, but it was an imperfect approach and the lock could cause some # performance issues. - fields = ('name', 'status', 'usertype') - q = """SELECT %s FROM users WHERE id=%%(user_id)s""" % ','.join(fields) - c.execute(q, session_data) - user_data = dict(zip(fields, c.fetchone())) + query = QueryProcessor(tables=['users'], columns=['name', 'status', 'usertype'], + clauses=['id=%(user_id)s'], + values={'user_id': session_data['user_id']}) + user_data = query.executeOne() if user_data['status'] != koji.USER_STATUS['NORMAL']: raise koji.AuthError('logins by %s are not allowed' % user_data['name']) @@ -177,13 +169,13 @@ class Session(object): self.exclusive = True else: # see if an exclusive session exists - q = """SELECT id FROM sessions WHERE user_id=%(user_id)s - AND "exclusive" = TRUE AND expired = FALSE""" - # should not return multiple rows (unique constraint) - c.execute(q, session_data) - row = c.fetchone() - if row: - (excl_id,) = row + query = QueryProcessor(tables=['sessions'], columns=['id'], + clauses=['user_id=%(user_id)s', 'exclusive = TRUE', + 'expired = FALSE'], + values=session_data) + excl_id = query.singleValue(strict=False) + + if excl_id: if excl_id == session_data['master']: # (note excl_id cannot be None) # our master session has the lock @@ -196,16 +188,16 @@ class Session(object): # an exclusive session with the force option). # update timestamp - q = """UPDATE sessions SET update_time=NOW() WHERE id = %(id)i""" - c.execute(q, locals()) - # save update time + update = UpdateProcessor('sessions', rawdata={'update_time': 'NOW()'}, + clauses=['id = %(id)i'], values={'id': id}) + update.execute() context.cnx.commit() - # update callnum (this is deliberately after the commit) # see earlier note near RetryError if callnum is not None: - q = """UPDATE sessions SET callnum=%(callnum)i WHERE id = %(id)i""" - c.execute(q, locals()) + update = UpdateProcessor('sessions', rawdata={'callnum': callnum}, + clauses=['id = %(id)i'], values={'id': id}) + update.execute() # record the login data self.id = id @@ -266,16 +258,14 @@ class Session(object): def checkLoginAllowed(self, user_id): """Verify that the user is allowed to login""" - cursor = context.cnx.cursor() - query = """SELECT name, usertype, status FROM users WHERE id = %(user_id)i""" - cursor.execute(query, locals()) - result = cursor.fetchone() + query = QueryProcessor(tables=['users'], columns=['name', 'usertype', 'status'], + clauses=['id = %(user_id)i'], values={'user_id': user_id}) + result = query.executeOne(strict=False) if not result: raise koji.AuthError('invalid user_id: %s' % user_id) - name, usertype, status = result - if status != koji.USER_STATUS['NORMAL']: - raise koji.AuthError('logins by %s are not allowed' % name) + if result['status'] != koji.USER_STATUS['NORMAL']: + raise koji.AuthError('logins by %s are not allowed' % result['name']) def login(self, user, password, opts=None): """create a login session""" @@ -288,20 +278,17 @@ class Session(object): hostip = self.get_remote_ip(override=opts.get('hostip')) # check passwd - c = context.cnx.cursor() - q = """SELECT id FROM users - WHERE name = %(user)s AND password = %(password)s""" - c.execute(q, locals()) - r = c.fetchone() - if not r: + query = QueryProcessor(tables=['users'], columns=['id'], + clauses=['name = %(user)s', 'password = %(password)s'], + values={'user': user, 'password': password}) + user_id = query.singleValue(strict=False) + if not user_id: raise koji.AuthError('invalid username or password') - user_id = r[0] self.checkLoginAllowed(user_id) # create session and return sinfo = self.createSession(user_id, hostip, koji.AUTHTYPES['NORMAL']) - session_id = sinfo['session-id'] context.cnx.commit() return sinfo @@ -417,7 +404,6 @@ class Session(object): def makeExclusive(self, force=False): """Make this session exclusive""" - c = context.cnx.cursor() if self.master is not None: raise koji.GenericError("subsessions cannot become exclusive") if self.exclusive: @@ -426,33 +412,35 @@ class Session(object): user_id = self.user_id session_id = self.id # acquire a row lock on the user entry - q = """SELECT id FROM users WHERE id=%(user_id)s FOR UPDATE""" - c.execute(q, locals()) + query = QueryProcessor(tables=['users'], columns=['id'], clauses=['id=%(user_id)s'], + values={'user_id': user_id}, opts={'rowlock': True}) + query.execute() # check that no other sessions for this user are exclusive - q = """SELECT id FROM sessions WHERE user_id=%(user_id)s - AND expired = FALSE AND "exclusive" = TRUE - FOR UPDATE""" - c.execute(q, locals()) - row = c.fetchone() - if row: + query = QueryProcessor(tables=['sessions'], columns=['id'], + clauses=['user_id=%(user_id)s', 'expired = FALSE', + 'exclusive = TRUE'], + values={'user_id': user_id}, opts={'rowlock': True}) + excl_id = query.singleValue(strict=False) + if excl_id: if force: # expire the previous exclusive session and try again - (excl_id,) = row - q = """UPDATE sessions SET expired=TRUE,"exclusive"=NULL WHERE id=%(excl_id)s""" - c.execute(q, locals()) + update = UpdateProcessor('sessions', data={'expired': True, 'exclusive': None}, + clauses=['id=%(excl_id)s'], values={'excl_id': excl_id},) + update.execute() else: raise koji.AuthLockError("Cannot get exclusive session") # mark this session exclusive - q = """UPDATE sessions SET "exclusive"=TRUE WHERE id=%(session_id)s""" - c.execute(q, locals()) + update = UpdateProcessor('sessions', data={'exclusive': True}, + clauses=['id=%(session_id)s'], values={'session_id': session_id}) + update.execute() context.cnx.commit() def makeShared(self): """Drop out of exclusive mode""" - c = context.cnx.cursor() session_id = self.id - q = """UPDATE sessions SET "exclusive"=NULL WHERE id=%(session_id)s""" - c.execute(q, locals()) + update = UpdateProcessor('sessions', data={'exclusive': None}, + clauses=['id=%(session_id)s'], values={'session_id': session_id}) + update.execute() context.cnx.commit() def logout(self): @@ -460,12 +448,10 @@ class Session(object): if not self.logged_in: # XXX raise an error? raise koji.AuthError("Not logged in") - update = """UPDATE sessions - SET expired=TRUE,exclusive=NULL - WHERE id = %(id)i OR master = %(id)i""" - # note we expire subsessions as well - c = context.cnx.cursor() - c.execute(update, {'id': self.id}) + update = UpdateProcessor('sessions', data={'expired': True, 'exclusive': None}, + clauses=['id = %(id)i OR master = %(id)i'], + values={'id': self.id}) + update.execute() context.cnx.commit() self.logged_in = False @@ -474,12 +460,10 @@ class Session(object): if not self.logged_in: # XXX raise an error? raise koji.AuthError("Not logged in") - update = """UPDATE sessions - SET expired=TRUE,exclusive=NULL - WHERE id = %(session_id)i AND master = %(master)i""" - master = self.id - c = context.cnx.cursor() - c.execute(update, locals()) + update = UpdateProcessor('sessions', data={'expired': True, 'exclusive': None}, + clauses=['id = %(session_id)i', 'master = %(master)i'], + values={'session_id': session_id, 'master': self.id}) + update.execute() context.cnx.commit() def createSession(self, user_id, hostip, authtype, master=None): @@ -488,8 +472,6 @@ class Session(object): Return a map containing the session-id and session-key. If master is specified, create a subsession """ - c = context.cnx.cursor() - # generate a random key alnum = string.ascii_letters + string.digits key = "%s-%s" % (user_id, @@ -497,16 +479,13 @@ class Session(object): # use sha? sha.new(phrase).hexdigest() # get a session id - q = """SELECT nextval('sessions_id_seq')""" - c.execute(q, {}) - (session_id,) = c.fetchone() + session_id = nextval('sessions_id_seq') # add session id to database - q = """ - INSERT INTO sessions (id, user_id, key, hostip, authtype, master) - VALUES (%(session_id)i, %(user_id)i, %(key)s, %(hostip)s, %(authtype)i, %(master)s) - """ - c.execute(q, locals()) + insert = InsertProcessor('sessions', + data={'id': session_id, 'user_id': user_id, 'key': key, + 'hostip': hostip, 'authtype': authtype, 'master': master}) + insert.execute() context.cnx.commit() # return session info @@ -564,15 +543,9 @@ class Session(object): '''Using session data, find host id (if there is one)''' if self.user_id is None: return None - c = context.cnx.cursor() - q = """SELECT id FROM host WHERE user_id = %(uid)d""" - c.execute(q, {'uid': self.user_id}) - r = c.fetchone() - c.close() - if r: - return r[0] - else: - return None + query = QueryProcessor(tables=['host'], columns=['id'], clauses=['user_id = %(uid)d'], + values={'uid': self.user_id}) + return query.singleValue(strict=False) def getHostId(self): # for compatibility @@ -581,32 +554,20 @@ class Session(object): def getUserId(self, username): """Return the user ID associated with a particular username. If no user with the given username if found, return None.""" - c = context.cnx.cursor() - q = """SELECT id FROM users WHERE name = %(username)s""" - c.execute(q, locals()) - r = c.fetchone() - c.close() - if r: - return r[0] - else: - return None + query = QueryProcessor(tables=['users'], columns=['id'], clauses=['name = %(username)s'], + values={'username': username}) + return query.singleValue(strict=False) def getUserIdFromKerberos(self, krb_principal): """Return the user ID associated with a particular Kerberos principal. If no user with the given princpal if found, return None.""" self.checkKrbPrincipal(krb_principal) - c = context.cnx.cursor() - q = """SELECT id FROM users - JOIN user_krb_principals - ON users.id = user_krb_principals.user_id - WHERE krb_principal = %(krb_principal)s""" - c.execute(q, locals()) - r = c.fetchone() - c.close() - if r: - return r[0] - else: - return None + query = QueryProcessor(tables=['users'], columns=['id'], + joins=['user_krb_principals ON ' + 'users.id = user_krb_principals.user_id'], + clauses=['krb_principal = %(krb_principal)s'], + values={'krb_principal': krb_principal}) + return query.singleValue(strict=False) def createUser(self, name, usertype=None, status=None, krb_principal=None, krb_princ_check=True): @@ -631,18 +592,16 @@ class Session(object): if krb_princ_check: self.checkKrbPrincipal(krb_principal) - cursor = context.cnx.cursor() - select = """SELECT nextval('users_id_seq')""" - cursor.execute(select, locals()) - user_id = cursor.fetchone()[0] + user_id = nextval('users_id_seq') - insert = """INSERT INTO users (id, name, usertype, status) - VALUES (%(user_id)i, %(name)s, %(usertype)i, %(status)i)""" - cursor.execute(insert, locals()) + insert = InsertProcessor('users', + data={'id': user_id, 'name': name, 'usertype': usertype, + 'status': status}) + insert.execute() if krb_principal: - insert = """INSERT INTO user_krb_principals (user_id, krb_principal) - VALUES (%(user_id)i, %(krb_principal)s)""" - cursor.execute(insert, locals()) + insert = InsertProcessor('user_krb_principals', + data={'user_id': user_id, 'krb_principal': krb_principal}) + insert.execute() context.cnx.commit() return user_id @@ -650,47 +609,40 @@ 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 %s""" if isinstance(name, six.integer_types): - user_condition = 'id = %(name)i' + clauses = ['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: + clauses = ['name = %(name)s'] + query = QueryProcessor(tables=['users'], columns=['id'], clauses=clauses, + values={'name': name}) + user_id = query.singleValue(strict=False) + if not user_id: context.cnx.rollback() raise koji.AuthError('No such user: %s' % name) - else: - user_id = r[0] - insert = """INSERT INTO user_krb_principals (user_id, krb_principal) - VALUES (%(user_id)i, %(krb_principal)s)""" - cursor.execute(insert, locals()) + insert = InsertProcessor('user_krb_principals', + data={'user_id': user_id, 'krb_principal': krb_principal}) + insert.execute() context.cnx.commit() return user_id def removeKrbPrincipal(self, name, krb_principal): - select = """SELECT id FROM users - JOIN user_krb_principals - ON users.id = user_krb_principals.user_id - WHERE %s - AND krb_principal = %%(krb_principal)s""" + clauses = ['krb_principal = %(krb_principal)s'] if isinstance(name, six.integer_types): - user_condition = 'id = %(name)i' + clauses.extend(['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: + clauses.extend(['name = %(name)s']) + query = QueryProcessor(tables=['users'], columns=['id'], + joins=['user_krb_principals ' + 'ON users.id = user_krb_principals.user_id'], + clauses=clauses, + values={'krb_principal': krb_principal, 'name': name}) + user_id = query.singleValue(strict=False) + if not user_id: context.cnx.rollback() raise koji.AuthError( 'cannot remove Kerberos Principal:' ' %(krb_principal)s with user %(name)s' % locals()) - else: - user_id = r[0] + cursor = context.cnx.cursor() delete = """DELETE FROM user_krb_principals WHERE user_id = %(user_id)i AND krb_principal = %(krb_principal)s""" @@ -708,23 +660,21 @@ class Session(object): user_name = krb_principal[:atidx] # check if user already exists - c = context.cnx.cursor() - q = """SELECT id, krb_principal FROM users - LEFT JOIN user_krb_principals - ON users.id = user_krb_principals.user_id - WHERE name = %(user_name)s""" - c.execute(q, locals()) - r = c.fetchall() + query = QueryProcessor(tables=['users'], columns=['id', 'krb_principal'], + joins=['LEFT JOIN user_krb_principals ON ' + 'users.id = user_krb_principals.user_id'], + clauses=['name = %(user_name)s'], + values={'user_name': user_name}) + r = query.execute() if not r: return self.createUser(user_name, krb_principal=krb_principal, krb_princ_check=False) else: - existing_user_krb_princs = [row[1] for row in r] + existing_user_krb_princs = [row['krb_principal'] for row in r] if krb_principal in existing_user_krb_princs: # do not set Kerberos principal if it already exists - return r[0][0] - return self.setKrbPrincipal(user_name, krb_principal, - krb_princ_check=False) + return r[0]['id'] + return self.setKrbPrincipal(user_name, krb_principal, krb_princ_check=False) def checkKrbPrincipal(self, krb_principal): """Check if the Kerberos principal is allowed""" @@ -749,35 +699,28 @@ def get_user_groups(user_id): returns a dictionary where the keys are the group ids and the values are the group names""" - c = context.cnx.cursor() t_group = koji.USERTYPES['GROUP'] - q = """SELECT group_id,name - FROM user_groups JOIN users ON group_id = users.id - WHERE active = TRUE AND users.usertype=%(t_group)i - AND user_id=%(user_id)i""" - c.execute(q, locals()) - return dict(c.fetchall()) + query = QueryProcessor(tables=['user_groups'], columns=['group_id', 'name'], + clauses=['active = TRUE', 'users.usertype=%(t_group)i', + 'user_id=%(user_id)i'], + joins=['users ON group_id = users.id'], + values={'t_group': t_group, 'user_id': user_id}) + return query.execute() def get_user_perms(user_id): - c = context.cnx.cursor() - q = """SELECT name - FROM user_perms JOIN permissions ON perm_id = permissions.id - WHERE active = TRUE AND user_id=%(user_id)s""" - c.execute(q, locals()) - # return a list of permissions by name - return [row[0] for row in c.fetchall()] + query = QueryProcessor(tables=['user_perms'], columns=['name'], + clauses=['active = TRUE', 'user_id=%(user_id)s'], + joins=['permissions ON perm_id = permissions.id'], + values={'user_id': user_id}) + result = query.execute() + return [r['name'] for r in result] def get_user_data(user_id): - c = context.cnx.cursor() - fields = ('name', 'status', 'usertype') - q = """SELECT %s FROM users WHERE id=%%(user_id)s""" % ','.join(fields) - c.execute(q, locals()) - row = c.fetchone() - if not row: - return None - return dict(zip(fields, row)) + query = QueryProcessor(tables=['users'], columns=['name', 'status', 'usertype'], + clauses=['id=%(user_id)s'], values={'user_id': user_id}) + return query.executeOne(strict=False) def login(*args, **opts): diff --git a/tests/test_lib/test_auth.py b/tests/test_lib/test_auth.py index f3318d4..d94aea9 100644 --- a/tests/test_lib/test_auth.py +++ b/tests/test_lib/test_auth.py @@ -6,9 +6,43 @@ import unittest import koji import koji.auth +import koji.db +import datetime + +UP = koji.auth.UpdateProcessor +QP = koji.auth.QueryProcessor class TestAuthSession(unittest.TestCase): + def getUpdate(self, *args, **kwargs): + update = UP(*args, **kwargs) + update.execute = mock.MagicMock() + self.updates.append(update) + return update + + def getQuery(self, *args, **kwargs): + query = QP(*args, **kwargs) + query.execute = self.query_execute + query.executeOne = self.query_executeOne + query.singleValue = self.query_singleValue + self.queries.append(query) + return query + + def setUp(self): + self.context = mock.patch('kojihub.context').start() + self.UpdateProcessor = mock.patch('koji.auth.UpdateProcessor', + side_effect=self.getUpdate).start() + self.updates = [] + self.query_execute = mock.MagicMock() + self.query_executeOne = mock.MagicMock() + self.query_singleValue = mock.MagicMock() + self.QueryProcessor = mock.patch('koji.auth.QueryProcessor', + side_effect=self.getQuery).start() + self.queries = [] + # It seems MagicMock will not automatically handle attributes that + # start with "assert" + self.context.session.assertLogin = mock.MagicMock() + def test_instance(self): """Simple auth.Session instance""" s = koji.auth.Session() @@ -23,54 +57,120 @@ class TestAuthSession(unittest.TestCase): 'QUERY_STRING': 'session-id=123&session-key=xyz&callnum=345', 'REMOTE_ADDR': 'remote-addr', } - cursor = mock.MagicMock(name='cursor') - context.cnx.cursor.return_value = cursor - cursor.fetchone.side_effect = [ - # get session - [koji.AUTHTYPES['NORMAL'], 344, False, False, 'master', 'start_time', - 'start_ts', 'update_time', 'update_ts', 'user_id'], - # get user - ['name', koji.USER_STATUS['NORMAL'], koji.USERTYPES['NORMAL']], - # get excl.session - None, - # upd. timestamp - None, - # upd callnum - None, - ] + self.query_executeOne.side_effect = [ + {'authtype': 2, 'callnum': 1, "date_part('epoch', start_time)": 1666599426.227002, + "date_part('epoch', update_time)": 1666599426.254308, 'exclusive': None, + 'expired': False, 'master': None, + 'start_time': datetime.datetime(2022, 10, 24, 8, 17, 6, 227002, + tzinfo=datetime.timezone.utc), + 'update_time': datetime.datetime(2022, 10, 24, 8, 17, 6, 254308, + tzinfo=datetime.timezone.utc), + 'user_id': 1}, + {'name': 'kojiadmin', 'status': 0, 'usertype': 0}] + self.query_singleValue.return_value = 123 s = koji.auth.Session() - return s, context, cursor + return s, context - @mock.patch('koji.auth.context') - def test_basic_instance(self, context): + def test_basic_instance(self): """auth.Session instance""" - s, cntext, cursor = self.get_session() - context.cnx = cntext.cnx - - self.assertEqual(s.id, 123) - self.assertEqual(s.key, 'xyz') - self.assertEqual(s.hostip, 'remote-addr') - self.assertEqual(s.callnum, 345) - self.assertEqual(s.user_id, 'user_id') - self.assertEqual(s.authtype, koji.AUTHTYPES['NORMAL']) - self.assertEqual(s.master, 'master') - self.assertTrue(s.logged_in) - - # 5 SQL calls: get session, get user, get excl. session, - # update timestamp, update callnum - self.assertEqual(cursor.execute.call_count, 5) - - @mock.patch('koji.auth.context') - def test_getattr(self, context): + s, cntext = self.get_session() + self.assertEqual(len(self.updates), 2) + update = self.updates[0] + + self.assertEqual(update.table, 'sessions') + self.assertEqual(update.values['id'], 123) + self.assertEqual(update.clauses, ['id = %(id)i']) + self.assertEqual(update.data, {}) + self.assertEqual(update.rawdata, {'update_time': 'NOW()'}) + + update = self.updates[1] + + self.assertEqual(update.table, 'sessions') + self.assertEqual(update.values['id'], 123) + self.assertEqual(update.clauses, ['id = %(id)i']) + self.assertEqual(update.data, {}) + self.assertEqual(update.rawdata, {'callnum': 345}) + + self.assertEqual(len(self.queries), 3) + + query = self.queries[0] + self.assertEqual(query.tables, ['sessions']) + self.assertEqual(query.joins, None) + self.assertEqual(query.clauses, ['hostip = %(hostip)s', 'id = %(id)i', 'key = %(key)s']) + self.assertEqual(query.columns, ['authtype', 'callnum', 'exclusive', 'expired', 'master', + 'start_time', "date_part('epoch', start_time)", + 'update_time', "date_part('epoch', update_time)", + 'user_id']) + self.assertEqual(query.aliases, ['authtype', 'callnum', 'exclusive', 'expired', 'master', + 'start_time', 'start_ts', 'update_time', 'update_ts', + 'user_id']) + self.assertEqual(query.values, {'id': 123, 'key': 'xyz', 'hostip': 'remote-addr'}) + + query = self.queries[1] + self.assertEqual(query.tables, ['users']) + self.assertEqual(query.joins, None) + self.assertEqual(query.clauses, ['id=%(user_id)s']) + self.assertEqual(query.columns, ['name', 'status', 'usertype']) + self.assertEqual(query.values, {'user_id': 1}) + + query = self.queries[2] + self.assertEqual(query.tables, ['sessions']) + self.assertEqual(query.joins, None) + self.assertEqual(query.clauses, ['exclusive = TRUE', 'expired = FALSE', + 'user_id=%(user_id)s']) + self.assertEqual(query.columns, ['id']) + + def test_getattr(self): """auth.Session instance""" - s, cntext, cursor = self.get_session() - context.cnx = cntext.cnx + s, cntext = self.get_session() + + self.assertEqual(len(self.updates), 2) + update = self.updates[0] + + self.assertEqual(update.table, 'sessions') + self.assertEqual(update.values['id'], 123) + self.assertEqual(update.clauses, ['id = %(id)i']) + self.assertEqual(update.data, {}) + self.assertEqual(update.rawdata, {'update_time': 'NOW()'}) + + update = self.updates[1] + + self.assertEqual(update.table, 'sessions') + self.assertEqual(update.values['id'], 123) + self.assertEqual(update.clauses, ['id = %(id)i']) + self.assertEqual(update.data, {}) + self.assertEqual(update.rawdata, {'callnum': 345}) + + self.assertEqual(len(self.queries), 3) + + query = self.queries[0] + self.assertEqual(query.tables, ['sessions']) + self.assertEqual(query.joins, None) + self.assertEqual(query.clauses, ['hostip = %(hostip)s', 'id = %(id)i', 'key = %(key)s']) + self.assertEqual(query.columns, ['authtype', 'callnum', 'exclusive', 'expired', 'master', + 'start_time', "date_part('epoch', start_time)", + 'update_time', "date_part('epoch', update_time)", + 'user_id']) + self.assertEqual(query.aliases, ['authtype', 'callnum', 'exclusive', 'expired', 'master', + 'start_time', 'start_ts', 'update_time', 'update_ts', + 'user_id']) + self.assertEqual(query.values, {'id': 123, 'key': 'xyz', 'hostip': 'remote-addr'}) + + query = self.queries[1] + self.assertEqual(query.tables, ['users']) + self.assertEqual(query.joins, None) + self.assertEqual(query.clauses, ['id=%(user_id)s']) + self.assertEqual(query.columns, ['name', 'status', 'usertype']) + self.assertEqual(query.values, {'user_id': 1}) + + query = self.queries[2] + self.assertEqual(query.tables, ['sessions']) + self.assertEqual(query.joins, None) + self.assertEqual(query.clauses, ['exclusive = TRUE', 'expired = FALSE', + 'user_id=%(user_id)s']) + self.assertEqual(query.columns, ['id']) - # test - self.assertEqual(s.perms, {}) - self.assertEqual(s.groups, {}) - self.assertEqual(s.host_id, None) # all other names should raise error with self.assertRaises(AttributeError): s.non_existing_attribute @@ -78,7 +178,7 @@ class TestAuthSession(unittest.TestCase): @mock.patch('koji.auth.context') def test_str(self, context): """auth.Session string representation""" - s, cntext, cursor = self.get_session() + s, cntext = self.get_session() context.cnx = cntext.cnx s.logged_in = False @@ -90,7 +190,7 @@ class TestAuthSession(unittest.TestCase): @mock.patch('koji.auth.context') def test_validate(self, context): """Session.validate""" - s, cntext, cursor = self.get_session() + s, cntext = self.get_session() context.cnx = cntext.cnx s.lockerror = True @@ -103,20 +203,26 @@ class TestAuthSession(unittest.TestCase): @mock.patch('koji.auth.context') def test_makeShared(self, context): """Session.makeShared""" - s, cntext, cursor = self.get_session() - context.cnx = cntext.cnx - + s, _ = self.get_session() s.makeShared() - c = cursor.execute.call_args[0] - self.assertEqual(c[0], - 'UPDATE sessions SET "exclusive"=NULL WHERE id=%(session_id)s') - self.assertEqual(c[1]['session_id'], 123) + self.assertEqual(len(self.updates), 3) + # check only last update query, first two are tested in test_basic_instance + update = self.updates[2] + + self.assertEqual(update.table, 'sessions') + self.assertEqual(update.values['session_id'], 123) + self.assertEqual(update.clauses, ['id=%(session_id)s']) + self.assertEqual(update.data, {'exclusive': None}) + self.assertEqual(update.rawdata, {}) + + self.assertEqual(len(self.queries), 3) + # all queries are tested in test_basic_instance @mock.patch('socket.gethostbyname') @mock.patch('koji.auth.context') def test_get_remote_ip(self, context, gethostbyname): """Session.get_remote_ip""" - s, cntext, cursor = self.get_session() + s, cntext = self.get_session() context.opts = {'CheckClientIP': False} self.assertEqual(s.get_remote_ip(), '-') @@ -133,8 +239,7 @@ class TestAuthSession(unittest.TestCase): @mock.patch('koji.auth.context') def test_login(self, context): - s, cntext, cursor = self.get_session() - context.cnx = cntext.cnx + s, cntext = self.get_session() # already logged in with self.assertRaises(koji.GenericError): @@ -153,22 +258,35 @@ class TestAuthSession(unittest.TestCase): s.checkLoginAllowed.return_value = True s.createSession = mock.MagicMock() s.createSession.return_value = {'session-id': 'session-id'} - cursor.fetchone = mock.MagicMock() - cursor.fetchone.return_value = ['user_id'] + self.query_singleValue.return_value = 123 + result = s.login('user', 'password') + self.assertEqual(len(self.queries), 4) + + # check only last update query, first three are tested in test_basic_instance + query = self.queries[3] + self.assertEqual(query.tables, ['users']) + self.assertEqual(query.joins, None) + self.assertEqual(query.clauses, ['name = %(user)s', 'password = %(password)s']) + self.assertEqual(query.columns, ['id']) + self.assertEqual(query.values, {'user': 'user', 'password': 'password'}) + + self.assertEqual(len(self.updates), 2) + # all updates are tested in test_basic_instance + self.assertEqual(s.get_remote_ip.call_count, 1) - self.assertEqual(s.checkLoginAllowed.call_args, mock.call('user_id')) + self.assertEqual(s.checkLoginAllowed.call_args, mock.call(123)) self.assertEqual(result, s.createSession.return_value) # one more try for non-existing user - cursor.fetchone.return_value = None + self.query_singleValue.return_value = None with self.assertRaises(koji.AuthError): s.login('user', 'password') @mock.patch('koji.auth.context') def test_checkKrbPrincipal(self, context): - s, cntext, cursor = self.get_session() + s, cntext = self.get_session() self.assertIsNone(s.checkKrbPrincipal(None)) context.opts = {'AllowedKrbRealms': '*'} self.assertIsNone(s.checkKrbPrincipal('any')) @@ -191,18 +309,318 @@ class TestAuthSession(unittest.TestCase): ' , example.org'} self.assertIsNone(s.checkKrbPrincipal('user@example.net')) - # functions outside Session object + def test_getUserIdFromKerberos(self): + krb_principal = 'test-krb-principal' + self.query_singleValue.return_value = 135 + s, cntext = self.get_session() + s.checkKrbPrincipal = mock.MagicMock() + s.checkKrbPrincipal.return_value = True + + s.getUserIdFromKerberos(krb_principal) + + self.assertEqual(len(self.queries), 4) + # check only last update query, first three are tested in test_basic_instance + query = self.queries[3] + self.assertEqual(query.tables, ['users']) + self.assertEqual(query.joins, ['user_krb_principals ON ' + 'users.id = user_krb_principals.user_id']) + self.assertEqual(query.clauses, ['krb_principal = %(krb_principal)s']) + self.assertEqual(query.columns, ['id']) + self.assertEqual(query.values, {'krb_principal': krb_principal}) + + self.assertEqual(len(self.updates), 2) + # all updates are tested in test_basic_instance + + def test_getUserId(self): + self.query_singleValue.return_value = 135 + s, cntext = self.get_session() + username = 'test-user' + + s.getUserId(username) + + self.assertEqual(len(self.queries), 4) + # check only last update query, first three are tested in test_basic_instance + query = self.queries[3] + self.assertEqual(query.tables, ['users']) + self.assertEqual(query.joins, None) + self.assertEqual(query.clauses, ['name = %(username)s']) + self.assertEqual(query.columns, ['id']) + self.assertEqual(query.values, {'username': username}) + + self.assertEqual(len(self.updates), 2) + # all updates are tested in test_basic_instance + + def test_getHostId(self): + self.query_singleValue.return_value = 199 + s, cntext = self.get_session() + + s._getHostId() + + self.assertEqual(len(self.queries), 4) + # check only last update query, first three are tested in test_basic_instance + query = self.queries[3] + self.assertEqual(query.tables, ['host']) + self.assertEqual(query.joins, None) + self.assertEqual(query.clauses, ['user_id = %(uid)d']) + self.assertEqual(query.columns, ['id']) + self.assertEqual(query.values, {'uid': 1}) + + self.assertEqual(len(self.updates), 2) + # all updates are tested in test_basic_instance + + def test_logout_not_logged(self): + s, cntext = self.get_session() + s.logged_in = False + with self.assertRaises(koji.AuthError) as cm: + s.logout() + self.assertEqual(cm.exception.args[0], 'Not logged in') @mock.patch('koji.auth.context') - def test_get_user_data(self, context): - """koji.auth.get_user_data""" - cursor = mock.MagicMock(name='cursor') - context.cnx.cursor.return_value = cursor - cursor.fetchone.return_value = ['name', 'status', 'usertype'] + def test_logout_logged(self, context): + s, cntext = self.get_session() + s.logged_in = True + s.logout() + + self.assertEqual(len(self.queries), 3) + # all queries are tested in test_basic_instance + + self.assertEqual(len(self.updates), 3) + # check only last update query, first two are tested in test_basic_instance + update = self.updates[2] + + self.assertEqual(update.table, 'sessions') + self.assertEqual(update.values, {'id': 123, 'id': 123}) + self.assertEqual(update.clauses, ['id = %(id)i OR master = %(id)i']) + self.assertEqual(update.data, {'expired': True, 'exclusive': None}) + self.assertEqual(update.rawdata, {}) + + def test_logoutChild_not_logged(self): + s, cntext = self.get_session() + s.logged_in = False + with self.assertRaises(koji.AuthError) as cm: + s.logoutChild(111) + self.assertEqual(cm.exception.args[0], 'Not logged in') + + @mock.patch('koji.auth.context') + def test_logoutChild_logged(self, context): + s, cntext = self.get_session() + s.logged_in = True + s.logoutChild(111) + + self.assertEqual(len(self.queries), 3) + # all queries are tested in test_basic_instance + + self.assertEqual(len(self.updates), 3) + # check only last update query, first two are tested in test_basic_instance + update = self.updates[2] + + self.assertEqual(update.table, 'sessions') + self.assertEqual(update.values, {'session_id': 111, 'master': 123}) + self.assertEqual(update.clauses, ['id = %(session_id)i', 'master = %(master)i']) + self.assertEqual(update.data, {'expired': True, 'exclusive': None}) + self.assertEqual(update.rawdata, {}) + + def test_makeExclusive_not_master(self): + s, cntext = self.get_session() + s.master = 333 + with self.assertRaises(koji.GenericError) as cm: + s.makeExclusive() + self.assertEqual(cm.exception.args[0], 'subsessions cannot become exclusive') + + def test_makeExclusive_already_exclusive(self): + s, cntext = self.get_session() + s.master = None + s.exclusive = True + with self.assertRaises(koji.GenericError) as cm: + s.makeExclusive() + self.assertEqual(cm.exception.args[0], 'session is already exclusive') + + def test_makeExclusive_without_force(self): + s, cntext = self.get_session() + s.master = None + s.exclusive = False + self.query_singleValue.return_value = 123 + + with self.assertRaises(koji.AuthLockError) as cm: + s.makeExclusive() + self.assertEqual(cm.exception.args[0], 'Cannot get exclusive session') + + self.assertEqual(len(self.queries), 5) + self.assertEqual(len(self.updates), 2) + + @mock.patch('koji.auth.context') + def test_makeExclusive(self, context): + s, cntext = self.get_session() + s.master = None + s.exclusive = False + self.query_singleValue.return_value = 123 + + s.makeExclusive(force=True) + + self.assertEqual(len(self.queries), 5) + # check only last two queries, first two are tested in test_basic_instance + + query = self.queries[3] + self.assertEqual(query.tables, ['users']) + self.assertEqual(query.joins, None) + self.assertEqual(query.clauses, ['id=%(user_id)s']) + self.assertEqual(query.columns, ['id']) + self.assertEqual(query.values, {'user_id': 1}) + + query = self.queries[4] + self.assertEqual(query.tables, ['sessions']) + self.assertEqual(query.joins, None) + self.assertEqual(query.clauses, ['exclusive = TRUE', 'expired = FALSE', + 'user_id=%(user_id)s']) + self.assertEqual(query.columns, ['id']) + self.assertEqual(query.values, {'user_id': 1}) + + self.assertEqual(len(self.updates), 4) + # check only last two update queries, first two are tested in test_basic_instance + + update = self.updates[2] + self.assertEqual(update.table, 'sessions') + self.assertEqual(update.values, {'excl_id': 123}) + self.assertEqual(update.clauses, ['id=%(excl_id)s']) + self.assertEqual(update.data, {'expired': True, 'exclusive': None}) + self.assertEqual(update.rawdata, {}) + + update = self.updates[3] + self.assertEqual(update.table, 'sessions') + self.assertEqual(update.values, {'session_id': 123}) + self.assertEqual(update.clauses, ['id=%(session_id)s']) + self.assertEqual(update.data, {'exclusive': True}) + self.assertEqual(update.rawdata, {}) + + def test_checkLoginAllowed(self): + s, cntext = self.get_session() + self.query_executeOne.side_effect = [{'name': 'testuser', 'status': 0, 'usertype': 0}] + s.checkLoginAllowed(2) + + self.assertEqual(len(self.queries), 4) + query = self.queries[3] + self.assertEqual(query.tables, ['users']) + self.assertEqual(query.joins, None) + self.assertEqual(query.clauses, ['id = %(user_id)i']) + self.assertEqual(query.columns, ['name', 'status', 'usertype']) + self.assertEqual(query.values, {'user_id': 2}) + + self.assertEqual(len(self.updates), 2) + + def test_checkLoginAllowed_not_normal_status(self): + s, cntext = self.get_session() + self.query_executeOne.side_effect = [{'name': 'testuser', 'status': 1, 'usertype': 0}] + + with self.assertRaises(koji.AuthError) as cm: + s.checkLoginAllowed(2) + self.assertEqual(cm.exception.args[0], 'logins by testuser are not allowed') - self.assertEqual(sorted(koji.auth.get_user_data(1).items()), - sorted({'name': 'name', 'status': 'status', - 'usertype': 'usertype'}.items())) + self.assertEqual(len(self.queries), 4) + query = self.queries[3] + self.assertEqual(query.tables, ['users']) + self.assertEqual(query.joins, None) + self.assertEqual(query.clauses, ['id = %(user_id)i']) + self.assertEqual(query.columns, ['name', 'status', 'usertype']) + self.assertEqual(query.values, {'user_id': 2}) - cursor.fetchone.return_value = None - self.assertEqual(koji.auth.get_user_data(1), None) + self.assertEqual(len(self.updates), 2) + + def test_checkLoginAllowed_not_exist_user(self): + s, cntext = self.get_session() + self.query_executeOne.side_effect = [None] + + with self.assertRaises(koji.AuthError) as cm: + s.checkLoginAllowed(2) + self.assertEqual(cm.exception.args[0], 'invalid user_id: 2') + + self.assertEqual(len(self.queries), 4) + query = self.queries[3] + self.assertEqual(query.tables, ['users']) + self.assertEqual(query.joins, None) + self.assertEqual(query.clauses, ['id = %(user_id)i']) + self.assertEqual(query.columns, ['name', 'status', 'usertype']) + self.assertEqual(query.values, {'user_id': 2}) + + self.assertEqual(len(self.updates), 2) + + def test_createUserFromKerberos_invalid_krb(self): + s, cntext = self.get_session() + krb_principal = 'test-krb-princ' + with self.assertRaises(koji.AuthError) as cm: + s.createUserFromKerberos(krb_principal) + self.assertEqual(cm.exception.args[0], 'invalid Kerberos principal: %s' % krb_principal) + + def test_createUserFromKerberos_user_not_exists(self): + self.query_execute.return_value = None + s, cntext = self.get_session() + krb_principal = 'test-krb-princ@redhat.com' + s.createUser = mock.MagicMock() + s.createUser.return_value = 3 + s.createUserFromKerberos(krb_principal) + self.assertEqual(len(self.queries), 4) + self.assertEqual(len(self.updates), 2) + + query = self.queries[3] + self.assertEqual(query.tables, ['users']) + self.assertEqual(query.joins, ['LEFT JOIN user_krb_principals ON ' + 'users.id = user_krb_principals.user_id']) + self.assertEqual(query.clauses, ['name = %(user_name)s']) + self.assertEqual(query.columns, ['id', 'krb_principal']) + self.assertEqual(query.values, {'user_name': 'test-krb-princ'}) + + def test_createUserFromKerberos_valid(self): + self.query_execute.return_value = [{'id': 1, 'krb_principal': 'krb-user-1@redhat.com'}, + {'id': 1, 'krb_principal': 'krb-user-2@redhat.com'}] + s, cntext = self.get_session() + krb_principal = 'test-krb-princ@redhat.com' + s.setKrbPrincipal = mock.MagicMock() + s.setKrbPrincipal.return_value = 1 + s.createUserFromKerberos(krb_principal) + self.assertEqual(len(self.queries), 4) + self.assertEqual(len(self.updates), 2) + + query = self.queries[3] + self.assertEqual(query.tables, ['users']) + self.assertEqual(query.joins, ['LEFT JOIN user_krb_principals ON ' + 'users.id = user_krb_principals.user_id']) + self.assertEqual(query.clauses, ['name = %(user_name)s']) + self.assertEqual(query.columns, ['id', 'krb_principal']) + self.assertEqual(query.values, {'user_name': 'test-krb-princ'}) + + # functions outside Session object + + def test_get_user_data(self): + """koji.auth.get_user_data""" + self.query_executeOne.return_value = None + self.assertEqual(len(self.queries), 0) + + self.query_executeOne.return_value = {'name': 'name', 'status': 'status', + 'usertype': 'usertype'} + koji.auth.get_user_data(1) + self.assertEqual(len(self.queries), 1) + query = self.queries[0] + self.assertEqual(query.tables, ['users']) + self.assertEqual(query.joins, None) + self.assertEqual(query.clauses, ['id=%(user_id)s']) + self.assertEqual(query.columns, ['name', 'status', 'usertype']) + + def test_get_user_groups(self): + """koji.auth.get_user_groups""" + koji.auth.get_user_groups(1) + self.assertEqual(len(self.queries), 1) + query = self.queries[0] + self.assertEqual(query.tables, ['user_groups']) + self.assertEqual(query.joins, ['users ON group_id = users.id']) + self.assertEqual(query.clauses, ['active = TRUE', 'user_id=%(user_id)i', + 'users.usertype=%(t_group)i']) + self.assertEqual(query.columns, ['group_id', 'name']) + + def test_get_user_perms(self): + """koji.auth.get_user_perms""" + koji.auth.get_user_perms(1) + self.assertEqual(len(self.queries), 1) + query = self.queries[0] + self.assertEqual(query.tables, ['user_perms']) + self.assertEqual(query.joins, ['permissions ON perm_id = permissions.id']) + self.assertEqual(query.clauses, ['active = TRUE', 'user_id=%(user_id)s']) + self.assertEqual(query.columns, ['name'])