adding comments for some issues and points

refactor retry method

apply the comments notes

adding test cases

adding more comments

adding test cases

adding more comments , refactoring, tests
Change-Id: I0949fcaed2b88f3cf75e66b6a90e6e9d7ca156b1
Signed-off-by: sgdt6900 <rhanafy.ext@orange.com>

6 files changed, 127 insertions, 42 deletions

diff --git a/moon_interface/moon_interface/api/authz.py b/moon_interface/moon_interface/api/authz.py
index bd60d3f6..b82a14f1 100644
--- a/moon_interface/moon_interface/api/authz.py
+++ b/moon_interface/moon_interface/api/authz.py
@@ -12,6 +12,7 @@ import logging
 import pickle
 import time
 from uuid import uuid4
+from python_moonutilities import exceptions
 
 from moon_interface.authz_requests import AuthzRequest
 
@@ -29,18 +30,13 @@ def get_pdp_from_cache(cache, uuid):
     """
     if uuid in cache.pdp:
         return cache.pdp.get(uuid)
-    return None
 
+    cache.update()
 
-def get_pdp_from_manager(cache, uuid):
-    """Check if a PDP exist with this ID in the Manager component
+    if uuid in cache.pdp:
+        return cache.pdp.get(uuid)
 
-    :param cache: Cache to use
-    :param uuid: Keystone Project ID
-    :return: True or False
-    """
-    cache.update()
-    return get_pdp_from_cache(cache, uuid)
+    raise exceptions.PdpUnknown
 
 
 def create_authz_request(cache, interface_name, manager_url, pdp_id, subject_name, object_name, action_name):
@@ -92,7 +88,7 @@ class Authz(Resource):
         self.MANAGER_URL = kwargs.get("manager_url", "http://manager:8080")
         self.TIMEOUT = 5
 
-    def get(self, pdp_id=None, subject_name=None, object_name=None, action_name=None):
+    def get(self, pdp_id, subject_name=None, object_name=None, action_name=None):
         """Get a response on an authorization request
 
         :param pdp_id: uuid of a tenant or an intra_extension
