From a13ff5d5a73c9c4c4c7b56fe60c6152402127f10 Mon Sep 17 00:00:00 2001
From: Goutham Pacha Ravi <gouthampravi@gmail.com>
Date: Tue, 2 Mar 2021 15:53:02 -0800
Subject: [PATCH] Fix traceback in scheduler-stats API

There was a traceback being included in the
error message body. This is unhelpful to
end users.

The error message that included the traceback
was for this corner case where the RBAC policy
isn't aligned with the internal "context_is_admin"
policy - an unlikely combination of decisions
that a deployer would make - nevertheless,
this is an opportunity for us to fix this
code path.

Change-Id: I888d684acac2133425f986ec7cef5e4f5cdcc5b6
Closes-Bug: #1917520
Signed-off-by: Goutham Pacha Ravi <gouthampravi@gmail.com>
---
 manila/api/v1/scheduler_stats.py              |  8 ++++++--
 manila/tests/api/v1/test_scheduler_stats.py   | 19 +++++++++++++++++++
 ...-if-action-forbidden-0da51825756fd5fc.yaml |  7 +++++++
 3 files changed, 32 insertions(+), 2 deletions(-)
 create mode 100644 releasenotes/notes/bug-1917520-avoid-sending-traceback-to-user-if-action-forbidden-0da51825756fd5fc.yaml

diff --git a/manila/api/v1/scheduler_stats.py b/manila/api/v1/scheduler_stats.py
index 335ef4cc2e..38a0add01a 100644
--- a/manila/api/v1/scheduler_stats.py
+++ b/manila/api/v1/scheduler_stats.py
@@ -71,8 +71,12 @@ class SchedulerStatsController(wsgi.Controller):
                     msg = _("Share type %s not found.") % req_share_type
                     raise exc.HTTPBadRequest(explanation=msg)
 
-        pools = self.scheduler_api.get_pools(context, filters=search_opts,
-                                             cached=True)
+        try:
+            pools = self.scheduler_api.get_pools(context,
+                                                 filters=search_opts,
+                                                 cached=True)
+        except exception.NotAuthorized:
+            raise exc.HTTPForbidden()
         detail = (action == 'detail')
         return self._view_builder.pools(pools, detail=detail)
 
diff --git a/manila/tests/api/v1/test_scheduler_stats.py b/manila/tests/api/v1/test_scheduler_stats.py
index c9fa440fe0..5757688c55 100644
--- a/manila/tests/api/v1/test_scheduler_stats.py
+++ b/manila/tests/api/v1/test_scheduler_stats.py
@@ -21,6 +21,7 @@ from webob import exc
 from manila.api.openstack import api_version_request as api_version
 from manila.api.v1 import scheduler_stats
 from manila import context
+from manila import exception
 from manila import policy
 from manila.scheduler import rpcapi
 from manila.share import share_types
@@ -333,6 +334,24 @@ class SchedulerStatsControllerTestCase(test.TestCase):
         self.mock_policy_check.assert_called_once_with(
             self.ctxt, self.resource_name, 'detail')
 
+    @ddt.data('index', 'detail')
+    def test_pools_forbidden(self, subresource):
+        mock_get_pools = self.mock_object(
+            rpcapi.SchedulerAPI, 'get_pools',
+            mock.Mock(side_effect=exception.AdminRequired(
+                "some traceback here")))
+        path = '/v1/fake_project/scheduler_stats/pools'
+        path = path + ('/%s' % subresource if subresource == 'detail' else '')
+        req = fakes.HTTPRequest.blank(path)
+        req.environ['manila.context'] = self.ctxt
+
+        self.assertRaises(exc.HTTPForbidden,
+                          getattr(self.controller, 'pools_%s' % subresource),
+                          req)
+        mock_get_pools.assert_called_once_with(self.ctxt,
+                                               filters={},
+                                               cached=True)
+
 
 class SchedulerStatsTestCase(test.TestCase):
 
diff --git a/releasenotes/notes/bug-1917520-avoid-sending-traceback-to-user-if-action-forbidden-0da51825756fd5fc.yaml b/releasenotes/notes/bug-1917520-avoid-sending-traceback-to-user-if-action-forbidden-0da51825756fd5fc.yaml
new file mode 100644
index 0000000000..a7f5c6d9bc
--- /dev/null
+++ b/releasenotes/notes/bug-1917520-avoid-sending-traceback-to-user-if-action-forbidden-0da51825756fd5fc.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    The scheduler stats resource APIs (/scheduler-stats/pools and
+    /scheduler-stats/pools/detail) have been fixed to not return an
+    arbitrary traceback in the error message body to the caller when access to
+    the resource has been denied.