From f8c38509a6adb4b48bcfcee6855a3bc873e5d294 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Nov 09 2022 14:45:35 +0000
Subject: [PATCH 1/5] cookies-based sessions


Related: https://pagure.io/koji/issue/3393

---

diff --git a/hub/kojixmlrpc.py b/hub/kojixmlrpc.py
index 7853c83..f11bb4e 100644
--- a/hub/kojixmlrpc.py
+++ b/hub/kojixmlrpc.py
@@ -19,6 +19,7 @@
 #       Mike McLean <mikem@redhat.com>
 
 import datetime
+import http.cookies
 import inspect
 import logging
 import os
@@ -811,6 +812,19 @@ def application(environ, start_response):
                 ('Content-Length', str(len(response))),
                 ('Content-Type', "text/xml"),
             ]
+            cookies = http.cookies.SimpleCookie(environ.get('HTTP_SET_COOKIE'))
+            if hasattr(context, 'session') and context.session.logged_in:
+                cookies['session-id'] = context.session.id
+                cookies['session-key'] = context.session.key
+                cookies['callnum'] = context.session.callnum
+                cookies['logged_in'] = "1"
+            else:
+                # good for not mistaking for the older hub with small overhead
+                cookies['logged_in'] = "0"
+            for c in cookies.values():
+                c.path = '/'
+                c.secure = True
+                headers.append(('Set-Cookie', c.OutputString()))
             start_response('200 OK', headers)
             if h.traceback:
                 # rollback
diff --git a/koji/__init__.py b/koji/__init__.py
index 3a02419..4c38ee3 100644
--- a/koji/__init__.py
+++ b/koji/__init__.py
@@ -57,6 +57,7 @@ except ImportError:  # pragma: no cover
 from fnmatch import fnmatch
 
 import dateutil.parser
+import http.cookies
 import requests
 import six
 import six.moves.configparser
@@ -2701,10 +2702,11 @@ class ClientSession(object):
             return self._prepUpload(*args, **kwargs)
         args = encode_args(*args, **kwargs)
         if self.logged_in:
-            sinfo = self.sinfo.copy()
-            sinfo['callnum'] = self.callnum
+            for c in self.rsession.cookies:
+                if c.name == 'callnum':
+                    c.value = str(self.callnum)
             self.callnum += 1
-            handler = "%s?%s" % (self.baseurl, six.moves.urllib.parse.urlencode(sinfo))
+            handler = self.baseurl
         elif name == 'sslLogin':
             handler = self.baseurl + '/ssllogin'
         else:
@@ -2799,6 +2801,13 @@ class ClientSession(object):
             warnings.simplefilter("ignore")
             r = self.rsession.post(handler, **callopts)
             r.raise_for_status()
+            if self.logged_in and len(r.cookies.items()) == 0:
+                # we have session, sent the cookies, but server is old
+                # and didn't sent them back, use old url-encoded style
+                sinfo = self.sinfo.copy()
+                handler = "%s?%s" % (handler, six.moves.urllib.parse.urlencode(sinfo))
+                r = self.rsession.post(handler, **callopts)
+                r.raise_for_status()
             try:
                 ret = self._read_xmlrpc_response(r)
             finally:
diff --git a/koji/auth.py b/koji/auth.py
index d1595a5..8db5f91 100644
--- a/koji/auth.py
+++ b/koji/auth.py
@@ -29,7 +29,6 @@ import string
 
 import six
 from six.moves import range, urllib
-
 import koji
 from .context import context
 from .util import to_list
@@ -79,24 +78,23 @@ class Session(object):
         self._perms = None
         self._groups = None
         self._host_id = ''
-        # get session data from request
-        if args is None:
-            environ = getattr(context, 'environ', {})
-            args = environ.get('QUERY_STRING', '')
-            if not args:
-                self.message = 'no session args'
-                return
-            args = urllib.parse.parse_qs(args, strict_parsing=True)
+        environ = getattr(context, 'environ', {})
+        # prefer new cookie-based sessions
+        if 'HTTP_COOKIE' in environ:
+            cookies = http.cookies.SimpleCookie(environ['HTTP_COOKIE'])
+            try:
+                id = int(cookies['session-id'].value)
+                key = str(cookies['session-key'].value)
+            except KeyError as field:
+                raise koji.AuthError('%s not specified in session args' % field)
+            try:
+                callnum = int(cookies['callnum'].value)
+            except KeyError:
+                callnum = None
+        else:
+            self.message = 'no session cookies'
+            return
         hostip = self.get_remote_ip(override=hostip)
