From 9b08ea8013dd9dce741b774bb1d36892f999829d Mon Sep 17 00:00:00 2001 From: Angelo Fenoglio Date: Thu, 17 Oct 2024 12:27:34 -0300 Subject: [PATCH 1/6] Catch `ForbiddenException` It happens when attempting to assume a role that does not exists or the user doesn't have permission to assume it --- leverage/modules/auth.py | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/leverage/modules/auth.py b/leverage/modules/auth.py index 62daf33f..f85326e2 100644 --- a/leverage/modules/auth.py +++ b/leverage/modules/auth.py @@ -1,10 +1,11 @@ import time -from configparser import NoSectionError, NoOptionError from pathlib import Path +from configparser import NoSectionError, NoOptionError -import boto3 import hcl2 +import boto3 from configupdater import ConfigUpdater +from botocore.exceptions import ClientError from leverage import logger from leverage._utils import key_finder, ExitError, get_or_create_section @@ -40,7 +41,7 @@ def get_layer_profile(raw_profile: str, config_updater: ConfigUpdater, tf_profil except NoSectionError: raise ExitError(40, f"Missing {sso_profile} permission for account {account_name}.") - # if we are processing a profile from a different layer, we need to built it + # if we are processing a profile from a different layer, we need to build it layer_profile = layer_profile or f"{project}-{account_name}-{sso_role.lower()}" return account_id, account_name, sso_role, layer_profile @@ -104,7 +105,7 @@ def refresh_layer_credentials(cli): expiration = int(config_updater.get(f"profile {layer_profile}", "expiration").value) / 1000 except (NoSectionError, NoOptionError): # first time using this profile, skip into the credential's retrieval step - logger.debug(f"No cached credentials found.") + logger.debug("No cached credentials found.") else: # we reduce the validity 30 minutes, to avoid expiration over long-standing tasks renewal = time.time() + (30 * 60) @@ -117,11 +118,20 @@ def refresh_layer_credentials(cli): # retrieve credentials logger.debug(f"Retrieving role credentials for {sso_role}...") - credentials = client.get_role_credentials( - roleName=sso_role, - accountId=account_id, - accessToken=cli.get_sso_access_token(), - )["roleCredentials"] + try: + credentials = client.get_role_credentials( + roleName=sso_role, + accountId=account_id, + accessToken=cli.get_sso_access_token(), + )["roleCredentials"] + except ClientError as error: + if error.response["Error"]["Code"] == "ForbiddenException": + raise ExitError( + 40, + f"User does not have permission to assume role [bold]{sso_role}[/bold]" + " in this account.\nPlease check with your administrator or try" + " running [bold]leverage aws configure sso[/bold].", + ) # update expiration on aws//config logger.info(f"Writing {layer_profile} profile") From 4e37e8f350a29d61a24e34a382fcd647120c88ca Mon Sep 17 00:00:00 2001 From: Angelo Fenoglio Date: Thu, 17 Oct 2024 12:27:39 -0300 Subject: [PATCH 2/6] Add tests --- tests/test_modules/test_auth.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/test_modules/test_auth.py b/tests/test_modules/test_auth.py index b295d954..b0fddfd5 100644 --- a/tests/test_modules/test_auth.py +++ b/tests/test_modules/test_auth.py @@ -3,6 +3,7 @@ from unittest.mock import Mock, MagicMock, PropertyMock import pytest +from botocore.exceptions import ClientError from configupdater import ConfigUpdater from leverage._utils import ExitError @@ -265,3 +266,19 @@ def test_refresh_layer_credentials(mock_boto, mock_open, mock_update_conf, sso_c "aws_secret_access_key": "secret-key", "aws_session_token": "session-token", } + + +b3_client_error = Mock() +b3_client_error.get_role_credentials.side_effect = ClientError( + {"Error": {"Code": "ForbiddenException", "Message": "No access"}}, "GetRoleCredentials" +) + + +@mock.patch("leverage.modules.auth.update_config_section") +@mock.patch("builtins.open", side_effect=open_side_effect) +@mock.patch("boto3.client", return_value=b3_client_error) +@mock.patch("time.time", new=Mock(return_value=1705859000)) +@mock.patch("pathlib.Path.touch", new=Mock()) +def test_refresh_layer_credentials_no_access(mock_boto, mock_open, mock_update_conf, sso_container, propagate_logs): + with pytest.raises(ExitError): + refresh_layer_credentials(sso_container) From 0b48d5f254fd5c7f5e1c44d5eb31b967096113d0 Mon Sep 17 00:00:00 2001 From: Angelo Fenoglio Date: Thu, 17 Oct 2024 15:52:33 -0300 Subject: [PATCH 3/6] Add `AccessDeniedException` as possible error code --- leverage/modules/auth.py | 2 +- tests/test_modules/test_auth.py | 26 +++++++++++++++----------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/leverage/modules/auth.py b/leverage/modules/auth.py index f85326e2..8a4b74e3 100644 --- a/leverage/modules/auth.py +++ b/leverage/modules/auth.py @@ -125,7 +125,7 @@ def refresh_layer_credentials(cli): accessToken=cli.get_sso_access_token(), )["roleCredentials"] except ClientError as error: - if error.response["Error"]["Code"] == "ForbiddenException": + if error.response["Error"]["Code"] in ("AccessDeniedException", "ForbiddenException"): raise ExitError( 40, f"User does not have permission to assume role [bold]{sso_role}[/bold]" diff --git a/tests/test_modules/test_auth.py b/tests/test_modules/test_auth.py index b0fddfd5..83329111 100644 --- a/tests/test_modules/test_auth.py +++ b/tests/test_modules/test_auth.py @@ -250,7 +250,6 @@ def test_refresh_layer_credentials_still_valid(mock_open, mock_boto, sso_contain @mock.patch("leverage.modules.auth.update_config_section") @mock.patch("builtins.open", side_effect=open_side_effect) -@mock.patch("boto3.client", return_value=b3_client) @mock.patch("time.time", new=Mock(return_value=1705859000)) @mock.patch("pathlib.Path.touch", new=Mock()) def test_refresh_layer_credentials(mock_boto, mock_open, mock_update_conf, sso_container, propagate_logs): @@ -268,17 +267,22 @@ def test_refresh_layer_credentials(mock_boto, mock_open, mock_update_conf, sso_c } -b3_client_error = Mock() -b3_client_error.get_role_credentials.side_effect = ClientError( - {"Error": {"Code": "ForbiddenException", "Message": "No access"}}, "GetRoleCredentials" -) - - @mock.patch("leverage.modules.auth.update_config_section") @mock.patch("builtins.open", side_effect=open_side_effect) -@mock.patch("boto3.client", return_value=b3_client_error) @mock.patch("time.time", new=Mock(return_value=1705859000)) @mock.patch("pathlib.Path.touch", new=Mock()) -def test_refresh_layer_credentials_no_access(mock_boto, mock_open, mock_update_conf, sso_container, propagate_logs): - with pytest.raises(ExitError): - refresh_layer_credentials(sso_container) +@pytest.mark.parametrize( + "error", + [ + ClientError({"Error": {"Code": "AccessDeniedException", "Message": "No access"}}, "GetRoleCredentials"), + ClientError({"Error": {"Code": "ForbiddenException", "Message": "No access"}}, "GetRoleCredentials"), + ] +) +def test_refresh_layer_credentials_no_access(mock_open, mock_update_conf, sso_container, error): + with mock.patch("boto3.client") as mocked_client: + mocked_client_obj = MagicMock() + mocked_client_obj.get_role_credentials.side_effect = error + mocked_client.return_value = mocked_client_obj + + with pytest.raises(ExitError): + refresh_layer_credentials(sso_container) From 134ffa18e2a3e1ee24d3ccb75d326849e9172ff7 Mon Sep 17 00:00:00 2001 From: Angelo Fenoglio Date: Thu, 17 Oct 2024 15:56:17 -0300 Subject: [PATCH 4/6] Fix test --- tests/test_modules/test_auth.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_modules/test_auth.py b/tests/test_modules/test_auth.py index 83329111..cb033812 100644 --- a/tests/test_modules/test_auth.py +++ b/tests/test_modules/test_auth.py @@ -251,6 +251,7 @@ def test_refresh_layer_credentials_still_valid(mock_open, mock_boto, sso_contain @mock.patch("leverage.modules.auth.update_config_section") @mock.patch("builtins.open", side_effect=open_side_effect) @mock.patch("time.time", new=Mock(return_value=1705859000)) +@mock.patch("boto3.client", return_value=b3_client) @mock.patch("pathlib.Path.touch", new=Mock()) def test_refresh_layer_credentials(mock_boto, mock_open, mock_update_conf, sso_container, propagate_logs): refresh_layer_credentials(sso_container) From 8b6144e86b09fb208b097fecac7524917899efb2 Mon Sep 17 00:00:00 2001 From: Angelo Fenoglio Date: Thu, 17 Oct 2024 16:23:00 -0300 Subject: [PATCH 5/6] Reorder test parameters to match patches --- tests/test_modules/test_auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_modules/test_auth.py b/tests/test_modules/test_auth.py index cb033812..f19b647b 100644 --- a/tests/test_modules/test_auth.py +++ b/tests/test_modules/test_auth.py @@ -279,7 +279,7 @@ def test_refresh_layer_credentials(mock_boto, mock_open, mock_update_conf, sso_c ClientError({"Error": {"Code": "ForbiddenException", "Message": "No access"}}, "GetRoleCredentials"), ] ) -def test_refresh_layer_credentials_no_access(mock_open, mock_update_conf, sso_container, error): +def test_refresh_layer_credentials_no_access(mock_update_conf, mock_open, sso_container, error): with mock.patch("boto3.client") as mocked_client: mocked_client_obj = MagicMock() mocked_client_obj.get_role_credentials.side_effect = error From df74769fe8f3b0ca180e5f02b888ef4e8d998e3b Mon Sep 17 00:00:00 2001 From: Angelo Fenoglio Date: Thu, 17 Oct 2024 16:24:15 -0300 Subject: [PATCH 6/6] Format --- tests/test_modules/test_auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_modules/test_auth.py b/tests/test_modules/test_auth.py index f19b647b..0692f988 100644 --- a/tests/test_modules/test_auth.py +++ b/tests/test_modules/test_auth.py @@ -277,7 +277,7 @@ def test_refresh_layer_credentials(mock_boto, mock_open, mock_update_conf, sso_c [ ClientError({"Error": {"Code": "AccessDeniedException", "Message": "No access"}}, "GetRoleCredentials"), ClientError({"Error": {"Code": "ForbiddenException", "Message": "No access"}}, "GetRoleCredentials"), - ] + ], ) def test_refresh_layer_credentials_no_access(mock_update_conf, mock_open, sso_container, error): with mock.patch("boto3.client") as mocked_client: