Refactor get-and-validate-requested-expiration pattern into util fn

* Clearer intentions, better names, less repetition, less cognitive load, etc

fence/blueprints/data/blueprint.py

@@ -9,7 +9,7 @@
     get_signed_url_for_file,
 )
 from fence.errors import Forbidden, InternalError, UserError, Forbidden
-from fence.utils import is_valid_expiration
+from fence.utils import get_valid_expiration
 from fence.authz.auth import check_arborist_auth
 
 
@@ -165,11 +165,13 @@ def upload_data_file():
     blank_index = BlankIndex(
         file_name=params["file_name"], authz=params.get("authz"), uploader=uploader
     )
-    expires_in = flask.current_app.config.get("MAX_PRESIGNED_URL_TTL", 3600)
+    default_expires_in = flask.current_app.config.get("MAX_PRESIGNED_URL_TTL", 3600)
 
-    if "expires_in" in params:
-        is_valid_expiration(params["expires_in"])
-        expires_in = min(params["expires_in"], expires_in)
+    expires_in = get_valid_expiration(
+        params.get("expires_in"),
+        max_limit=default_expires_in,
+        default=default_expires_in,
+    )
 
     response = {
         "guid": blank_index.guid,
@@ -193,10 +195,14 @@ def init_multipart_upload():
     if "file_name" not in params:
         raise UserError("missing required argument `file_name`")
     blank_index = BlankIndex(file_name=params["file_name"])
-    expires_in = flask.current_app.config.get("MAX_PRESIGNED_URL_TTL", 3600)
-    if "expires_in" in params:
-        is_valid_expiration(params["expires_in"])
-        expires_in = min(params["expires_in"], expires_in)
+
+    default_expires_in = flask.current_app.config.get("MAX_PRESIGNED_URL_TTL", 3600)
+    expires_in = get_valid_expiration(
+        params.get("expires_in"),
+        max_limit=default_expires_in,
+        default=default_expires_in,
+    )
+
     response = {
         "guid": blank_index.guid,
         "uploadId": BlankIndex.init_multipart_upload(
@@ -222,10 +228,13 @@ def generate_multipart_upload_presigned_url():
     if missing:
         raise UserError("missing required arguments: {}".format(list(missing)))
 
-    expires_in = flask.current_app.config.get("MAX_PRESIGNED_URL_TTL", 3600)
-    if "expires_in" in params:
-        is_valid_expiration(params["expires_in"])
-        expires_in = min(params["expires_in"], expires_in)
+    default_expires_in = flask.current_app.config.get("MAX_PRESIGNED_URL_TTL", 3600)
+    expires_in = get_valid_expiration(
+        params.get("expires_in"),
+        max_limit=default_expires_in,
+        default=default_expires_in,
+    )
+
     response = {
         "presigned_url": BlankIndex.generate_aws_presigned_url_for_part(
             params["key"],
@@ -253,10 +262,12 @@ def complete_multipart_upload():
     if missing:
         raise UserError("missing required arguments: {}".format(list(missing)))
 
-    expires_in = flask.current_app.config.get("MAX_PRESIGNED_URL_TTL", 3600)
-    if "expires_in" in params:
-        is_valid_expiration(params["expires_in"])
-        expires_in = min(params["expires_in"], expires_in)
+    default_expires_in = flask.current_app.config.get("MAX_PRESIGNED_URL_TTL", 3600)
+    expires_in = get_valid_expiration(
+        params.get("expires_in"),
+        max_limit=default_expires_in,
+        default=default_expires_in,
+    )
 
     try:
         BlankIndex.complete_multipart_upload(

fence/blueprints/data/indexd.py

@@ -63,10 +63,11 @@ def get_signed_url_for_file(action, file_id, file_name=None):
         force_signed_url = False
 
     indexed_file = IndexedFile(file_id)
-    expires_in = config.get("MAX_PRESIGNED_URL_TTL", 3600)
-    requested_expires_in = get_valid_expiration_from_request()
-    if requested_expires_in:
-        expires_in = min(requested_expires_in, expires_in)
+    default_expires_in = config.get("MAX_PRESIGNED_URL_TTL", 3600)
+    expires_in = get_valid_expiration_from_request(
+        max_limit=default_expires_in,
+        default=default_expires_in,
+    )
 
     signed_url = indexed_file.get_signed_url(
         requested_protocol,

fence/blueprints/storage_creds/google.py

@@ -232,10 +232,11 @@ def handle_user_service_account_creds(self, key, service_account):
         """
         # requested time (in seconds) during which the access key will be valid
         # x days * 24 hr/day * 60 min/hr * 60 s/min = y seconds
-        expires_in = cirrus_config.SERVICE_KEY_EXPIRATION_IN_DAYS * 24 * 60 * 60
-        requested_expires_in = get_valid_expiration_from_request()
-        if requested_expires_in:
-            expires_in = min(expires_in, requested_expires_in)
+        default_expires_in = cirrus_config.SERVICE_KEY_EXPIRATION_IN_DAYS * 24 * 60 * 60
+        expires_in = get_valid_expiration_from_request(
+            max_limit=default_expires_in,
+            default=default_expires_in,
+        )
 
         expiration_time = int(time.time()) + int(expires_in)
         key_id = key.get("private_key_id")

fence/oidc/grants/oidc_code_grant.py

@@ -39,14 +39,10 @@ def create_authorization_code(client, grant_user, request):
 
         # requested lifetime (in seconds) for the refresh token
         refresh_token_expires_in = get_valid_expiration_from_request(
-            expiry_param="refresh_token_expires_in"
+            expiry_param="refresh_token_expires_in",
+            max_limit=config["REFRESH_TOKEN_EXPIRES_IN"],
+            default=config["REFRESH_TOKEN_EXPIRES_IN"],
         )
-        if refresh_token_expires_in:
-            refresh_token_expires_in = min(
-                refresh_token_expires_in, config["REFRESH_TOKEN_EXPIRES_IN"]
-            )
-        else:
-            refresh_token_expires_in = config["REFRESH_TOKEN_EXPIRES_IN"]
 
         code = AuthorizationCode(
             code=generate_token(50),

fence/resources/google/access_utils.py

@@ -851,18 +851,19 @@ def add_user_service_account_to_db(session, to_add_project_ids, service_account)
 
         access_groups = _get_google_access_groups(session, project_id)
 
-        # timestamp at which the SA will lose bucket access
+        # time until the SA will lose bucket access
         # by default: use configured time or 7 days
-        expiration_time = int(time.time()) + config.get(
+        default_expires_in = config.get(
             "GOOGLE_USER_SERVICE_ACCOUNT_ACCESS_EXPIRES_IN", 604800
         )
-        requested_expires_in = (
-            get_valid_expiration_from_request()
-        )  # requested time (in seconds)
-        if requested_expires_in:
-            # convert it to timestamp
-            requested_expiration = int(time.time()) + requested_expires_in
-            expiration_time = min(expiration_time, requested_expiration)
+        # use expires_in from request query params if it was provided and
+        # it was not greater than the default
+        expires_in = get_valid_expiration_from_request(
+            max_limit=default_expires_in,
+            default=default_expires_in,
+        )
+        # convert expires_in to timestamp
+        expiration_time = int(time.time() + expires_in)
 
         for access_group in access_groups:
             sa_to_group = ServiceAccountToGoogleBucketAccessGroup(

fence/scripting/fence_create.py

@@ -52,7 +52,7 @@
 from fence.scripting.google_monitor import email_users_without_access, validation_check
 from fence.config import config
 from fence.sync.sync_users import UserSyncer
-from fence.utils import create_client, is_valid_expiration
+from fence.utils import create_client, get_valid_expiration
 
 logger = get_logger(__name__)
 
@@ -1444,16 +1444,17 @@ def force_update_google_link(DB, username, google_email, expires_in=None):
                 user_id, google_email, session
             )
 
-        # timestamp at which the SA will lose bucket access
+        # time until the SA will lose bucket access
         # by default: use configured time or 7 days
-        expiration = int(time.time()) + config.get(
+        default_expires_in = config.get(
             "GOOGLE_USER_SERVICE_ACCOUNT_ACCESS_EXPIRES_IN", 604800
         )
-        if expires_in:
-            is_valid_expiration(expires_in)
-            # convert it to timestamp
-            requested_expiration = int(time.time()) + expires_in
-            expiration = min(expiration, requested_expiration)
+        # use expires_in from arg if it was provided and it was not greater than the default
+        expires_in = get_valid_expiration(
+            expires_in, max_limit=default_expires_in, default=default_expires_in
+        )
+        # convert expires_in to timestamp
+        expiration = int(time.time() + expires_in)
 
         force_update_user_google_account_expiration(
             user_google_account, proxy_group_id, google_email, expiration, session

fence/utils.py

@@ -275,29 +275,38 @@ def send_email(from_email, to_emails, subject, text, smtp_domain):
     )
 
 
-def get_valid_expiration_from_request(expiry_param="expires_in"):
+def get_valid_expiration_from_request(
+    expiry_param="expires_in", max_limit=None, default=None
+):
     """
-    Get the specified expiry_param (default "expires_in") from the request params.
-    Throw an error if the result is not a positive integer.
+    Thin wrapper around get_valid_expiration; looks for default query parameter "expires_in"
+    in flask request, unless a different parameter name was specified.
     """
-    if expiry_param in flask.request.args:
-        is_valid_expiration(flask.request.args[expiry_param])
-        return int(flask.request.args[expiry_param])
-    else:
-        return None
+    return get_valid_expiration(
+        flask.request.args.get(expiry_param), max_limit=max_limit, default=default
+    )
 
 
-def is_valid_expiration(expires_in):
+def get_valid_expiration(requested_expiration, max_limit=None, default=None):
     """
-    Throw an error if expires_in is not a positive integer.
+    If requested_expiration is not a positive integer and not None, throw error.
+    If max_limit is provided and requested_expiration exceeds max_limit,
+      return max_limit.
+    If requested_expiration is None, return default (which may also be None).
+    Else return requested_expiration.
     """
+    if requested_expiration is None:
+        return default
     try:
-        expires_in = int(expires_in)
-        assert expires_in > 0
+        rv = int(requested_expiration)
+        assert rv > 0
+        if max_limit:
+            rv = min(rv, max_limit)
+        return rv
     except (ValueError, AssertionError):
         raise UserError(
             "Requested expiry must be a positive integer; instead got {}".format(
-                expires_in
+                requested_expiration
             )
         )