From bb1ca345636bb576f113d878d6b042b4082bea70 Mon Sep 17 00:00:00 2001
From: Aurélien Bompard <aurelien@bompard.org>
Date: Jun 22 2022 09:21:10 +0000
Subject: [PATCH 1/3] Expire refreshable tokens in a configurable amount of time


Signed-off-by: Aurélien Bompard <aurelien@bompard.org>

---

diff --git a/ipsilon/providers/openidc/store.py b/ipsilon/providers/openidc/store.py
index ae9518b..e7873e2 100644
--- a/ipsilon/providers/openidc/store.py
+++ b/ipsilon/providers/openidc/store.py
@@ -27,9 +27,10 @@ class OpenIDCStaticStore(Store):
 
 
 class OpenIDCStore(Store):
-    def __init__(self, database_url, static_store):
+    def __init__(self, database_url, static_store, token_lifetime):
         Store.__init__(self, database_url=database_url)
         self.static_store = static_store
+        self.token_lifetime = token_lifetime
 
     def registerDynamicClient(self, client):
         data = {}
@@ -216,14 +217,15 @@ class OpenIDCStore(Store):
 
         token_security_check = generate_random_secure_string()
         refresh_security_check = generate_random_secure_string(128)
-        expires_in = 3600
-        # TODO: Figure out values for this
-        refreshable_until = None
+        expires_in = self.token_lifetime['access']
 
         token['security_check'] = token_security_check
         token['refresh_security_check'] = refresh_security_check
         token['expires_at'] = int(time.time()) + expires_in
-        token['refreshable_until'] = refreshable_until
+        token['refreshable_until'] = (
+            None if self.token_lifetime['refresh'] is None else
+            int(time.time()) + self.token_lifetime['refresh']
+        )
 
         self.update_token(token)
 
@@ -240,7 +242,7 @@ class OpenIDCStore(Store):
                    userinfocode):
         token_security_check = generate_random_secure_string()
 
