From 2c347b4b77be90dbfa424e2f275fbe9525decb65 Mon Sep 17 00:00:00 2001
From: Jana Cupova <root@localhost>
Date: Dec 12 2022 11:44:12 +0000
Subject: [PATCH 1/9] Create new session when old session was timeout


Fixes: https://pagure.io/koji/issue/3394

---

diff --git a/koji/__init__.py b/koji/__init__.py
index e1422d9..db92f49 100644
--- a/koji/__init__.py
+++ b/koji/__init__.py
@@ -2406,6 +2406,9 @@ def grab_session_options(options):
         'upload_blocksize',
         'no_ssl_verify',
         'serverca',
+        'keytab',
+        'principal',
+        'ccache',
     )
     # cert is omitted for now
     if isinstance(options, dict):
@@ -2442,6 +2445,8 @@ class ClientSession(object):
         self.rsession = None
         self.new_session()
         self.opts.setdefault('timeout', DEFAULT_REQUEST_TIMEOUT)
+        self.exclusive = False
+        self.hostip = None
 
     @property
     def multicall(self):
@@ -2473,13 +2478,16 @@ class ClientSession(object):
             self.callnum = None
             # do we need to do anything else here?
             self.authtype = None
+            self.session_key = None
         else:
             self.logged_in = True
             self.callnum = 0
+            self.session_key = sinfo['session-key']
         self.sinfo = sinfo
 
     def login(self, opts=None):
-        sinfo = self.callMethod('login', self.opts['user'], self.opts['password'], opts)
+        sinfo = self.callMethod('login', self.opts['user'], self.opts['password'],
+                                self.opts['session_key'], opts)
         if not sinfo:
             return False
         self.setSession(sinfo)
@@ -2492,7 +2500,7 @@ class ClientSession(object):
         return type(self)(self.baseurl, self.opts, sinfo)
 
     def gssapi_login(self, principal=None, keytab=None, ccache=None,
-                     proxyuser=None, proxyauthtype=None):
+                     proxyuser=None, proxyauthtype=None, session_key=None):
         if not reqgssapi:
             raise PythonImportError(
                 "Please install python-requests-gssapi to use GSSAPI."
@@ -2537,7 +2545,7 @@ class ClientSession(object):
                 # will fail with a handshake failure, which is retried by default.
                 # For this case we're now using retry=False and test errors for
                 # this exact usecase.
-                kwargs = {'proxyuser': proxyuser}
+                kwargs = {'proxyuser': proxyuser, 'session_key': session_key}
                 if proxyauthtype is not None:
                     kwargs['proxyauthtype'] = proxyauthtype
                 for tries in range(self.opts.get('max_retries', 30)):
@@ -2585,7 +2593,8 @@ 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,
+                  session_key=None):
         cert = cert or self.opts.get('cert')
         serverca = serverca or self.opts.get('serverca')
         if cert is None:
@@ -2614,7 +2623,7 @@ class ClientSession(object):
         self.opts['serverca'] = serverca
         e_str = None
         try:
-            kwargs = {'proxyuser': proxyuser}
+            kwargs = {'proxyuser': proxyuser, 'session_key': session_key}
             if proxyauthtype is not None:
                 kwargs['proxyauthtype'] = proxyauthtype
             sinfo = self._callMethod('sslLogin', [], kwargs)
@@ -2831,6 +2840,32 @@ class ClientSession(object):
             result = result[0]
         return result
 
+    def _renew_session(self):
+        session_key = self.session_key
+        self.setSession(None)
+        if self.authtype == 'SSL' or \
+                (self.opts.get('cert') and os.path.isfile(self.opts['cert'])):
+            self.ssl_login(cert=self.opts['cert'],
+                           serverca=self.opts['serverca'],
+                           session_key=session_key)
+        elif self.authtype == 'NORMAL' or self.opts.get('user'):
+            self.login(user=self.opts['user'], password=self.opts['password'],
+                       session_key=session_key)
+        elif self.authtype in ['KERBEROS', 'GSSAPI'] or \
+                self.opts.get('krb_principal'):
+            authtype = self.authtype or AUTHTYPES['GSSAPI']
+            principal = self.opts.get('principal')
+            keytab = self.opts.get('keytab')
+            ccache = self.opts.get('ccache')
+            if authtype == 'KERBEROS':
+                self.krb_login(principal=principal, keytab=keytab,
+                               ccache=ccache, session_key=session_key)
+            elif authtype == 'GSSAPI':
+                self.gssapi_login(self, principal=principal, keytab=keytab,
+                                  ccache=ccache, session_key=session_key)
+        if self.exclusive:
+            self.exclusiveSession()
+
     def _callMethod(self, name, args, kwargs=None, retry=True):
         """Make a call to the hub with retries and other niceties"""
 
