PR#3664: Recreate timeouted session

Merges #3664
https://pagure.io/koji/pull-request/3664

References #3394
https://pagure.io/koji/issue/3394
This commit is contained in:
Tomas Kopecek 2023-02-01 15:01:21 +01:00
commit d5ed48f56a
5 changed files with 213 additions and 81 deletions

View file

@ -0,0 +1,13 @@
-- upgrade script to migrate the Koji database schema
-- from version 1.31 to 1.32
BEGIN;
-- fix duplicate extension in archivetypes
UPDATE archivetypes SET extensions = 'vhdx.gz vhdx.xz' WHERE name = 'vhdx-compressed';
-- for tag if session is closed or not
ALTER TABLE sessions ADD COLUMN closed BOOLEAN NOT NULL DEFAULT FALSE;
ALTER TABLE sessions ADD CONSTRAINT no_closed_exclusive CHECK (closed IS FALSE OR "exclusive" IS NULL);
ALTER TABLE sessions DROP CONSTRAINT exclusive_expired_sane;
COMMIT;

View file

@ -119,10 +119,11 @@ CREATE TABLE sessions (
start_time TIMESTAMPTZ NOT NULL DEFAULT NOW(),
update_time TIMESTAMPTZ NOT NULL DEFAULT NOW(),
exclusive BOOLEAN CHECK (exclusive),
closed BOOLEAN NOT NULL DEFAULT FALSE,
CONSTRAINT no_exclusive_subsessions CHECK (
master IS NULL OR "exclusive" IS NULL),
CONSTRAINT exclusive_expired_sane CHECK (
expired IS FALSE OR "exclusive" IS NULL),
CONSTRAINT no_closed_exclusive CHECK (
closed IS FALSE OR "exclusive" IS NULL),
UNIQUE (user_id,exclusive)
) WITHOUT OIDS;
CREATE INDEX sessions_master ON sessions(master);

View file

