From 6228ab5a51d3d5477c6b1307d6a508772899e5bb Mon Sep 17 00:00:00 2001
From: Joseph Schorr <joseph.schorr@coreos.com>
Date: Fri, 2 Feb 2018 15:14:43 -0500
Subject: [PATCH] We cannot cache the full expiration for an app specific
 token, as it would include the datetime when the cache is created, rather
 than `now`

Fixes https://jira.coreos.com/browse/QUAY-819
---
 data/model/appspecifictoken.py           | 14 ++++++-----
 data/model/test/test_appspecifictoken.py | 32 ++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/data/model/appspecifictoken.py b/data/model/appspecifictoken.py
index 5cb619390..282fa458c 100644
--- a/data/model/appspecifictoken.py
+++ b/data/model/appspecifictoken.py
@@ -12,20 +12,22 @@ from util.timedeltastring import convert_to_timedelta
 logger = logging.getLogger(__name__)
 
 
-@lru_cache(maxsize=1)
-def _default_expiration():
+def _default_expiration_duration():
   expiration_str = config.app_config.get('APP_SPECIFIC_TOKEN_EXPIRATION')
-  return datetime.now() + convert_to_timedelta(expiration_str) if expiration_str else expiration_str
+  return convert_to_timedelta(expiration_str) if expiration_str else None
 
 
 # Define a "unique" value so that callers can specifiy an expiration of None and *not* have it
 # use the default.
-_default_expiration_opt = '__deo'
+_default_expiration_duration_opt = '__deo'
 
-def create_token(user, title, expiration=_default_expiration_opt):
+def create_token(user, title, expiration=_default_expiration_duration_opt):
   """ Creates and returns an app specific token for the given user. If no expiration is specified
       (including `None`), then the default from config is used. """
-  expiration = expiration if expiration != _default_expiration_opt else _default_expiration()
+  if expiration == _default_expiration_duration_opt:
+    duration = _default_expiration_duration()
+    expiration = duration + datetime.now() if duration else None
+
   return AppSpecificAuthToken.create(user=user, title=title, expiration=expiration)
 
 
diff --git a/data/model/test/test_appspecifictoken.py b/data/model/test/test_appspecifictoken.py
index 1cf6fb70a..ef0498cc0 100644
--- a/data/model/test/test_appspecifictoken.py
+++ b/data/model/test/test_appspecifictoken.py
@@ -1,7 +1,9 @@
 from datetime import datetime
+from mock import patch
 
 import pytest
 
+from data.model import config as _config
 from data import model
 from data.model.appspecifictoken import create_token, revoke_token, access_valid_token
 from data.model.appspecifictoken import gc_expired_tokens, get_expiring_tokens
@@ -75,3 +77,33 @@ def test_expiring_soon(initialized_db):
   expiring_soon = get_expiring_tokens(user, convert_to_timedelta('49h'))
   assert expiring_soon
   assert len(expiring_soon) == 2
+
+
+@pytest.fixture(scope='function')
+def app_config():
+  with patch.dict(_config.app_config, {}, clear=True):
+    yield _config.app_config
+
+@pytest.mark.parametrize('expiration', [
+  (None),
+  ('10m'),
+  ('10d'),
+  ('10w'),
+])
+@pytest.mark.parametrize('default_expiration', [
+  (None),
+  ('10m'),
+  ('10d'),
+  ('10w'),
+])
+def test_create_access_token(expiration, default_expiration, initialized_db, app_config):
+  user = model.user.get_user('devtable')
+  expiration_date = datetime.now() + convert_to_timedelta(expiration) if expiration else None
+  with patch.dict(_config.app_config, {}, clear=True):
+    app_config['APP_SPECIFIC_TOKEN_EXPIRATION'] = default_expiration
+    if expiration:
+      exp_token = create_token(user, 'Some token', expiration=expiration_date)
+      assert exp_token.expiration == expiration_date
+    else:
+      exp_token = create_token(user, 'Some token')
+      assert (exp_token.expiration is None) == (default_expiration is None)