From e1d12a4e49ab720f69b8a82a573faf68d1d8761f Mon Sep 17 00:00:00 2001 From: Gabriel Hurley Date: Sun, 17 Mar 2013 12:14:29 -0700 Subject: [PATCH] Improve UX around "unauthorized" API exceptions. Instead of blindly logging out the user when any API returns a 401 or 403 response (which in most cases is due to a service being down or misconfigured) we catch the error and inform the user that they are not authorized for that data. This is separate from being unauthorized for a dashboard or panel in Horizon, since those are within our control and involve security concerns around exposing admin functionalities to end users. Those checks function as they have previously. Fixes bug 1060426. Change-Id: Ied800f10926ac5fb3b9ac1c1c26bbb4fa94a2557 --- horizon/exceptions.py | 13 ++++++++++--- .../dashboards/admin/overview/views.py | 11 +++++++++-- .../dashboards/project/overview/tests.py | 9 +++++++-- openstack_dashboard/test/test_data/exceptions.py | 3 +++ 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/horizon/exceptions.py b/horizon/exceptions.py index 5f17a4e897..0402d60a6b 100644 --- a/horizon/exceptions.py +++ b/horizon/exceptions.py @@ -274,15 +274,22 @@ def handle(request, message=None, redirect=None, ignore=False, if issubclass(exc_type, UNAUTHORIZED): if ignore: return NotAuthorized - logout(request) if not force_silence and not handled: log_method(error_color("Unauthorized: %s" % exc_value)) if not handled: + if message: + message = _("Unauthorized: %s") % message # We get some pretty useless error messages back from # some clients, so let's define our own fallback. fallback = _("Unauthorized. Please try logging in again.") - messages.error(request, message or fallback, extra_tags="login") - raise NotAuthorized # Redirect handled in middleware + messages.error(request, message or fallback) + # Escalation means logging the user out and raising NotAuthorized + # so the middleware will redirect them appropriately. + if escalate: + logout(request) + raise NotAuthorized + # Otherwise continue and present our "unauthorized" error message. + return NotAuthorized if issubclass(exc_type, NOT_FOUND): wrap = True diff --git a/openstack_dashboard/dashboards/admin/overview/views.py b/openstack_dashboard/dashboards/admin/overview/views.py index 679250f615..8f77148229 100644 --- a/openstack_dashboard/dashboards/admin/overview/views.py +++ b/openstack_dashboard/dashboards/admin/overview/views.py @@ -19,6 +19,9 @@ # under the License. from django.conf import settings +from django.utils.translation import ugettext as _ + +from horizon import exceptions from openstack_dashboard import api from openstack_dashboard import usage @@ -37,8 +40,12 @@ class GlobalOverview(usage.UsageView): def get_data(self): data = super(GlobalOverview, self).get_data() # Pre-fill tenant names - tenants = api.keystone.tenant_list(self.request, - admin=True) + try: + tenants = api.keystone.tenant_list(self.request, admin=True) + except: + tenants = [] + exceptions.handle(self.request, + _('Unable to retrieve project list.')) for instance in data: tenant = filter(lambda t: t.id == instance.tenant_id, tenants) if tenant: diff --git a/openstack_dashboard/dashboards/project/overview/tests.py b/openstack_dashboard/dashboards/project/overview/tests.py index 811f7286e5..386ce88d2b 100644 --- a/openstack_dashboard/dashboards/project/overview/tests.py +++ b/openstack_dashboard/dashboards/project/overview/tests.py @@ -55,18 +55,23 @@ class UsageViewTests(test.TestCase): self.assertContains(res, 'form-horizontal') def test_unauthorized(self): - exc = self.exceptions.keystone_unauthorized + exc = self.exceptions.nova_unauthorized now = timezone.now() + quota_data = self.quota_usages.first() self.mox.StubOutWithMock(api.nova, 'usage_get') + self.mox.StubOutWithMock(quotas, 'tenant_quota_usages') api.nova.usage_get(IsA(http.HttpRequest), self.tenant.id, datetime.datetime(now.year, now.month, 1, 0, 0, 0), Func(usage.almost_now)) \ .AndRaise(exc) + quotas.tenant_quota_usages(IsA(http.HttpRequest)).AndReturn(quota_data) self.mox.ReplayAll() url = reverse('horizon:project:overview:index') res = self.client.get(url) - self.assertRedirects(res, reverse("login") + "?next=" + url) + self.assertTemplateUsed(res, 'project/overview/usage.html') + self.assertMessageCount(res, error=1) + self.assertContains(res, 'Unauthorized:') def test_usage_csv(self): now = timezone.now() diff --git a/openstack_dashboard/test/test_data/exceptions.py b/openstack_dashboard/test/test_data/exceptions.py index 9f58692033..413bacf95e 100644 --- a/openstack_dashboard/test/test_data/exceptions.py +++ b/openstack_dashboard/test/test_data/exceptions.py @@ -54,6 +54,9 @@ def data(TEST): nova_exception = nova_exceptions.ClientException TEST.exceptions.nova = create_stubbed_exception(nova_exception) + nova_unauth = nova_exceptions.Unauthorized + TEST.exceptions.nova_unauthorized = create_stubbed_exception(nova_unauth) + glance_exception = glance_exceptions.ClientException TEST.exceptions.glance = create_stubbed_exception(glance_exception)