Skip to content

Commit

Permalink
Fixes test in response to optional hostname fix
Browse files Browse the repository at this point in the history
Several tests have had their behavior changed
with the client appending the default host.
With that new behavior reversed in the client,
some tests use a username that contains an @,
causing an ambiguous case for the API to parse.
Deliberately including the host in the tests
fixes this.

Fixes Bug #1171559

Change-Id: I967eb4b21d8ca3e2c2721db6f362b8dac2551346
  • Loading branch information
Ed Cranford committed Apr 23, 2013
1 parent df4405c commit f224dd5
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 43 deletions.
2 changes: 1 addition & 1 deletion reddwarf/common/wsgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@
'flavor': {'id': '', 'ram': '', 'name': ''},
'link': {'href': '', 'rel': ''},
'database': {'name': ''},
'user': {'name': '', 'password': ''},
'user': {'name': '', 'password': '', 'host': ''},
'account': {'id': ''},
'security_group': {'id': '', 'name': '', 'description': '', 'user': '',
'tenant_id': ''},
Expand Down
27 changes: 12 additions & 15 deletions reddwarf/extensions/mysql/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,18 @@ def populate_databases(dbs):

def populate_users(users):
"""Create a serializable request containing users"""
try:
users_data = []
for user in users:
u = guest_models.MySQLUser()
u.name = user.get('name', '')
u.host = user.get('host', '%')
u.password = user.get('password', '')
dbs = user.get('databases', '')
if dbs:
for db in dbs:
u.databases = db.get('name', '')
users_data.append(u.serialize())
return users_data
except ValueError as ve:
raise exception.BadRequest(str(ve))
users_data = []
for user in users:
u = guest_models.MySQLUser()
u.name = user.get('name', '')
u.host = user.get('host')
u.password = user.get('password', '')
dbs = user.get('databases', '')
if dbs:
for db in dbs:
u.databases = db.get('name', '')
users_data.append(u.serialize())
return users_data


