Keystone-related improvements.

* Repairs updating for users and tenants. Fixes bug 912143.

  * Makes connection caching work for admin keystoneclient calls.
    Adds unit tests for horizon.api.keystone.keystoneclient.
    Fixes bug 933170.

  * In conjunction with this keystoneclient review
    https://review.openstack.org/#change,4133
    it takes care of the following bugs as well:

    Fixes bug 922394. Fixes bug 881606. Fixes bug 918997.

Change-Id: Id72c99772cd214c33fd1aacf357176cf67c6f473
This commit is contained in:
Gabriel Hurley 2012-02-15 15:43:19 -08:00
parent bcb4166eb7
commit d2df3ee6ed
9 changed files with 252 additions and 121 deletions
horizon/horizon
api
dashboards/syspanel
test.py
tests/api_tests
views

@ -26,6 +26,7 @@ from keystoneclient import service_catalog
from keystoneclient.v2_0 import client as keystone_client
from keystoneclient.v2_0 import tokens
from horizon.api import base
from horizon import exceptions
@ -33,13 +34,18 @@ LOG = logging.getLogger(__name__)
DEFAULT_ROLE = None
def _get_endpoint_url(request):
def _get_endpoint_url(request, endpoint_type, catalog=None):
if getattr(request.user, "service_catalog", None):
return base.url_for(request,
service_type='identity',
endpoint_type=endpoint_type)
return request.session.get('region_endpoint',
getattr(settings, 'OPENSTACK_KEYSTONE_URL'))
def keystoneclient(request, username=None, password=None, tenant_id=None,
token_id=None, endpoint=None, endpoint_type=None):
token_id=None, endpoint=None, endpoint_type='publicURL',
admin=False):
"""Returns a client connected to the Keystone backend.
Several forms of authentication are supported:
@ -55,75 +61,76 @@ def keystoneclient(request, username=None, password=None, tenant_id=None,
Lazy authentication if an ``endpoint`` parameter is provided.
Calls requiring the admin endpoint should have ``admin=True`` passed in
as a keyword argument.
The client is cached so that subsequent API calls during the same
request/response cycle don't have to be re-authenticated.
"""
# Take care of client connection caching/fetching a new client
user = request.user
if hasattr(request, '_keystone') and \
request._keystone.auth_token == token_id:
if admin:
if not user.is_admin():
raise exceptions.NotAuthorized
endpoint_type = 'adminURL'
# Take care of client connection caching/fetching a new client.
# Admin vs. non-admin clients are cached separately for token matching.
cache_attr = "_keystone_admin" if admin else "_keystone"
if hasattr(request, cache_attr) and (not token_id
or getattr(request, cache_attr).auth_token == token_id):
LOG.debug("Using cached client for token: %s" % user.token)
conn = request._keystone
conn = getattr(request, cache_attr)
else:
LOG.debug("Creating a new client connection with endpoint: %s."
% endpoint)
endpoint_lookup = _get_endpoint_url(request, endpoint_type)
auth_url = endpoint or endpoint_lookup
LOG.debug("Creating a new keystoneclient connection to %s." % auth_url)
conn = keystone_client.Client(username=username or user.username,
password=password,
tenant_id=tenant_id or user.tenant_id,
token=token_id or user.token,
auth_url=_get_endpoint_url(request),
auth_url=auth_url,
endpoint=endpoint)
request._keystone = conn
setattr(request, cache_attr, conn)
# Fetch the correct endpoint for the user type
# Fetch the correct endpoint if we've re-scoped the token.
catalog = getattr(conn, 'service_catalog', None)
if catalog and "serviceCatalog" in catalog.catalog.keys():
if endpoint_type:
endpoint = catalog.url_for(service_type='identity',
endpoint_type=endpoint_type)
elif user.is_admin():
endpoint = catalog.url_for(service_type='identity',
endpoint_type='adminURL')
else:
endpoint = catalog.url_for(service_type='identity',
endpoint_type='publicURL')
else:
endpoint = _get_endpoint_url(request)
catalog = catalog.catalog['serviceCatalog']
endpoint = _get_endpoint_url(request, endpoint_type, catalog)
conn.management_url = endpoint
return conn
def tenant_create(request, tenant_name, description, enabled):
return keystoneclient(request).tenants.create(tenant_name,
return keystoneclient(request, admin=True).tenants.create(tenant_name,
description,
enabled)
def tenant_get(request, tenant_id):
return keystoneclient(request).tenants.get(tenant_id)
def tenant_get(request, tenant_id, admin=False):
return keystoneclient(request, admin=admin).tenants.get(tenant_id)
def tenant_delete(request, tenant_id):
keystoneclient(request).tenants.delete(tenant_id)
keystoneclient(request, admin=True).tenants.delete(tenant_id)
def tenant_list(request):
return keystoneclient(request).tenants.list()
def tenant_list(request, admin=False):
return keystoneclient(request, admin=admin).tenants.list()
def tenant_update(request, tenant_id, tenant_name, description, enabled):
return keystoneclient(request).tenants.update(tenant_id,
tenant_name,
description,
enabled)
return keystoneclient(request, admin=True).tenants.update(tenant_id,
tenant_name,
description,
enabled)
def tenant_list_for_token(request, token, endpoint_type=None):
def tenant_list_for_token(request, token, endpoint_type='publicURL'):
c = keystoneclient(request,
token_id=token,
endpoint=_get_endpoint_url(request),
endpoint=_get_endpoint_url(request, endpoint_type),
endpoint_type=endpoint_type)
return c.tenants.list()
@ -139,7 +146,7 @@ def token_create(request, tenant, username, password):
username=username,
password=password,
tenant_id=tenant,
endpoint=_get_endpoint_url(request))
endpoint=_get_endpoint_url(request, 'publicURL'))
token = c.tokens.authenticate(username=username,
password=password,
tenant_id=tenant)
@ -156,7 +163,7 @@ def token_create_scoped(request, tenant, token):
c = keystoneclient(request,
tenant_id=tenant,
token_id=token,
endpoint=_get_endpoint_url(request))
endpoint=_get_endpoint_url(request, 'publicURL'))
raw_token = c.tokens.authenticate(tenant_id=tenant,
token=token,
return_raw=True)
@ -172,56 +179,59 @@ def token_create_scoped(request, tenant, token):
def user_list(request, tenant_id=None):
return keystoneclient(request).users.list(tenant_id=tenant_id)
return keystoneclient(request, admin=True).users.list(tenant_id=tenant_id)
def user_create(request, user_id, email, password, tenant_id, enabled):
return keystoneclient(request).users.create(user_id,
password,
email,
tenant_id,
enabled)
return keystoneclient(request, admin=True).users.create(user_id,
password,
email,
tenant_id,
enabled)
def user_delete(request, user_id):
keystoneclient(request).users.delete(user_id)
keystoneclient(request, admin=True).users.delete(user_id)
def user_get(request, user_id):
return keystoneclient(request).users.get(user_id)
def user_get(request, user_id, admin=True):
return keystoneclient(request, admin=admin).users.get(user_id)
def user_update_email(request, user_id, email):
return keystoneclient(request).users.update_email(user_id, email)
def user_update(request, user, **data):
return keystoneclient(request, admin=True).users.update(user, **data)
def user_update_enabled(request, user_id, enabled):
return keystoneclient(request).users.update_enabled(user_id, enabled)
return keystoneclient(request, admin=True).users.update_enabled(user_id,
enabled)
def user_update_password(request, user_id, password):
return keystoneclient(request).users.update_password(user_id, password)
def user_update_password(request, user_id, password, admin=True):
return keystoneclient(request, admin=admin).users.update_password(user_id,
password)
def user_update_tenant(request, user_id, tenant_id):
return keystoneclient(request).users.update_tenant(user_id, tenant_id)
def user_update_tenant(request, user_id, tenant_id, admin=True):
return keystoneclient(request, admin=admin).users.update_tenant(user_id,
tenant_id)
def role_list(request):
""" Returns a global list of available roles. """
return keystoneclient(request).roles.list()
return keystoneclient(request, admin=True).roles.list()
def add_tenant_user_role(request, tenant_id, user_id, role_id):
""" Adds a role for a user on a tenant. """
return keystoneclient(request).roles.add_user_role(user_id,
role_id,
tenant_id)
return keystoneclient(request, admin=True).roles.add_user_role(user_id,
role_id,
tenant_id)
def remove_tenant_user(request, tenant_id, user_id):
""" Removes all roles from a user on a tenant, removing them from it. """
client = keystoneclient(request)
client = keystoneclient(request, admin=True)
roles = client.roles.roles_for_user(user_id, tenant_id)
for role in roles:
client.roles.remove_user_role(user_id, role.id, tenant_id)
@ -237,7 +247,7 @@ def get_default_role(request):
default = getattr(settings, "OPENSTACK_KEYSTONE_DEFAULT_ROLE", None)
if default and DEFAULT_ROLE is None:
try:
roles = keystoneclient(request).roles.list()
roles = keystoneclient(request, admin=True).roles.list()
except:
exceptions.handle(request)
for role in roles:

