Merge "Fix token cache expires_at check" into stable/train

tripleo_common/image/image_uploader.py

@@ -316,12 +316,36 @@ class RegistrySessionHelper(object):
             return None
         with lock.get_lock():
             data = lock.sessions().get(scope)
-            if data and data.get('issued_at'):
-                token_time = dt_parse(data.get('issued_at'))
-                now = datetime.now(tzlocal())
-                expires_in = data.get('expires_in')
-                if not expires_in or (now - token_time).seconds < expires_in:
+            if data:
+                # expires_in is an integer value from the issued_at time.
+                # We will use this to determine if the token should be expired
+                # https://www.rfc-editor.org/rfc/rfc6749#section-4.2.2
+                if data.get('expires_in'):
+                    now = datetime.now(tzlocal())
+                    expires_in = data.get('expires_in')
+                    token_time = dt_parse(data.get('issued_at'))
+                    if (now - token_time).seconds < expires_in:
+                        return data['token']
+
+                # TODO(bshephar) Remove once Satellite returns expected expiry
+                # expires_at uses UTC date and time format. Although, this
+                # field doesn't appear in the RFC for OAuth. It appears some
+                # API's return a response with expires_at instead of expires_in
+                # https://www.rfc-editor.org/rfc/rfc6749
+                # https://bugzilla.redhat.com/show_bug.cgi?id=2134075
+                # https://bugzilla.redhat.com/show_bug.cgi?id=2138743
+                elif data.get('expires_at'):
+                    now = datetime.utcnow()
+                    expires = datetime.strptime(data.get('expires_at'),
+                                                "%Y-%m-%dT%H:%M:%S.%fZ")
+                    if now < expires:
+                        return data['token']
+
+                # If we don't have expires_in or expires_at. We can just
+                # return the token since it probably doesn't expire.
+                else:
                     return data['token']
+
         return None
 
     @staticmethod

tripleo_common/tests/image/test_image_uploader.py

@@ -13,6 +13,7 @@
 #   under the License.
 #
 
+import datetime
 import hashlib
 import io
 import json
@@ -27,6 +28,7 @@ import tempfile
 import zlib
 
 from oslo_concurrency import processutils
+from dateutil.tz import tzlocal
 from tripleo_common.image.exception import ImageNotFoundException
 from tripleo_common.image.exception import ImageRateLimitedException
 from tripleo_common.image.exception import ImageUploaderException
@@ -245,6 +247,100 @@ class TestRegistrySessionHelper(base.TestCase):
         mock_action.assert_called_once_with('put',
                                             request_session)
 
+    @mock.patch('tripleo_common.utils.locks.threadinglock.ThreadingLock',
+                name='mock_lock')
+    def test_get_cached_bearer_token_expired(self, mock_lock):
+        issued = datetime.datetime.now(tzlocal()) - datetime.timedelta(
+            minutes=1
+        )
+
+        resp = {"fakeimage": {
+            'token': 'blah',
+            'expires_in': 1,
+            'issued_at': '%s' % issued}
+        }
+
+        scope = "fakeimage"
+        mock_lock.sessions.return_value = resp
+
+        ret = image_uploader.RegistrySessionHelper.get_cached_bearer_token(
+            mock_lock, scope)
+
+        self.assertIsNone(ret)
+
+    @mock.patch('tripleo_common.utils.locks.threadinglock.ThreadingLock',
+                name='mock_lock')
+    def test_get_cached_bearer_token_expires_at_current(self, mock_lock):
+        expiry = datetime.datetime.utcnow() + datetime.timedelta(minutes=60)
+        expiry_time = "{}Z".format(expiry.isoformat())
+
+        resp = {"fakeimage": {
+            'token': 'blah',
+            'expires_at': expiry_time,
+            'issued_at': '2022-10-17T23:45:09.306Z'}
+        }
+
+        scope = "fakeimage"
+        mock_lock.sessions.return_value = resp
+
+        ret = image_uploader.RegistrySessionHelper.get_cached_bearer_token(
+            mock_lock, scope)
+
+        self.assertEqual(ret, "blah")
+
+    @mock.patch('tripleo_common.utils.locks.threadinglock.ThreadingLock',
+                name='mock_lock')
+    def test_get_cached_bearer_token_no_expires_in(self, mock_lock):
+
+        resp = {"fakeimage": {
+            'token': 'blah',
+            'issued_at': '2022-10-17T23:45:09.306Z'}
+        }
+
+        scope = "fakeimage"
+        mock_lock.sessions.return_value = resp
+
+        ret = image_uploader.RegistrySessionHelper.get_cached_bearer_token(
+            mock_lock, scope)
+
+        self.assertEqual(ret, "blah")
+
+    @mock.patch('tripleo_common.utils.locks.threadinglock.ThreadingLock',
+                name='mock_lock')
+    def test_get_cached_bearer_token_no_issued_at(self, mock_lock):
+
+        resp = {"fakeimage": {
+            'token': 'blah',
+            'expires_at': '2022-10-17T23:45:09.306Z'}
+        }
+
+        scope = "fakeimage"
+        mock_lock.sessions.return_value = resp
+
+        ret = image_uploader.RegistrySessionHelper.get_cached_bearer_token(
+            mock_lock, scope)
+
+        self.assertIsNone(ret)
+
+    @mock.patch('tripleo_common.utils.locks.threadinglock.ThreadingLock',
+                name='mock_lock')
+    def test_get_cached_bearer_token_expires_in_current(self, mock_lock):
+        issued = datetime.datetime.now(tzlocal())
+
+        resp = {"fakeimage": {
+            'token': 'blah',
+            'expires_in': 10000,
+            'issued_at': '%s' % issued}
+        }
+
+        scope = "fakeimage"
+        mock_lock.sessions.return_value = resp
+
+        ret = image_uploader.RegistrySessionHelper.get_cached_bearer_token(
+            mock_lock, scope)
+
+        self.assertEqual(ret, "blah")
+
 
 class TestImageUploadManager(base.TestCase):
     def setUp(self):