From 645a9ad4178e9c554ef2faceae8fd5c3f104cfef Mon Sep 17 00:00:00 2001 From: Ed Cranford Date: Mon, 11 Mar 2013 10:53:36 -0500 Subject: [PATCH] Adds optional host parameter to users. All calls that involve calling a user by name now also allow for the host to be specified optionally. Similarly, all calls that respond with a user also include the host, defaulting to '%', MySQL's shorthand for 'anywhere'. Hosts with dots in the name are escaped to avoid a problem with the way our routing parses URLs. Change-Id: Idc5d514a7d862a723469ca8b49f1c51ae07f741b --- reddwarfclient/cli.py | 22 ++++++++++++++-------- reddwarfclient/common.py | 11 +++++++++++ reddwarfclient/users.py | 29 +++++++++++++++-------------- tests/test_users.py | 36 ++++++++++++++++++++++++++++++++---- 4 files changed, 72 insertions(+), 26 deletions(-) diff --git a/reddwarfclient/cli.py b/reddwarfclient/cli.py index 2daa5aa5..4f94f38e 100644 --- a/reddwarfclient/cli.py +++ b/reddwarfclient/cli.py @@ -140,6 +140,7 @@ class UserCommands(common.AuthedCommandsBase): 'id', 'database', 'databases', + 'hostname', 'name', 'password', ] @@ -149,19 +150,20 @@ class UserCommands(common.AuthedCommandsBase): self._require('id', 'name', 'password', 'databases') self._make_list('databases') databases = [{'name': dbname} for dbname in self.databases] - users = [{'name': self.name, 'password': self.password, - 'databases': databases}] + users = [{'name': self.name, 'host': self.hostname, + 'password': self.password, 'databases': databases}] self.dbaas.users.create(self.id, users) def delete(self): """Delete the specified user""" self._require('id', 'name') - self.dbaas.users.delete(self.id, self.name) + self.dbaas.users.delete(self.id, self.name, self.hostname) def get(self): """Get a single user.""" self._require('id', 'name') - self._pretty_print(self.dbaas.users.get, self.id, self.name) + self._pretty_print(self.dbaas.users.get, self.id, + self.name, self.hostname) def list(self): """List all the users for an instance""" @@ -171,25 +173,29 @@ class UserCommands(common.AuthedCommandsBase): def access(self): """Show all databases the user has access to.""" self._require('id', 'name') - self._pretty_list(self.dbaas.users.list_access, self.id, self.name) + self._pretty_list(self.dbaas.users.list_access, self.id, + self.name, self.hostname) def grant(self): """Allow an existing user permissions to access one or more databases.""" self._require('id', 'name', 'databases') self._make_list('databases') - self.dbaas.users.grant(self.id, self.name, self.databases) + self.dbaas.users.grant(self.id, self.name, self.databases, + self.hostname) def revoke(self): """Revoke from an existing user access permissions to a database.""" self._require('id', 'name', 'database') - self.dbaas.users.revoke(self.id, self.name, self.database) + self.dbaas.users.revoke(self.id, self.name, self.database, + self.hostname) def change_password(self): """Change the password of a single user.""" self._require('id', 'name', 'password') users = [{'name': self.name, - 'password': self.password}] + 'host': self.hostname, + 'password': self.password}] self.dbaas.users.change_passwords(self.id, users) diff --git a/reddwarfclient/common.py b/reddwarfclient/common.py index cd5cf5bf..015d7e07 100644 --- a/reddwarfclient/common.py +++ b/reddwarfclient/common.py @@ -23,6 +23,8 @@ from reddwarfclient import client from reddwarfclient.xml import ReddwarfXmlClient from reddwarfclient import exceptions +from urllib import quote + def methods_of(obj): """Get all callable methods of an object that don't start with underscore @@ -68,6 +70,15 @@ def limit_url(url, limit=None, marker=None): return url + query +def quote_user_host(user, host): + quoted = '' + if host: + quoted = quote("%s@%s" % (user, host)) + else: + quoted = quote("%s@%%" % user) + return quoted.replace('.', '%2e') + + class CliOptions(object): """A token object containing the user, apikey and token which is pickleable.""" diff --git a/reddwarfclient/users.py b/reddwarfclient/users.py index 505703c4..344252c1 100644 --- a/reddwarfclient/users.py +++ b/reddwarfclient/users.py @@ -18,9 +18,9 @@ from reddwarfclient import databases from reddwarfclient.common import check_for_exceptions from reddwarfclient.common import limit_url from reddwarfclient.common import Paginated +from reddwarfclient.common import quote_user_host import exceptions import urlparse -from urllib import quote class User(base.Resource): @@ -46,8 +46,9 @@ class Users(base.ManagerWithFind): resp, body = self.api.client.post(url, body=body) check_for_exceptions(resp, body) - def delete(self, instance_id, user): + def delete(self, instance_id, username, hostname=None): """Delete an existing user in the specified instance""" + user = quote_user_host(username, hostname) url = "/instances/%s/users/%s" % (instance_id, user) resp, body = self.api.client.delete(url) check_for_exceptions(resp, body) @@ -78,41 +79,41 @@ class Users(base.ManagerWithFind): return self._list("/instances/%s/users" % base.getid(instance), "users", limit, marker) - def get(self, instance_id, user): + def get(self, instance_id, username, hostname=None): """ Get a single User from the instance's Database. :rtype: :class:`User`. """ - username = quote(user) - url = "/instances/%s/users/%s" % (instance_id, username) + user = quote_user_host(username, hostname) + url = "/instances/%s/users/%s" % (instance_id, user) return self._get(url, "user") - def list_access(self, instance, user): + def list_access(self, instance, username, hostname=None): """Show all databases the given user has access to. """ instance_id = base.getid(instance) - username = quote(user) - url = "/instances/%(instance_id)s/users/%(username)s/databases" + user = quote_user_host(username, hostname) + url = "/instances/%(instance_id)s/users/%(user)s/databases" resp, body = self.api.client.get(url % locals()) check_for_exceptions(resp, body) if not body: raise Exception("Call to %s did not return to a body" % url) return [databases.Database(self, db) for db in body['databases']] - def grant(self, instance, user, databases): + def grant(self, instance, username, databases, hostname=None): """Allow an existing user permissions to access a database.""" instance_id = base.getid(instance) - username = quote(user) - url = "/instances/%(instance_id)s/users/%(username)s/databases" + user = quote_user_host(username, hostname) + url = "/instances/%(instance_id)s/users/%(user)s/databases" dbs = {'databases': [{'name': db} for db in databases]} resp, body = self.api.client.put(url % locals(), body=dbs) check_for_exceptions(resp, body) - def revoke(self, instance, user, database): + def revoke(self, instance, username, database, hostname=None): """Revoke from an existing user access permissions to a database.""" instance_id = base.getid(instance) - username = quote(user) - url = ("/instances/%(instance_id)s/users/%(username)s/" + user = quote_user_host(username, hostname) + url = ("/instances/%(instance_id)s/users/%(user)s/" "databases/%(database)s") resp, body = self.api.client.delete(url % locals()) check_for_exceptions(resp, body) diff --git a/tests/test_users.py b/tests/test_users.py index 266d9fd5..2023d60c 100644 --- a/tests/test_users.py +++ b/tests/test_users.py @@ -56,20 +56,48 @@ class UsersTest(TestCase): return Mock(side_effect=side_effect_func) + def _build_fake_user(self, name, hostname=None, password=None, + databases=None): + return {'name': name, + 'password': password if password else 'password', + 'host': hostname, + 'databases': databases if databases else [], + } + def test_create(self): self.users.api.client.post = self._get_mock_method() self._resp.status = 200 - self.users.create(23, 'user1') + user = self._build_fake_user('user1') + + self.users.create(23, [user]) self.assertEqual('/instances/23/users', self._url) - self.assertEqual({"users": 'user1'}, self._body) + self.assertEqual({"users": [user]}, self._body) + + # Even if host isn't supplied originally, + # the default is supplied. + del user['host'] + self.users.create(23, [user]) + self.assertEqual('/instances/23/users', self._url) + user['host'] = '%' + self.assertEqual({"users": [user]}, self._body) + + # If host is supplied, of course it's put into the body. + user['host'] = '127.0.0.1' + self.users.create(23, [user]) + self.assertEqual({"users": [user]}, self._body) + + # Make sure that response of 400 is recognized as an error. + user['host'] = '%' self._resp.status = 400 - self.assertRaises(Exception, self.users.create, 12, ['user1']) + self.assertRaises(Exception, self.users.create, 12, [user]) def test_delete(self): self.users.api.client.delete = self._get_mock_method() self._resp.status = 200 self.users.delete(27, 'user1') - self.assertEqual('/instances/27/users/user1', self._url) + # The client appends the host to remove ambiguity. + # urllib.unquote('%40%25') == '@%' + self.assertEqual('/instances/27/users/user1%40%25', self._url) self._resp.status = 400 self.assertRaises(Exception, self.users.delete, 34, 'user1')