Merge branch 'master' into fix/usersync-update-role

fence/blueprints/data/blueprint.py

@@ -9,13 +9,12 @@
     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
 
 
 logger = get_logger(__name__)
 
-
 blueprint = flask.Blueprint("data", __name__)
 
 
@@ -165,11 +164,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 +194,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 +227,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 +261,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

@@ -64,10 +64,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,
@@ -471,15 +472,14 @@ def delete_files(self, urls=None, delete_all=True):
             urls (Optional[List[str]])
 
         Return:
-            None
+            Response (str: message, int: status code)
         """
         locations_to_delete = []
-        if urls is None and delete_all:
+        if not urls and delete_all:
             locations_to_delete = self.indexed_file_locations
         else:
-            locations_to_delete = [
-                location for location in locations_to_delete if location.url in urls
-            ]
+            locations_to_delete = list(map(IndexedFileLocation.from_url, urls))
+        response = ("No URLs to delete", 200)
         for location in locations_to_delete:
             bucket = location.bucket_name()
 
@@ -495,7 +495,11 @@ def delete_files(self, urls=None, delete_all=True):
                     file_suffix, bucket
                 )
             )
-            return location.delete(bucket, file_suffix)
+            response = location.delete(bucket, file_suffix)
+            # check status code not in 200s
+            if response[1] > 399:
+                break
+        return response
 
     @login_required({"data"})
     def delete(self):

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/models.py

@@ -245,6 +245,8 @@ class AuthorizationCode(Base, OAuth2AuthorizationCodeMixin):
 
     nonce = Column(String, nullable=True)
 
+    refresh_token_expires_in = Column(Integer, nullable=True)
+
     _scope = Column(Text, default="")
 
     def __init__(self, **kwargs):
@@ -660,6 +662,13 @@ def migrate(driver):
         metadata=md,
     )
 
+    add_column_if_not_exist(
+        table_name=AuthorizationCode.__tablename__,
+        column=Column("refresh_token_expires_in", Integer),
+        driver=driver,
+        metadata=md,
+    )
+
     drop_foreign_key_column_if_exist(
         table_name=GoogleProxyGroup.__tablename__,
         column_name="user_id",

fence/oidc/grants/oidc_code_grant.py

@@ -7,7 +7,8 @@
 )
 from authlib.oauth2.rfc6749 import InvalidRequestError
 import flask
-
+from fence.utils import get_valid_expiration_from_request
+from fence.config import config
 from fence.models import AuthorizationCode, ClientAuthType, User
 
 
@@ -35,13 +36,22 @@ def create_authorization_code(client, grant_user, request):
         the arguments passed from the OAuth request (the redirect URI, scope,
         and nonce).
         """
+
+        # requested lifetime (in seconds) for the refresh token
+        refresh_token_expires_in = get_valid_expiration_from_request(
+            expiry_param="refresh_token_expires_in",
+            max_limit=config["REFRESH_TOKEN_EXPIRES_IN"],
+            default=config["REFRESH_TOKEN_EXPIRES_IN"],
+        )
+
         code = AuthorizationCode(
             code=generate_token(50),
             client_id=client.client_id,
             redirect_uri=request.redirect_uri,
             scope=request.scope,
             user_id=grant_user.id,
             nonce=request.data.get("nonce"),
+            refresh_token_expires_in=refresh_token_expires_in,
         )
 
         with flask.current_app.db.session as session:
@@ -63,6 +73,7 @@ def create_token_response(self):
 
         scope = authorization_code.scope
         nonce = authorization_code.nonce
+        refresh_token_expires_in = authorization_code.refresh_token_expires_in
 
         token = self.generate_token(
             client,
@@ -71,6 +82,7 @@ def create_token_response(self):
             scope=scope,
             include_refresh_token=client.has_client_secret(),
             nonce=nonce,
+            refresh_token_expires_in=refresh_token_expires_in,
         )
 
         self.request.user = user

fence/oidc/jwt_generator.py

@@ -132,6 +132,7 @@ def generate_token_response(
     client,
     grant_type,
     expires_in=None,
+    refresh_token_expires_in=None,
     user=None,
     scope=None,
     include_refresh_token=True,
@@ -201,11 +202,13 @@ def generate_token_response(
     # If ``refresh_token`` was passed (for instance from the refresh
     # grant), use that instead of generating a new one.
     if refresh_token is None:
+        if refresh_token_expires_in is None:
+            refresh_token_expires_in = config["REFRESH_TOKEN_EXPIRES_IN"]
         refresh_token = generate_signed_refresh_token(
             kid=keypair.kid,
             private_key=keypair.private_key,
             user=user,
-            expires_in=config["REFRESH_TOKEN_EXPIRES_IN"],
+            expires_in=refresh_token_expires_in,
             scopes=scope,
             client_id=client.client_id,
         ).token

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,27 +275,40 @@ def send_email(from_email, to_emails, subject, text, smtp_domain):
     )
 
 
-def get_valid_expiration_from_request():
+def get_valid_expiration_from_request(
+    expiry_param="expires_in", max_limit=None, default=None
+):
     """
-    Return the expires_in param if it is in the request, None otherwise.
-    Throw an error if the requested expires_in 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 "expires_in" in flask.request.args:
-        is_valid_expiration(flask.request.args["expires_in"])
-        return int(flask.request.args["expires_in"])
-    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(flask.request.args["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("expires_in must be a positive integer")
+        raise UserError(
+            "Requested expiry must be a positive integer; instead got {}".format(
+                requested_expiration
+            )
+        )
 
 
 def _print_func_name(function):