-        try:
-            id = int(args['session-id'][0])
-            key = args['session-key'][0]
-        except KeyError as field:
-            raise koji.AuthError('%s not specified in session args' % field)
-        try:
-            callnum = args['callnum'][0]
-        except Exception:
-            callnum = None
         # lookup the session
         # sort for stability (unittests)
 
@@ -488,6 +486,12 @@ class Session(object):
         insert.execute()
         context.cnx.commit()
 
+        # update it here, so it can be propagated to the cookies in kojixmlrpc.py
+        context.session.id = session_id
+        context.session.key = key
+        context.session.logged_in = True
+        context.session.callnum = 0
+
         # return session info
         return {'session-id': session_id, 'session-key': key}
 

From 4e9c1fe07ebd2315b3f08f4e49687365811604a6 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Nov 09 2022 14:45:35 +0000
Subject: [PATCH 2/5] use header-based auth


---

diff --git a/docs/source/hub_conf.rst b/docs/source/hub_conf.rst
index 158d739..fbe503c 100644
--- a/docs/source/hub_conf.rst
+++ b/docs/source/hub_conf.rst
@@ -153,6 +153,17 @@ The following options control aspects of authentication when using ``mod_auth_gs
       option.
       The default value of False is recommended.
 
+   DisableURLSessions
+      Type: boolean
+
+      Default: ``False``
+
+      If set to ``False``, it enables older clients to log in via session parameters
+      encoded in URL. New behaviour uses header-based parameteres. This default
+      will be changed in future to ``True`` effectively disabling older clients. It is
+      encouraged to set it to ``True`` as soon as possible when no older clients are
+      using the hub. (Added in 1.30)
+
 Enabling gssapi auth also requires settings in the httpd config.
 
 SSL client certificate auth configuration
diff --git a/hub/hub.conf b/hub/hub.conf
index 783cdd7..90fd9a6 100644
--- a/hub/hub.conf
+++ b/hub/hub.conf
@@ -35,6 +35,8 @@ KojiDir = /mnt/koji
 # AllowedKrbRealms = *
 ## TODO: this option should be removed in future release
 # DisableGSSAPIProxyDNFallback = False
+## TODO: this option should be turned True in future release
+# DisableURLSessions = False
 
 ## end Kerberos auth configuration
 
diff --git a/hub/kojixmlrpc.py b/hub/kojixmlrpc.py
index f11bb4e..5596d05 100644
--- a/hub/kojixmlrpc.py
+++ b/hub/kojixmlrpc.py
@@ -19,7 +19,7 @@
 #       Mike McLean <mikem@redhat.com>
 
 import datetime
-import http.cookies
+import email
 import inspect
 import logging
 import os
@@ -449,6 +449,8 @@ def load_config(environ):
         ['AllowedKrbRealms', 'string', '*'],
         # TODO: this option should be removed in future release
         ['DisableGSSAPIProxyDNFallback', 'boolean', False],
+        # TODO:  this option should be turned True in future release
+        ['DisableURLSessions', 'boolean', False],
 
         ['DNUsernameComponent', 'string', 'CN'],
         ['ProxyDNs', 'string', ''],
@@ -808,24 +810,18 @@ def application(environ, start_response):
             except RequestTimeout as e:
                 return error_reply(start_response, '408 Request Timeout', str(e) + '\n')
             response = response.encode()
-            headers = [
-                ('Content-Length', str(len(response))),
-                ('Content-Type', "text/xml"),
-            ]
-            cookies = http.cookies.SimpleCookie(environ.get('HTTP_SET_COOKIE'))
+            headers = email.message.EmailMessage()
+            headers['Content-Length'] = str(len(response))
+            headers['Content-Type'] = "text/xml"
             if hasattr(context, 'session') and context.session.logged_in:
