Merge pull request #60 from Clariteia/issue-59-improve-validation-errors

#59 - Raise an exception when token validations fails

Author: Sergio García Prado

minos/api_gateway/rest/__init__.py

@@ -6,7 +6,6 @@
 from .exceptions import (
     ApiGatewayConfigException,
     ApiGatewayException,
-    InvalidAuthenticationException,
     NoTokenException,
 )
 from .launchers import (

minos/api_gateway/rest/exceptions.py

@@ -2,10 +2,6 @@ class ApiGatewayException(Exception):
     """Base Api Gateway Exception."""
 
 
-class InvalidAuthenticationException(ApiGatewayException):
-    """Exception to be raised when authentication is not valid."""
-
-
 class NoTokenException(ApiGatewayException):
     """Exception to be raised when token is not available."""
 

minos/api_gateway/rest/handler.py

@@ -15,7 +15,6 @@
 )
 
 from .exceptions import (
-    InvalidAuthenticationException,
     NoTokenException,
 )
 
@@ -53,11 +52,8 @@ async def get_user(request: web.Request) -> Optional[str]:
     except NoTokenException:
         return None
 
-    try:
-        original_headers = dict(request.headers.copy())
-        return await authenticate(auth.host, auth.port, auth.method, auth.path, original_headers)
-    except InvalidAuthenticationException:
-        return None
+    original_headers = dict(request.headers.copy())
+    return await authenticate(auth.host, auth.port, auth.method, auth.path, original_headers)
 
 
 async def discover(host: str, port: int, path: str, verb: str, endpoint: str) -> dict[str, Any]:
@@ -76,10 +72,13 @@ async def discover(host: str, port: int, path: str, verb: str, endpoint: str) ->
         async with ClientSession() as session:
             async with session.get(url=url) as response:
                 if not response.ok:
-                    raise web.HTTPBadGateway(text="The discovery response is not okay.")
+                    if response.status == 404:
+                        raise web.HTTPNotFound(text=f"The {endpoint!r} path is not available for {verb!r} method.")
+                    raise web.HTTPBadGateway(text="The Discovery Service response is wrong.")
+
                 data = await response.json()
     except ClientConnectorError:
-        raise web.HTTPGatewayTimeout(text="The discovery is not available.")
+        raise web.HTTPGatewayTimeout(text="The Discovery Service is not available.")
 
     data["port"] = int(data["port"])
 
@@ -127,20 +126,30 @@ async def _clone_response(response: ClientResponse) -> web.Response:
 
 
 async def authenticate(host: str, port: str, method: str, path: str, authorization_headers: dict[str, str]) -> str:
+    """Authenticate a request based on its headers.
+
+    :param host: The authentication service host.
+    :param port: The authentication Service port.
+    :param method: The Authentication Service method.
+    :param path: The Authentication Service path.
+    :param authorization_headers: The headers that contain the authentication metadata.
+    :return: The authenticated user identifier.
+    """
     authentication_url = URL(f"http://{host}:{port}{path}")
     authentication_method = method
     logger.info("Authenticating request...")
 
     try:
         async with ClientSession(headers=authorization_headers) as session:
             async with session.request(method=authentication_method, url=authentication_url) as response:
-                if response.ok:
-                    jwt_payload = await response.json()
-                    return jwt_payload["sub"]
-                else:
-                    raise InvalidAuthenticationException
-    except (ClientConnectorError, web.HTTPError):
-        raise InvalidAuthenticationException
+                if not response.ok:
+                    raise web.HTTPUnauthorized(text="The given request does not have authorization to be forwarded.")
+
+                payload = await response.json()
+                return payload["sub"]
+
+    except ClientConnectorError:
+        raise web.HTTPGatewayTimeout(text="The Authentication Service is not available.")
 
 
 async def get_token(request: web.Request) -> str:

tests/test_api_gateway/test_rest/test_authentication.py

@@ -1,4 +1,5 @@
 import os
+import unittest
 from unittest import (
     mock,
 )
@@ -10,6 +11,9 @@
     AioHTTPTestCase,
     unittest_run_loop,
 )
