From 38268e57a091ee77c11e0a110b751bddc3f572f5 Mon Sep 17 00:00:00 2001 From: wallrony Date: Mon, 23 May 2022 10:58:47 -0300 Subject: [PATCH 1/2] Cherry-Pick Merge: Add validation for duplicated access requests (#252) Co-authored-by: vassalo --- e2e/slack/test_accessbot_slack_access.py | 8 ++++++++ plugins/sdm/accessbot.py | 6 ++++++ plugins/sdm/lib/helper/grant_request_helper.py | 8 ++++++++ .../sdm/lib/helper/test_grant_request_helper.py | 17 +++++++++++++++++ 4 files changed, 39 insertions(+) diff --git a/e2e/slack/test_accessbot_slack_access.py b/e2e/slack/test_accessbot_slack_access.py index 89bb976..2fc9446 100644 --- a/e2e/slack/test_accessbot_slack_access.py +++ b/e2e/slack/test_accessbot_slack_access.py @@ -225,6 +225,14 @@ def test_access_command_grant_with_strange_casing(self, mocked_testbot): assert "access request" in mocked_testbot.pop_message() assert "Granting" in mocked_testbot.pop_message() + def test_access_command_fails_when_duplicating_request(self, mocked_testbot): + mocked_testbot.push_message(f"access to {resource_name}") + mocked_testbot.push_message(f"access to {resource_name}") + assert "valid request" in mocked_testbot.pop_message() + assert "access request" in mocked_testbot.pop_message() + assert "already have a pending grant request" in mocked_testbot.pop_message() + + class Test_access_flow_from_access_form(ErrBotExtraTestSettings): @pytest.fixture def mocked_testbot(self, testbot): diff --git a/plugins/sdm/accessbot.py b/plugins/sdm/accessbot.py index a52cf94..669369b 100644 --- a/plugins/sdm/accessbot.py +++ b/plugins/sdm/accessbot.py @@ -183,6 +183,7 @@ def access_resource(self, message, match): try: self.get_arguments_helper().check_required_flags(flags_validators.keys(), self.config['REQUIRED_FLAGS'], flags) self.check_requester_flag(message, flags.get('requester')) + self.__grant_requests_helper.check_request_already_exists(resource_name, GrantRequestType.ACCESS_RESOURCE, message.frm.person) except Exception as e: yield str(e) return @@ -198,6 +199,11 @@ def assign_role(self, message, match): return self.__metrics_helper.increment_access_requests() role_name = re.sub(ASSIGN_ROLE_REGEX, "\\1", match.string.replace("*", ""), flags=re.IGNORECASE) + try: + self.__grant_requests_helper.check_request_already_exists(role_name, GrantRequestType.ASSIGN_ROLE, message.frm.person) + except Exception as e: + yield str(e) + return yield from self.get_role_grant_helper().request_access(message, role_name) self.__metrics_helper.reset_consecutive_errors() diff --git a/plugins/sdm/lib/helper/grant_request_helper.py b/plugins/sdm/lib/helper/grant_request_helper.py index 06dc8b1..9eb88c6 100644 --- a/plugins/sdm/lib/helper/grant_request_helper.py +++ b/plugins/sdm/lib/helper/grant_request_helper.py @@ -113,6 +113,14 @@ def remove(self, request_id: str): self.__grant_requests.pop(request_id, None) self.save_state() + def check_request_already_exists(self, sdm_object_name: str, grant_request_type: GrantRequestType, user: str): + for grant_request in self.__grant_requests.values(): + if grant_request["type"] == grant_request_type.value and grant_request["message"].frm.person == user \ + and grant_request["sdm_object"].name.lower() == sdm_object_name.lower(): + obj_type_name = "resource" if grant_request_type == GrantRequestType.ACCESS_RESOURCE else "role" + raise Exception( + f"You already have a pending grant request for that {obj_type_name}. Please, wait for an admin evaluation.") + def __sdm_model_to_dict(self, object): return object if type(object) is dict else object.to_dict() diff --git a/plugins/sdm/lib/helper/test_grant_request_helper.py b/plugins/sdm/lib/helper/test_grant_request_helper.py index 3e987ab..f526329 100644 --- a/plugins/sdm/lib/helper/test_grant_request_helper.py +++ b/plugins/sdm/lib/helper/test_grant_request_helper.py @@ -60,6 +60,23 @@ def test_restore_state_when_has_stored_requests(self): assert helper.get(request_id) is None assert not helper.exists(request_id) + def test_raise_exception_when_add_duplicated_request(self): + bot = get_mocked_bot() + with patch("builtins.open", mock_open(read_data=mocked_file_data)) as handle: + helper = GrantRequestHelper(bot) + assert helper.get(request_id) is not None + assert helper.exists(request_id) + assert len(helper.get_request_ids()) == 1 + file = handle() + file.read.assert_called_once() + with pytest.raises(Exception) as e: + helper.add(request_id, get_mocked_message(), get_mock_sdm_object(), get_mock_sdm_account(), + GrantRequestType.ACCESS_RESOURCE) + assert "already have a pending grant request" in e.value + helper.remove(request_id) + assert helper.get(request_id) is None + assert not helper.exists(request_id) + def test_dont_restore_state_when_has_stored_requests_and_is_disabled(self): with patch("os.path.isfile") as mock_isfile: mock_isfile.side_effect = [True] From b201a4c70bd46e3f7f68d926101a7d8232ec67eb Mon Sep 17 00:00:00 2001 From: vassalo Date: Wed, 25 May 2022 14:26:21 -0300 Subject: [PATCH 2/2] Fix grant request helper unit test file existence verification (#252) Co-authored-by: vassalo --- .../lib/helper/test_grant_request_helper.py | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/plugins/sdm/lib/helper/test_grant_request_helper.py b/plugins/sdm/lib/helper/test_grant_request_helper.py index f526329..fb448e5 100644 --- a/plugins/sdm/lib/helper/test_grant_request_helper.py +++ b/plugins/sdm/lib/helper/test_grant_request_helper.py @@ -61,21 +61,23 @@ def test_restore_state_when_has_stored_requests(self): assert not helper.exists(request_id) def test_raise_exception_when_add_duplicated_request(self): - bot = get_mocked_bot() - with patch("builtins.open", mock_open(read_data=mocked_file_data)) as handle: - helper = GrantRequestHelper(bot) - assert helper.get(request_id) is not None - assert helper.exists(request_id) - assert len(helper.get_request_ids()) == 1 - file = handle() - file.read.assert_called_once() - with pytest.raises(Exception) as e: - helper.add(request_id, get_mocked_message(), get_mock_sdm_object(), get_mock_sdm_account(), - GrantRequestType.ACCESS_RESOURCE) - assert "already have a pending grant request" in e.value - helper.remove(request_id) - assert helper.get(request_id) is None - assert not helper.exists(request_id) + with patch("os.path.isfile") as mock_isfile: + mock_isfile.side_effect = [True] + bot = get_mocked_bot() + with patch("builtins.open", mock_open(read_data=mocked_file_data)) as handle: + helper = GrantRequestHelper(bot) + assert helper.get(request_id) is not None + assert helper.exists(request_id) + assert len(helper.get_request_ids()) == 1 + file = handle() + file.read.assert_called_once() + with pytest.raises(Exception) as e: + helper.add(request_id, get_mocked_message(), get_mock_sdm_object(), get_mock_sdm_account(), + GrantRequestType.ACCESS_RESOURCE) + assert "already have a pending grant request" in e.value + helper.remove(request_id) + assert helper.get(request_id) is None + assert not helper.exists(request_id) def test_dont_restore_state_when_has_stored_requests_and_is_disabled(self): with patch("os.path.isfile") as mock_isfile: