From: Gorka Eguileor <geguileo@redhat.com>
Date: Tue, 18 Aug 2015 09:29:15 +0000 (+0200)
Subject: Fix backup list all_tenants=0 filtering for admin
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=34101027c6fea35f8acbc0463a7e6c95f6dfeb66;p=openstack-build%2Fcinder-build.git

Fix backup list all_tenants=0 filtering for admin

When a user with admin role lists backups specifying all_tenants=0 in
the request, it will not behave as expected, it will return backups for
all tenants just as if all_tenants=1 had been specified.

This patch fixes this issue and will only return all tenants in backup
listings if all_tenants is set to a value meaning True (1, true, yes,
y).

Change-Id: Ia74bc258a86f6dc48483dfe17bf5a312aec4058d
Closes-Bug: #1485900
---

diff --git a/cinder/backup/api.py b/cinder/backup/api.py
index cc4c24ef4..ddfa871a4 100644
--- a/cinder/backup/api.py
+++ b/cinder/backup/api.py
@@ -24,6 +24,7 @@ from eventlet import greenthread
 from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_utils import excutils
+from oslo_utils import strutils
 
 from cinder.backup import rpcapi as backup_rpcapi
 from cinder import context
@@ -103,9 +104,12 @@ class API(base.Base):
 
         search_opts = search_opts or {}
 
-        if (context.is_admin and 'all_tenants' in search_opts):
-            # Need to remove all_tenants to pass the filtering below.
-            search_opts.pop('all_tenants', None)
+        all_tenants = search_opts.pop('all_tenants', '0')
+        if not utils.is_valid_boolstr(all_tenants):
+            msg = _("all_tenants must be a boolean, got '%s'.") % all_tenants
+            raise exception.InvalidParameterValue(err=msg)
+
+        if context.is_admin and strutils.bool_from_string(all_tenants):
             backups = objects.BackupList.get_all(context, filters=search_opts)
         else:
             backups = objects.BackupList.get_all_by_project(
diff --git a/cinder/tests/unit/test_backup.py b/cinder/tests/unit/test_backup.py
index 52278d715..238b52709 100644
--- a/cinder/tests/unit/test_backup.py
+++ b/cinder/tests/unit/test_backup.py
@@ -22,6 +22,7 @@ from oslo_config import cfg
 from oslo_utils import importutils
 from oslo_utils import timeutils
 
+from cinder.backup import api
 from cinder.backup import manager
 from cinder import context
 from cinder import db
@@ -890,3 +891,42 @@ class BackupTestCaseWithVerify(BaseBackupTest):
         self.backup_mgr.reset_status(self.ctxt, backup, 'error')
         backup = db.backup_get(self.ctxt, backup['id'])
         self.assertEqual('error', backup['status'])
+
+
+class BackupAPITestCase(BaseBackupTest):
+    def setUp(self):
+        super(BackupAPITestCase, self).setUp()
+        self.api = api.API()
+
+    def test_get_all_wrong_all_tenants_value(self):
+        self.assertRaises(exception.InvalidParameterValue,
+                          self.api.get_all, self.ctxt, {'all_tenants': 'bad'})
+
+    @mock.patch.object(objects, 'BackupList')
+    def test_get_all_no_all_tenants_value(self, mock_backuplist):
+        result = self.api.get_all(self.ctxt, {'key': 'value'})
+        self.assertFalse(mock_backuplist.get_all.called)
+        self.assertEqual(mock_backuplist.get_all_by_project.return_value,
+                         result)
+        mock_backuplist.get_all_by_project.assert_called_once_with(
+            self.ctxt, self.ctxt.project_id, filters={'key': 'value'})
+
+    @mock.patch.object(objects, 'BackupList')
+    def test_get_all_false_value_all_tenants(self, mock_backuplist):
+        result = self.api.get_all(self.ctxt, {'all_tenants': '0',
+                                              'key': 'value'})
+        self.assertFalse(mock_backuplist.get_all.called)
+        self.assertEqual(mock_backuplist.get_all_by_project.return_value,
+                         result)
+        mock_backuplist.get_all_by_project.assert_called_once_with(
+            self.ctxt, self.ctxt.project_id, filters={'key': 'value'})
+
+    @mock.patch.object(objects, 'BackupList')
+    def test_get_all_true_value_all_tenants(self, mock_backuplist):
+        result = self.api.get_all(self.ctxt, {'all_tenants': 'true',
+                                              'key': 'value'})
+        self.assertFalse(mock_backuplist.get_all_by_project.called)
+        self.assertEqual(mock_backuplist.get_all.return_value,
+                         result)
+        mock_backuplist.get_all.assert_called_once_with(
+            self.ctxt, filters={'key': 'value'})