From 8f6023b826d8b10e19018c4d0585f87d27314be4 Mon Sep 17 00:00:00 2001 From: sgdt6900 Date: Tue, 9 Jan 2018 11:33:47 +0200 Subject: 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 --- moon_interface/moon_interface/api/authz.py | 26 ++++------ moon_interface/moon_interface/authz_requests.py | 56 ++++++++++---------- moon_interface/moon_interface/http_server.py | 7 +++ moon_interface/moon_interface/server.py | 7 +++ moon_interface/tests/unit_python/api/test_authz.py | 60 +++++++++++++++++++++- moon_interface/tests/unit_python/conftest.py | 13 +++++ 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): -- cgit 1.2.3-korg