Island: Raise exception instead of returning bool in authenticate()

monkey/common/utils/exceptions.py

@@ -18,6 +18,10 @@ class AlreadyRegisteredError(RegistrationNotNeededError):
     """ Raise to indicate the reason why registration is not required """
 
 
+class IncorrectCredentialsError(Exception):
+    """ Raise to indicate that authentication failed """
+
+
 class RulePathCreatorNotFound(Exception):
     """ Raise to indicate that ScoutSuite rule doesn't have a path creator"""
 

monkey/monkey_island/cc/resources/auth/auth.py

@@ -7,6 +7,7 @@ from flask import make_response, request
 from flask_jwt_extended.exceptions import JWTExtendedException
 from jwt import PyJWTError
 
+from common.utils.exceptions import IncorrectCredentialsError
 from monkey_island.cc.resources.auth.credential_utils import get_username_password_from_request
 from monkey_island.cc.services.authentication import AuthenticationService
 
@@ -37,11 +38,13 @@ class Authenticate(flask_restful.Resource):
         """
         username, password = get_username_password_from_request(request)
 
-        if AuthenticationService.authenticate(username, password):
+        try:
+            AuthenticationService.authenticate(username, password)
             access_token = _create_access_token(username)
-            return make_response({"access_token": access_token, "error": ""}, 200)
+        except IncorrectCredentialsError:
+            return make_response({"error": "Invalid credentials"}, 401)
 
-        return make_response({"error": "Invalid credentials"}, 401)
+        return make_response({"access_token": access_token, "error": ""}, 200)
 
 
 def _create_access_token(username):

monkey/monkey_island/cc/services/authentication.py

@@ -1,6 +1,7 @@
 import bcrypt
 
 import monkey_island.cc.environment.environment_singleton as env_singleton
+from common.utils.exceptions import IncorrectCredentialsError
 from monkey_island.cc.environment.user_creds import UserCreds
 from monkey_island.cc.server_utils.encryption import (
     reset_datastore_encryptor,
@@ -31,12 +32,11 @@ class AuthenticationService:
         reset_database()
 
     @classmethod
-    def authenticate(cls, username: str, password: str) -> bool:
-        if _credentials_match_registered_user(username, password):
-            cls._unlock_datastore_encryptor(username, password)
-            return True
+    def authenticate(cls, username: str, password: str):
+        if not _credentials_match_registered_user(username, password):
+            raise IncorrectCredentialsError()
 
-        return False
+        cls._unlock_datastore_encryptor(username, password)
 
     @classmethod
     def _unlock_datastore_encryptor(cls, username: str, password: str):

monkey/tests/unit_tests/monkey_island/cc/resources/auth/test_auth.py

@@ -3,6 +3,8 @@ from unittest.mock import MagicMock
 
 import pytest
 
+from common.utils.exceptions import IncorrectCredentialsError
+
 USERNAME = "test_user"
 PASSWORD = "test_password"
 TEST_REQUEST = f'{{"username": "{USERNAME}", "password": "{PASSWORD}"}}'
@@ -51,7 +53,7 @@ def test_authentication_successful(make_auth_request, mock_authentication_servic
 
 
 def test_authentication_failure(make_auth_request, mock_authentication_service):
-    mock_authentication_service.authenticate = MagicMock(return_value=False)
+    mock_authentication_service.authenticate = MagicMock(side_effect=IncorrectCredentialsError())
 
     response = make_auth_request(TEST_REQUEST)