-                cookies['session-id'] = context.session.id
-                cookies['session-key'] = context.session.key
-                cookies['callnum'] = context.session.callnum
-                cookies['logged_in'] = "1"
-            else:
-                # good for not mistaking for the older hub with small overhead
-                cookies['logged_in'] = "0"
-            for c in cookies.values():
-                c.path = '/'
-                c.secure = True
-                headers.append(('Set-Cookie', c.OutputString()))
-            start_response('200 OK', headers)
+                headers.add_header(
+                    'X-Session-Data',
+                    '1',  # logged in
+                    session_id=str(context.session.id),
+                    session_key=str(context.session.key),
+                    callnum=str(context.session.callnum),
+                )
+            start_response('200 OK', headers.items())
             if h.traceback:
                 # rollback
                 context.cnx.rollback()
diff --git a/koji/__init__.py b/koji/__init__.py
index 4c38ee3..5ae933f 100644
--- a/koji/__init__.py
+++ b/koji/__init__.py
@@ -26,6 +26,7 @@ from __future__ import absolute_import, division
 
 import base64
 import datetime
+import email
 import errno
 import hashlib
 import json
@@ -57,7 +58,6 @@ except ImportError:  # pragma: no cover
 from fnmatch import fnmatch
 
 import dateutil.parser
-import http.cookies
 import requests
 import six
 import six.moves.configparser
@@ -2701,12 +2701,19 @@ class ClientSession(object):
         if name == 'rawUpload':
             return self._prepUpload(*args, **kwargs)
         args = encode_args(*args, **kwargs)
+        headers = email.message.EmailMessage()
         if self.logged_in:
-            for c in self.rsession.cookies:
-                if c.name == 'callnum':
-                    c.value = str(self.callnum)
+            sinfo = self.sinfo.copy()
+            sinfo['callnum'] = self.callnum
             self.callnum += 1
-            handler = self.baseurl
+            if sinfo.get('header-auth'):
+                for k, v in sinfo.items():
+                    sinfo[k] = str(v)
+                handler = self.baseurl
+                headers.add_header('X-Session-Data', '1', **sinfo)
+            else:
+                # old server
+                handler = "%s?%s" % (self.baseurl, six.moves.urllib.parse.urlencode(sinfo))
         elif name == 'sslLogin':
             handler = self.baseurl + '/ssllogin'
         else:
@@ -2717,12 +2724,9 @@ class ClientSession(object):
             # encoded as UTF-8. For python3 it means "return a str with an appropriate
             # xml declaration for encoding as UTF-8".
             request = request.encode('utf-8')
-        headers = [
-            # connection class handles Host
-            ('User-Agent', 'koji/1'),
-            ('Content-Type', 'text/xml'),
-            ('Content-Length', str(len(request))),
-        ]
+        headers['User-Agent'] = 'koji/1'
+        headers['Content-Type'] = 'text/xml'
+        headers['Content-Length'] = str(len(request))
         return handler, headers, request
 
     def _sanitize_url(self, url):
@@ -2801,13 +2805,6 @@ class ClientSession(object):
             warnings.simplefilter("ignore")
             r = self.rsession.post(handler, **callopts)
             r.raise_for_status()
-            if self.logged_in and len(r.cookies.items()) == 0:
-                # we have session, sent the cookies, but server is old
-                # and didn't sent them back, use old url-encoded style
-                sinfo = self.sinfo.copy()
-                handler = "%s?%s" % (handler, six.moves.urllib.parse.urlencode(sinfo))
-                r = self.rsession.post(handler, **callopts)
-                r.raise_for_status()
             try:
                 ret = self._read_xmlrpc_response(r)
             finally:
@@ -3039,24 +3036,31 @@ class ClientSession(object):
         """prep a rawUpload call"""
         if not self.logged_in:
             raise ActionNotAllowed("you must be logged in to upload")
-        args = self.sinfo.copy()
-        args['callnum'] = self.callnum
-        args['filename'] = name
-        args['filepath'] = path
-        args['fileverify'] = verify
-        args['offset'] = str(offset)
+        sinfo = self.sinfo.copy()
+        sinfo['callnum'] = self.callnum
+        args = {
+            'filename': name,
+            'filepath': path,
+            'fileverify': verify,
+            'offset': str(offset),
+        }
         if overwrite:
             args['overwrite'] = "1"
         if volume is not None:
             args['volume'] = volume
         size = len(chunk)
         self.callnum += 1
