Restrict changing subscription of different tenant

In current tacker multi-tenant environment, users are able to perform
show and delete operations on subscriptions belonging to different
tenants.

This patch fixes the above problem by adding tenant_id in where clause
of the existing SQL queries used to show and delete subscriptions in
tacker.

Note: The functional test cases for the proposed changes are covered
in [1].

[1] https://review.opendev.org/c/openstack/tacker/+/846130

Closes-Bug: #1983575
Change-Id: I202bbcb2b8294993c4961afce0599e0737b0464d

releasenotes/notes/bug-1983575-disallow-subscription-change-of-diff-tenant-06a03559bfca3431.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    Fixes `bug 1983575`_. In a multi-tenant environment, tacker disallows
+    deletion operation and showing of subscription details belonging to
+    different tenants.
+
+    .. _bug 1983575: https://bugs.launchpad.net/tacker/+bug/1983575

tacker/objects/vnf_lcm_subscriptions.py

@@ -160,7 +160,12 @@ def _vnf_lcm_subscriptions_get(context,
 
 @db_api.context_manager.reader
 def _vnf_lcm_subscriptions_show(context, subscriptionId):
+    """Query to retrieve desired subscription details
 
+    The SQL query fetches subscription details only when
+    the requester's tenant details match with the target
+    subscription.
+    """
     sql = text(
         "select "
         "t1.id,t1.callback_uri,t1.tenant_id,t2.filter "
@@ -168,14 +173,17 @@ def _vnf_lcm_subscriptions_show(context, subscriptionId):
         "(select distinct subscription_uuid,filter from vnf_lcm_filters) t2 "
         "where t1.id = t2.subscription_uuid "
         "and deleted = 0 "
-        "and t1.id = :subsc_id")
+        "and t1.id = :subsc_id "
+        "and t1.tenant_id = :tenant_id")
     result_line = ""
     try:
-        result = context.session.execute(sql, {'subsc_id': subscriptionId})
+        result = context.session.execute(sql, {'subsc_id': subscriptionId,
+            'tenant_id': context.project_id})
         for line in result:
             result_line = line
     except exceptions.NotFound:
-        return ''
+        LOG.error('Subscription %(id) not found.', {"id": subscriptionId})
+        return None
     except Exception as e:
         raise e
     return result_line
@@ -208,15 +216,20 @@ def _get_by_subscriptionid(context, subscriptionsId):
     sql = text("select id "
              "from vnf_lcm_subscriptions "
              "where id = :subsc_id "
-             "and deleted = 0 ")
+             "and deleted = 0 "
+             "and tenant_id = :tenant_id")
+    result_line = ""
     try:
-        result = context.session.execute(sql, {'subsc_id': subscriptionsId})
+        result = context.session.execute(sql, {'subsc_id': subscriptionsId,
+            'tenant_id': context.project_id})
+        for line in result:
+            result_line = line
     except exceptions.NotFound:
-        return ''
+        LOG.error('Subscription %(id) not found.', {"id": subscriptionsId})
+        return None
     except Exception as e:
         raise e
-
+    return result_line
-    return result
 
 
 @db_api.context_manager.reader

tacker/tests/unit/objects/test_vnf_lcm_subscriptions.py

@@ -204,6 +204,17 @@ class TestVnfLcmSubScriptions(SqlTestCase):
             self.context, self.subscription.id)
         self.assertTrue(filter, result)
 
+    @mock.patch.object(objects.vnf_lcm_subscriptions,
+                       '_vnf_lcm_subscriptions_show')
+    def test_show_for_invalid_tenant_id(self, mock_vnf_lcm_subscriptions_show):
+        mock_vnf_lcm_subscriptions_show.return_value = None
+        self.context.tenant_id = 12345
+        subs_obj = objects.vnf_lcm_subscriptions.LccnSubscriptionRequest(
+            context=self.context)
+        result = subs_obj.vnf_lcm_subscriptions_show(self.context,
+            self.subscription.id)
+        self.assertIsNone(result)
+
     @mock.patch.object(objects.vnf_lcm_subscriptions,
                        '_vnf_lcm_subscriptions_all')
     def test_list(self, mock_vnf_lcm_subscriptions_all):
@@ -277,6 +288,18 @@ class TestVnfLcmSubScriptions(SqlTestCase):
         mock_vnf_lcm_subscriptions_destroy.assert_called_with(
             self.context, self.subscription.id)
 
+    @mock.patch.object(objects.vnf_lcm_subscriptions,
+                       '_destroy_vnf_lcm_subscription')
+    @mock.patch.object(objects.vnf_lcm_subscriptions, '_get_by_subscriptionid')
+    def test_destroy_for_invalid_tenant_id(self,
+            mock_get_by_subscriptionid,
+            mock_vnf_lcm_subscriptions_destroy):
+        mock_get_by_subscriptionid.result_value = None
+        self.context.tenant_id = 12345
+        self.subscription.destroy(self.context, self.subscription.id)
+        mock_vnf_lcm_subscriptions_destroy.assert_called_with(
+            self.context, self.subscription.id)
+
     @mock.patch.object(objects.vnf_lcm_subscriptions,
                        '_vnf_lcm_subscriptions_get')
     def test_get(self, mock_vnf_lcm_subscriptions_get):