+from werkzeug.exceptions import (
+    abort,
+)
 
 from minos.api_gateway.rest import (
     ApiGatewayConfig,
@@ -131,7 +135,7 @@ async def get_application(self):
         return await rest_service.create_application()
 
     @unittest_run_loop
-    async def test_auth_unreachable(self):
+    async def test_auth_disabled(self):
         url = "/order"
         headers = {"Authorization": "Bearer test_token"}
         response = await self.client.request("POST", url, headers=headers)
@@ -140,6 +144,59 @@ async def test_auth_unreachable(self):
         self.assertIn("Microservice call correct!!!", await response.text())
 
 
+class TestAuthFailed(AioHTTPTestCase):
+    CONFIG_FILE_PATH = BASE_PATH / "config.yml"
+
+    @mock.patch.dict(os.environ, {"API_GATEWAY_REST_CORS_ENABLED": "true"})
+    def setUp(self) -> None:
+        self.config = ApiGatewayConfig(self.CONFIG_FILE_PATH)
+
+        self.discovery = MockServer(host=self.config.discovery.host, port=self.config.discovery.port,)
+        self.discovery.add_json_response(
+            "/microservices", {"address": "localhost", "port": "5568", "status": True},
+        )
+
+        self.microservice = MockServer(host="localhost", port=5568)
+        self.microservice.add_json_response(
+            "/order/5", "Microservice call correct!!!", methods=("GET", "PUT", "PATCH", "DELETE",)
+        )
+        self.microservice.add_json_response("/order", "Microservice call correct!!!", methods=("POST",))
+
+        self.authentication_service = MockServer(host="localhost", port=8082)
+
+        self.authentication_service.add_callback_response("/token", lambda: abort(400), methods=("POST",))
+
+        self.discovery.start()
+        self.microservice.start()
+        self.authentication_service.start()
+        super().setUp()
+
+    def tearDown(self) -> None:
+        self.discovery.shutdown_server()
+        self.microservice.shutdown_server()
+        self.authentication_service.shutdown_server()
+        super().tearDown()
+
+    async def get_application(self):
+        """
+        Override the get_app method to return your application.
+        """
+        rest_service = ApiGatewayRestService(
+            address=self.config.rest.host, port=self.config.rest.port, config=self.config
+        )
+
+        return await rest_service.create_application()
+
+    @unittest_run_loop
+    async def test_auth_unauthorized(self):
+        url = "/order"
+        headers = {"Authorization": "Bearer test_token"}
+        response = await self.client.request("POST", url, headers=headers)
+
+        self.assertEqual(401, response.status)
+        self.assertIn("The given request does not have authorization to be forwarded", await response.text())
+
+
 class TestAuthUnreachable(AioHTTPTestCase):
     CONFIG_FILE_PATH = BASE_PATH / "config.yml"
 
@@ -182,5 +239,9 @@ async def test_auth_unreachable(self):
         headers = {"Authorization": "Bearer test_token"}
         response = await self.client.request("POST", url, headers=headers)
 
-        self.assertEqual(200, response.status)
-        self.assertIn("Microservice call correct!!!", await response.text())
+        self.assertEqual(504, response.status)
+        self.assertIn("The Authentication Service is not available", await response.text())
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/test_api_gateway/test_rest/test_service.py

@@ -6,6 +6,9 @@
     AioHTTPTestCase,
     unittest_run_loop,
 )
+from werkzeug.exceptions import (
+    abort,
+)
 
 from minos.api_gateway.rest import (
     ApiGatewayConfig,
@@ -95,13 +98,48 @@ async def test_delete(self):
         self.assertIn("Microservice call correct!!!", await response.text())
 
 
+class TestApiGatewayRestServiceNotFoundDiscovery(AioHTTPTestCase):
+    CONFIG_FILE_PATH = BASE_PATH / "config.yml"
+
+    def setUp(self) -> None:
+        self.config = ApiGatewayConfig(self.CONFIG_FILE_PATH)
+
+        self.discovery = MockServer(host=self.config.discovery.host, port=self.config.discovery.port,)
+
+        self.discovery.start()
+        super().setUp()
+
+    def tearDown(self) -> None:
+        self.discovery.shutdown_server()
+        super().tearDown()
+
+    async def get_application(self):
+        """
+        Override the get_app method to return your application.
+        """
+        rest_service = ApiGatewayRestService(
+            address=self.config.rest.host, port=self.config.rest.port, config=self.config
+        )
+
+        return await rest_service.create_application()
+
+    @unittest_run_loop
+    async def test_get(self):
+        url = "/order/5?verb=GET&path=12324"
+        response = await self.client.request("GET", url)
+
+        self.assertEqual(404, response.status)
+        self.assertIn("The '/order/5' path is not available for 'GET' method.", await response.text())
+
+
 class TestApiGatewayRestServiceFailedDiscovery(AioHTTPTestCase):
     CONFIG_FILE_PATH = BASE_PATH / "config.yml"
 
     def setUp(self) -> None:
         self.config = ApiGatewayConfig(self.CONFIG_FILE_PATH)
 
         self.discovery = MockServer(host=self.config.discovery.host, port=self.config.discovery.port,)
+        self.discovery.add_callback_response("/microservices", lambda: abort(400), methods=("GET",))
 
         self.discovery.start()
         super().setUp()
@@ -126,7 +164,7 @@ async def test_get(self):
         response = await self.client.request("GET", url)
 
         self.assertEqual(502, response.status)
-        self.assertIn("The discovery response is not okay.", await response.text())
+        self.assertIn("The Discovery Service response is wrong.", await response.text())
 
 
 class TestApiGatewayRestServiceUnreachableDiscovery(AioHTTPTestCase):
@@ -152,7 +190,7 @@ async def test_get(self):
         response = await self.client.request("GET", url)
 
         self.assertEqual(504, response.status)
-        self.assertIn("The discovery is not available.", await response.text())
+        self.assertIn("The Discovery Service is not available.", await response.text())
 
 
 class TestApiGatewayRestServiceUnreachableMicroservice(AioHTTPTestCase):