+        headers = email.message.EmailMessage()
+        if sinfo.get('header-auth'):
+            for k, v in sinfo.items():
+                sinfo[k] = str(v)
+            headers.add_header('X-Session-Data', '1', **sinfo)
+        else:
+            args.update(sinfo)
         handler = "%s?%s" % (self.baseurl, six.moves.urllib.parse.urlencode(args))
-        headers = [
-            ('User-Agent', 'koji/1'),
-            ("Content-Type", "application/octet-stream"),
-            ("Content-length", str(size)),
-        ]
+        headers['User-Agent'] = 'koji/1'
+        headers["Content-Type"] = "application/octet-stream"
+        headers["Content-length"] = str(size)
         request = chunk
         if six.PY3 and isinstance(chunk, str):
             request = chunk.encode('utf-8')
diff --git a/koji/auth.py b/koji/auth.py
index 8db5f91..82f45a3 100644
--- a/koji/auth.py
+++ b/koji/auth.py
@@ -79,20 +79,47 @@ class Session(object):
         self._groups = None
         self._host_id = ''
         environ = getattr(context, 'environ', {})
-        # prefer new cookie-based sessions
-        if 'HTTP_COOKIE' in environ:
-            cookies = http.cookies.SimpleCookie(environ['HTTP_COOKIE'])
+        # prefer new header-based sessions
+        if 'HTTP_X_SESSION_DATA' in environ:
+            header = environ['HTTP_X_SESSION_DATA']
+            params = header.split(';')
+            id, key, callnum = None, None, None
+            for p in params[1:]:
+                k, v = [x.strip() for x in p.split('=')]
+                v = v.strip('"')
+                if k == 'session-id':
+                    id = int(v)
+                elif k == 'session-key':
+                    key = v
+                elif k == 'callnum':
+                    callnum = v
+                elif k == 'header-auth':
+                    pass
+                else:
+                    raise koji.AuthError("Unexpected key in X-Session-Data: %s" % k)
+            if id is None:
+                raise koji.AuthError('session-id not specified in session args')
+            elif key is None:
+                raise koji.AuthError('session-key not specified in session args')
+        elif not context.opts['DisableURLSessions'] and args is not None:
+            # old deprecated method with session values in query string
+            # Option will be turned off by default in future release and removed later
+            args = environ.get('QUERY_STRING', '')
+            if not args:
+                self.message = 'nor session header nor session args'
+                return
+            args = urllib.parse.parse_qs(args, strict_parsing=True)
             try:
-                id = int(cookies['session-id'].value)
-                key = str(cookies['session-key'].value)
+                id = int(args['session-id'][0])
+                key = args['session-key'][0]
             except KeyError as field:
                 raise koji.AuthError('%s not specified in session args' % field)
             try:
-                callnum = int(cookies['callnum'].value)
-            except KeyError:
+                callnum = args['callnum'][0]
+            except Exception:
                 callnum = None
         else:
-            self.message = 'no session cookies'
+            self.message = 'no X-Session-Data header'
             return
         hostip = self.get_remote_ip(override=hostip)
         # lookup the session
@@ -486,14 +513,18 @@ class Session(object):
         insert.execute()
         context.cnx.commit()
 
-        # update it here, so it can be propagated to the cookies in kojixmlrpc.py
+        # update it here, so it can be propagated to the headers in kojixmlrpc.py
         context.session.id = session_id
         context.session.key = key
         context.session.logged_in = True
         context.session.callnum = 0
 
         # return session info
-        return {'session-id': session_id, 'session-key': key}
+        return {
+            'session-id': session_id,
+            'session-key': key,
+            'header-auth': True,  # signalize to client to use new session handling in 1.30
+        }
 
     def subsession(self):
         "Create a subsession"

From cb202f0207a3b411424c58825c3cd07566a70079 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Nov 09 2022 14:45:35 +0000
Subject: [PATCH 3/5] separate headers


---

