diff --git a/monkey/monkey_island/cc/resources/auth/auth.py b/monkey/monkey_island/cc/resources/auth/auth.py index 811f4238b..69aa1ffcd 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -29,6 +29,9 @@ class Authenticate(AbstractResource): urls = ["/api/auth"] + def __init__(self, authentication_service: AuthenticationService): + self._authentication_service = authentication_service + def post(self): """ Example request: \ @@ -41,7 +44,7 @@ class Authenticate(AbstractResource): username, password = get_username_password_from_request(request) try: - AuthenticationService.authenticate(username, password) + self._authentication_service.authenticate(username, password) access_token = create_access_token(username) except IncorrectCredentialsError: return make_response({"error": "Invalid credentials"}, 401) diff --git a/monkey/monkey_island/cc/resources/auth/registration.py b/monkey/monkey_island/cc/resources/auth/registration.py index 9ef640915..e4928165f 100644 --- a/monkey/monkey_island/cc/resources/auth/registration.py +++ b/monkey/monkey_island/cc/resources/auth/registration.py @@ -14,14 +14,17 @@ class Registration(AbstractResource): urls = ["/api/registration"] + def __init__(self, authentication_service: AuthenticationService): + self._authentication_service = authentication_service + def get(self): - return {"needs_registration": AuthenticationService.needs_registration()} + return {"needs_registration": self._authentication_service.needs_registration()} def post(self): username, password = get_username_password_from_request(request) try: - AuthenticationService.register_new_user(username, password) + self._authentication_service.register_new_user(username, password) return make_response({"error": ""}, 200) # API Spec: HTTP status code for AlreadyRegisteredError should be 409 (CONFLICT) except (InvalidRegistrationCredentialsError, AlreadyRegisteredError) as e: diff --git a/monkey/monkey_island/cc/services/authentication/authentication_service.py b/monkey/monkey_island/cc/services/authentication/authentication_service.py index 93fbf32f4..9e52a2649 100644 --- a/monkey/monkey_island/cc/services/authentication/authentication_service.py +++ b/monkey/monkey_island/cc/services/authentication/authentication_service.py @@ -18,52 +18,40 @@ from .user_creds import UserCreds class AuthenticationService: - DATA_DIR = None - user_datastore = None + def __init__(self, data_dir: Path, user_datastore: IUserDatastore): + self._data_dir = data_dir + self._user_datastore = user_datastore - # TODO: A number of these services should be instance objects instead of - # static/singleton hybrids. At the moment, this requires invasive refactoring that's - # not a priority. - @classmethod - def initialize(cls, data_dir: Path, user_datastore: IUserDatastore): - cls.DATA_DIR = data_dir - cls.user_datastore = user_datastore + def needs_registration(self) -> bool: + return not self._user_datastore.has_registered_users() - @classmethod - def needs_registration(cls) -> bool: - return not cls.user_datastore.has_registered_users() - - @classmethod - def register_new_user(cls, username: str, password: str): + def register_new_user(self, username: str, password: str): if not username or not password: raise InvalidRegistrationCredentialsError("Username or password can not be empty.") credentials = UserCreds(username, _hash_password(password)) - cls.user_datastore.add_user(credentials) - cls._reset_datastore_encryptor(username, password) + self._user_datastore.add_user(credentials) + self._reset_datastore_encryptor(username, password) reset_database() - @classmethod - def authenticate(cls, username: str, password: str): + def authenticate(self, username: str, password: str): try: - registered_user = cls.user_datastore.get_user_credentials(username) + registered_user = self._user_datastore.get_user_credentials(username) except UnknownUserError: raise IncorrectCredentialsError() if not _credentials_match_registered_user(username, password, registered_user): raise IncorrectCredentialsError() - cls._unlock_datastore_encryptor(username, password) + self._unlock_datastore_encryptor(username, password) - @classmethod - def _unlock_datastore_encryptor(cls, username: str, password: str): + def _unlock_datastore_encryptor(self, username: str, password: str): secret = _get_secret_from_credentials(username, password) - unlock_datastore_encryptor(cls.DATA_DIR, secret) + unlock_datastore_encryptor(self._data_dir, secret) - @classmethod - def _reset_datastore_encryptor(cls, username: str, password: str): + def _reset_datastore_encryptor(self, username: str, password: str): secret = _get_secret_from_credentials(username, password) - reset_datastore_encryptor(cls.DATA_DIR, secret) + reset_datastore_encryptor(self._data_dir, secret) def _hash_password(plaintext_password: str) -> str: diff --git a/monkey/monkey_island/cc/services/initialize.py b/monkey/monkey_island/cc/services/initialize.py index 7d9bde9e6..54148e2a1 100644 --- a/monkey/monkey_island/cc/services/initialize.py +++ b/monkey/monkey_island/cc/services/initialize.py @@ -41,6 +41,7 @@ from monkey_island.cc.services.telemetry.processing.processing import ( from monkey_island.cc.setup.mongo.mongo_setup import MONGO_URL from . import AuthenticationService, JsonFileUserDatastore +from .authentication.i_user_datastore import IUserDatastore from .reporting.report import ReportService logger = logging.getLogger(__name__) @@ -64,7 +65,6 @@ def initialize_services(data_dir: Path) -> DIContainer: # This is temporary until we get DI all worked out. PostBreachFilesService.initialize(container.resolve(IFileRepository)) - AuthenticationService.initialize(data_dir, JsonFileUserDatastore(data_dir)) ReportService.initialize(container.resolve(AWSService)) return container @@ -95,6 +95,7 @@ def _register_repositories(container: DIContainer, data_dir: Path): container.register_instance( ICredentialsRepository, container.resolve(MongoCredentialsRepository) ) + container.register_instance(IUserDatastore, container.resolve(JsonFileUserDatastore)) def _decorate_file_repository(file_repository: IFileRepository) -> IFileRepository: @@ -140,6 +141,7 @@ def _register_services(container: DIContainer): container.register_instance(AWSService, container.resolve(AWSService)) container.register_instance(LocalMonkeyRunService, container.resolve(LocalMonkeyRunService)) container.register_instance(IslandModeService, container.resolve(IslandModeService)) + container.register_instance(AuthenticationService, container.resolve(AuthenticationService)) def _patch_credentials_parser(container: DIContainer): diff --git a/monkey/tests/unit_tests/monkey_island/cc/resources/auth/conftest.py b/monkey/tests/unit_tests/monkey_island/cc/resources/auth/conftest.py new file mode 100644 index 000000000..3cb37c638 --- /dev/null +++ b/monkey/tests/unit_tests/monkey_island/cc/resources/auth/conftest.py @@ -0,0 +1,24 @@ +from unittest.mock import MagicMock + +import pytest +from tests.common import StubDIContainer + +from monkey_island.cc.services import AuthenticationService + + +@pytest.fixture +def mock_authentication_service(): + mock_service = MagicMock(spec=AuthenticationService) + mock_service.authenticate = MagicMock() + + return mock_service + + +@pytest.fixture +def flask_client(build_flask_client, mock_authentication_service): + container = StubDIContainer() + + container.register_instance(AuthenticationService, mock_authentication_service) + + with build_flask_client(container) as flask_client: + yield flask_client diff --git a/monkey/tests/unit_tests/monkey_island/cc/resources/auth/test_auth.py b/monkey/tests/unit_tests/monkey_island/cc/resources/auth/test_auth.py index 73466a47d..b3f10f5df 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/resources/auth/test_auth.py +++ b/monkey/tests/unit_tests/monkey_island/cc/resources/auth/test_auth.py @@ -11,16 +11,6 @@ PASSWORD = "test_password" TEST_REQUEST = f'{{"username": "{USERNAME}", "password": "{PASSWORD}"}}' -@pytest.fixture -def mock_authentication_service(monkeypatch): - mock_service = MagicMock() - mock_service.authenticate = MagicMock() - - monkeypatch.setattr("monkey_island.cc.resources.auth.auth.AuthenticationService", mock_service) - - return mock_service - - @pytest.fixture def make_auth_request(flask_client): url = Authenticate.urls[0] diff --git a/monkey/tests/unit_tests/monkey_island/cc/resources/auth/test_registration.py b/monkey/tests/unit_tests/monkey_island/cc/resources/auth/test_registration.py index e7bab544c..c10f0b011 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/resources/auth/test_registration.py +++ b/monkey/tests/unit_tests/monkey_island/cc/resources/auth/test_registration.py @@ -12,19 +12,6 @@ USERNAME = "test_user" PASSWORD = "test_password" -@pytest.fixture -def mock_authentication_service(monkeypatch): - mock_service = MagicMock() - mock_service.register_new_user = MagicMock() - mock_service.needs_registration = MagicMock() - - monkeypatch.setattr( - "monkey_island.cc.resources.auth.registration.AuthenticationService", mock_service - ) - - return mock_service - - @pytest.fixture def make_registration_request(flask_client): def inner(request_body): diff --git a/monkey/tests/unit_tests/monkey_island/cc/services/test_authentication_service.py b/monkey/tests/unit_tests/monkey_island/cc/services/test_authentication_service.py index 8df77e00a..402f77616 100644 --- a/monkey/tests/unit_tests/monkey_island/cc/services/test_authentication_service.py +++ b/monkey/tests/unit_tests/monkey_island/cc/services/test_authentication_service.py @@ -69,8 +69,7 @@ def test_needs_registration__true(tmp_path): has_registered_users = False mock_user_datastore = MockUserDatastore(lambda: has_registered_users, None, None) - a_s = AuthenticationService() - a_s.initialize(tmp_path, mock_user_datastore) + a_s = AuthenticationService(tmp_path, mock_user_datastore) assert a_s.needs_registration() @@ -79,8 +78,7 @@ def test_needs_registration__false(tmp_path): has_registered_users = True mock_user_datastore = MockUserDatastore(lambda: has_registered_users, None, None) - a_s = AuthenticationService() - a_s.initialize(tmp_path, mock_user_datastore) + a_s = AuthenticationService(tmp_path, mock_user_datastore) assert not a_s.needs_registration() @@ -92,8 +90,7 @@ def test_register_new_user__fails( ): mock_user_datastore = MockUserDatastore(lambda: True, MagicMock(side_effect=error), None) - a_s = AuthenticationService() - a_s.initialize(tmp_path, mock_user_datastore) + a_s = AuthenticationService(tmp_path, mock_user_datastore) with pytest.raises(error): a_s.register_new_user(USERNAME, PASSWORD) @@ -107,8 +104,7 @@ def test_register_new_user__empty_password_fails( ): mock_user_datastore = MockUserDatastore(lambda: False, None, None) - a_s = AuthenticationService() - a_s.initialize(tmp_path, mock_user_datastore) + a_s = AuthenticationService(tmp_path, mock_user_datastore) with pytest.raises(InvalidRegistrationCredentialsError): a_s.register_new_user(USERNAME, "") @@ -122,8 +118,7 @@ def test_register_new_user(tmp_path, mock_reset_datastore_encryptor, mock_reset_ mock_add_user = MagicMock() mock_user_datastore = MockUserDatastore(lambda: False, mock_add_user, None) - a_s = AuthenticationService() - a_s.initialize(tmp_path, mock_user_datastore) + a_s = AuthenticationService(tmp_path, mock_user_datastore) a_s.register_new_user(USERNAME, PASSWORD) @@ -144,8 +139,7 @@ def test_authenticate__success(tmp_path, mock_unlock_datastore_encryptor): lambda _: UserCreds(USERNAME, PASSWORD_HASH), ) - a_s = AuthenticationService() - a_s.initialize(tmp_path, mock_user_datastore) + a_s = AuthenticationService(tmp_path, mock_user_datastore) # If authentication fails, this function will raise an exception and the test will fail. a_s.authenticate(USERNAME, PASSWORD) @@ -165,8 +159,7 @@ def test_authenticate__failed_wrong_credentials( lambda _: UserCreds(USERNAME, PASSWORD_HASH), ) - a_s = AuthenticationService() - a_s.initialize(tmp_path, mock_user_datastore) + a_s = AuthenticationService(tmp_path, mock_user_datastore) with pytest.raises(IncorrectCredentialsError): a_s.authenticate(username, password) @@ -179,8 +172,7 @@ def test_authenticate__failed_no_registered_user(tmp_path, mock_unlock_datastore lambda: True, None, MagicMock(side_effect=UnknownUserError) ) - a_s = AuthenticationService() - a_s.initialize(tmp_path, mock_user_datastore) + a_s = AuthenticationService(tmp_path, mock_user_datastore) with pytest.raises(IncorrectCredentialsError): a_s.authenticate(USERNAME, PASSWORD)