@@ -2869,7 +2904,16 @@ class ClientSession(object):
                             # server correctly reporting an outage
                             tries = 0
                             continue
-                    raise err
+                    elif isinstance(err, AuthExpired):
+                        if self.logged_in:
+                            self._renew_session()
+                            return self._callMethod(name, args, kwargs, retry)
+                        else:
+                            raise AuthError("Session ID %s is unlogged and expired." %
+                                            self.sinfo['session-id'])
+                    else:
+                        raise err
+
                 except (SystemExit, KeyboardInterrupt):
                     # (depending on the python version, these may or may not be subclasses of
                     # Exception)
@@ -3181,6 +3225,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
diff --git a/koji/auth.py b/koji/auth.py
index c9a4aee..5486f05 100644
--- a/koji/auth.py
+++ b/koji/auth.py
@@ -278,7 +278,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, session_key=None, opts=None):
         """create a login session"""
         if opts is None:
             opts = {}
@@ -299,7 +299,8 @@ 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'],
+                                   session_key=session_key)
         context.cnx.commit()
         return sinfo
 
@@ -324,7 +325,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, session_key=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
@@ -410,7 +411,7 @@ class Session(object):
 
         hostip = self.get_remote_ip()
 
-        sinfo = self.createSession(user_id, hostip, authtype)
+        sinfo = self.createSession(user_id, hostip, authtype, session_key=session_key)
         return sinfo
 
     def makeExclusive(self, force=False):
@@ -489,12 +490,22 @@ class Session(object):
         update.execute()
         context.cnx.commit()
 
-    def createSession(self, user_id, hostip, authtype, master=None):
+    def createSession(self, user_id, hostip, authtype, master=None, session_key=None):
         """Create a new session for the given user.
 
         Return a map containing the session-id and session-key.
         If master is specified, create a subsession
         """
+        if session_key:
+            query = QueryProcessor(tables=['sessions'], columns=['master'],
+                                   clauses=['key=%(session_key)d'],
+                                   values={'session_key': session_key})
+            row = query.executeOne(strict=False)
+            if not row:
+                raise koji.GenericError("Don't allow to renew subsession, "
+                                        "subsession doesn't exist.")
+            master = row['master']
+
         # generate a random key
         alnum = string.ascii_letters + string.digits
         key = "%s-%s" % (user_id,
diff --git a/tests/test_lib/test_gssapi.py b/tests/test_lib/test_gssapi.py
index 6249221..1c7d918 100644
--- a/tests/test_lib/test_gssapi.py
+++ b/tests/test_lib/test_gssapi.py
@@ -27,7 +27,7 @@ class TestGSSAPI(unittest.TestCase):
         old_environ = dict(**os.environ)
         self.session.gssapi_login()
         self.session._callMethod.assert_called_with(
-            'sslLogin', [], {'proxyuser': None}, retry=False)
+            'sslLogin', [], {'proxyuser': None, 'session_key': None}, retry=False)
         self.assertEqual(old_environ, dict(**os.environ))
 
     @mock.patch('koji.reqgssapi.HTTPKerberosAuth')
@@ -47,7 +47,7 @@ class TestGSSAPI(unittest.TestCase):
             koji.reqgssapi.__version__ = accepted_version
             rv = self.session.gssapi_login(principal, keytab, ccache)
             self.session._callMethod.assert_called_with(
-                'sslLogin', [], {'proxyuser': None}, retry=False)
+                'sslLogin', [], {'proxyuser': None, 'session_key': None}, retry=False)
             self.assertEqual(old_environ, dict(**os.environ))
             self.assertTrue(rv)
             self.session._callMethod.reset_mock()