@@ -119,13 +115,13 @@ class Authz(Resource):
         }
         :internal_api: authz
         """
-        pdp_value = get_pdp_from_cache(self.CACHE, pdp_id)
-        if not pdp_id:
-            pdp_value = get_pdp_from_manager(self.CACHE, pdp_id)
-            if not pdp_id:
-                return {
+        try:
+            get_pdp_from_cache(self.CACHE, pdp_id)
+        except exceptions.PdpUnknown:
+            return {
                    "result": False,
                    "message": "Unknown PDP ID."}, 403
+
         authz_request = create_authz_request(
             cache=self.CACHE,
             pdp_id=pdp_id,
diff --git a/moon_interface/moon_interface/authz_requests.py b/moon_interface/moon_interface/authz_requests.py
index 87e21152..12c190c7 100644
--- a/moon_interface/moon_interface/authz_requests.py
+++ b/moon_interface/moon_interface/authz_requests.py
@@ -7,6 +7,7 @@ import logging
 import itertools
 import pickle
 import requests
+import sys
 from python_moonutilities import exceptions
 from python_moonutilities.context import Context
 from python_moonutilities.cache import Cache
@@ -31,51 +32,53 @@ class AuthzRequest:
         if ctx['project_id'] not in CACHE.container_chaining:
             raise exceptions.KeystoneProjectError("Unknown Project ID {}".format(ctx['project_id']))
         self.container_chaining = CACHE.container_chaining[ctx['project_id']]
-        if len(self.container_chaining) == 0:
+
+        if len(self.container_chaining) == 0 or not all(k in self.container_data for k in ("container_id", "hostname", "hostip", "port")):
             raise exceptions.MoonError('Void container chaining')
+
         self.pdp_container = self.container_chaining[0]["container_id"]
         self.run()
 
     def run(self):
         self.context.delete_cache()
         req = None
-        try:
-            req = requests.post("http://{}:{}/authz".format(
-                self.container_chaining[0]["hostip"],
-                self.container_chaining[0]["port"],
-            ), data=pickle.dumps(self.context))
-            if req.status_code != 200:
-                raise exceptions.AuthzException(
-                    "Receive bad response from Authz function "
-                    "(with IP address - {})".format(
-                        req.status_code
-                    ))
-        except requests.exceptions.ConnectionError:
-            logger.error("Cannot connect to {}".format(
-                "http://{}:{}/authz".format(
-                    self.container_chaining[0]["hostip"],
-                    self.container_chaining[0]["port"]
-                )))
-        except ValueError:
+        tries = 0
+        success = False
+
+        if "hostip" in self.container_chaining[0]:
+            hostname = self.container_chaining[0]["hostip"]
+        elif "hostname" in self.container_chaining[0]:
+            hostname = self.container_chaining[0]["hostname"]
+        else:
+            raise exceptions.AuthzException(
+                "error in address no hostname or hostip"
+            )
+        while tries < 2:
             try:
                 req = requests.post("http://{}:{}/authz".format(
-                    self.container_chaining[0]["hostname"],
+                    hostname,
                     self.container_chaining[0]["port"],
                 ), data=pickle.dumps(self.context))
                 if req.status_code != 200:
                     raise exceptions.AuthzException(
                         "Receive bad response from Authz function "
-                        "(with hostname - {})".format(
-                            req.status_code
-                        ))
+                        "(with address - {})".format(req.status_code)
+                    )
+                success = True
             except requests.exceptions.ConnectionError:
                 logger.error("Cannot connect to {}".format(
                     "http://{}:{}/authz".format(
-                        self.container_chaining[0]["hostname"],
+                        hostname,
                         self.container_chaining[0]["port"]
                     )))
-                raise exceptions.AuthzException(
-                    "Cannot connect to Authz function")
+            except:
+                logger.error("Unexpected error:", sys.exc_info()[0])
+            hostname = self.container_chaining[0]["hostname"],
+            tries += 1
+
+        if not success:
+            raise exceptions.AuthzException("Cannot connect to Authz function")
+
         self.context.set_cache(CACHE)
         if req and len(self.container_chaining) == 1:
             self.result = pickle.loads(req.content)
@@ -132,6 +135,7 @@ class AuthzRequest:
         authz_results = []
         for key in self.result.pdp_set:
             if "effect" in self.result.pdp_set[key]:
+
                 if self.result.pdp_set[key]["effect"] == "grant":
                     # the pdp is a authorization PDP and grant the request
                     authz_results.append(True)
diff --git a/moon_interface/moon_interface/http_server.py b/moon_interface/moon_interface/http_server.py
index 57170985..82ee5d91 100644
--- a/moon_interface/moon_interface/http_server.py
+++ b/moon_interface/moon_interface/http_server.py
@@ -113,6 +113,13 @@ class HTTPServer(Server):
 
         def get_400_json(e):
             return jsonify({"result": False, "code": 400, "description": str(e)}), 400
+
+        '''
+            [Note] i have tried to simulate authz post request (authz_Requests/run) to return 500 response code
+            and an AuthzException thrown from their [Line 63] and catched here , then the server here return response 
+            with 403 code [Forbidden] , is it correct if so why sometime at authz [from Line 137] return a response 
+            with error code , i think we can do it in the same way as the one mentioned?
+        '''
         self.app.register_error_handler(400, lambda e: get_400_json)
         self.app.register_error_handler(403, exceptions.AuthException)
 
diff --git a/moon_interface/moon_interface/server.py b/moon_interface/moon_interface/server.py
index 0af1fd06..d38dae28 100644
--- a/moon_interface/moon_interface/server.py
+++ b/moon_interface/moon_interface/server.py
@@ -13,8 +13,12 @@ logger = logging.getLogger("moon.interface.server")
 def create_server():
     configuration.init_logging()
     try:
+
         conf = configuration.get_configuration("components/pipeline").get(
             "components/pipeline", {}).get("interface", {})
+        '''
+        [Note] i think pipeline should be changed to interface
+        '''
         hostname = conf.get("hostname", "pipeline")
         port = conf.get("port", 80)
         bind = conf.get("bind", "127.0.0.1")
@@ -22,6 +26,9 @@ def create_server():
         hostname = "interface"
         bind = "127.0.0.1"
         port = 80
+        '''
+        [Note] i think pipeline should be changed to interface
+        '''
         configuration.add_component(uuid="pipeline",
                                     name=hostname,
                                     port=port,
diff --git a/moon_interface/tests/unit_python/api/test_authz.py b/moon_interface/tests/unit_python/api/test_authz.py
index 10957218..052bc9c9 100644
--- a/moon_interface/tests/unit_python/api/test_authz.py
+++ b/moon_interface/tests/unit_python/api/test_authz.py
@@ -1,4 +1,5 @@
 import json
+import conftest
 
 
 def get_json(data):
@@ -6,6 +7,7 @@ def get_json(data):
 
 
 def test_authz_true(context):
+
     import moon_interface.server
     server = moon_interface.server.create_server()
     client = server.app.test_client()
@@ -19,5 +21,61 @@ def test_authz_true(context):
     data = get_json(req.data)
     assert data
     assert "result" in data
-    assert data['result'] == True
+    assert data['result'] is True
+
+def test_authz_False(context):
+    import moon_interface.server
+    server = moon_interface.server.create_server()
+    client = server.app.test_client()
 
+    req = client.get("/authz/{p_id}/{s_id}/{o_id}/{a_id}".format(
+        p_id=None,
+        s_id=context["subject_name"],
+        o_id=context["object_name"],
+        a_id=context["action_name"],
+    ))
+    assert req.status_code == 403
+    data = get_json(req.data)
+    assert data
+    assert "result" in data
+    assert data['result'] is False
+
+
+def test_authz_effect_unset(context, set_consul_and_db):
+    import moon_interface.server
+    server = moon_interface.server.create_server()
+    client = server.app.test_client()
+
+    set_consul_and_db.register_uri(
+        'POST', 'http://127.0.0.1:8081/authz',
+        content = conftest.get_pickled_context_invalid()
+    )
+
+    req = client.get("/authz/{p_id}/{s_id}/{o_id}/{a_id}".format(
+        p_id=context["pdp_id"],
+        s_id=context["subject_name"],
+        o_id=context["object_name"],
+        a_id=context["action_name"],
+    ))
+    assert req.status_code == 401
+    data = get_json(req.data)
+    assert data
+    assert "result" in data
+    assert data['result'] is False
+
+def test_authz_invalid_ip(context, set_consul_and_db):
+    import moon_interface.server
+    server = moon_interface.server.create_server()
+    client = server.app.test_client()
+
+    set_consul_and_db.register_uri(
+        'POST', 'http://127.0.0.1:8081/authz', status_code=500
+    )
+
+    req = client.get("/authz/{p_id}/{s_id}/{o_id}/{a_id}".format(
+        p_id=context["pdp_id"],
+        s_id=context["subject_name"],
+        o_id=context["object_name"],
+        a_id=context["action_name"],
+    ))
+    assert req.status_code == 403
diff --git a/moon_interface/tests/unit_python/conftest.py b/moon_interface/tests/unit_python/conftest.py
index a6acbcdd..893a8637 100644
--- a/moon_interface/tests/unit_python/conftest.py
+++ b/moon_interface/tests/unit_python/conftest.py
@@ -214,6 +214,19 @@ def get_pickled_context():
     print(_context.pdp_set)
     return pickle.dumps(_context)
 
+def get_pickled_context_invalid():
+    from python_moonutilities.context import Context
+    from python_moonutilities.cache import Cache
+    CACHE = Cache()
+    CACHE.update()
+    _context = Context(context(), CACHE)
+    _context.increment_index()
+    _context.pdp_set['effect'] = 'invalid'
+    _context.pdp_set[os.environ['META_RULE_ID']]['effect'] = 'invalid'
+    print(_context.pdp_set)
+    return pickle.dumps(_context)
+
+
 
 @pytest.fixture(autouse=True)
 def set_consul_and_db(monkeypatch):