add test_list_user_keys_status

gen3workflow/aws_utils.py

@@ -12,17 +12,31 @@
 
 
 def get_iam_user_name(user_id):
-    # TODO alphanumeric characters and/or the following: +=,.@_-
+    """
+    Generate a valid IAM user name for the specified user.
+    IAM user names can contain up to 64 characters. They can only contain alphanumeric characters
+    and/or the following: +=,.@_-
+
+    Args:
+        user_id (str): The user's unique Gen3 ID
+
+    Returns:
+        str: IAM user name
+    """
     escaped_hostname = config["HOSTNAME"].replace(".", "-")
-    iam_user_name = f"gen3wf-{escaped_hostname}-{user_id}"  # TODO max length 64
+    iam_user_name = f"gen3wf-{escaped_hostname}"
+    max = 64 - len(f"-{user_id}")
+    if len(iam_user_name) > max:
+        iam_user_name = iam_user_name[:max]
+    iam_user_name = f"{iam_user_name}-{user_id}"
     return iam_user_name
 
 
 def get_user_bucket_info(user_id):
     """TODO
 
     Args:
-        user_id (_type_): _description_
+        user_id (str): The user's unique Gen3 ID
 
     Returns:
         tuple: (bucket name, prefix where the user stores objects in the bucket, bucket region)
@@ -152,15 +166,3 @@ def delete_iam_user_key(user_id, key_id):
         UserName=get_iam_user_name(user_id),
         AccessKeyId=key_id,
     )
-
-
-# def delete_expired_iam_user_keys(user_id, keys):
-#     iam_user_name = get_iam_user_name(user_id)
-#     now = datetime.now()
-#     for key in keys:
-#         if key["Status"] == "Inactive" or key["CreateDate"] - now > "TODO":
-#             logger.info(f"Deleting user {user_id}'s expired IAM key '{key["AccessKeyId"]}'")
-#             iam_client.delete_access_key(
-#                 UserName=iam_user_name,
-#                 AccessKeyId=key["AccessKeyId"],
-#             )

gen3workflow/config-default.yaml

@@ -6,7 +6,7 @@ HOSTNAME: localhost
 DEBUG: true
 DOCS_URL_PREFIX: /gen3workflow
 
-MAX_IAM_KEYS_PER_USER: 3
+MAX_IAM_KEYS_PER_USER: 2  # the default AWS AccessKeysPerUser quota is 2
 IAM_KEYS_LIFETIME_DAYS: 30
 
 # override the default Arborist URL; ignored if already set as an environment variable

gen3workflow/routes/storage.py

@@ -43,33 +43,6 @@ async def generate_user_key(request: Request, auth=Depends(Auth)):
     }
 
 
-# def get_refresh_token_expirations(username, idps):
-#     """
-#     Returns:
-#         dict: IdP to expiration of the most recent refresh token, or None if it's expired.
-#     """
-#     now = int(time.time())
-#     refresh_tokens = (
-#         db.session.query(RefreshToken)
-#         .filter_by(username=username)
-#         .filter(RefreshToken.idp.in_(idps))
-#         .order_by(RefreshToken.expires.asc())
-#     )
-#     if not refresh_tokens:
-#         return {}
-#     # the tokens are ordered by oldest to most recent, because we only want
-#     # to return None if the most recent token is expired
-#     expirations = {idp: None for idp in idps}
-#     expirations.update(
-#         {
-#             t.idp: seconds_to_human_time(t.expires - now)
-#             for t in refresh_tokens
-#             if t.expires > now
-#         }
-#     )
-#     return expirations
-
-
 def seconds_to_human_time(seconds):
     if seconds < 0:
         return None
@@ -92,14 +65,13 @@ async def get_user_keys(request: Request, auth=Depends(Auth)):
     now = datetime.now(timezone.utc)
 
     def get_key_expiration(key_status, key_creation_date):
-        # TODO unit tests for this
         if (
             key_status == "Inactive"
             or (now - key_creation_date).days > config["IAM_KEYS_LIFETIME_DAYS"]
         ):
             return "expired"
         expires_in = (
-            now + timedelta(days=config["IAM_KEYS_LIFETIME_DAYS"]) - key_creation_date
+            key_creation_date + timedelta(days=config["IAM_KEYS_LIFETIME_DAYS"]) - now
         )
         return f"expires in {seconds_to_human_time(expires_in.total_seconds())}"
 

pyproject.toml

@@ -24,6 +24,7 @@ jsonschema = "<5"
 uvicorn = "<1"
 
 [tool.poetry.dev-dependencies]
+freezegun = "<2"
 moto = "<6"
 pytest = "<9"
 pytest-asyncio = "<1"

tests/test-gen3workflow-config.yaml

@@ -1,3 +1,2 @@
-MAX_IAM_KEYS_PER_USER: 2
 ARBORIST_URL: http://test-arborist-server
 TES_SERVER_URL: http://external-tes-server/tes

tests/test_misc.py

@@ -0,0 +1,31 @@
+import pytest
+
+from gen3workflow.aws_utils import get_iam_user_name
+from gen3workflow.config import config
+
+
+@pytest.fixture(scope="function")
+def reset_config_hostname():
+    original_hostname = config["HOSTNAME"]
+    yield
+    config["HOSTNAME"] = original_hostname
+
+
+def test_get_iam_user_name(reset_config_hostname):
+    user_id = "asdfgh"
+
+    # test a hostname with a `.`; it should be replaced by a `-`
+    config["HOSTNAME"] = "qwert.qwert"
+    escaped_shortened_hostname = "qwert-qwert"
+    iam_user_id = get_iam_user_name(user_id)
+    assert len(iam_user_id) < 64
+    assert iam_user_id == f"gen3wf-{escaped_shortened_hostname}-{user_id}"
+
+    # test with a hostname that would result in a name longer than the max (64 chars)
+    config["HOSTNAME"] = (
+        "qwertqwert.qwertqwert.qwertqwert.qwertqwert.qwertqwert.qwertqwert"
+    )
+    escaped_shortened_hostname = "qwertqwert-qwertqwert-qwertqwert-qwertqwert-qwertq"
+    iam_user_id = get_iam_user_name(user_id)
+    assert len(iam_user_id) == 64
+    assert iam_user_id == f"gen3wf-{escaped_shortened_hostname}-{user_id}"

tests/test_storage.py

@@ -1,9 +1,12 @@
 import boto3
+from freezegun import freeze_time
 from moto import mock_aws
 import pytest
 
 from conftest import TEST_USER_ID
 from gen3workflow import aws_utils
+from gen3workflow.config import config
+from gen3workflow.aws_utils import get_iam_user_name
 
 
 @pytest.mark.asyncio
@@ -38,8 +41,14 @@ async def test_create_and_list_user_keys(client, access_token_patcher):
         )
         assert res.status_code == 200, res.text
         assert res.json() == [
-            {"aws_key_id": keys[0]["aws_key_id"], "status": "expires in 30 days"},
-            {"aws_key_id": keys[1]["aws_key_id"], "status": "expires in 30 days"},
+            {
+                "aws_key_id": keys[0]["aws_key_id"],
+                "status": f"expires in {config['IAM_KEYS_LIFETIME_DAYS'] - 1} days",
+            },
+            {
+                "aws_key_id": keys[1]["aws_key_id"],
+                "status": f"expires in {config['IAM_KEYS_LIFETIME_DAYS'] - 1} days",
+            },
         ]
 
         # delete the 2st key
@@ -55,7 +64,71 @@ async def test_create_and_list_user_keys(client, access_token_patcher):
         )
         assert res.status_code == 200, res.text
         assert res.json() == [
-            {"aws_key_id": keys[1]["aws_key_id"], "status": "expires in 30 days"}
+            {
+                "aws_key_id": keys[1]["aws_key_id"],
+                "status": f"expires in {config['IAM_KEYS_LIFETIME_DAYS'] - 1} days",
+            }
+        ]
+
+
+@pytest.mark.asyncio
+async def test_list_user_keys_status(client, access_token_patcher):
+    """
+    Keys that are deactivated or past the expiration date should be listed as "expired" when listing a user's keys.
+    """
+    with mock_aws():
+        aws_utils.iam_client = boto3.client("iam")
+
+        # create 2 keys. The 1st one has a mocked creation date of more than IAM_KEYS_LIFETIME_DAYS
+        # days ago, so it should be expired.
+        keys = []
+        import datetime
+
+        for i in range(2):
+            if i == 0:
+                with freeze_time(
+                    datetime.date.today()
+                    - datetime.timedelta(days=config["IAM_KEYS_LIFETIME_DAYS"] + 1)
+                ):
+                    res = await client.post(
+                        "/storage/credentials", headers={"Authorization": f"bearer 123"}
+                    )
+            else:
+                res = await client.post(
+                    "/storage/credentials", headers={"Authorization": f"bearer 123"}
+                )
+            assert res.status_code == 200, res.text
+            key_data = res.json()
+            assert "aws_key_id" in key_data and "aws_key_secret" in key_data
+            keys.append(key_data)
+
+        # list the user's key; the 1st key should show as expired
+        res = await client.get(
+            "/storage/credentials", headers={"Authorization": f"bearer 123"}
+        )
+        assert res.status_code == 200, res.text
+        assert res.json() == [
+            {"aws_key_id": keys[0]["aws_key_id"], "status": f"expired"},
+            {
+                "aws_key_id": keys[1]["aws_key_id"],
+                "status": f"expires in {config['IAM_KEYS_LIFETIME_DAYS'] - 1} days",
+            },
+        ]
+
+        # deactivate the 2nd key
+        access_key = boto3.resource("iam").AccessKey(
+            get_iam_user_name(TEST_USER_ID), keys[1]["aws_key_id"]
+        )
+        access_key.deactivate()
+
+        # list the user's key; both keys should now show as expired
+        res = await client.get(
+            "/storage/credentials", headers={"Authorization": f"bearer 123"}
+        )
+        assert res.status_code == 200, res.text
+        assert res.json() == [
+            {"aws_key_id": keys[0]["aws_key_id"], "status": "expired"},
+            {"aws_key_id": keys[1]["aws_key_id"], "status": "expired"},
         ]
 
 
@@ -67,16 +140,16 @@ async def test_too_many_user_keys(client, access_token_patcher):
     with mock_aws():
         aws_utils.iam_client = boto3.client("iam")
 
-        # create 2 keys
-        for _ in range(2):
+        # create the max number of keys
+        for _ in range(config["MAX_IAM_KEYS_PER_USER"]):
             res = await client.post(
                 "/storage/credentials", headers={"Authorization": f"bearer 123"}
             )
             assert res.status_code == 200, res.text
             key_data = res.json()
             assert "aws_key_id" in key_data and "aws_key_secret" in key_data
 
-        # attempt to create another key; this should fail since `MAX_IAM_KEYS_PER_USER` is 2
+        # attempt to create another key; this should fail since `MAX_IAM_KEYS_PER_USER` is reached
         res = await client.post(
             "/storage/credentials", headers={"Authorization": f"bearer 123"}
         )