@@ -84,7 +84,7 @@ class TestGSSAPI(unittest.TestCase):
         with self.assertRaises(koji.GSSAPIAuthError):
             self.session.gssapi_login()
         self.session._callMethod.assert_called_with(
-            'sslLogin', [], {'proxyuser': None}, retry=False)
+            'sslLogin', [], {'proxyuser': None, 'session_key': None}, retry=False)
         self.assertEqual(old_environ, dict(**os.environ))
 
     def test_gssapi_login_http(self):

From 4f1bf08455698ccf7431a7cb9f0de5567376eb9d Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Dec 12 2022 11:44:14 +0000
Subject: [PATCH 2/9] store original auth method


---

diff --git a/koji/__init__.py b/koji/__init__.py
index db92f49..9f2e65e 100644
--- a/koji/__init__.py
+++ b/koji/__init__.py
@@ -2406,9 +2406,6 @@ def grab_session_options(options):
         'upload_blocksize',
         'no_ssl_verify',
         'serverca',
-        'keytab',
-        'principal',
-        'ccache',
     )
     # cert is omitted for now
     if isinstance(options, dict):
@@ -2425,7 +2422,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 = {}
@@ -2446,7 +2449,7 @@ class ClientSession(object):
         self.new_session()
         self.opts.setdefault('timeout', DEFAULT_REQUEST_TIMEOUT)
         self.exclusive = False
-        self.hostip = None
+        self.auth_method = auth_method
 
     @property
     def multicall(self):
@@ -2485,9 +2488,18 @@ class ClientSession(object):
             self.session_key = sinfo['session-key']
         self.sinfo = sinfo
 
-    def login(self, opts=None):
+    def login(self, opts=None, session_key=None):
+        """
+        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}}
         sinfo = self.callMethod('login', self.opts['user'], self.opts['password'],
-                                self.opts['session_key'], opts)
+                                opts=opts, session_key=session_key)
         if not sinfo:
             return False
         self.setSession(sinfo)
@@ -2497,14 +2509,33 @@ 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, session_key=None):
+        """
+        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)
+        :param str session_key: used for session renewal
+        :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, 'session_key': session_key
+            }
+        }
         # force https
         old_baseurl = self.baseurl
         uri = six.moves.urllib.parse.urlsplit(self.baseurl)
@@ -2595,6 +2626,26 @@ class ClientSession(object):
 
     def ssl_login(self, cert=None, ca=None, serverca=None, proxyuser=None, proxyauthtype=None,
                   session_key=None):
+        """
+        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)
+        :param str session_key: used for session renewal
+        :returns bool: success
+        """
+        # store calling parameters
+        self.logger.error("ssl_login---------------")
+        self.auth_method = {
+            'method': 'ssl_login',
+            'kwargs': {
+                'cert': cert, 'ca': ca, 'serverca': serverca, 'proxyuser': proxyuser,
+                'proxyauthtype': proxyauthtype, 'session_key': session_key,
+            }
+        }
         cert = cert or self.opts.get('cert')
         serverca = serverca or self.opts.get('serverca')
         if cert is None:
@@ -2841,28 +2892,15 @@ class ClientSession(object):
         return result
 
     def _renew_session(self):
-        session_key = self.session_key
+        if not hasattr(self, 'auth_method'):
+            raise GenericError("Missing info for reauthentication")
+        # will be deleted by setSession
+        auth_method = self.auth_method['method']
+        args = self.auth_method.get('args', [])
+        kwargs = self.auth_method.get('kwargs', {})
+        kwargs['session_key'] = self.session_key
         self.setSession(None)
-        if self.authtype == 'SSL' or \
-                (self.opts.get('cert') and os.path.isfile(self.opts['cert'])):
-            self.ssl_login(cert=self.opts['cert'],
-                           serverca=self.opts['serverca'],
-                           session_key=session_key)
-        elif self.authtype == 'NORMAL' or self.opts.get('user'):
-            self.login(user=self.opts['user'], password=self.opts['password'],
-                       session_key=session_key)
-        elif self.authtype in ['KERBEROS', 'GSSAPI'] or \
-                self.opts.get('krb_principal'):
-            authtype = self.authtype or AUTHTYPES['GSSAPI']
-            principal = self.opts.get('principal')
-            keytab = self.opts.get('keytab')
-            ccache = self.opts.get('ccache')
-            if authtype == 'KERBEROS':
-                self.krb_login(principal=principal, keytab=keytab,
-                               ccache=ccache, session_key=session_key)
-            elif authtype == 'GSSAPI':
-                self.gssapi_login(self, principal=principal, keytab=keytab,
-                                  ccache=ccache, session_key=session_key)
+        auth_method(*args, **kwargs)
         if self.exclusive:
             self.exclusiveSession()
 