-        expires_in = 3600
+        expires_in = self.token_lifetime['access']
 
         token = {
             'type': 'Bearer',
@@ -256,8 +258,10 @@ class OpenIDCStore(Store):
 
         if issue_refresh:
             token['refreshable'] = True
-            # TODO: Figure out time for this
-            token['refreshable_until'] = None
+            token['refreshable_until'] = (
+                None if self.token_lifetime['refresh'] is None else
+                int(time.time()) + self.token_lifetime['refresh']
+            )
             token['refresh_security_check'] = \
                 generate_random_secure_string(128)
 
diff --git a/ipsilon/providers/openidcp.py b/ipsilon/providers/openidcp.py
index aee776b..7d3ddb3 100644
--- a/ipsilon/providers/openidcp.py
+++ b/ipsilon/providers/openidcp.py
@@ -88,6 +88,14 @@ Provides OpenID Connect authentication infrastructure. """
                 'default allowed attributes',
                 'Defines a list of allowed attributes, applied after mapping',
                 ['*']),
+            pconfig.Integer(
+                'access token lifetime',
+                'The access tokens will expire after this number of seconds',
+                3600),
+            pconfig.Integer(
+                'refresh token lifetime',
+                'The refresh tokens will expire after this number of seconds',
+                3600 * 24 * 365),  # 1 year
         )
 
     @property
@@ -175,10 +183,18 @@ Provides OpenID Connect authentication infrastructure. """
             for key in loaded_keys['keys']:
                 self.keyset.add(JWK(**key))
 
+        token_lifetime = {
+            "access": self.get_config_value("access token lifetime"),
+            "refresh": self.get_config_value("refresh token lifetime"),
+        }
         static_store = OpenIDCStaticStore(
-            self.get_config_value('static database url'))
-        self.datastore = OpenIDCStore(self.get_config_value('database url'),
-                                      static_store)
+            self.get_config_value('static database url'),
+        )
+        self.datastore = OpenIDCStore(
+            self.get_config_value('database url'),
+            static_store,
+            token_lifetime=token_lifetime
+        )
 
     def openid_connect_issuer_wf_rel(self, resource):
         link = {
diff --git a/tests/testcleanup.py b/tests/testcleanup.py
index 3ef02f5..ce92b5c 100755
--- a/tests/testcleanup.py
+++ b/tests/testcleanup.py
@@ -113,6 +113,8 @@ if __name__ == '__main__':
     sess.add_server(idpname, 'https://127.0.0.10:45080', user, 'ipsilon')
     sess.add_server(sp1name, 'https://127.0.0.11:45081')
 
+    db_url_base = f"sqlite:///{os.environ['TESTDIR']}lib/idp1"
+
     with TC.case('Verify logged out state'):
         page = sess.fetch_page(idpname, 'https://127.0.0.10:45080/idp1/')
         page.expected_value('//div[@id="content"]/p/a/text()', 'Log In')
@@ -161,15 +163,23 @@ if __name__ == '__main__':
 
 
     with TC.case('Checking that refreshable OpenIDC tokens are not expired'):
-        static_db_path = os.path.join(os.environ['TESTDIR'], 'lib/idp1/openidc.static.sqlite')
-        db_path = os.path.join(os.environ['TESTDIR'], 'lib/idp1/openidc.sqlite')
-        static_store = OpenIDCStaticStore(database_url=f"sqlite:///{static_db_path}")
+        static_store = OpenIDCStaticStore(database_url=f"{db_url_base}/openidc.static.sqlite")
         store = OpenIDCStore(
-            database_url=f"sqlite:///{db_path}", static_store=static_store
+                database_url=f"{db_url_base}/openidc.sqlite",
+                static_store=static_store,
+                token_lifetime={"access": 3600, "refresh": None},
         )
 
+        # Remove existing tokens and userinfo
+        for token_id in store.get_unique_data("token"):
+            store.del_unique_data("token", token_id)
+        for ui_id in store.get_unique_data("userinfo"):
+            store.del_unique_data("userinfo", ui_id)
+
+        # Prepare userinfo
         userinfocode = store.storeUserInfo({"name": "dummy"})
 
+        # Create tokens
         token_refreshable = store.issueToken(
             client_id="client-id", username="username", scope=["openid"],
             issue_refresh=True, userinfocode=userinfocode
@@ -182,7 +192,7 @@ if __name__ == '__main__':
 
         assert len(store.get_unique_data("token")) == 2
 
-        conn = sqlite3.connect(db_path)
+        conn = sqlite3.connect(f"{os.environ['TESTDIR']}lib/idp1/openidc.sqlite")
         cur = conn.cursor()
 
         expired_ts = int(time.time()) - 1
@@ -195,11 +205,7 @@ if __name__ == '__main__':
         conn.commit()
         conn.close()
 
-        try:
-            cleanup_count = store._cleanupExpiredTokens()
-        except Exception as e:
-            print(e)
-            raise
+        cleanup_count = store._cleanupExpiredTokens()
 
         if cleanup_count != 1:
             raise Exception(
@@ -215,3 +221,33 @@ if __name__ == '__main__':
         userinfo = store.get_unique_data("userinfo")
         if len(userinfo) != 1:
             raise Exception("The userinfo data has been cleaned up")
+
+
+    with TC.case('Checking that access tokens expire in the right amount of time'):
+        static_store = OpenIDCStaticStore(database_url=f"{db_url_base}/openidc.static.sqlite")
+        store = OpenIDCStore(
+                database_url=f"{db_url_base}/openidc.sqlite",
+                static_store=static_store,
+                token_lifetime={"access": 1, "refresh": 1},
+        )
+        token = store.issueToken(
+            client_id="client-id", username="username", scope=["openid"],
+            issue_refresh=False, userinfocode=userinfocode
+        )
+
+        time.sleep(3)
+        cleanup_count = store._cleanupExpiredTokens()
+
+        assert cleanup_count == 1, f"{cleanup_count} tokens were cleaned up"
+        assert token["token_id"] not in store.get_unique_data("token").keys()
+
+    with TC.case("Checking that refresh tokens expire in the right amount of time"):
+        token = store.issueToken(
+            client_id="client-id", username="username", scope=["openid"],
+            issue_refresh=True, userinfocode=userinfocode
+        )
+        time.sleep(3)
+        cleanup_count = store._cleanupExpiredTokens()
+
+        assert cleanup_count == 1, f"{cleanup_count} tokens were cleaned up"
+        assert token["token_id"] not in store.get_unique_data("token").keys()

From 81f6b3960374ffe28d84454b44f6b4c4adb86b30 Mon Sep 17 00:00:00 2001
From: Aurélien Bompard <aurelien@bompard.org>
Date: Jun 22 2022 09:21:10 +0000
Subject: [PATCH 2/3] Make the tests outputs more readable


Signed-off-by: Aurélien Bompard <aurelien@bompard.org>

---

diff --git a/tests/helpers/common.py b/tests/helpers/common.py
index 87d62e4..afbe09c 100755
--- a/tests/helpers/common.py
+++ b/tests/helpers/common.py
@@ -486,8 +486,9 @@ basicConstraints = CA:false""" % {'certdir': os.path.join(self.testdir,
         return self.run_and_collect([self.pycmd, exe], env)
 
     def run_and_collect(self, cmd, env):
-        p = subprocess.Popen(cmd, env=env,
-                             stdout=subprocess.PIPE, stderr=self.stderr)
+        p = subprocess.Popen(
+            cmd, env=env, universal_newlines=True,
+            stdout=subprocess.PIPE, stderr=self.stderr)
         results = []
         for line in p.stdout:
             line = line[:-1]  # Strip newline
diff --git a/tests/helpers/control.py b/tests/helpers/control.py
index 001e279..dd2b104 100644
--- a/tests/helpers/control.py
+++ b/tests/helpers/control.py
@@ -3,6 +3,7 @@
 from __future__ import print_function
 
 import sys
+from traceback import print_exception
 
 
 class TC(object):
@@ -37,6 +38,7 @@ class TC(object):
             if exc is None and not self.should_fail:
                 TC.output_method(TC.prefix + 'done')
             elif not self.should_fail:
+                print_exception(exc)
                 TC.output_method(TC.prefix + 'fail:' + repr(exc))
                 sys.exit(1)
             elif not exc:
@@ -80,8 +82,8 @@ class TC(object):
         case done:  ('done',)
         case fail:  ('fail', 'some error')
         """
-        if line.startswith(TC.prefix.encode('utf-8')):
-            return tuple(line[len(TC.prefix):].split(b':'))
+        if line.startswith(TC.prefix):
+            return tuple(line[len(TC.prefix):].split(':'))
         else:
             return None
 

From e8b89638d4670da713f52baa83d027a6ac1b0b1b Mon Sep 17 00:00:00 2001
From: Aurélien Bompard <aurelien@bompard.org>
Date: Jun 22 2022 09:21:10 +0000
Subject: [PATCH 3/3] Make the Integer config class actually return an integer


Signed-off-by: Aurélien Bompard <aurelien@bompard.org>

---

diff --git a/ipsilon/util/config.py b/ipsilon/util/config.py
index bbc171a..1707daf 100644
--- a/ipsilon/util/config.py
+++ b/ipsilon/util/config.py
@@ -195,6 +195,15 @@ class Integer(String):
         if value:
             self._assigned_value = int(value)
 
+    def get_value(self, default=True):
+        value = super().get_value(default)
+        return int(value) if value is not None else None
+
+    def export_value(self):
+        if self._assigned_value is None:
+            return None
+        return int(self._assigned_value)
+
 
 class Image(Option):
     """

