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

@@ -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:
+    except ConnectionError as ex:
-        print("connection to %s failed!" % synergy_url)
+        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)
 
 

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()
 

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')
 

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_auth_url='auth_url',
-            os_cacert=None,
+            os_cacert='cacert',
-            os_password=None,
+            os_password='password',
-            os_project_domain_id=None,
+            os_project_domain_id='project_domain_id',
-            os_project_domain_name=None,
+            os_project_domain_name='project_domain_name',
             os_project_id=None,
-            os_project_name=None,
+            os_project_name='project_name',
-            os_user_domain_id=None,
+            os_user_domain_id='user_domain_id',
-            os_user_domain_name=None,
+            os_user_domain_name='user_domain_name',
-            os_username=None)
+            os_username='username')
         mock_httpcmd.assert_called_once_with("bypass_url", ns)