diff --git a/koji/auth.py b/koji/auth.py
index 5486f05..412d03a 100644
--- a/koji/auth.py
+++ b/koji/auth.py
@@ -278,7 +278,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, session_key=None, opts=None):
+    def login(self, user, password, opts=None, session_key=None):
         """create a login session"""
         if opts is None:
             opts = {}
@@ -497,14 +497,14 @@ class Session(object):
         If master is specified, create a subsession
         """
         if session_key:
+            if master:
+                raise koji.GenericError("Can't call createSession with both master + session_key.")
             query = QueryProcessor(tables=['sessions'], columns=['master'],
                                    clauses=['key=%(session_key)d'],
                                    values={'session_key': session_key})
-            row = query.executeOne(strict=False)
-            if not row:
-                raise koji.GenericError("Don't allow to renew subsession, "
-                                        "subsession doesn't exist.")
-            master = row['master']
+            master = query.singleValue(strict=False)
+            if not master:
+                raise koji.GenericError("Don't allow to renew non-existent subsession")
 
         # generate a random key
         alnum = string.ascii_letters + string.digits

From 06d98adf074730a6c65065f719ea6f7b16f7b883 Mon Sep 17 00:00:00 2001
From: Jana Cupova <root@localhost>
Date: Dec 12 2022 11:44:14 +0000
Subject: [PATCH 3/9] Fix call auth_method


---

diff --git a/koji/__init__.py b/koji/__init__.py
index 9f2e65e..816050a 100644
--- a/koji/__init__.py
+++ b/koji/__init__.py
@@ -2895,7 +2895,7 @@ class ClientSession(object):
         if not hasattr(self, 'auth_method'):
             raise GenericError("Missing info for reauthentication")
         # will be deleted by setSession
-        auth_method = self.auth_method['method']
+        auth_method = getattr(self, self.auth_method['method'])
         args = self.auth_method.get('args', [])
         kwargs = self.auth_method.get('kwargs', {})
         kwargs['session_key'] = self.session_key
diff --git a/koji/auth.py b/koji/auth.py
index 412d03a..649c25e 100644
--- a/koji/auth.py
+++ b/koji/auth.py
@@ -502,9 +502,10 @@ class Session(object):
             query = QueryProcessor(tables=['sessions'], columns=['master'],
                                    clauses=['key=%(session_key)d'],
                                    values={'session_key': session_key})
-            master = query.singleValue(strict=False)
-            if not master:
+            row = query.executeOne(strict=False)
+            if not row:
                 raise koji.GenericError("Don't allow to renew non-existent subsession")
+            master = row['master']
 
         # generate a random key
         alnum = string.ascii_letters + string.digits

From fd96b4073ec2f48c8c15ef213139364afe3769a1 Mon Sep 17 00:00:00 2001
From: Jana Cupova <root@localhost>
Date: Dec 12 2022 11:44:51 +0000
Subject: [PATCH 4/9] Add closed column to session table and use it in session


---

diff --git a/docs/schema-update-1.31-1.32.sql b/docs/schema-update-1.31-1.32.sql
index 2c25886..e8b242f 100644
--- a/docs/schema-update-1.31-1.32.sql
+++ b/docs/schema-update-1.31-1.32.sql
@@ -2,8 +2,9 @@
 -- 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';
 COMMIT;
diff --git a/docs/schema.sql b/docs/schema.sql
index 39b7893..74fde66 100644
--- a/docs/schema.sql
+++ b/docs/schema.sql
@@ -119,6 +119,7 @@ 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 (
diff --git a/koji/auth.py b/koji/auth.py
index 649c25e..e42fe62 100644
--- a/koji/auth.py
+++ b/koji/auth.py
@@ -471,7 +471,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()
@@ -500,11 +501,11 @@ class Session(object):
             if master:
                 raise koji.GenericError("Can't call createSession with both master + session_key.")
             query = QueryProcessor(tables=['sessions'], columns=['master'],
-                                   clauses=['key=%(session_key)d'],
+                                   clauses=['key=%(session_key)d', 'closed=FALSE'],
                                    values={'session_key': session_key})
             row = query.executeOne(strict=False)
             if not row:
-                raise koji.GenericError("Don't allow to renew non-existent subsession")
+                raise koji.GenericError("Don't allow to renew non-existent or logged out session")
             master = row['master']
 
         # generate a random key

From e02d0d1d580b4b7fc3a2449d59253462229c52f8 Mon Sep 17 00:00:00 2001
From: Jana Cupova <root@localhost>
Date: Dec 12 2022 11:44:55 +0000
Subject: [PATCH 5/9] Add decorator for renew expired session


---

diff --git a/koji/__init__.py b/koji/__init__.py
index 816050a..a326a5c 100644
--- a/koji/__init__.py
+++ b/koji/__init__.py
@@ -2686,6 +2686,7 @@ class ClientSession(object):
             sinfo = None
         finally:
             self.opts = old_opts
+
         if not sinfo:
             err = 'unable to obtain a session'
             if e_str:
@@ -2892,6 +2893,7 @@ class ClientSession(object):
         return result
 
     def _renew_session(self):
+        """Renew expirated session or subsession."""
         if not hasattr(self, 'auth_method'):
             raise GenericError("Missing info for reauthentication")
         # will be deleted by setSession
@@ -2904,9 +2906,19 @@ class ClientSession(object):
         if self.exclusive:
             self.exclusiveSession()
 
+    def renew_expired_session(func):
+        """Decorator to renew expirated session or subsession."""
+        def _renew_expired_session(*args, **kwargs):
+            try:
+                return func(*args, **kwargs)
+            except AuthExpired:
+                args[0]._renew_session()
+                return func(*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 = {}
@@ -2942,13 +2954,6 @@ class ClientSession(object):
                             # server correctly reporting an outage
                             tries = 0
                             continue
-                    elif isinstance(err, AuthExpired):
-                        if self.logged_in:
-                            self._renew_session()
-                            return self._callMethod(name, args, kwargs, retry)
-                        else:
-                            raise AuthError("Session ID %s is unlogged and expired." %
-                                            self.sinfo['session-id'])
                     else:
                         raise err
 

From 08c15e96d08c0af8e2ab67e3030d6c1a3de7b829 Mon Sep 17 00:00:00 2001
From: Jana Cupova <root@localhost>
Date: Dec 12 2022 11:44:55 +0000
Subject: [PATCH 6/9] Fix unit tests


---

diff --git a/tests/test_lib/test_auth.py b/tests/test_lib/test_auth.py
index 3568107..963d068 100644
--- a/tests/test_lib/test_auth.py
+++ b/tests/test_lib/test_auth.py
@@ -434,7 +434,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):
@@ -668,15 +668,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()

From c60eb7cc0c72c268f69c75357b39dd057d9abe99 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Dec 12 2022 11:45:40 +0000
Subject: [PATCH 7/9] remove passing session-id


---

diff --git a/docs/schema-update-1.31-1.32.sql b/docs/schema-update-1.31-1.32.sql
index e8b242f..55a45e4 100644
--- a/docs/schema-update-1.31-1.32.sql
+++ b/docs/schema-update-1.31-1.32.sql
@@ -6,5 +6,6 @@ BEGIN;
     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 COLUMN closed BOOLEAN NOT NULL DEFAULT FALSE;
+    ALTER TABLE sessions ADD CONSTRAINT no_closed_exclusive CHECK (closed IS FALSE OR "exclusive" IS NULL);
 COMMIT;
diff --git a/koji/__init__.py b/koji/__init__.py
index a326a5c..546efd7 100644
--- a/koji/__init__.py
+++ b/koji/__init__.py
@@ -2481,14 +2481,12 @@ class ClientSession(object):
             self.callnum = None
             # do we need to do anything else here?
             self.authtype = None
-            self.session_key = None
         else:
             self.logged_in = True
             self.callnum = 0
-            self.session_key = sinfo['session-key']
         self.sinfo = sinfo
 
-    def login(self, opts=None, session_key=None):
+    def login(self, opts=None, renew=False):
         """
         Username/password based login method
 