diff --git a/docs/source/hub_conf.rst b/docs/source/hub_conf.rst
index fbe503c..0870a98 100644
--- a/docs/source/hub_conf.rst
+++ b/docs/source/hub_conf.rst
@@ -162,7 +162,7 @@ The following options control aspects of authentication when using ``mod_auth_gs
       encoded in URL. New behaviour uses header-based parameteres. This default
       will be changed in future to ``True`` effectively disabling older clients. It is
       encouraged to set it to ``True`` as soon as possible when no older clients are
-      using the hub. (Added in 1.30)
+      using the hub. (Added in 1.30, will be removed in 1.34)
 
 Enabling gssapi auth also requires settings in the httpd config.
 
diff --git a/hub/hub.conf b/hub/hub.conf
index 90fd9a6..0ea5164 100644
--- a/hub/hub.conf
+++ b/hub/hub.conf
@@ -35,7 +35,7 @@ KojiDir = /mnt/koji
 # AllowedKrbRealms = *
 ## TODO: this option should be removed in future release
 # DisableGSSAPIProxyDNFallback = False
-## TODO: this option should be turned True in future release
+## TODO: this option should be turned True in 1.34
 # DisableURLSessions = False
 
 ## end Kerberos auth configuration
diff --git a/hub/kojixmlrpc.py b/hub/kojixmlrpc.py
index 5596d05..e74138a 100644
--- a/hub/kojixmlrpc.py
+++ b/hub/kojixmlrpc.py
@@ -449,7 +449,7 @@ def load_config(environ):
         ['AllowedKrbRealms', 'string', '*'],
         # TODO: this option should be removed in future release
         ['DisableGSSAPIProxyDNFallback', 'boolean', False],
-        # TODO:  this option should be turned True in future release
+        # TODO:  this option should be turned True in 1.34
         ['DisableURLSessions', 'boolean', False],
 
         ['DNUsernameComponent', 'string', 'CN'],
@@ -810,18 +810,17 @@ def application(environ, start_response):
             except RequestTimeout as e:
                 return error_reply(start_response, '408 Request Timeout', str(e) + '\n')
             response = response.encode()
-            headers = email.message.EmailMessage()
-            headers['Content-Length'] = str(len(response))
-            headers['Content-Type'] = "text/xml"
+            headers = [
+                ('Content-Length', str(len(response))),
+                ('Content-Type', "text/xml"),
+            ]
             if hasattr(context, 'session') and context.session.logged_in:
-                headers.add_header(
-                    'X-Session-Data',
-                    '1',  # logged in
-                    session_id=str(context.session.id),
-                    session_key=str(context.session.key),
-                    callnum=str(context.session.callnum),
-                )
-            start_response('200 OK', headers.items())
+                headers += [
+                    ('Koji-Session-Id', str(context.session.id)),
+                    ('Koji-Session-Key', str(context.session.key)),
+                    ('Koji-Session-Callnum', str(context.session.callnum)),
+                ]
+            start_response('200 OK', headers)
             if h.traceback:
                 # rollback
                 context.cnx.rollback()
diff --git a/koji/__init__.py b/koji/__init__.py
index 5ae933f..8494020 100644
--- a/koji/__init__.py
+++ b/koji/__init__.py
@@ -26,7 +26,6 @@ from __future__ import absolute_import, division
 
 import base64
 import datetime
-import email
 import errno
 import hashlib
 import json
@@ -2701,16 +2700,18 @@ class ClientSession(object):
         if name == 'rawUpload':
             return self._prepUpload(*args, **kwargs)
         args = encode_args(*args, **kwargs)
-        headers = email.message.EmailMessage()
+        headers = []
         if self.logged_in:
             sinfo = self.sinfo.copy()
             sinfo['callnum'] = self.callnum
             self.callnum += 1
             if sinfo.get('header-auth'):
-                for k, v in sinfo.items():
-                    sinfo[k] = str(v)
                 handler = self.baseurl
-                headers.add_header('X-Session-Data', '1', **sinfo)
+                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))
@@ -2724,9 +2725,12 @@ class ClientSession(object):
             # encoded as UTF-8. For python3 it means "return a str with an appropriate
             # xml declaration for encoding as UTF-8".
             request = request.encode('utf-8')