def unquote_user_host(user_hostname):
Expand Down
5 changes: 5 additions & 0 deletions reddwarf/extensions/mysql/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ def __init__(self, name, host, password, databases):
@classmethod
def load(cls, context, instance_id, username, hostname):
load_and_verify(context, instance_id)
validate = guest_models.MySQLUser()
validate.name = username
validate.host = hostname
client = create_guest_client(context, instance_id)
found_user = client.get_user(username=username, hostname=hostname)
if not found_user:
Expand All @@ -79,6 +82,8 @@ def create(cls, context, instance_id, users):
for user in users:
user_name = user['_name']
host_name = user['_host']
if host_name is None:
host_name = '%'
userhost = "%s@%s" % (user_name, host_name)
existing_users, _nadda = Users.load_with_client(
client,
Expand Down
51 changes: 33 additions & 18 deletions reddwarf/extensions/mysql/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,27 +91,29 @@ def create(self, req, body, tenant_id, instance_id):
context = req.environ[wsgi.CONTEXT_KEY]
self.validate(body)
users = body['users']
model_users = populate_users(users)
models.User.create(context, instance_id, model_users)
try:
model_users = populate_users(users)
models.User.create(context, instance_id, model_users)
except (ValueError, AttributeError) as e:
raise exception.BadRequest(msg=e)
return wsgi.Result(None, 202)

def delete(self, req, tenant_id, instance_id, id):
LOG.info(_("Deleting user for instance '%s'") % instance_id)
LOG.info(_("req : '%s'\n\n") % req)
context = req.environ[wsgi.CONTEXT_KEY]
username, host = unquote_user_host(id)

user = None
try:
user = guest_models.MySQLUser()
user.name = username
user.host = host
except ValueError as ve:
raise exception.BadRequest(str(ve))

found_user = models.User.load(context, instance_id, username,
host)
except (ValueError, AttributeError) as e:
raise exception.BadRequest(msg=e)
if not user:
raise exception.UserNotFound(uuid=id)

models.User.delete(context, instance_id, user.serialize())
return wsgi.Result(None, 202)

Expand All @@ -121,7 +123,11 @@ def show(self, req, tenant_id, instance_id, id):
LOG.info(_("req : '%s'\n\n") % req)
context = req.environ[wsgi.CONTEXT_KEY]
username, host = unquote_user_host(id)
user = models.User.load(context, instance_id, username, host)
user = None
try:
user = models.User.load(context, instance_id, username, host)
except (ValueError, AttributeError) as e:
raise exception.BadRequest(msg=e)
if not user:
raise exception.UserNotFound(uuid=id)
view = views.UserView(user)
Expand All @@ -136,11 +142,14 @@ def update(self, req, body, tenant_id, instance_id):
users = body['users']
model_users = []
for user in users:
mu = guest_models.MySQLUser()
mu.name = user['name']
mu.host = user.get('host')
mu.password = user['password']
model_users.append(mu)
try:
mu = guest_models.MySQLUser()
mu.name = user['name']
mu.host = user.get('host')
mu.password = user['password']
model_users.append(mu)
except (ValueError, AttributeError) as e:
raise exception.BadRequest(msg=e)
models.User.change_password(context, instance_id, model_users)
return wsgi.Result(None, 202)

Expand All @@ -153,8 +162,10 @@ def validate(cls, body):
"""Validate that the request has all the required parameters"""
if not body:
raise exception.BadRequest("The request contains an empty body")
if not body.get('databases', ''):
if not body.get('databases', []):
raise exception.MissingKey(key='databases')
if type(body['databases']) is not list:
raise exception.BadRequest("Databases must be provided as a list.")
for database in body.get('databases'):
if not database.get('name', ''):
raise exception.MissingKey(key='name')
Expand All @@ -163,8 +174,8 @@ def _get_user(self, context, instance_id, user_id):
username, hostname = unquote_user_host(user_id)
try:
user = models.User.load(context, instance_id, username, hostname)
except ValueError as ve:
raise exception.BadRequest(str(ve))
except (ValueError, AttributeError) as e:
raise exception.BadRequest(msg=e)
if not user:
raise exception.UserNotFound(uuid=user_id)
return user
Expand All @@ -183,6 +194,8 @@ def index(self, req, tenant_id, instance_id, user_id):

def update(self, req, body, tenant_id, instance_id, user_id):
"""Grant access for a user to one or more databases."""
LOG.info(_("Granting user access for instance '%s'") % instance_id)
LOG.info(_("req : '%s'\n\n") % req)
context = req.environ[wsgi.CONTEXT_KEY]
self.validate(body)
user = self._get_user(context, instance_id, user_id)
Expand All @@ -193,6 +206,8 @@ def update(self, req, body, tenant_id, instance_id, user_id):

def delete(self, req, tenant_id, instance_id, user_id, id):
"""Revoke access for a user."""
LOG.info(_("Revoking user access for instance '%s'") % instance_id)
LOG.info(_("req : '%s'\n\n") % req)
context = req.environ[wsgi.CONTEXT_KEY]
user = self._get_user(context, instance_id, user_id)
username, hostname = unquote_user_host(user_id)
Expand Down Expand Up @@ -249,8 +264,8 @@ def delete(self, req, tenant_id, instance_id, id):
schema = guest_models.MySQLDatabase()
schema.name = id
models.Schema.delete(context, instance_id, schema.serialize())
except ValueError as ve:
raise exception.BadRequest(str(ve))
except (ValueError, AttributeError) as e:
raise exception.BadRequest(msg=e)
return wsgi.Result(None, 202)

def show(self, req, tenant_id, instance_id, id):
Expand Down
6 changes: 5 additions & 1 deletion reddwarf/instance/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,11 @@ def create(self, req, body, tenant_id):
flavor_ref = body['instance']['flavorRef']
flavor_id = utils.get_id_from_href(flavor_ref)
databases = populate_databases(body['instance'].get('databases', []))
users = populate_users(body['instance'].get('users', []))
users = None
try:
users = populate_users(body['instance'].get('users', []))
except ValueError as ve:
raise exception.BadRequest(msg=ve)
if body['instance'].get('volume', None) is not None:
try:
volume_size = int(body['instance']['volume']['size'])
Expand Down
28 changes: 26 additions & 2 deletions reddwarf/tests/api/user_access.py
Original file line number Diff line number Diff line change
Expand Up @@ -432,11 +432,35 @@ def test_user_withperiod(self):
self._negative_user_test("test.user", self.databases)

@test
def test_user_empty(self):
def test_user_empty_no_host(self):
# This creates a request to .../<instance-id>/users//databases,
# which is parsed to mean "show me user 'databases', which in this
# case is a valid username, but not one of an extant user.
self._negative_user_test("", self.databases, 400, 400, 400, 400)
self._negative_user_test("", self.databases, 400, 500, 404, 404)

@test
def test_user_empty_with_host(self):
#self._negative_user_test("", self.databases, 400, 400, 400, 400)
# Try and fail to create the user.
empty_user = {"name": "", "host": "%",
"password": "password", "databases": []}
assert_raises(exceptions.BadRequest,
self.dbaas.users.create,
instance_info.id,
[empty_user])
assert_equal(400, self.dbaas.last_http_code)

assert_raises(exceptions.BadRequest, self.dbaas.users.grant,
instance_info.id, "", [], "%")
assert_equal(400, self.dbaas.last_http_code)

assert_raises(exceptions.BadRequest, self.dbaas.users.list_access,
instance_info.id, "", "%")
assert_equal(400, self.dbaas.last_http_code)

assert_raises(exceptions.BadRequest, self.dbaas.users.revoke,
instance_info.id, "", "db", "%")
assert_equal(400, self.dbaas.last_http_code)

@test
def test_user_nametoolong(self):
Expand Down
11 changes: 5 additions & 6 deletions reddwarf/tests/api/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,8 @@ class TestUsers(object):
"""

username = "tes!@#tuser"
username_urlencoded = "tes%21%40%23tuser"
password = "testpa$^%ssword"
username1 = "anous*&^er"
username1_urlendcoded = "anous%2A%26%5Eer"
password1 = "anopas*?.sword"
db1 = "usersfirstdb"
db2 = "usersseconddb"
Expand Down Expand Up @@ -136,7 +134,8 @@ def test_cannot_create_root_user(self):

@test(depends_on=[test_create_users_list])
def test_get_one_user(self):
user = self.dbaas.users.get(instance_info.id, username=self.username)
user = self.dbaas.users.get(instance_info.id, username=self.username,
hostname='%')
assert_equal(200, self.dbaas.last_http_code)
assert_equal(user.name, self.username)
assert_equal(1, len(user.databases))
Expand All @@ -159,9 +158,9 @@ def test_create_users_list_system(self):
@test(depends_on=[test_create_users_list],
runs_after=[test_fails_when_creating_user_twice])
def test_delete_users(self):
self.dbaas.users.delete(instance_info.id, self.username_urlencoded)
self.dbaas.users.delete(instance_info.id, self.username, hostname='%')
assert_equal(202, self.dbaas.last_http_code)
self.dbaas.users.delete(instance_info.id, self.username1_urlendcoded)
self.dbaas.users.delete(instance_info.id, self.username1, hostname='%')
assert_equal(202, self.dbaas.last_http_code)
if not FAKE:
time.sleep(5)
Expand Down Expand Up @@ -239,7 +238,7 @@ def check_database_for_user(self, user, password, dbs):
fail("User %s not added to collection." % user)

# Confirm via API get.
result = self.dbaas.users.get(instance_info.id, user)
result = self.dbaas.users.get(instance_info.id, user, '%')
assert_equal(200, self.dbaas.last_http_code)
if result.name != user:
fail("User %s not found via get." % user)
Expand Down

0 comments on commit f224dd5

Please sign in to comment.