@@ -2498,8 +2496,8 @@ class ClientSession(object):
         """
         # store calling parameters
         self.auth_method = {'method': 'login', 'kwargs': {'opts': opts}}
-        sinfo = self.callMethod('login', self.opts['user'], self.opts['password'],
-                                opts=opts, session_key=session_key)
+        sinfo = self.callMethod('login', self.opts['user'], self.opts['password'], opts=opts,
+                                renew=renew)
         if not sinfo:
             return False
         self.setSession(sinfo)
@@ -2512,7 +2510,7 @@ class ClientSession(object):
         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, session_key=None):
+                     proxyuser=None, proxyauthtype=None, renew=False):
         """
         GSSAPI/Kerberos login method
 
@@ -2521,7 +2519,6 @@ class ClientSession(object):
         :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)
-        :param str session_key: used for session renewal
         :returns bool True: success or raises exception
         """
         if not reqgssapi:
@@ -2533,7 +2530,7 @@ class ClientSession(object):
             'method': 'gssapi_login',
             'kwargs': {
                 'principal': principal, 'keytab': keytab, 'ccache': ccache, 'proxyuser': proxyuser,
-                'proxyauthtype': proxyauthtype, 'session_key': session_key
+                'proxyauthtype': proxyauthtype
             }
         }
         # force https
@@ -2576,7 +2573,7 @@ class ClientSession(object):
                 # will fail with a handshake failure, which is retried by default.
                 # For this case we're now using retry=False and test errors for
                 # this exact usecase.
-                kwargs = {'proxyuser': proxyuser, 'session_key': session_key}
+                kwargs = {'proxyuser': proxyuser, 'renew': renew}
                 if proxyauthtype is not None:
                     kwargs['proxyauthtype'] = proxyauthtype
                 for tries in range(self.opts.get('max_retries', 30)):
@@ -2625,7 +2622,7 @@ class ClientSession(object):
         return True
 
     def ssl_login(self, cert=None, ca=None, serverca=None, proxyuser=None, proxyauthtype=None,
-                  session_key=None):
+                  renew=False):
         """
         SSL cert based login
 