-        headers['User-Agent'] = 'koji/1'
-        headers['Content-Type'] = 'text/xml'
-        headers['Content-Length'] = str(len(request))
+        headers += [
+            # connection class handles Host
+            ('User-Agent', 'koji/1'),
+            ('Content-Type', 'text/xml'),
+            ('Content-Length', str(len(request))),
+        ]
         return handler, headers, request
 
     def _sanitize_url(self, url):
@@ -3050,17 +3054,21 @@ class ClientSession(object):
             args['volume'] = volume
         size = len(chunk)
         self.callnum += 1
-        headers = email.message.EmailMessage()
+        headers = []
         if sinfo.get('header-auth'):
-            for k, v in sinfo.items():
-                sinfo[k] = str(v)
-            headers.add_header('X-Session-Data', '1', **sinfo)
+            headers += [
+                ('Koji-Session-Id', str(self.sinfo['session-id'])),
+                ('Koji-Session-Key', str(self.sinfo['session-key'])),
+                ('Koji-Session-Callnum', str(sinfo['callnum'])),
+            ]
         else:
             args.update(sinfo)
         handler = "%s?%s" % (self.baseurl, six.moves.urllib.parse.urlencode(args))
-        headers['User-Agent'] = 'koji/1'
-        headers["Content-Type"] = "application/octet-stream"
-        headers["Content-length"] = str(size)
+        headers += [
+            ('User-Agent', 'koji/1'),
+            ("Content-Type", "application/octet-stream"),
+            ("Content-length", str(size)),
+        ]
         request = chunk
         if six.PY3 and isinstance(chunk, str):
             request = chunk.encode('utf-8')
diff --git a/koji/auth.py b/koji/auth.py
index 82f45a3..d635338 100644
--- a/koji/auth.py
+++ b/koji/auth.py
@@ -80,27 +80,13 @@ class Session(object):
         self._host_id = ''
         environ = getattr(context, 'environ', {})
         # prefer new header-based sessions
-        if 'HTTP_X_SESSION_DATA' in environ:
-            header = environ['HTTP_X_SESSION_DATA']
-            params = header.split(';')
-            id, key, callnum = None, None, None
-            for p in params[1:]:
-                k, v = [x.strip() for x in p.split('=')]
-                v = v.strip('"')
-                if k == 'session-id':
-                    id = int(v)
-                elif k == 'session-key':
-                    key = v
-                elif k == 'callnum':
-                    callnum = v
-                elif k == 'header-auth':
-                    pass
-                else:
-                    raise koji.AuthError("Unexpected key in X-Session-Data: %s" % k)
-            if id is None:
-                raise koji.AuthError('session-id not specified in session args')
-            elif key is None:
-                raise koji.AuthError('session-key not specified in session args')
+        if 'HTTP_KOJI_SESSION_ID' in environ:
+            id = int(environ['HTTP_KOJI_SESSION_ID'])
+            key = environ['HTTP_KOJI_SESSION_KEY']
+            try:
+                callnum = int(environ['HTTP_KOJI_CALLNUM'])
+            except KeyError:
+                callnum = None
         elif not context.opts['DisableURLSessions'] and args is not None:
             # old deprecated method with session values in query string
             # Option will be turned off by default in future release and removed later
@@ -119,7 +105,7 @@ class Session(object):
             except Exception:
                 callnum = None
         else:
-            self.message = 'no X-Session-Data header'
+            self.message = 'no Koji-Session-* headers'
             return
         hostip = self.get_remote_ip(override=hostip)
         # lookup the session

From 9eafc38b62509e9cc15c6ce06779da1eb66450f1 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Nov 09 2022 14:45:35 +0000
Subject: [PATCH 4/5] remove unused code


---

diff --git a/hub/kojixmlrpc.py b/hub/kojixmlrpc.py
index e74138a..746b5b2 100644
--- a/hub/kojixmlrpc.py
+++ b/hub/kojixmlrpc.py
@@ -19,7 +19,6 @@
 #       Mike McLean <mikem@redhat.com>
 
 import datetime
-import email
 import inspect
 import logging
 import os
@@ -814,12 +813,6 @@ def application(environ, start_response):
                 ('Content-Length', str(len(response))),
                 ('Content-Type', "text/xml"),
             ]