@ -82,8 +82,7 @@ class CreateTenant(forms.SelfHandlingForm):
class UpdateTenant(forms.SelfHandlingForm):
id = forms.CharField(label=_("ID"),
widget=forms.TextInput(attrs={'readonly': 'readonly'}))
name = forms.CharField(label=_("Name"),
widget=forms.TextInput(attrs={'readonly': 'readonly'}))
name = forms.CharField(label=_("Name"))
description = forms.CharField(
widget=forms.widgets.Textarea(),
label=_("Description"))

@ -27,8 +27,9 @@ INDEX_URL = reverse('horizon:syspanel:projects:index')
class TenantsViewTests(test.BaseAdminViewTests):
def test_index(self):
self.mox.StubOutWithMock(api, 'tenant_list')
api.tenant_list(IsA(http.HttpRequest)).AndReturn(self.tenants.list())
self.mox.StubOutWithMock(api.keystone, 'tenant_list')
api.keystone.tenant_list(IsA(http.HttpRequest), admin=True) \
.AndReturn(self.tenants.list())
self.mox.ReplayAll()
res = self.client.get(INDEX_URL)
@ -50,7 +51,8 @@ class TenantsViewTests(test.BaseAdminViewTests):
self.mox.StubOutWithMock(api.keystone, 'tenant_get')
self.mox.StubOutWithMock(api.nova, 'tenant_quota_get')
self.mox.StubOutWithMock(api.nova, 'tenant_quota_update')
api.keystone.tenant_get(IgnoreArg(), tenant.id).AndReturn(tenant)
api.keystone.tenant_get(IgnoreArg(), tenant.id, admin=True) \
.AndReturn(tenant)
api.nova.tenant_quota_get(IgnoreArg(), tenant.id).AndReturn(quota)
api.nova.tenant_quota_update(IgnoreArg(), tenant.id, **quota_data)
self.mox.ReplayAll()
@ -65,12 +67,12 @@ class TenantsViewTests(test.BaseAdminViewTests):
def test_modify_users(self):
self.mox.StubOutWithMock(api.keystone, 'tenant_get')
self.mox.StubOutWithMock(api.keystone, 'user_list')
api.keystone.tenant_get(IgnoreArg(), self.tenant.id) \
api.keystone.tenant_get(IgnoreArg(), self.tenant.id, admin=True) \
.AndReturn(self.tenant)
api.keystone.user_list(IsA(http.HttpRequest)) \
.AndReturn(self.users.list())
api.keystone.user_list(IsA(http.HttpRequest),
self.tenant.id).AndReturn([self.user])
.AndReturn(self.users.list())
api.keystone.user_list(IsA(http.HttpRequest), self.tenant.id) \
.AndReturn([self.user])
self.mox.ReplayAll()
url = reverse('horizon:syspanel:projects:users',
args=(self.tenant.id,))

