From 51e6dde4e5b244b41880ad7d6676884bee758576 Mon Sep 17 00:00:00 2001 From: Lisa Zangrando Date: Mon, 17 Jul 2017 12:16:14 +0200 Subject: [PATCH] Fixes on the authorization mechanism This commit fixes some errors introduced with the support for the authorization in Synergy. The files involved are: service.py and shell.py and related unit tests. Change-Id: I84587e48927307d85b9a3f96ea49154805da7bcd Sem-Ver: bugfix --- synergy/client/shell.py | 78 +++++++++++++----------- synergy/service.py | 30 ++++----- synergy/tests/functional/test_synergy.py | 12 ++-- synergy/tests/unit/test_client_shell.py | 27 +++++--- 4 files changed, 83 insertions(+), 64 deletions(-) diff --git a/synergy/client/shell.py b/synergy/client/shell.py index 41ba896..70a8521 100644 --- a/synergy/client/shell.py +++ b/synergy/client/shell.py @@ -128,55 +128,65 @@ def main(): command_name = args.command_name token = None + if not os_username: + raise ValueError("'os-username' not defined!") + + if not os_password: + raise ValueError("'os-password' not defined!") + + if not os_project_name: + raise ValueError("'os-project-name' not defined!") + + if not os_auth_url: + raise ValueError("'os-auth-url' not defined!") + + if not os_user_domain_name: + os_user_domain_name = "default" + + if not os_project_domain_name: + os_project_domain_name = "default" + + client = keystone_v3.KeystoneClient( + auth_url=os_auth_url, + username=os_username, + password=os_password, + ca_cert=os_cacert, + user_domain_id=os_user_domain_id, + user_domain_name=os_user_domain_name, + project_name=os_project_name, + project_domain_id=os_project_domain_id, + project_domain_name=os_project_domain_name) + + token = client.authenticate() + if bypass_url: synergy_url = bypass_url else: - if not os_username: - raise ValueError("'os-username' not defined!") - - if not os_password: - raise ValueError("'os-password' not defined!") - - if not os_project_name: - raise ValueError("'os-project-name' not defined!") - - if not os_auth_url: - raise ValueError("'os-auth-url' not defined!") - - if not os_user_domain_name: - os_user_domain_name = "default" - - if not os_project_domain_name: - os_project_domain_name = "default" - - client = keystone_v3.KeystoneClient( - auth_url=os_auth_url, - username=os_username, - password=os_password, - ca_cert=os_cacert, - user_domain_id=os_user_domain_id, - user_domain_name=os_user_domain_name, - project_name=os_project_name, - project_domain_id=os_project_domain_id, - project_domain_name=os_project_domain_name) - - token = client.authenticate() synergy_endpoint = client.getEndpoint("synergy") synergy_url = synergy_endpoint["url"] if command_name not in commands: - print("command %r not found!" % command_name) + print("Command %r not found!" % command_name) commands[command_name].setToken(token) commands[command_name].execute(synergy_url, args) except KeyboardInterrupt: print("Shutting down synergyclient") sys.exit(1) - except (RequestException, ConnectionError, HTTPError) as ex: - print("connection to %s failed!" % synergy_url) + except ConnectionError as ex: + print("Failed to establish a new connection to %s" % synergy_url) + sys.exit(1) + except HTTPError as ex: + if ex.response._content: + print("%s" % ex.response._content) + else: + print("%s" % ex.message) + sys.exit(1) + except RequestException as ex: + print("%s" % ex.response._content) sys.exit(1) except Exception as ex: - print(ex.message) + print(ex) sys.exit(1) diff --git a/synergy/service.py b/synergy/service.py index 92b8dba..6a7c7cc 100644 --- a/synergy/service.py +++ b/synergy/service.py @@ -155,6 +155,9 @@ class Synergy(Service): def parseParameters(f): def wrapper(self, *args, **kw): + if not args: + return f(self, *args, **kw) + context = args[0] query = context.get("QUERY_STRING", None) @@ -177,24 +180,24 @@ class Synergy(Service): return wrapper - def checkParameters(paremeters): + def checkParameters(parameters): def check(f): def wrapper(self, *args, **kw): context = args[0] start_response = args[1] - for parameter in paremeters: + for parameter in parameters: value = context.get(parameter, None) if not value: start_response("400 BAD REQUEST", [("Content-Type", "text/plain")]) - return "parameter %s not found!" % parameter + return ["parameter %s not found!" % parameter] if parameter == "manager" and value not in self.managers: start_response("404 NOT FOUND", [("Content-Type", "text/plain")]) - return "manager %s not found!" % value + return ["manager %s not found!" % value] return f(self, *args, **kw) return wrapper @@ -211,7 +214,7 @@ class Synergy(Service): except AuthorizationError as ex: args[1]("401 Unauthorized", [("Content-Type", "text/plain")]) - return [ex.message] + return ["%s" % ex.message] return f(self, *args, **kw) @@ -265,7 +268,7 @@ class Synergy(Service): return [json.dumps(result, cls=SynergyEncoder)] @parseParameters - @checkParameters(["manager", "command", "args"]) + @checkParameters(["manager", "command"]) @authorize def executeCommand(self, environ, start_response): manager_name = environ["manager"] @@ -284,14 +287,14 @@ class Synergy(Service): LOG.error(message) start_response("500 INTERNAL SERVER ERROR", [("Content-Type", "text/plain")]) - return message + return [message] except SynergyError as ex: LOG.debug("execute command: error=%s" % ex) - start_response("500 INTERNAL SERVER ERROR", + start_response("422 Unprocessable Entity", [("Content-Type", "text/plain")]) - return "%s" % ex + return ["%s" % ex.message] @parseParameters @checkParameters(["manager"]) @@ -320,7 +323,7 @@ class Synergy(Service): result.set("message", "wrong state") start_response("200 OK", [("Content-Type", "text/html")]) - return json.dumps(result, cls=SynergyEncoder) + return [json.dumps(result, cls=SynergyEncoder)] @parseParameters @checkParameters(["manager"]) @@ -348,7 +351,7 @@ class Synergy(Service): result.set("message", "wrong state") start_response("200 OK", [("Content-Type", "text/html")]) - return json.dumps(result, cls=SynergyEncoder) + return [json.dumps(result, cls=SynergyEncoder)] def start(self): self.model_disconnected = False @@ -438,16 +441,13 @@ def main(): ", /etc/) and the '--config-file' option!") setLogger(name="synergy") + setLogger(name="oslo.messaging._drivers") global LOG - # LOG = logging.getLogger(None) LOG = logging.getLogger(__name__) LOG.info("Starting Synergy...") - # set session ID to this process so we can kill group in sigterm - # os.setsid() - server = Synergy() server.start() diff --git a/synergy/tests/functional/test_synergy.py b/synergy/tests/functional/test_synergy.py index 32e8efa..98990c4 100644 --- a/synergy/tests/functional/test_synergy.py +++ b/synergy/tests/functional/test_synergy.py @@ -99,12 +99,12 @@ class SynergyTests(unittest.TestCase): result = self.synergy.startManager(environ, start_response) - self.assertEqual(result, "manager NONE not found!") + self.assertEqual(result[0], "manager NONE not found!") environ = {'QUERY_STRING': 'manager=TimerManager'} result = self.synergy.startManager(environ, start_response) - result = json.loads(result, object_hook=objectHookHandler) + result = json.loads(result[0], object_hook=objectHookHandler) self.assertEqual(result.getStatus(), 'RUNNING') self.assertEqual(result.get("message"), 'started successfully') @@ -112,7 +112,7 @@ class SynergyTests(unittest.TestCase): time.sleep(0.5) result = self.synergy.startManager(environ, start_response) - result = json.loads(result, object_hook=objectHookHandler) + result = json.loads(result[0], object_hook=objectHookHandler) self.assertEqual(result.getStatus(), 'RUNNING') self.assertEqual(result.get("message"), 'WARN: already started') @@ -124,17 +124,17 @@ class SynergyTests(unittest.TestCase): result = self.synergy.startManager(environ, stop_response) - self.assertEqual(result, "manager NONE not found!") + self.assertEqual(result[0], "manager NONE not found!") environ = {'QUERY_STRING': 'manager=TimerManager'} result = self.synergy.startManager(environ, stop_response) - result = json.loads(result, object_hook=objectHookHandler) + result = json.loads(result[0], object_hook=objectHookHandler) time.sleep(0.5) result = self.synergy.stopManager(environ, stop_response) - result = json.loads(result, object_hook=objectHookHandler) + result = json.loads(result[0], object_hook=objectHookHandler) self.assertEqual(result.getStatus(), 'ACTIVE') diff --git a/synergy/tests/unit/test_client_shell.py b/synergy/tests/unit/test_client_shell.py index e5f84b2..8053fdf 100644 --- a/synergy/tests/unit/test_client_shell.py +++ b/synergy/tests/unit/test_client_shell.py @@ -162,6 +162,15 @@ class TestHTTPCommand(base.TestCase): def test_bypass_url(self, mock_argv, mock_httpcmd): """Filling bypass-url should set other params as defaults.""" args = [ + '--os-username', 'username', + '--os-password', 'password', + '--os-user-domain-id', 'user_domain_id', + '--os-user-domain-name', 'user_domain_name', + '--os-project-name', 'project_name', + '--os-project-domain-id', 'project_domain_id', + '--os-project-domain-name', 'project_domain_name', + '--os-cacert', 'cacert', + '--os-auth-url', 'auth_url', '--bypass-url', 'bypass_url', 'manager', 'list'] mock_argv.__getitem__.return_value = args @@ -174,14 +183,14 @@ class TestHTTPCommand(base.TestCase): command='list', command_name='manager', debug=False, - os_auth_url=None, - os_cacert=None, - os_password=None, - os_project_domain_id=None, - os_project_domain_name=None, + os_auth_url='auth_url', + os_cacert='cacert', + os_password='password', + os_project_domain_id='project_domain_id', + os_project_domain_name='project_domain_name', os_project_id=None, - os_project_name=None, - os_user_domain_id=None, - os_user_domain_name=None, - os_username=None) + os_project_name='project_name', + os_user_domain_id='user_domain_id', + os_user_domain_name='user_domain_name', + os_username='username') mock_httpcmd.assert_called_once_with("bypass_url", ns)