@@ -2634,16 +2631,14 @@ class ClientSession(object):
         :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)
-        :param str session_key: used for session renewal
         :returns bool: success
         """
         # store calling parameters
-        self.logger.error("ssl_login---------------")
         self.auth_method = {
             'method': 'ssl_login',
             'kwargs': {
-                'cert': cert, 'ca': ca, 'serverca': serverca, 'proxyuser': proxyuser,
-                'proxyauthtype': proxyauthtype, 'session_key': session_key,
+                'cert': cert, 'ca': ca, 'serverca': serverca,
+                'proxyuser': proxyuser, 'proxyauthtype': proxyauthtype,
             }
         }
         cert = cert or self.opts.get('cert')
@@ -2674,7 +2669,7 @@ class ClientSession(object):
         self.opts['serverca'] = serverca
         e_str = None
         try:
-            kwargs = {'proxyuser': proxyuser, 'session_key': session_key}
+            kwargs = {'proxyuser': proxyuser, 'renew': renew}
             if proxyauthtype is not None:
                 kwargs['proxyauthtype'] = proxyauthtype
             sinfo = self._callMethod('sslLogin', [], kwargs)
@@ -2766,24 +2761,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:
+            # session renewal (not logged in, but have session data)
+            # makes sense only for new method/server
             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))
-        elif name == 'sslLogin':
+            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 in '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
@@ -2896,12 +2895,11 @@ class ClientSession(object):
         """Renew expirated session or subsession."""
         if not hasattr(self, 'auth_method'):
             raise GenericError("Missing info for reauthentication")
-        # will be deleted by setSession
         auth_method = getattr(self, self.auth_method['method'])
         args = self.auth_method.get('args', [])
         kwargs = self.auth_method.get('kwargs', {})
-        kwargs['session_key'] = self.session_key
-        self.setSession(None)
+        kwargs['renew'] = True
+        self.logged_in = False
         auth_method(*args, **kwargs)
         if self.exclusive:
             self.exclusiveSession()
diff --git a/koji/auth.py b/koji/auth.py
index e42fe62..55758ee 100644
--- a/koji/auth.py
+++ b/koji/auth.py
@@ -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:
@@ -119,23 +121,26 @@ class Session(object):
 
         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},
+                               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(f'session "{self.id}" has expired')
+
         # check for callnum sanity
         if callnum is not None:
             try:
@@ -145,8 +150,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(f"{lastcall} > {callnum} (session {self.id})")
                 elif lastcall == callnum:
                     # Some explanation:
                     # This function is one of the few that performs its own commit.
@@ -159,8 +163,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))
+                            f"unable to retry call {callnum} "
+                            f"(method {method}) for session {self.id}")
+
+        if session_data['expired']:
+            return
 
         # read user data
         # historical note:
@@ -200,19 +207,17 @@ 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', rawdata={'callnum': callnum},
-                                     clauses=['id = %(id)i'], values={'id': id})
+                                     clauses=['id = %(id)i'], values={'id': self.id})
             update.execute()
 
         # record the login data
-        self.id = id
-        self.key = key
         self.hostip = hostip
         self.callnum = callnum
         self.user_id = session_data['user_id']
@@ -325,7 +330,7 @@ class Session(object):
 
         return (local_ip, local_port, remote_ip, remote_port)
 
-    def sslLogin(self, proxyuser=None, proxyauthtype=None, session_key=None):
+    def sslLogin(self, proxyuser=None, proxyauthtype=None, renew=False):
 
         """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