@ -2424,7 +2424,13 @@ def grab_session_options(options):
class ClientSession(object):
def __init__(self, baseurl, opts=None, sinfo=None):
def __init__(self, baseurl, opts=None, sinfo=None, auth_method=None):
"""
:param baseurl str: hub url
:param dict opts: dictionary with content varying according to authentication method
:param dict sinfo: session info returned by login method
:param dict auth_method: method for reauthentication, shouldn't be ever set manually
"""
assert baseurl, "baseurl argument must not be empty"
if opts is None:
opts = {}
@ -2444,6 +2450,8 @@ class ClientSession(object):
self.rsession = None
self.new_session()
self.opts.setdefault('timeout', DEFAULT_REQUEST_TIMEOUT)
self.exclusive = False
self.auth_method = auth_method
@property
def multicall(self):
@ -2480,8 +2488,21 @@ class ClientSession(object):
self.callnum = 0
self.sinfo = sinfo
def login(self, opts=None):
sinfo = self.callMethod('login', self.opts['user'], self.opts['password'], opts)
def login(self, opts=None, renew=False):
"""
Username/password based login method
:param dict opts: dict used by hub "login" call, currently can
contain only "host_ip" key.
:returns bool True: success or raises exception
"""
# store calling parameters
self.auth_method = {'method': 'login', 'kwargs': {'opts': opts}}
kwargs = {'opts': opts}
if renew:
kwargs['renew'] = True
kwargs['exclusive'] = self.exclusive
sinfo = self.callMethod('login', self.opts['user'], self.opts['password'], **kwargs)
if not sinfo:
return False
self.setSession(sinfo)
@ -2491,14 +2512,32 @@ class ClientSession(object):
def subsession(self):
"Create a subsession"
sinfo = self.callMethod('subsession')
return type(self)(self.baseurl, self.opts, sinfo)
return type(self)(self.baseurl, opts=self.opts, sinfo=sinfo, auth_method=self.auth_method)
def gssapi_login(self, principal=None, keytab=None, ccache=None,
proxyuser=None, proxyauthtype=None):
proxyuser=None, proxyauthtype=None, renew=False):
"""
GSSAPI/Kerberos login method
:param str principal: Kerberos principal
:param str keytab: path to keytab file
:param str ccache: path to ccache file/dir
:param str proxyuser: name of proxied user (e.g. forwarding by web ui)
:param int proxyauthtype: AUTHTYPE used by proxied user (can be different from ours)
:returns bool True: success or raises exception
"""
if not reqgssapi:
raise PythonImportError(
"Please install python-requests-gssapi to use GSSAPI."
)
# store calling parameters
self.auth_method = {
'method': 'gssapi_login',
'kwargs': {
'principal': principal, 'keytab': keytab, 'ccache': ccache, 'proxyuser': proxyuser,
'proxyauthtype': proxyauthtype
}
}
# force https
old_baseurl = self.baseurl
uri = six.moves.urllib.parse.urlsplit(self.baseurl)
@ -2540,6 +2579,9 @@ class ClientSession(object):
# For this case we're now using retry=False and test errors for
# this exact usecase.
kwargs = {'proxyuser': proxyuser}
if renew:
kwargs['renew'] = True
kwargs['exclusive'] = self.exclusive
if proxyauthtype is not None:
kwargs['proxyauthtype'] = proxyauthtype
for tries in range(self.opts.get('max_retries', 30)):
@ -2587,7 +2629,26 @@ class ClientSession(object):
self.authtype = AUTHTYPES['GSSAPI']
return True
def ssl_login(self, cert=None, ca=None, serverca=None, proxyuser=None, proxyauthtype=None):
def ssl_login(self, cert=None, ca=None, serverca=None, proxyuser=None, proxyauthtype=None,
renew=False):
"""
SSL cert based login
:param str cert: path to SSL certificate
:param str ca: deprecated, not used anymore
:param str serverca: path for CA public cert, otherwise system-wide CAs are used
:param str proxyuser: name of proxied user (e.g. forwarding by web ui)
:param int proxyauthtype: AUTHTYPE used by proxied user (can be different from ours)
:returns bool: success
"""
# store calling parameters
self.auth_method = {
'method': 'ssl_login',
'kwargs': {
'cert': cert, 'ca': ca, 'serverca': serverca,
'proxyuser': proxyuser, 'proxyauthtype': proxyauthtype,
}
}
cert = cert or self.opts.get('cert')
serverca = serverca or self.opts.get('serverca')
if cert is None:
@ -2617,6 +2678,9 @@ class ClientSession(object):
e_str = None
try:
kwargs = {'proxyuser': proxyuser}
if renew:
kwargs['renew'] = True
kwargs['exclusive'] = self.exclusive
if proxyauthtype is not None:
kwargs['proxyauthtype'] = proxyauthtype
sinfo = self._callMethod('sslLogin', [], kwargs)
@ -2628,6 +2692,7 @@ class ClientSession(object):
sinfo = None
finally:
self.opts = old_opts
if not sinfo:
err = 'unable to obtain a session'
if e_str:
@ -2680,8 +2745,6 @@ class ClientSession(object):
self.new_session()
# forget our login session, if any
if not self.logged_in:
return
self.setSession(None)
# we've had some trouble with this method causing strange problems
@ -2707,24 +2770,28 @@ class ClientSession(object):
return self._prepUpload(*args, **kwargs)
args = encode_args(*args, **kwargs)
headers = []
if self.logged_in:
sinfo = None
if getattr(self, 'sinfo') is not None:
# send sinfo in headers if we have it
# still needed if not logged in for renewal case
sinfo = self.sinfo.copy()
sinfo['callnum'] = self.callnum
self.callnum += 1
if sinfo.get('header-auth'):
handler = self.baseurl
headers += [
('Koji-Session-Id', str(self.sinfo['session-id'])),
('Koji-Session-Key', str(self.sinfo['session-key'])),
('Koji-Session-Callnum', str(sinfo['callnum'])),
]
else:
# old server
handler = "%s?%s" % (self.baseurl, six.moves.urllib.parse.urlencode(sinfo))
headers += [
('Koji-Session-Id', str(sinfo['session-id'])),
('Koji-Session-Key', str(sinfo['session-key'])),
('Koji-Session-Callnum', str(sinfo['callnum'])),
]
if self.logged_in and not self.sinfo.get('header-auth'):
# old server
handler = "%s?%s" % (self.baseurl, six.moves.urllib.parse.urlencode(sinfo))
elif name == 'sslLogin':
handler = self.baseurl + '/ssllogin'
else:
handler = self.baseurl
request = dumps(args, name, allow_none=1)
if six.PY3:
# For python2, dumps() without encoding specified means return a str
@ -2833,9 +2900,30 @@ class ClientSession(object):
result = result[0]
return result
def _renew_session(self):
"""Renew expirated session or subsession."""
if not hasattr(self, 'auth_method'):
raise GenericError("Missing info for reauthentication")
auth_method = getattr(self, self.auth_method['method'])
args = self.auth_method.get('args', [])
kwargs = self.auth_method.get('kwargs', {})
kwargs['renew'] = True
self.logged_in = False
auth_method(*args, **kwargs)
def renew_expired_session(func):
"""Decorator to renew expirated session or subsession."""
def _renew_expired_session(self, *args, **kwargs):
try:
return func(self, *args, **kwargs)
except AuthExpired:
self._renew_session()
return func(self, *args, **kwargs)
return _renew_expired_session
@renew_expired_session
def _callMethod(self, name, args, kwargs=None, retry=True):
"""Make a call to the hub with retries and other niceties"""
if self.multicall:
if kwargs is None:
kwargs = {}
@ -3183,6 +3271,11 @@ class ClientSession(object):
result = self.callMethod('downloadTaskOutput', taskID, fileName, **dlopts)
return base64.b64decode(result)
def exclusiveSession(self, force=False):
"""Make this session exclusive"""
self._callMethod('exclusiveSession', {'force': force})
self.exclusive = True
class MultiCallHack(object):
"""Workaround of a terribly overloaded namespace

View file

@ -56,6 +56,8 @@ RetryWhitelist = [
'repoProblem',
]
AUTH_METHODS = ['login', 'sslLogin']
logger = logging.getLogger('koji.auth')
@ -82,8 +84,8 @@ class Session(object):
args = environ.get('QUERY_STRING', '')
# prefer new header-based sessions
if 'HTTP_KOJI_SESSION_ID' in environ:
id = int(environ['HTTP_KOJI_SESSION_ID'])
key = environ['HTTP_KOJI_SESSION_KEY']
self.id = int(environ['HTTP_KOJI_SESSION_ID'])
self.key = environ['HTTP_KOJI_SESSION_KEY']
try:
callnum = int(environ['HTTP_KOJI_CALLNUM'])
except KeyError:
@ -96,8 +98,8 @@ class Session(object):
return
args = urllib.parse.parse_qs(args, strict_parsing=True)
try:
id = int(args['session-id'][0])
key = args['session-key'][0]
self.id = int(args['session-id'][0])
self.key = args['session-key'][0]
except KeyError as field:
raise koji.AuthError('%s not specified in session args' % field)
try:
@ -118,24 +120,28 @@ class Session(object):
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},
clauses=['id = %(id)i', 'key = %(key)s', 'hostip = %(hostip)s',
'closed IS FALSE'],
values={'id': self.id, 'key': self.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})
clauses=['id = %(id)i'], values={'id': self.id})
row = query.executeOne(strict=False)
if row:
if key != row['key']:
logger.warning("Session ID %s is not related to session key %s.", id, key)
if self.key != row['key']:
logger.warning("Session ID %s is not related to session key %s.",
self.id, self.key)
elif hostip != row['hostip']:
logger.warning("Session ID %s is not related to host IP %s.", id, hostip)
logger.warning("Session ID %s is not related to host IP %s.", self.id, hostip)
raise koji.AuthError('Invalid session or bad credentials')
# check for expiration
if session_data['expired']:
raise koji.AuthExpired('session "%i" has expired' % id)
if getattr(context, 'method') not in AUTH_METHODS:
raise koji.AuthExpired('session "%s" has expired' % self.id)
# check for callnum sanity
if callnum is not None:
try:
@ -145,8 +151,7 @@ class Session(object):
lastcall = session_data['callnum']
if lastcall is not None:
if lastcall > callnum:
raise koji.SequenceError("%d > %d (session %d)"
% (lastcall, callnum, id))
raise koji.SequenceError("%s > %s (session %s)" % (lastcall, callnum, self.id))
elif lastcall == callnum:
# Some explanation:
# This function is one of the few that performs its own commit.
@ -159,8 +164,11 @@ class Session(object):
method = getattr(context, 'method', 'UNKNOWN')
if method not in RetryWhitelist:
raise koji.RetryError(
"unable to retry call %d (method %s) for session %d"
% (callnum, method, id))
"unable to retry call %s (method %s) for session %s" %
(callnum, method, self.id))
if session_data['expired']:
return
# read user data
# historical note:
@ -182,7 +190,7 @@ class Session(object):
# see if an exclusive session exists
query = QueryProcessor(tables=['sessions'], columns=['id'],
clauses=['user_id=%(user_id)s', 'exclusive = TRUE',
'expired = FALSE'],
'closed = FALSE'],
values=session_data)
excl_id = query.singleValue(strict=False)
@ -200,21 +208,19 @@ class Session(object):
# update timestamp
update = UpdateProcessor('sessions', rawdata={'update_time': 'NOW()'},
clauses=['id = %(id)i'], values={'id': id})
clauses=['id = %(id)i'], values={'id': self.id})
update.execute()
context.cnx.commit()
# update callnum (this is deliberately after the commit)
# see earlier note near RetryError
if callnum is not None:
update = UpdateProcessor('sessions', data={'callnum': callnum},
clauses=['id = %(id)i'], values={'id': id})
clauses=['id = %(id)i'], values={'id': self.id})
update.execute()
# we only want to commit the callnum change if there are other commits
context.commit_pending = False
# record the login data
self.id = id
self.key = key
self.hostip = hostip
self.callnum = callnum
self.user_id = session_data['user_id']
@ -280,7 +286,7 @@ class Session(object):
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):
def login(self, user, password, opts=None, renew=False, exclusive=False):
"""create a login session"""
if opts is None:
opts = {}
@ -301,7 +307,9 @@ class Session(object):
self.checkLoginAllowed(user_id)
# create session and return
sinfo = self.createSession(user_id, hostip, koji.AUTHTYPES['NORMAL'])
sinfo = self.createSession(user_id, hostip, koji.AUTHTYPES['NORMAL'], renew=renew)
if sinfo and exclusive and not self.exclusive:
self.makeExclusive()
context.cnx.commit()
return sinfo
@ -326,7 +334,7 @@ class Session(object):
return (local_ip, local_port, remote_ip, remote_port)
def sslLogin(self, proxyuser=None, proxyauthtype=None):
def sslLogin(self, proxyuser=None, proxyauthtype=None, renew=False, exclusive=None):
"""Login into brew via SSL. proxyuser name can be specified and if it is
allowed in the configuration file then connection is allowed to login as
@ -404,7 +412,9 @@ class Session(object):
hostip = self.get_remote_ip()
sinfo = self.createSession(user_id, hostip, authtype)
sinfo = self.createSession(user_id, hostip, authtype, renew=renew)
if sinfo and exclusive and not self.exclusive:
self.makeExclusive()
return sinfo
def makeExclusive(self, force=False):
@ -420,16 +430,17 @@ class Session(object):
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
# check that no other sessions for this user are exclusive (including expired)
query = QueryProcessor(tables=['sessions'], columns=['id'],
clauses=['user_id=%(user_id)s', 'expired = FALSE',
clauses=['user_id=%(user_id)s', 'closed = 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
update = UpdateProcessor('sessions', data={'expired': True, 'exclusive': None},
# close the previous exclusive sessions and try again
update = UpdateProcessor('sessions',
data={'expired': True, 'exclusive': None, 'closed': True},
clauses=['id=%(excl_id)s'], values={'excl_id': excl_id},)
update.execute()
else:
@ -449,7 +460,7 @@ class Session(object):
context.cnx.commit()
def logout(self, session_id=None):
"""expire a login session"""
"""close a login session"""
if not self.logged_in:
# XXX raise an error?
raise koji.AuthError("Not logged in")
@ -464,7 +475,8 @@ class Session(object):
ses_id = session_id
else:
ses_id = self.id
update = UpdateProcessor('sessions', data={'expired': True, 'exclusive': None},
update = UpdateProcessor('sessions',
data={'expired': True, 'exclusive': None, 'closed': True},
clauses=['id = %(id)i OR master = %(id)i'],
values={'id': ses_id})
update.execute()
@ -473,17 +485,18 @@ class Session(object):
self.logged_in = False
def logoutChild(self, session_id):
"""expire a subsession"""
"""close a subsession"""
if not self.logged_in:
# XXX raise an error?
raise koji.AuthError("Not logged in")
update = UpdateProcessor('sessions', data={'expired': True, 'exclusive': None},
update = UpdateProcessor('sessions',
data={'expired': True, 'exclusive': None, 'closed': True},
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):
def createSession(self, user_id, hostip, authtype, master=None, renew=False):
"""Create a new session for the given user.
Return a map containing the session-id and session-key.
@ -495,14 +508,34 @@ class Session(object):
''.join([random.choice(alnum) for x in range(1, 20)]))
# use sha? sha.new(phrase).hexdigest()
# get a session id
session_id = nextval('sessions_id_seq')
if renew and self.id is not None:
# just update key
session_id = self.id
self.key = key
if self.master:
# check if master session died meanwhile (expired is ok)
query = QueryProcessor(tables=['sessions'],
clauses=['id = %(master_id)d', 'closed IS FALSE'],
values={'master_id': self.master},
opts={'countOnly': True})
if query.executeOne() == 0:
return None
# add session id to database
insert = InsertProcessor('sessions',
data={'id': session_id, 'user_id': user_id, 'key': key,
'hostip': hostip, 'authtype': authtype, 'master': master})
insert.execute()
update = UpdateProcessor('sessions',
clauses=['id=%(id)i'],
rawdata={'update_time': 'NOW()'},
data={'key': self.key, 'expired': False},
values={'id': self.id})
update.execute()
else:
# get a session id
session_id = nextval('sessions_id_seq')
# add session id to database
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
@ -519,8 +552,7 @@ class Session(object):
master = self.master
if master is None:
master = self.id
return self.createSession(self.user_id, self.hostip, self.authtype,
master=master)
return self.createSession(self.user_id, self.hostip, self.authtype, master=master)
def getPerms(self):
if not self.logged_in:

View file

@ -140,7 +140,8 @@ class TestAuthSession(unittest.TestCase):
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.clauses, ['closed IS FALSE', '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)",
@ -160,7 +161,7 @@ class TestAuthSession(unittest.TestCase):
query = self.queries[2]
self.assertEqual(query.tables, ['sessions'])
self.assertEqual(query.joins, None)
self.assertEqual(query.clauses, ['exclusive = TRUE', 'expired = FALSE',
self.assertEqual(query.clauses, ['closed = FALSE', 'exclusive = TRUE',
'user_id=%(user_id)s'])
self.assertEqual(query.columns, ['id'])
@ -190,7 +191,8 @@ class TestAuthSession(unittest.TestCase):
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.clauses, ['closed IS FALSE', '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)",
@ -210,7 +212,7 @@ class TestAuthSession(unittest.TestCase):
query = self.queries[2]
self.assertEqual(query.tables, ['sessions'])
self.assertEqual(query.joins, None)
self.assertEqual(query.clauses, ['exclusive = TRUE', 'expired = FALSE',
self.assertEqual(query.clauses, ['closed = FALSE', 'exclusive = TRUE',
'user_id=%(user_id)s'])
self.assertEqual(query.columns, ['id'])
@ -434,7 +436,7 @@ class TestAuthSession(unittest.TestCase):
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.data, {'closed': True, 'expired': True, 'exclusive': None})
self.assertEqual(update.rawdata, {})
def test_logoutChild_not_logged(self):
@ -460,7 +462,7 @@ class TestAuthSession(unittest.TestCase):
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.data, {'expired': True, 'exclusive': None, 'closed': True})
self.assertEqual(update.rawdata, {})
def test_makeExclusive_not_master(self):
@ -513,7 +515,7 @@ class TestAuthSession(unittest.TestCase):
query = self.queries[4]
self.assertEqual(query.tables, ['sessions'])
self.assertEqual(query.joins, None)
self.assertEqual(query.clauses, ['exclusive = TRUE', 'expired = FALSE',
self.assertEqual(query.clauses, ['closed = FALSE', 'exclusive = TRUE',
'user_id=%(user_id)s'])
self.assertEqual(query.columns, ['id'])
self.assertEqual(query.values, {'user_id': 1})
@ -525,7 +527,7 @@ class TestAuthSession(unittest.TestCase):
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.data, {'expired': True, 'exclusive': None, 'closed': True})
self.assertEqual(update.rawdata, {})
update = self.updates[3]
@ -668,15 +670,6 @@ class TestAuthSession(unittest.TestCase):
self.assertEqual(query.clauses, ['active = TRUE', 'user_id=%(user_id)s'])
self.assertEqual(query.columns, ['name'])
def test_logout_not_logged(self):
s, cntext = self.get_session()
# not logged
s.logged_in = False
with self.assertRaises(koji.AuthError) as ex:
s.logout()
self.assertEqual("Not logged in", str(ex.exception))
@mock.patch('koji.auth.context')
def test_logout_logged_not_owner(self, context):
s, cntext = self.get_session()