From 6e47d3272dd31e7c390ad87389f01f9e16cc56a4 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Thu, 15 Sep 2022 11:48:12 -0400 Subject: [PATCH 1/7] Island: Use monkey_island database in MongoCredentialsRepository --- .../monkey_island/cc/repository/mongo_credentials_repository.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/monkey/monkey_island/cc/repository/mongo_credentials_repository.py b/monkey/monkey_island/cc/repository/mongo_credentials_repository.py index 3fdc306a8..20caa288b 100644 --- a/monkey/monkey_island/cc/repository/mongo_credentials_repository.py +++ b/monkey/monkey_island/cc/repository/mongo_credentials_repository.py @@ -20,7 +20,7 @@ class MongoCredentialsRepository(ICredentialsRepository): """ def __init__(self, mongo: MongoClient, repository_encryptor: ILockableEncryptor): - self._database = mongo.monkeyisland + self._database = mongo.monkey_island self._repository_encryptor = repository_encryptor def get_configured_credentials(self) -> Sequence[Credentials]: From 5aff1c62c3c174dd152da5dd220a3c058f5263a7 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Thu, 15 Sep 2022 11:54:06 -0400 Subject: [PATCH 2/7] Island: Use drop() to clear mongo credentials collections --- .../monkey_island/cc/repository/mongo_credentials_repository.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/monkey/monkey_island/cc/repository/mongo_credentials_repository.py b/monkey/monkey_island/cc/repository/mongo_credentials_repository.py index 20caa288b..20f0ca4d7 100644 --- a/monkey/monkey_island/cc/repository/mongo_credentials_repository.py +++ b/monkey/monkey_island/cc/repository/mongo_credentials_repository.py @@ -109,6 +109,6 @@ class MongoCredentialsRepository(ICredentialsRepository): @staticmethod def _remove_credentials_fom_collection(collection): try: - collection.delete_many({}) + collection.drop() except RemovalError as err: raise err From 84c8de6a7cf88d76e775fea354d22f00e825bcd4 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Thu, 15 Sep 2022 11:55:48 -0400 Subject: [PATCH 3/7] UT: Test error conditions in MongoCredentialsRepository Increases test coverage to 100% for MongoCredentialsRepository --- .../test_mongo_credentials_repository.py | 53 ++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/monkey/tests/unit_tests/monkey_island/cc/repository/test_mongo_credentials_repository.py b/monkey/tests/unit_tests/monkey_island/cc/repository/test_mongo_credentials_repository.py index ffd3c3bfa..88c584534 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/repository/test_mongo_credentials_repository.py +++ b/monkey/tests/unit_tests/monkey_island/cc/repository/test_mongo_credentials_repository.py @@ -9,7 +9,13 @@ from pymongo.database import Database from tests.data_for_tests.propagation_credentials import CREDENTIALS from common.credentials import Credentials -from monkey_island.cc.repository import MongoCredentialsRepository +from monkey_island.cc.repository import ( + ICredentialsRepository, + MongoCredentialsRepository, + RemovalError, + RetrievalError, + StorageError, +) from monkey_island.cc.server_utils.encryption import ILockableEncryptor CONFIGURED_CREDENTIALS = CREDENTIALS[0:3] @@ -40,6 +46,36 @@ def mongo_repository(mongo_client, repository_encryptor): return MongoCredentialsRepository(mongo_client, repository_encryptor) +@pytest.fixture +def error_raising_mock_mongo_client() -> mongomock.MongoClient: + mongo_client = MagicMock(spec=mongomock.MongoClient) + mongo_client.monkey_island = MagicMock(spec=mongomock.Database) + mongo_client.monkey_island.stolen_credentials = MagicMock(spec=mongomock.Collection) + mongo_client.monkey_island.configured_credentials = MagicMock(spec=mongomock.Collection) + + mongo_client.monkey_island.configured_credentials.find = MagicMock( + side_effect=Exception("some exception") + ) + mongo_client.monkey_island.stolen_credentials.find = MagicMock( + side_effect=Exception("some exception") + ) + mongo_client.monkey_island.stolen_credentials.insert_one = MagicMock( + side_effect=Exception("some exception") + ) + mongo_client.monkey_island.stolen_credentials.drop = MagicMock( + side_effect=Exception("some exception") + ) + + return mongo_client + + +@pytest.fixture +def error_raising_credentials_repository( + error_raising_mock_mongo_client: mongomock.MongoClient, repository_encryptor: ILockableEncryptor +) -> ICredentialsRepository: + return MongoCredentialsRepository(error_raising_mock_mongo_client, repository_encryptor) + + def test_mongo_repository_get_configured(mongo_repository): actual_configured_credentials = mongo_repository.get_configured_credentials() @@ -91,6 +127,21 @@ def test_mongo_repository_all(mongo_repository): assert mongo_repository.get_configured_credentials() == [] +def test_mongo_repository_get__retrieval_error(error_raising_credentials_repository): + with pytest.raises(RetrievalError): + error_raising_credentials_repository.get_all_credentials() + + +def test_mongo_repository_save__storage_error(error_raising_credentials_repository): + with pytest.raises(StorageError): + error_raising_credentials_repository.save_stolen_credentials(STOLEN_CREDENTIALS) + + +def test_mongo_repository_remove_credentials__removal_error(error_raising_credentials_repository): + with pytest.raises(RemovalError): + error_raising_credentials_repository.remove_stolen_credentials() + + @pytest.mark.parametrize("credentials", CREDENTIALS) def test_configured_secrets_encrypted( mongo_repository: MongoCredentialsRepository, From 07815eed933d06a42110fbb7e0f77e353c0378a7 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Thu, 15 Sep 2022 11:56:58 -0400 Subject: [PATCH 4/7] Island: Wrap drop() Exceptions with RemovalError --- .../cc/repository/mongo_credentials_repository.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/monkey/monkey_island/cc/repository/mongo_credentials_repository.py b/monkey/monkey_island/cc/repository/mongo_credentials_repository.py index 20f0ca4d7..ab7c9f18b 100644 --- a/monkey/monkey_island/cc/repository/mongo_credentials_repository.py +++ b/monkey/monkey_island/cc/repository/mongo_credentials_repository.py @@ -110,5 +110,5 @@ class MongoCredentialsRepository(ICredentialsRepository): def _remove_credentials_fom_collection(collection): try: collection.drop() - except RemovalError as err: - raise err + except Exception as err: + raise RemovalError(f"Error removing credentials: {err}") From 3fd27c650388ee7f52135337ea173d5fcd4bf0e4 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Thu, 15 Sep 2022 12:00:04 -0400 Subject: [PATCH 5/7] Island: Add ICredentialsRepository.reset() --- .../cc/repository/i_credentials_repository.py | 7 +++++++ .../cc/repository/mongo_credentials_repository.py | 3 +++ .../monkey_island/in_memory_credentials_repository.py | 3 +++ .../cc/repository/test_mongo_credentials_repository.py | 5 +++++ 4 files changed, 18 insertions(+) diff --git a/monkey/monkey_island/cc/repository/i_credentials_repository.py b/monkey/monkey_island/cc/repository/i_credentials_repository.py index 381782533..ad891ff39 100644 --- a/monkey/monkey_island/cc/repository/i_credentials_repository.py +++ b/monkey/monkey_island/cc/repository/i_credentials_repository.py @@ -84,3 +84,10 @@ class ICredentialsRepository(ABC): :raises RemovalError: If an error is encountered while attempting to remove the credentials """ pass + + def reset(self): + """ + An alias for remove_all_credentials() + + :raises RemovalError: If an error is encountered while attempting to remove the credentials + """ diff --git a/monkey/monkey_island/cc/repository/mongo_credentials_repository.py b/monkey/monkey_island/cc/repository/mongo_credentials_repository.py index ab7c9f18b..438ce48b6 100644 --- a/monkey/monkey_island/cc/repository/mongo_credentials_repository.py +++ b/monkey/monkey_island/cc/repository/mongo_credentials_repository.py @@ -52,6 +52,9 @@ class MongoCredentialsRepository(ICredentialsRepository): self.remove_configured_credentials() self.remove_stolen_credentials() + def reset(self): + self.remove_all_credentials() + def _get_credentials_from_collection(self, collection) -> Sequence[Credentials]: try: collection_result = [] diff --git a/monkey/tests/monkey_island/in_memory_credentials_repository.py b/monkey/tests/monkey_island/in_memory_credentials_repository.py index 6eb8155e8..389a31e08 100644 --- a/monkey/tests/monkey_island/in_memory_credentials_repository.py +++ b/monkey/tests/monkey_island/in_memory_credentials_repository.py @@ -33,3 +33,6 @@ class InMemoryCredentialsRepository(ICredentialsRepository): def remove_all_credentials(self): self.remove_configured_credentials() self.remove_stolen_credentials() + + def reset(self): + self.remove_all_credentials() diff --git a/monkey/tests/unit_tests/monkey_island/cc/repository/test_mongo_credentials_repository.py b/monkey/tests/unit_tests/monkey_island/cc/repository/test_mongo_credentials_repository.py index 88c584534..f921c9df5 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/repository/test_mongo_credentials_repository.py +++ b/monkey/tests/unit_tests/monkey_island/cc/repository/test_mongo_credentials_repository.py @@ -142,6 +142,11 @@ def test_mongo_repository_remove_credentials__removal_error(error_raising_creden error_raising_credentials_repository.remove_stolen_credentials() +def test_mongo_repository_reset__removal_error(error_raising_credentials_repository): + with pytest.raises(RemovalError): + error_raising_credentials_repository.reset() + + @pytest.mark.parametrize("credentials", CREDENTIALS) def test_configured_secrets_encrypted( mongo_repository: MongoCredentialsRepository, From a7a2968a99fc028cea63371c5b67436b277c5e34 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Thu, 15 Sep 2022 12:00:24 -0400 Subject: [PATCH 6/7] Island: Remove superfluous "pass" from ICredentialsRepository --- .../cc/repository/i_credentials_repository.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/monkey/monkey_island/cc/repository/i_credentials_repository.py b/monkey/monkey_island/cc/repository/i_credentials_repository.py index ad891ff39..b49b0e60c 100644 --- a/monkey/monkey_island/cc/repository/i_credentials_repository.py +++ b/monkey/monkey_island/cc/repository/i_credentials_repository.py @@ -21,7 +21,6 @@ class ICredentialsRepository(ABC): credentials :return: Sequence of configured credentials """ - pass def get_stolen_credentials(self) -> Sequence[Credentials]: """ @@ -31,7 +30,6 @@ class ICredentialsRepository(ABC): credentials :return: Sequence of stolen credentials """ - pass def get_all_credentials(self) -> Sequence[Credentials]: """ @@ -41,7 +39,6 @@ class ICredentialsRepository(ABC): credentials :return: Sequence of stolen and configured credentials """ - pass def save_configured_credentials(self, credentials: Sequence[Credentials]): """ @@ -50,7 +47,6 @@ class ICredentialsRepository(ABC): :param credentials: Configured Credentials to store in the repository :raises StorageError: If an error is encountered while attempting to store the credentials """ - pass def save_stolen_credentials(self, credentials: Sequence[Credentials]): """ @@ -59,7 +55,6 @@ class ICredentialsRepository(ABC): :param credentials: Stolen Credentials to store in the repository :raises StorageError: If an error is encountered while attempting to store the credentials """ - pass def remove_configured_credentials(self): """ @@ -67,7 +62,6 @@ class ICredentialsRepository(ABC): :raises RemovalError: If an error is encountered while attempting to remove the credentials """ - pass def remove_stolen_credentials(self): """ @@ -75,7 +69,6 @@ class ICredentialsRepository(ABC): :raises RemovalError: If an error is encountered while attempting to remove the credentials """ - pass def remove_all_credentials(self): """ @@ -83,7 +76,6 @@ class ICredentialsRepository(ABC): :raises RemovalError: If an error is encountered while attempting to remove the credentials """ - pass def reset(self): """ From deacd18cbeed023cc76daaa356708fc1d4cde0a1 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Thu, 15 Sep 2022 12:03:38 -0400 Subject: [PATCH 7/7] Island: Make ICredentialsRepository methods abstract --- .../cc/repository/i_credentials_repository.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/monkey/monkey_island/cc/repository/i_credentials_repository.py b/monkey/monkey_island/cc/repository/i_credentials_repository.py index b49b0e60c..9affc52ea 100644 --- a/monkey/monkey_island/cc/repository/i_credentials_repository.py +++ b/monkey/monkey_island/cc/repository/i_credentials_repository.py @@ -1,4 +1,4 @@ -from abc import ABC +from abc import ABC, abstractmethod from typing import Sequence from common.credentials import Credentials @@ -13,6 +13,7 @@ class ICredentialsRepository(ABC): a simulation. """ + @abstractmethod def get_configured_credentials(self) -> Sequence[Credentials]: """ Retrieve credentials that were configured. @@ -22,6 +23,7 @@ class ICredentialsRepository(ABC): :return: Sequence of configured credentials """ + @abstractmethod def get_stolen_credentials(self) -> Sequence[Credentials]: """ Retrieve credentials that were stolen during a simulation. @@ -31,6 +33,7 @@ class ICredentialsRepository(ABC): :return: Sequence of stolen credentials """ + @abstractmethod def get_all_credentials(self) -> Sequence[Credentials]: """ Retrieve all credentials in the repository. @@ -40,6 +43,7 @@ class ICredentialsRepository(ABC): :return: Sequence of stolen and configured credentials """ + @abstractmethod def save_configured_credentials(self, credentials: Sequence[Credentials]): """ Save credentials that were configured. @@ -48,6 +52,7 @@ class ICredentialsRepository(ABC): :raises StorageError: If an error is encountered while attempting to store the credentials """ + @abstractmethod def save_stolen_credentials(self, credentials: Sequence[Credentials]): """ Save credentials that were stolen during a simulation. @@ -56,6 +61,7 @@ class ICredentialsRepository(ABC): :raises StorageError: If an error is encountered while attempting to store the credentials """ + @abstractmethod def remove_configured_credentials(self): """ Remove credentials that were configured from the repository. @@ -63,6 +69,7 @@ class ICredentialsRepository(ABC): :raises RemovalError: If an error is encountered while attempting to remove the credentials """ + @abstractmethod def remove_stolen_credentials(self): """ Remove stolen credentials from the repository. @@ -70,6 +77,7 @@ class ICredentialsRepository(ABC): :raises RemovalError: If an error is encountered while attempting to remove the credentials """ + @abstractmethod def remove_all_credentials(self): """ Remove all credentials in the repository. @@ -77,6 +85,7 @@ class ICredentialsRepository(ABC): :raises RemovalError: If an error is encountered while attempting to remove the credentials """ + @abstractmethod def reset(self): """ An alias for remove_all_credentials()