From 85ae8444142c889a1dc6289579ad0a1167e687fc Mon Sep 17 00:00:00 2001 From: Pauline Ribeyre <4224001+paulineribeyre@users.noreply.github.com> Date: Mon, 11 Nov 2024 12:42:19 -0600 Subject: [PATCH] add test_list_user_keys_status --- gen3workflow/aws_utils.py | 32 ++++++----- gen3workflow/config-default.yaml | 2 +- gen3workflow/routes/storage.py | 30 +--------- poetry.lock | 34 ++++++++---- pyproject.toml | 1 + tests/test-gen3workflow-config.yaml | 1 - tests/test_storage.py | 85 +++++++++++++++++++++++++++-- 7 files changed, 123 insertions(+), 62 deletions(-) diff --git a/gen3workflow/aws_utils.py b/gen3workflow/aws_utils.py index 3e01426..d470cdd 100644 --- a/gen3workflow/aws_utils.py +++ b/gen3workflow/aws_utils.py @@ -12,9 +12,23 @@ 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 @@ -22,7 +36,7 @@ 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"], -# ) diff --git a/gen3workflow/config-default.yaml b/gen3workflow/config-default.yaml index 016cd53..dd2296f 100644 --- a/gen3workflow/config-default.yaml +++ b/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 diff --git a/gen3workflow/routes/storage.py b/gen3workflow/routes/storage.py index 50bb3f7..82016af 100644 --- a/gen3workflow/routes/storage.py +++ b/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())}" diff --git a/poetry.lock b/poetry.lock index cc50b13..5a45af9 100644 --- a/poetry.lock +++ b/poetry.lock @@ -103,17 +103,17 @@ files = [ [[package]] name = "boto3" -version = "1.35.56" +version = "1.35.57" description = "The AWS SDK for Python" optional = false python-versions = ">=3.8" files = [ - {file = "boto3-1.35.56-py3-none-any.whl", hash = "sha256:d04608cf40f429025eb66b52b835bdc333436022918788853ed0bbbba6dd2f09"}, - {file = "boto3-1.35.56.tar.gz", hash = "sha256:6fcc510a4e747e85f84046b0ba0e5b178e89ba0f8ac9e2b6ebb4cc925c68c23b"}, + {file = "boto3-1.35.57-py3-none-any.whl", hash = "sha256:9edf49640c79a05b0a72f4c2d1e24dfc164344b680535a645f455ac624dc3680"}, + {file = "boto3-1.35.57.tar.gz", hash = "sha256:db58348849a5af061f0f5ec9c3b699da5221ca83354059fdccb798e3ddb6b62a"}, ] [package.dependencies] -botocore = ">=1.35.56,<1.36.0" +botocore = ">=1.35.57,<1.36.0" jmespath = ">=0.7.1,<2.0.0" s3transfer = ">=0.10.0,<0.11.0" @@ -122,21 +122,21 @@ crt = ["botocore[crt] (>=1.21.0,<2.0a0)"] [[package]] name = "botocore" -version = "1.35.56" +version = "1.35.57" description = "Low-level, data-driven core of boto 3." optional = false python-versions = ">=3.8" files = [ - {file = "botocore-1.35.56-py3-none-any.whl", hash = "sha256:4be97f7bc1fbf33ad71ee1b678cea0ecf9638e61d5f566a46f261cde969dd690"}, - {file = "botocore-1.35.56.tar.gz", hash = "sha256:8a9e752c8e87a423575ac528340a35d4318b8576ae4c6e0acfe5a3867f6bbccf"}, + {file = "botocore-1.35.57-py3-none-any.whl", hash = "sha256:92ddd02469213766872cb2399269dd20948f90348b42bf08379881d5e946cc34"}, + {file = "botocore-1.35.57.tar.gz", hash = "sha256:d96306558085baf0bcb3b022d7a8c39c93494f031edb376694d2b2dcd0e81327"}, ] [package.dependencies] jmespath = ">=0.7.1,<2.0.0" python-dateutil = ">=2.1,<3.0.0" urllib3 = [ - {version = ">=1.25.4,<2.2.0 || >2.2.0,<3", markers = "python_version >= \"3.10\""}, {version = ">=1.25.4,<1.27", markers = "python_version < \"3.10\""}, + {version = ">=1.25.4,<2.2.0 || >2.2.0,<3", markers = "python_version >= \"3.10\""}, ] [package.extras] @@ -569,6 +569,20 @@ typing-extensions = ">=4.8.0" all = ["email-validator (>=2.0.0)", "fastapi-cli[standard] (>=0.0.5)", "httpx (>=0.23.0)", "itsdangerous (>=1.1.0)", "jinja2 (>=2.11.2)", "orjson (>=3.2.1)", "pydantic-extra-types (>=2.0.0)", "pydantic-settings (>=2.0.0)", "python-multipart (>=0.0.7)", "pyyaml (>=5.3.1)", "ujson (>=4.0.1,!=4.0.2,!=4.1.0,!=4.2.0,!=4.3.0,!=5.0.0,!=5.1.0)", "uvicorn[standard] (>=0.12.0)"] standard = ["email-validator (>=2.0.0)", "fastapi-cli[standard] (>=0.0.5)", "httpx (>=0.23.0)", "jinja2 (>=2.11.2)", "python-multipart (>=0.0.7)", "uvicorn[standard] (>=0.12.0)"] +[[package]] +name = "freezegun" +version = "1.5.1" +description = "Let your Python tests travel through time" +optional = false +python-versions = ">=3.7" +files = [ + {file = "freezegun-1.5.1-py3-none-any.whl", hash = "sha256:bf111d7138a8abe55ab48a71755673dbaa4ab87f4cff5634a4442dfec34c15f1"}, + {file = "freezegun-1.5.1.tar.gz", hash = "sha256:b29dedfcda6d5e8e083ce71b2b542753ad48cfec44037b3fc79702e2980a89e9"}, +] + +[package.dependencies] +python-dateutil = ">=2.7" + [[package]] name = "gen3authz" version = "2.2.0" @@ -937,8 +951,8 @@ files = [ annotated-types = ">=0.6.0" pydantic-core = "2.23.4" typing-extensions = [ - {version = ">=4.12.2", markers = "python_version >= \"3.13\""}, {version = ">=4.6.1", markers = "python_version < \"3.13\""}, + {version = ">=4.12.2", markers = "python_version >= \"3.13\""}, ] [package.extras] @@ -1516,4 +1530,4 @@ files = [ [metadata] lock-version = "2.0" python-versions = ">=3.9,<4" -content-hash = "e25c90f8b76208e73dbc422ec88479b94d8e616c130996e49eec0826574cb6a1" +content-hash = "af80678a4da8f589eff65c78a9cb0a1221cae8cb27df0de67a248ece35f95536" diff --git a/pyproject.toml b/pyproject.toml index 1badd58..89551c6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -24,6 +24,7 @@ jsonschema = "<5" uvicorn = "<1" [tool.poetry.dev-dependencies] +freezegun = "<2" moto = "<6" pytest = "<9" pytest-asyncio = "<1" diff --git a/tests/test-gen3workflow-config.yaml b/tests/test-gen3workflow-config.yaml index b87b296..d76c994 100644 --- a/tests/test-gen3workflow-config.yaml +++ b/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 diff --git a/tests/test_storage.py b/tests/test_storage.py index d78d554..d5bf30f 100644 --- a/tests/test_storage.py +++ b/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']} days", + }, + { + "aws_key_id": keys[1]["aws_key_id"], + "status": f"expires in {config['IAM_KEYS_LIFETIME_DAYS']} 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']} 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,8 +140,8 @@ 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"} ) @@ -76,7 +149,7 @@ async def test_too_many_user_keys(client, access_token_patcher): 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"} )