@@ -411,7 +416,7 @@ class Session(object):
 
         hostip = self.get_remote_ip()
 
-        sinfo = self.createSession(user_id, hostip, authtype, session_key=session_key)
+        sinfo = self.createSession(user_id, hostip, authtype, renew=renew)
         return sinfo
 
     def makeExclusive(self, force=False):
@@ -491,37 +496,48 @@ class Session(object):
         update.execute()
         context.cnx.commit()
 
-    def createSession(self, user_id, hostip, authtype, master=None, session_key=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.
         If master is specified, create a subsession
         """
-        if session_key:
-            if master:
-                raise koji.GenericError("Can't call createSession with both master + session_key.")
-            query = QueryProcessor(tables=['sessions'], columns=['master'],
-                                   clauses=['key=%(session_key)d', 'closed=FALSE'],
-                                   values={'session_key': session_key})
-            row = query.executeOne(strict=False)
-            if not row:
-                raise koji.GenericError("Don't allow to renew non-existent or logged out session")
-            master = row['master']
-
         # generate a random key
         alnum = string.ascii_letters + string.digits
         key = "%s-%s" % (user_id,
                          ''.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')
-
-        # 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()
+        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
+                query = QueryProcessor(tables=['sessions'],
+                                       clauses=['id = %(master_id)d',
+                                                'expired IS FALSE',
+                                                'closed IS FALSE'],
+                                       values={'master_id': self.master},
+                                       opts={'countOnly': True})
+                if query.executeOne() == 0:
+                    return None
+
+            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
@@ -538,8 +554,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:

From 35be62cdf542d366042cf77be83eb8cf0409f66e Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Dec 12 2022 11:45:42 +0000
Subject: [PATCH 8/9] renew exclusive status as part of login


---

diff --git a/koji/__init__.py b/koji/__init__.py
index 546efd7..297abea 100644
--- a/koji/__init__.py
+++ b/koji/__init__.py
@@ -2496,8 +2496,11 @@ class ClientSession(object):
         """
         # store calling parameters
         self.auth_method = {'method': 'login', 'kwargs': {'opts': opts}}
-        sinfo = self.callMethod('login', self.opts['user'], self.opts['password'], opts=opts,
-                                renew=renew)
+        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)
@@ -2573,7 +2576,10 @@ class ClientSession(object):
                 # will fail with a handshake failure, which is retried by default.
                 # For this case we're now using retry=False and test errors for
                 # this exact usecase.