-            if hasattr(context, 'session') and context.session.logged_in:
-                headers += [
-                    ('Koji-Session-Id', str(context.session.id)),
-                    ('Koji-Session-Key', str(context.session.key)),
-                    ('Koji-Session-Callnum', str(context.session.callnum)),
-                ]
             start_response('200 OK', headers)
             if h.traceback:
                 # rollback
diff --git a/koji/auth.py b/koji/auth.py
index d635338..81c1397 100644
--- a/koji/auth.py
+++ b/koji/auth.py
@@ -79,6 +79,7 @@ class Session(object):
         self._groups = None
         self._host_id = ''
         environ = getattr(context, 'environ', {})
+        args = environ.get('QUERY_STRING', '')
         # prefer new header-based sessions
         if 'HTTP_KOJI_SESSION_ID' in environ:
             id = int(environ['HTTP_KOJI_SESSION_ID'])
@@ -90,9 +91,8 @@ class Session(object):
         elif not context.opts['DisableURLSessions'] and args is not None:
             # old deprecated method with session values in query string
             # Option will be turned off by default in future release and removed later
-            args = environ.get('QUERY_STRING', '')
             if not args:
-                self.message = 'nor session header nor session args'
+                self.message = 'no session header or session args'
                 return
             args = urllib.parse.parse_qs(args, strict_parsing=True)
             try:
@@ -499,12 +499,6 @@ class Session(object):
         insert.execute()
         context.cnx.commit()
 
-        # update it here, so it can be propagated to the headers in kojixmlrpc.py
-        context.session.id = session_id
-        context.session.key = key
-        context.session.logged_in = True
-        context.session.callnum = 0
-
         # return session info
         return {
             'session-id': session_id,

From 62e85e812da8174c109f2aaf90ba1746d93d2bcd Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Nov 10 2022 10:06:22 +0000
Subject: [PATCH 5/5] fix tests


---

diff --git a/tests/test_lib/test_auth.py b/tests/test_lib/test_auth.py
index d94aea9..35d7c8b 100644
--- a/tests/test_lib/test_auth.py
+++ b/tests/test_lib/test_auth.py
@@ -43,16 +43,27 @@ class TestAuthSession(unittest.TestCase):
         # start with "assert"
         self.context.session.assertLogin = mock.MagicMock()
 
-    def test_instance(self):
+    @mock.patch('koji.auth.context')
+    def test_instance(self, context):
         """Simple auth.Session instance"""
-        s = koji.auth.Session()
+        context.opts = {
+            'CheckClientIP': True,
+            'DisableURLSessions': False,
+        }
+        with self.assertRaises(koji.GenericError) as cm:
+            koji.auth.Session()
         # no args in request/environment
-        self.assertEqual(s.message, 'no session args')
+        self.assertEqual(cm.exception.args[0], "'session-id' not specified in session args")
 
     @mock.patch('koji.auth.context')
-    def get_session(self, context):
+    def get_session_old(self, context):
         """auth.Session instance"""
         # base session from test_basic_instance
+        # url-based auth - will be dropped in 1.34
+        context.opts = {
+            'CheckClientIP': True,
+            'DisableURLSessions': False,
+        }
         context.environ = {
             'QUERY_STRING': 'session-id=123&session-key=xyz&callnum=345',
             'REMOTE_ADDR': 'remote-addr',
@@ -72,6 +83,38 @@ class TestAuthSession(unittest.TestCase):
         s = koji.auth.Session()
         return s, context
 
+    @mock.patch('koji.auth.context')
+    def get_session(self, context):
+        # base session from test_basic_instance
+        # header-based auth
+        context.opts = {
+            'CheckClientIP': True,
+            'DisableURLSessions': True,
+        }
+        context.environ = {
+            'HTTP_KOJI_SESSION_ID': '123',
+            'HTTP_KOJI_SESSION_KEY': 'xyz',
+            'HTTP_KOJI_CALLNUM': '345',
+            'REMOTE_ADDR': 'remote-addr',
+        }
+
+        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
+
+    def test_session_old(self):
+        self.get_session_old()
+
     def test_basic_instance(self):
         """auth.Session instance"""
         s, cntext = self.get_session()