@ -21,11 +21,8 @@
import logging
import operator
from django import http
from django.contrib import messages
from django.core.urlresolvers import reverse
from django.utils.translation import ugettext as _
from keystoneclient import exceptions as api_exceptions
from horizon import api
from horizon import exceptions
@ -46,17 +43,9 @@ class IndexView(tables.DataTableView):
def get_data(self):
tenants = []
try:
tenants = api.tenant_list(self.request)
except api_exceptions.AuthorizationFailure, e:
LOG.exception("Unauthorized attempt to list tenants.")
messages.error(self.request, _('Unable to get tenant info: %s')
% e.message)
except Exception, e:
LOG.exception('Exception while getting tenant list')
if not hasattr(e, 'message'):
e.message = str(e)
messages.error(self.request, _('Unable to get tenant info: %s')
% e.message)
tenants = api.keystone.tenant_list(self.request, admin=True)
except:
exceptions.handle(self.request)
tenants.sort(key=lambda x: x.id, reverse=True)
return tenants
@ -74,12 +63,12 @@ class UpdateView(forms.ModalFormView):
def get_object(self, *args, **kwargs):
tenant_id = kwargs['tenant_id']
try:
return api.tenant_get(self.request, tenant_id)
except Exception as e:
LOG.exception('Error fetching tenant with id "%s"' % tenant_id)
messages.error(self.request, _('Unable to update tenant: %s')
% e.message)
raise http.Http404("Project with ID %s not found." % tenant_id)
return api.keystone.tenant_get(self.request, tenant_id, admin=True)
except:
redirect = reverse("horizon:syspanel:projects:index")
exceptions.handle(self.request,
_('Unable to retrieve tenant.'),
redirect=redirect)
def get_initial(self):
return {'id': self.object.id,
@ -96,7 +85,9 @@ class UsersView(tables.MultiTableView):
tenant_id = self.kwargs["tenant_id"]
if not hasattr(self, "_shared_data"):
try:
tenant = api.keystone.tenant_get(self.request, tenant_id)
tenant = api.keystone.tenant_get(self.request,
tenant_id,
admin=True)
all_users = api.keystone.user_list(self.request)
tenant_users = api.keystone.user_list(self.request, tenant_id)
self._shared_data = {'tenant': tenant,
@ -130,7 +121,9 @@ class AddUserView(forms.ModalFormView):
context_object_name = 'tenant'
def get_object(self, *args, **kwargs):
return api.keystone.tenant_get(self.request, kwargs["tenant_id"])
return api.keystone.tenant_get(self.request,
kwargs["tenant_id"],
admin=True)
def get_context_data(self, **kwargs):
context = super(AddUserView, self).get_context_data(**kwargs)
@ -165,11 +158,13 @@ class QuotasView(forms.ModalFormView):
context_object_name = 'tenant'
def get_object(self, *args, **kwargs):
return api.keystone.tenant_get(self.request, kwargs["tenant_id"])
return api.keystone.tenant_get(self.request,
kwargs["tenant_id"],
admin=True)
def get_initial(self):
quotas = api.nova.tenant_quota_get(self.request,
self.kwargs['tenant_id'])
self.kwargs['tenant_id'])
return {
'tenant_id': self.kwargs['tenant_id'],
'metadata_items': quotas.metadata_items,

@ -37,7 +37,7 @@ class BaseUserForm(forms.SelfHandlingForm):
super(BaseUserForm, self).__init__(*args, **kwargs)
# Populate tenant choices
tenant_choices = [('', "Select a tenant")]
for tenant in api.tenant_list(request):
for tenant in api.tenant_list(request, admin=True):
if tenant.enabled:
tenant_choices.append((tenant.id, tenant.name))
self.fields['tenant_id'].choices = tenant_choices
@ -49,7 +49,7 @@ class BaseUserForm(forms.SelfHandlingForm):
class CreateUserForm(BaseUserForm):
name = forms.CharField(label=_("Name"))
email = forms.CharField(label=_("Email"))
email = forms.EmailField(label=_("Email"))
password = forms.CharField(label=_("Password"),
widget=forms.PasswordInput(render_value=False),
required=False)
@ -84,28 +84,59 @@ class CreateUserForm(BaseUserForm):
class UpdateUserForm(BaseUserForm):
id = forms.CharField(label=_("ID"),
widget=forms.TextInput(attrs={'readonly': 'readonly'}))
# FIXME: keystone doesn't return the username from a get API call.
#name = forms.CharField(label=_("Name"))
email = forms.CharField(label=_("Email"))
id = forms.CharField(label=_("ID"), widget=forms.HiddenInput)
name = forms.CharField(label=_("User Name"))
email = forms.EmailField(label=_("Email"))
password = forms.CharField(label=_("Password"),
widget=forms.PasswordInput(render_value=False),
required=False)
tenant_id = forms.ChoiceField(label=_("Primary Project"))
def handle(self, request, data):
updated = []
if data['email']:
updated.append('email')
api.user_update_email(request, data['id'], data['email'])
if data['password']:
updated.append('password')
api.user_update_password(request, data['id'], data['password'])
if data['tenant_id']:
updated.append('tenant')
api.user_update_tenant(request, data['id'], data['tenant_id'])
messages.success(request,
_('Updated %(attrib)s for %(user)s.') %
{"attrib": ', '.join(updated), "user": data['id']})
failed, succeeded = [], []
user = data.pop('id')
password = data.pop('password')
tenant = data.pop('tenant_id')
# Discard the "method" param so we can pass kwargs to keystoneclient
data.pop('method')
# Update user details
msg_bits = (_('name'), _('email'))
try:
api.keystone.user_update(request, user, **data)
succeeded.extend(msg_bits)
except:
failed.extend(msg_bits)
exceptions.handle(request, ignore=True)
# Update default tenant
msg_bits = (_('primary project'),)
try:
api.user_update_tenant(request, user, tenant)
succeeded.extend(msg_bits)
except:
failed.append(msg_bits)
exceptions.handle(request, ignore=True)
# If present, update password
# FIXME(gabriel): password change should be its own form and view
if password:
msg_bits = (_('password'),)
try:
api.user_update_password(request, user, password)
succeeded.extend(msg_bits)
except:
failed.extend(msg_bits)
exceptions.handle(request, ignore=True)
if succeeded:
messages.success(request,
_('Updated %(attributes)s for "%(user)s".')
% {"user": data["name"],
"attributes": ", ".join(succeeded)})
if failed:
messages.error(request,
_('Unable to update %(attributes)s for "%(user)s".')
% {"user": data["name"],
"attributes": ", ".join(failed)})
return shortcuts.redirect('horizon:syspanel:users:index')

@ -20,12 +20,13 @@
import logging
from django import shortcuts
from django.contrib import messages
from django.core.urlresolvers import reverse
from django.utils.translation import ugettext as _
from keystoneclient import exceptions as api_exceptions
from horizon import api
from horizon import exceptions
from horizon import forms
from horizon import tables
from .forms import CreateUserForm, UpdateUserForm
@ -64,15 +65,16 @@ class UpdateView(forms.ModalFormView):
def get_object(self, *args, **kwargs):
user_id = kwargs['user_id']
try:
return api.user_get(self.request, user_id)
except Exception as e:
LOG.exception('Error fetching user with id "%s"' % user_id)
messages.error(self.request,
_('Unable to update user: %s') % e.message)
raise http.Http404("User with id %s not found." % user_id)
return api.user_get(self.request, user_id, admin=True)
except:
redirect = reverse("horizon:syspanel:users:index")
exceptions.handle(self.request,
_('Unable to update user.'),
redirect=redirect)
def get_initial(self):
return {'id': self.object.id,
'name': getattr(self.object, 'name', None),
'tenant_id': getattr(self.object, 'tenantId', None),
'email': getattr(self.object, 'email', '')}

@ -216,7 +216,8 @@ class APITestCase(TestCase):
super(APITestCase, self).setUp()
def fake_keystoneclient(request, username=None, password=None,
tenant_id=None, token_id=None, endpoint=None):
tenant_id=None, token_id=None, endpoint=None,
admin=False):
"""
Wrapper function which returns the stub keystoneclient. Only
necessary because the function takes too many arguments to

@ -20,8 +20,99 @@
from __future__ import absolute_import
from keystoneclient.v2_0 import client as keystone_client
from horizon import api
from horizon import test
from horizon import users
class FakeConnection(object):
pass
class ClientConnectionTests(test.TestCase):
def setUp(self):
super(ClientConnectionTests, self).setUp()
self.mox.StubOutWithMock(keystone_client, "Client")
self.test_user = users.User(id=self.user.id,
user=self.user.name,
service_catalog=self.service_catalog)
self.request.user = self.test_user
self.public_url = api.base.url_for(self.request,
'identity',
endpoint_type='publicURL')
self.admin_url = api.base.url_for(self.request,
'identity',
endpoint_type='adminURL')
self.conn = FakeConnection()
def test_connect(self):
keystone_client.Client(auth_url=self.public_url,
endpoint=None,
password=self.user.password,
tenant_id=None,
token=None,
username=self.user.name).AndReturn(self.conn)
self.mox.ReplayAll()
client = api.keystone.keystoneclient(self.request,
username=self.user.name,
password=self.user.password)
self.assertEqual(client.management_url, self.public_url)
def test_connect_admin(self):
self.test_user.roles = [{'name': 'admin'}]
keystone_client.Client(auth_url=self.admin_url,
endpoint=None,
password=self.user.password,
tenant_id=None,
token=None,
username=self.user.name).AndReturn(self.conn)
self.mox.ReplayAll()
client = api.keystone.keystoneclient(self.request,
username=self.user.name,
password=self.user.password,
admin=True)
self.assertEqual(client.management_url, self.admin_url)
def connection_caching(self):
self.test_user.roles = [{'name': 'admin'}]
# Regular connection
keystone_client.Client(auth_url=self.public_url,
endpoint=None,
password=self.user.password,
tenant_id=None,
token=None,
username=self.user.name).AndReturn(self.conn)
# Admin connection
keystone_client.Client(auth_url=self.admin_url,
endpoint=None,
password=self.user.password,
tenant_id=None,
token=None,
username=self.user.name).AndReturn(self.conn)
self.mox.ReplayAll()
# Request both admin and regular connections out of order,
# If the caching fails we would see UnexpectedMethodCall errors
# from mox.
client = api.keystone.keystoneclient(self.request,
username=self.user.name,
password=self.user.password)
self.assertEqual(client.management_url, self.public_url)
client = api.keystone.keystoneclient(self.request,
username=self.user.name,
password=self.user.password,
admin=True)
self.assertEqual(client.management_url, self.admin_url)
client = api.keystone.keystoneclient(self.request,
username=self.user.name,
password=self.user.password)
self.assertEqual(client.management_url, self.public_url)
client = api.keystone.keystoneclient(self.request,
username=self.user.name,
password=self.user.password,
admin=True)
self.assertEqual(client.management_url, self.admin_url)
class TokenApiTests(test.APITestCase):

@ -79,7 +79,7 @@ def switch_tenants(request, tenant_id):
_set_session_data(request, token)
user = users.User(users.get_user_from_request(request))
return shortcuts.redirect(Horizon.get_user_home(user))
except Exception, e:
except:
exceptions.handle(request,
_("You are not authorized for that tenant."))