-                kwargs = {'proxyuser': proxyuser, 'renew': renew}
+                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)):
@@ -2669,7 +2675,10 @@ class ClientSession(object):
         self.opts['serverca'] = serverca
         e_str = None
         try:
-            kwargs = {'proxyuser': proxyuser, 'renew': renew}
+            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)
@@ -2901,8 +2910,6 @@ class ClientSession(object):
         kwargs['renew'] = True
         self.logged_in = False
         auth_method(*args, **kwargs)
-        if self.exclusive:
-            self.exclusiveSession()
 
     def renew_expired_session(func):
         """Decorator to renew expirated session or subsession."""
diff --git a/koji/auth.py b/koji/auth.py
index 55758ee..d15a6be 100644
--- a/koji/auth.py
+++ b/koji/auth.py
@@ -120,7 +120,8 @@ 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'],
+                               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)
@@ -146,7 +147,7 @@ class Session(object):
             try:
                 callnum = int(callnum)
             except (ValueError, TypeError):
-                raise koji.AuthError("Invalid callnum: %r" % callnum)
+                raise koji.AuthError(f"Invalid callnum: {callnum!r}")
             lastcall = session_data['callnum']
             if lastcall is not None:
                 if lastcall > callnum:
@@ -283,7 +284,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, session_key=None):
+    def login(self, user, password, opts=None, renew=False, exclusive=False):
         """create a login session"""
         if opts is None:
             opts = {}
@@ -305,7 +306,9 @@ class Session(object):
 
         # create session and return
         sinfo = self.createSession(user_id, hostip, koji.AUTHTYPES['NORMAL'],
-                                   session_key=session_key)
+                                   renew=renew)
+        if sinfo and exclusive and not self.exclusive:
+            self.makeExclusive()
         context.cnx.commit()
         return sinfo
 
@@ -330,7 +333,7 @@ class Session(object):
 
         return (local_ip, local_port, remote_ip, remote_port)
 
-    def sslLogin(self, proxyuser=None, proxyauthtype=None, renew=False):
+    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
@@ -417,6 +420,8 @@ class Session(object):
         hostip = self.get_remote_ip()
 
         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):

From a56c10585ff401405a51a411ae27539408a6b96c Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Dec 12 2022 11:49:11 +0000
Subject: [PATCH 9/9] retain expired session exclusivity


---

diff --git a/docs/schema-update-1.31-1.32.sql b/docs/schema-update-1.31-1.32.sql
index 55a45e4..c676333 100644
--- a/docs/schema-update-1.31-1.32.sql
+++ b/docs/schema-update-1.31-1.32.sql
@@ -8,4 +8,5 @@ BEGIN;
     -- 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;
diff --git a/docs/schema.sql b/docs/schema.sql
index 74fde66..9de4e14 100644
--- a/docs/schema.sql
+++ b/docs/schema.sql
@@ -122,8 +122,8 @@ CREATE TABLE sessions (
 	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);
diff --git a/koji/auth.py b/koji/auth.py
index d15a6be..ba3dea8 100644
--- a/koji/auth.py
+++ b/koji/auth.py
@@ -190,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)
 
@@ -305,8 +305,7 @@ class Session(object):
         self.checkLoginAllowed(user_id)
 
         # create session and return
-        sinfo = self.createSession(user_id, hostip, koji.AUTHTYPES['NORMAL'],
-                                   renew=renew)
+        sinfo = self.createSession(user_id, hostip, koji.AUTHTYPES['NORMAL'], renew=renew)
         if sinfo and exclusive and not self.exclusive:
             self.makeExclusive()
         context.cnx.commit()
@@ -437,9 +436,9 @@ 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)
@@ -466,7 +465,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")
@@ -491,11 +490,12 @@ 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()
@@ -518,11 +518,9 @@ class Session(object):
             session_id = self.id
             self.key = key
             if self.master:
-                # check if master session died meanwhile
+                # check if master session died meanwhile (expired is ok)
                 query = QueryProcessor(tables=['sessions'],
-                                       clauses=['id = %(master_id)d',
-                                                'expired IS FALSE',
-                                                'closed IS FALSE'],
+                                       clauses=['id = %(master_id)d', 'closed IS FALSE'],
                                        values={'master_id': self.master},
                                        opts={'countOnly': True})
                 if query.executeOne() == 0:

