From b1f614bca8c33be78f0a5d395b5bdcbcd8a2c0f1 Mon Sep 17 00:00:00 2001 From: Jana Cupova Date: Apr 13 2023 11:22:46 +0000 Subject: [PATCH 1/3] Add renewal session timeout Fixes: https://pagure.io/koji/issue/3596 --- diff --git a/docs/source/hub_conf.rst b/docs/source/hub_conf.rst index 7738fbc..d8c13bf 100644 --- a/docs/source/hub_conf.rst +++ b/docs/source/hub_conf.rst @@ -112,6 +112,14 @@ General authentication options Whether or not to automatically create a new user from valid ssl or gssapi credentials. + SessionRenewalTimeout + Type: integer + + Default: ``1440`` + + The number of minutes before sessions are required to re-authenticate. + Set to 0 for no timeout. + GSSAPI authentication options ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -553,4 +561,4 @@ We have default checksums types for create rpm checksums. Default: ``md5 sha256`` - Set RPM default checksums type. Default value is set upt to ``md5 sha256``. + Set RPM default checksums type. Default value is set up to ``md5 sha256``. diff --git a/kojihub/app/hub.conf b/kojihub/app/hub.conf index 9ea7aa6..ea4d4c0 100644 --- a/kojihub/app/hub.conf +++ b/kojihub/app/hub.conf @@ -144,3 +144,6 @@ NotifyOnSuccess = True ## Determines default checksums # RPMDefaultChecksums = md5 sha256 + +# The number of minutes before sessions are required to re-authenticate. Set to 0 for no timeout. +# SessionRenewalTimeout = 1440 diff --git a/kojihub/auth.py b/kojihub/auth.py index 1547fd7..b3e2687 100644 --- a/kojihub/auth.py +++ b/kojihub/auth.py @@ -26,6 +26,7 @@ import random import re import socket import string +import time import six from six.moves import range, urllib @@ -137,7 +138,18 @@ class Session(object): 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 not session_data['expired'] and context.opts['SessionRenewalTimeout'] != 0: + renewal_cutoff = (session_data['start_ts'] + + context.opts['SessionRenewalTimeout'] * 60) + if time.time() > renewal_cutoff: + session_data['expired'] = True + update = UpdateProcessor('sessions', + data={'expired': True}, + clauses=['id = %(id)s OR master = %(id)s'], + values={'id': self.id}) + update.execute() + context.cnx.commit() + if session_data['expired']: if getattr(context, 'method') not in AUTH_METHODS: raise koji.AuthExpired('session "%s" has expired' % self.id) diff --git a/kojihub/kojixmlrpc.py b/kojihub/kojixmlrpc.py index 3139071..e9e0b3d 100644 --- a/kojihub/kojixmlrpc.py +++ b/kojihub/kojixmlrpc.py @@ -499,7 +499,9 @@ def load_config(environ): ['RegexNameInternal', 'string', r'^[A-Za-z0-9/_.+-]+$'], ['RegexUserName', 'string', r'^[A-Za-z0-9/_.@-]+$'], - ['RPMDefaultChecksums', 'string', 'md5 sha256'] + ['RPMDefaultChecksums', 'string', 'md5 sha256'], + + ['SessionRenewalTimeout', 'integer', 1440], ] opts = {} for name, dtype, default in cfgmap: diff --git a/tests/test_lib/test_auth.py b/tests/test_lib/test_auth.py index 2fae67a..606101b 100644 --- a/tests/test_lib/test_auth.py +++ b/tests/test_lib/test_auth.py @@ -49,6 +49,7 @@ class TestAuthSession(unittest.TestCase): self.context.opts = { 'CheckClientIP': True, 'DisableURLSessions': False, + 'SessionRenewalTimeout': 0, } with self.assertRaises(koji.GenericError) as cm: kojihub.auth.Session() @@ -62,6 +63,7 @@ class TestAuthSession(unittest.TestCase): self.context.opts = { 'CheckClientIP': True, 'DisableURLSessions': False, + 'SessionRenewalTimeout': 0, } self.context.environ = { 'QUERY_STRING': 'session-id=123&session-key=xyz&callnum=345', @@ -88,6 +90,7 @@ class TestAuthSession(unittest.TestCase): self.context.opts = { 'CheckClientIP': True, 'DisableURLSessions': True, + 'SessionRenewalTimeout': 0, } self.context.environ = { 'HTTP_KOJI_SESSION_ID': '123', @@ -113,6 +116,58 @@ class TestAuthSession(unittest.TestCase): def test_session_old(self): self.get_session_old() + def test_renewal_timeout(self): + """Simple kojihub.auth.Session instance""" + self.context.opts = { + 'CheckClientIP': True, + 'DisableURLSessions': False, + 'SessionRenewalTimeout': 1440, + } + self.context.environ = { + 'QUERY_STRING': 'session-id=123&session-key=xyz&callnum=345', + 'REMOTE_ADDR': 'remote-addr', + } + + self.query_executeOne.side_effect = [ + {'authtype': 2, 'callnum': 1, "start_ts": 1666599426.227002, + "update_ts": 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}] + with self.assertRaises(koji.GenericError) as cm: + kojihub.auth.Session() + # no args in request/environment + self.assertEqual(cm.exception.args[0], 'session "123" has expired') + + self.assertEqual(len(self.updates), 1) + self.assertEqual(len(self.queries), 1) + + update = self.updates[0] + + self.assertEqual(update.table, 'sessions') + self.assertEqual(update.values['id'], 123) + self.assertEqual(update.clauses, ['id = %(id)s OR master = %(id)s']) + self.assertEqual(update.data, {'expired': True}) + self.assertEqual(update.rawdata, {}) + + query = self.queries[0] + self.assertEqual(query.tables, ['sessions']) + self.assertEqual(query.joins, None) + 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)", + 'user_id']) + self.assertEqual(query.aliases, ['authtype', 'callnum', 'exclusive', 'expired', 'master', + 'start_time', 'start_ts', 'update_time', 'update_ts', + 'user_id']) + self.assertEqual(query.values, {'id': 123, 'key': 'xyz', 'hostip': 'remote-addr'}) + def test_basic_instance(self): """auth.Session instance""" s, cntext = self.get_session() From 13166bb22cfb63207b610af439b6281dd61b2683 Mon Sep 17 00:00:00 2001 From: Jana Cupova Date: Apr 21 2023 07:00:50 +0000 Subject: [PATCH 2/3] Add renew_ts column and check with renew_ts --- diff --git a/docs/schema-upgrade-1.32-1.33.sql b/docs/schema-upgrade-1.32-1.33.sql new file mode 100644 index 0000000..3dd5db9 --- /dev/null +++ b/docs/schema-upgrade-1.32-1.33.sql @@ -0,0 +1,7 @@ +-- upgrade script to migrate the Koji database schema +-- from version 1.32 to 1.33 + +BEGIN; + ALTER TABLE sessions ADD COLUMN renew_time TIMESTAMPTZ DEFAULT NULL; +COMMIT; + diff --git a/docs/schema.sql b/docs/schema.sql index 16e1cef..0ff74fd 100644 --- a/docs/schema.sql +++ b/docs/schema.sql @@ -120,6 +120,7 @@ CREATE TABLE sessions ( update_time TIMESTAMPTZ NOT NULL DEFAULT NOW(), exclusive BOOLEAN CHECK (exclusive), closed BOOLEAN NOT NULL DEFAULT FALSE, + renew_time TIMESTAMPTZ DEFAULT NULL, CONSTRAINT no_exclusive_subsessions CHECK ( master IS NULL OR "exclusive" IS NULL), CONSTRAINT no_closed_exclusive CHECK ( diff --git a/kojihub/auth.py b/kojihub/auth.py index b3e2687..5b95557 100644 --- a/kojihub/auth.py +++ b/kojihub/auth.py @@ -117,7 +117,8 @@ class Session(object): fields = (('authtype', 'authtype'), ('callnum', 'callnum'), ('exclusive', 'exclusive'), ('expired', 'expired'), ('master', 'master'), ('start_time', 'start_time'), ('update_time', 'update_time'), ("date_part('epoch', start_time)", 'start_ts'), - ("date_part('epoch', update_time)", 'update_ts'), ('user_id', 'user_id')) + ("date_part('epoch', update_time)", 'update_ts'), ('user_id', 'user_id'), + ("date_part('epoch', renew_time)", 'renew_ts')) columns, aliases = zip(*fields) query = QueryProcessor(tables=['sessions'], columns=columns, aliases=aliases, @@ -139,8 +140,12 @@ class Session(object): raise koji.AuthError('Invalid session or bad credentials') if not session_data['expired'] and context.opts['SessionRenewalTimeout'] != 0: - renewal_cutoff = (session_data['start_ts'] + - context.opts['SessionRenewalTimeout'] * 60) + if session_data['renew_ts']: + renewal_cutoff = (session_data['renew_ts'] + + context.opts['SessionRenewalTimeout'] * 60) + else: + renewal_cutoff = (session_data['start_ts'] + + context.opts['SessionRenewalTimeout'] * 60) if time.time() > renewal_cutoff: session_data['expired'] = True update = UpdateProcessor('sessions', @@ -535,7 +540,7 @@ class Session(object): update = UpdateProcessor('sessions', clauses=['id=%(id)i'], - rawdata={'update_time': 'NOW()'}, + rawdata={'update_time': 'NOW()', 'renew_time': 'NOW()'}, data={'key': self.key, 'expired': False}, values={'id': self.id}) update.execute() From 805fb2f727acf62d93ba964b299fe561295a20c1 Mon Sep 17 00:00:00 2001 From: Jana Cupova Date: Apr 26 2023 05:52:16 +0000 Subject: [PATCH 3/3] Drop default Null for renew_time in sql --- diff --git a/docs/schema-upgrade-1.32-1.33.sql b/docs/schema-upgrade-1.32-1.33.sql index 3dd5db9..1f9c3e8 100644 --- a/docs/schema-upgrade-1.32-1.33.sql +++ b/docs/schema-upgrade-1.32-1.33.sql @@ -2,6 +2,6 @@ -- from version 1.32 to 1.33 BEGIN; - ALTER TABLE sessions ADD COLUMN renew_time TIMESTAMPTZ DEFAULT NULL; + ALTER TABLE sessions ADD COLUMN renew_time TIMESTAMPTZ; COMMIT; diff --git a/docs/schema.sql b/docs/schema.sql index 0ff74fd..af4616d 100644 --- a/docs/schema.sql +++ b/docs/schema.sql @@ -120,7 +120,7 @@ CREATE TABLE sessions ( update_time TIMESTAMPTZ NOT NULL DEFAULT NOW(), exclusive BOOLEAN CHECK (exclusive), closed BOOLEAN NOT NULL DEFAULT FALSE, - renew_time TIMESTAMPTZ DEFAULT NULL, + renew_time TIMESTAMPTZ, CONSTRAINT no_exclusive_subsessions CHECK ( master IS NULL OR "exclusive" IS NULL), CONSTRAINT no_closed_exclusive CHECK ( diff --git a/kojihub/auth.py b/kojihub/auth.py index 5b95557..a858db2 100644 --- a/kojihub/auth.py +++ b/kojihub/auth.py @@ -118,7 +118,7 @@ class Session(object): ('expired', 'expired'), ('master', 'master'), ('start_time', 'start_time'), ('update_time', 'update_time'), ("date_part('epoch', start_time)", 'start_ts'), ("date_part('epoch', update_time)", 'update_ts'), ('user_id', 'user_id'), - ("date_part('epoch', renew_time)", 'renew_ts')) + ('renew_time', 'renew_time'), ("date_part('epoch', renew_time)", 'renew_ts')) columns, aliases = zip(*fields) query = QueryProcessor(tables=['sessions'], columns=columns, aliases=aliases, diff --git a/tests/test_hub/test_get_session_info.py b/tests/test_hub/test_get_session_info.py index 9777421..fbb72e9 100644 --- a/tests/test_hub/test_get_session_info.py +++ b/tests/test_hub/test_get_session_info.py @@ -41,8 +41,8 @@ class TestGetSessionInfo(DBQueryTestCase): self.assertEqual(query.clauses, ['expired is FALSE', 'user_id = %(user_id)i']) self.assertEqual(query.joins, None) self.assertEqual(query.columns, ['authtype', 'callnum', 'exclusive', 'expired', 'master', - "date_part('epoch', start_time)", 'update_time', - 'user_id']) + "date_part('epoch', start_time)", + 'update_time', 'user_id']) self.assertEqual(query.aliases, ['authtype', 'callnum', 'exclusive', 'expired', 'master', 'start_time', 'update_time', 'user_id']) @@ -71,8 +71,8 @@ class TestGetSessionInfo(DBQueryTestCase): self.assertEqual(query.clauses, ['expired is FALSE', 'user_id = %(user_id)i']) self.assertEqual(query.joins, None) self.assertEqual(query.columns, ['authtype', 'callnum', 'exclusive', 'expired', 'master', - "date_part('epoch', start_time)", 'update_time', - 'user_id']) + "date_part('epoch', start_time)", + 'update_time', 'user_id']) self.assertEqual(query.aliases, ['authtype', 'callnum', 'exclusive', 'expired', 'master', 'start_time', 'update_time', 'user_id']) diff --git a/tests/test_lib/test_auth.py b/tests/test_lib/test_auth.py index 606101b..6735664 100644 --- a/tests/test_lib/test_auth.py +++ b/tests/test_lib/test_auth.py @@ -136,6 +136,7 @@ class TestAuthSession(unittest.TestCase): tzinfo=datetime.timezone.utc), 'update_time': datetime.datetime(2022, 10, 24, 8, 17, 6, 254308, tzinfo=datetime.timezone.utc), + 'renew_ts': None, 'user_id': 1}, {'name': 'kojiadmin', 'status': 0, 'usertype': 0}] with self.assertRaises(koji.GenericError) as cm: @@ -160,12 +161,13 @@ class TestAuthSession(unittest.TestCase): 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', + 'renew_time', "date_part('epoch', renew_time)", 'start_time', "date_part('epoch', start_time)", 'update_time', "date_part('epoch', update_time)", 'user_id']) self.assertEqual(query.aliases, ['authtype', 'callnum', 'exclusive', 'expired', 'master', - 'start_time', 'start_ts', 'update_time', 'update_ts', - 'user_id']) + 'renew_time', 'renew_ts', 'start_time', 'start_ts', + 'update_time', 'update_ts', 'user_id']) self.assertEqual(query.values, {'id': 123, 'key': 'xyz', 'hostip': 'remote-addr'}) def test_basic_instance(self): @@ -196,12 +198,13 @@ class TestAuthSession(unittest.TestCase): 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', + 'renew_time', "date_part('epoch', renew_time)", 'start_time', "date_part('epoch', start_time)", 'update_time', "date_part('epoch', update_time)", 'user_id']) self.assertEqual(query.aliases, ['authtype', 'callnum', 'exclusive', 'expired', 'master', - 'start_time', 'start_ts', 'update_time', 'update_ts', - 'user_id']) + 'renew_time', 'renew_ts', 'start_time', 'start_ts', + 'update_time', 'update_ts', 'user_id']) self.assertEqual(query.values, {'id': 123, 'key': 'xyz', 'hostip': 'remote-addr'}) query = self.queries[1] @@ -247,12 +250,13 @@ class TestAuthSession(unittest.TestCase): 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', + 'renew_time', "date_part('epoch', renew_time)", 'start_time', "date_part('epoch', start_time)", 'update_time', "date_part('epoch', update_time)", 'user_id']) self.assertEqual(query.aliases, ['authtype', 'callnum', 'exclusive', 'expired', 'master', - 'start_time', 'start_ts', 'update_time', 'update_ts', - 'user_id']) + 'renew_time', 'renew_ts', 'start_time', 'start_ts', + 'update_time', 'update_ts', 'user_id']) self.assertEqual(query.values, {'id': 123, 'key': 'xyz', 'hostip': 'remote-addr'}) query = self.queries[1]