Merge "Fix failing v2 identity user tests by adding admin_only kwarg."
diff --git a/patrole_tempest_plugin/rbac_rule_validation.py b/patrole_tempest_plugin/rbac_rule_validation.py
index 33ee666..5e1e816 100644
--- a/patrole_tempest_plugin/rbac_rule_validation.py
+++ b/patrole_tempest_plugin/rbac_rule_validation.py
@@ -26,7 +26,37 @@
 LOG = logging.getLogger(__name__)
 
 
-def action(service, rule, expected_error_code=403):
+def action(service, rule, admin_only=False, expected_error_code=403):
+    """A decorator which does a policy check and matches it against test run.
+
+    A decorator which allows for positive and negative RBAC testing. Given
+    an OpenStack service and a policy action enforced by that service, an
+    oslo.policy lookup is performed by calling `authority.get_permission`.
+    The following cases are possible:
+
+    * If `allowed` is True and the test passes, this is a success.
+    * If `allowed` is True and the test fails, this is a failure.
+    * If `allowed` is False and the test passes, this is a failure.
+    * If `allowed` is False and the test fails, this is a success.
+
+    :param service: A OpenStack service: for example, "nova" or "neutron".
+    :param rule: A policy action defined in a policy.json file (or in code).
+    :param admin_only: Skips over oslo.policy check because the policy action
+                       defined by `rule` is not enforced by the service's
+                       policy enforcement logic. For example, Keystone v2
+                       performs an admin check for most of its endpoints. If
+                       True, `rule` is effectively ignored.
+    :param expected_error_code: Overrides default value of 403 (Forbidden)
+                                with endpoint-specific error code. Currently
+                                only supports 403 and 404. Support for 404
+                                is needed because some services, like Neutron,
+                                intentionally throw a 404 for security reasons.
+
+    :raises NotFound: if `service` is invalid or
+                      if Tempest credentials cannot be found.
+    :raises Forbidden: for bullet (2) above.
+    :raises RbacOverPermission: for bullet (3) above.
+    """
     def decorator(func):
         def wrapper(*args, **kwargs):
             try:
@@ -41,8 +71,17 @@
                 LOG.error(msg)
                 raise rbac_exceptions.RbacResourceSetupFailed(msg)
 
-            authority = rbac_auth.RbacAuthority(tenant_id, user_id, service)
-            allowed = authority.get_permission(rule, CONF.rbac.rbac_test_role)
+            if admin_only:
+                LOG.info("As admin_only is True, only admin role should be "
+                         "allowed to perform the API. Skipping oslo.policy "
+                         "check for policy action {0}.".format(rule))
+                allowed = CONF.rbac.rbac_test_role == 'admin'
+            else:
+                authority = rbac_auth.RbacAuthority(tenant_id, user_id,
+                                                    service)
+                allowed = authority.get_permission(rule,
+                                                   CONF.rbac.rbac_test_role)
+
             expected_exception, irregular_msg = _get_exception_type(
                 expected_error_code)
 
diff --git a/patrole_tempest_plugin/tests/api/identity/v2/test_endpoints_rbac.py b/patrole_tempest_plugin/tests/api/identity/v2/test_endpoints_rbac.py
index ccde9bd..ba04d06 100644
--- a/patrole_tempest_plugin/tests/api/identity/v2/test_endpoints_rbac.py
+++ b/patrole_tempest_plugin/tests/api/identity/v2/test_endpoints_rbac.py
@@ -54,7 +54,8 @@
         return endpoint
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:create_endpoint")
+                                 rule="identity:create_endpoint",
+                                 admin_only=True)
     @decorators.idempotent_id('6bdaecd4-0843-4ed6-ab64-3a57ab0cd124')
     def test_create_endpoint(self):
 
@@ -67,7 +68,8 @@
         self._create_endpoint()
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:delete_endpoint")
+                                 rule="identity:delete_endpoint",
+                                 admin_only=True)
     @decorators.idempotent_id('6bdaecd4-0843-4ed6-ab64-3a57ab0cd125')
     def test_delete_endpoint(self):
 
@@ -81,7 +83,8 @@
         self.endpoints_client.delete_endpoint(endpoint['endpoint']['id'])
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:list_endpoints")
+                                 rule="identity:list_endpoints",
+                                 admin_only=True)
     @decorators.idempotent_id('6bdaecd4-0843-4ed6-ab64-3a57ab0cd126')
     def test_list_endpoints(self):
 
diff --git a/patrole_tempest_plugin/tests/api/identity/v2/test_projects_rbac.py b/patrole_tempest_plugin/tests/api/identity/v2/test_projects_rbac.py
index 9c4ab33..dd0af46 100644
--- a/patrole_tempest_plugin/tests/api/identity/v2/test_projects_rbac.py
+++ b/patrole_tempest_plugin/tests/api/identity/v2/test_projects_rbac.py
@@ -25,7 +25,8 @@
 class IdentityProjectV2AdminRbacTest(rbac_base.BaseIdentityV2AdminRbacTest):
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:create_project")
+                                 rule="identity:create_project",
+                                 admin_only=True)
     @decorators.idempotent_id('0f148510-63bf-11e6-b348-080044d0d904')
     def test_create_project(self):
 
@@ -38,7 +39,8 @@
         self._create_tenant()
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:update_project")
+                                 rule="identity:update_project",
+                                 admin_only=True)
     @decorators.idempotent_id('0f148510-63bf-11e6-b348-080044d0d905')
     def test_update_project(self):
 
@@ -53,7 +55,8 @@
                                           description="Changed description")
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:delete_project")
+                                 rule="identity:delete_project",
+                                 admin_only=True)
     @decorators.idempotent_id('0f148510-63bf-11e6-b348-080044d0d906')
     def test_delete_project(self):
 
@@ -67,7 +70,8 @@
         self.tenants_client.delete_tenant(tenant['id'])
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:get_project")
+                                 rule="identity:get_project",
+                                 admin_only=True)
     @decorators.idempotent_id('0f148510-63bf-11e6-b348-080044d0d907')
     def test_get_project(self):
 
@@ -82,7 +86,8 @@
         self.tenants_client.show_tenant(tenant['id'])
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:list_projects")
+                                 rule="identity:list_projects",
+                                 admin_only=True)
     @decorators.idempotent_id('0f148510-63bf-11e6-b348-080044d0d908')
     def test_get_all_projects(self):
 
@@ -94,9 +99,10 @@
         self.tenants_client.list_tenants()
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:list_user_projects")
+                                 rule="identity:list_user_projects",
+                                 admin_only=True)
     @decorators.idempotent_id('0f148510-63bf-11e6-b348-080044d0d909')
-    def test_list_users_for_tenant(self):
+    def test_list_project_users(self):
 
         """Get Users of a Project Test
 
diff --git a/patrole_tempest_plugin/tests/api/identity/v2/test_roles_rbac.py b/patrole_tempest_plugin/tests/api/identity/v2/test_roles_rbac.py
index 74a423e..aeb8d05 100644
--- a/patrole_tempest_plugin/tests/api/identity/v2/test_roles_rbac.py
+++ b/patrole_tempest_plugin/tests/api/identity/v2/test_roles_rbac.py
@@ -53,7 +53,8 @@
             tenant['id'], user['id'], role['id'])
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:create_role")
+                                 rule="identity:create_role",
+                                 admin_only=True)
     @decorators.idempotent_id('0f148510-63bf-11e6-8674-080044d0d904')
     def test_create_role(self):
 
@@ -66,7 +67,8 @@
         self._create_role()
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:delete_role")
+                                 rule="identity:delete_role",
+                                 admin_only=True)
     @decorators.idempotent_id('0f148510-63bf-11e6-8674-080044d0d905')
     def test_delete_role(self):
 
@@ -80,7 +82,8 @@
         self.roles_client.delete_role(role['id'])
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:get_role")
+                                 rule="identity:get_role",
+                                 admin_only=True)
     @decorators.idempotent_id('0f148510-63bf-11e6-8674-080044d0d906')
     def test_show_role(self):
 
@@ -94,7 +97,8 @@
         self.roles_client.show_role(role['id'])
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:list_roles")
+                                 rule="identity:list_roles",
+                                 admin_only=True)
     @decorators.idempotent_id('0f148510-63bf-11e6-8674-080044d0d907')
     def test_list_roles(self):
 
@@ -106,7 +110,8 @@
         self.roles_client.list_roles()
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:add_role_to_user")
+                                 rule="identity:add_role_to_user",
+                                 admin_only=True)
     @decorators.idempotent_id('0f148510-63bf-11e6-8674-080044d0d908')
     def test_create_role_on_project(self):
 
@@ -119,7 +124,8 @@
         self._create_role_on_project(tenant, user, role)
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:remove_role_from_user")
+                                 rule="identity:remove_role_from_user",
+                                 admin_only=True)
     @decorators.idempotent_id('0f148510-63bf-11e6-8674-080044d0d909')
     def test_delete_role_from_user_on_project(self):
 
@@ -135,7 +141,8 @@
             tenant['id'], user['id'], role['id'])
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:get_user_roles")
+                                 rule="identity:get_user_roles",
+                                 admin_only=True)
     @decorators.idempotent_id('0f148510-63bf-11e6-8674-080044d0d90a')
     def test_list_user_roles_on_project(self):
 
diff --git a/patrole_tempest_plugin/tests/api/identity/v2/test_services_rbac.py b/patrole_tempest_plugin/tests/api/identity/v2/test_services_rbac.py
index 6ba60fa..bc3dae8 100644
--- a/patrole_tempest_plugin/tests/api/identity/v2/test_services_rbac.py
+++ b/patrole_tempest_plugin/tests/api/identity/v2/test_services_rbac.py
@@ -30,7 +30,8 @@
         cls.services_client = cls.os.identity_services_client
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:create_service")
+                                 rule="identity:create_service",
+                                 admin_only=True)
     @decorators.idempotent_id('370050f6-d271-4fb4-abc5-4de1d6dfbad2')
     def test_create_service(self):
         """Create Service Test
@@ -41,7 +42,8 @@
         self._create_service()
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:delete_service")
+                                 rule="identity:delete_service",
+                                 admin_only=True)
     @decorators.idempotent_id('f6c64fc3-6a1f-423e-af91-e411add3a384')
     def test_delete_service(self):
         """Delete Service Test
@@ -54,7 +56,8 @@
         self.services_client.delete_service(service_id)
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:get_service")
+                                 rule="identity:get_service",
+                                 admin_only=True)
     @decorators.idempotent_id('504d62bb-97d7-445e-9d6d-b1945a7c9e08')
     def test_show_service(self):
         """Show Service Test
@@ -67,7 +70,8 @@
         self.services_client.show_service(service_id)
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:list_services")
+                                 rule="identity:list_services",
+                                 admin_only=True)
     @decorators.idempotent_id('d7dc461d-51ad-48e0-9cd2-33add1b88de9')
     def test_list_services(self):
         """List all the services
diff --git a/patrole_tempest_plugin/tests/api/identity/v2/test_users_rbac.py b/patrole_tempest_plugin/tests/api/identity/v2/test_users_rbac.py
index a94a811..da5cdce 100644
--- a/patrole_tempest_plugin/tests/api/identity/v2/test_users_rbac.py
+++ b/patrole_tempest_plugin/tests/api/identity/v2/test_users_rbac.py
@@ -23,14 +23,16 @@
 class IdentityUserV2AdminRbacTest(rbac_base.BaseIdentityV2AdminRbacTest):
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:create_user")
+                                 rule="identity:create_user",
+                                 admin_only=True)
     @decorators.idempotent_id('0f148510-63bf-11e6-1342-080044d0d904')
     def test_create_user(self):
         self.rbac_utils.switch_role(self, switchToRbacRole=True)
         self._create_user()
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:update_user")
+                                 rule="identity:update_user",
+                                 admin_only=True)
     @decorators.idempotent_id('0f148510-63bf-11e6-1342-080044d0d905')
     def test_update_user(self):
         user = self._create_user()
@@ -39,7 +41,8 @@
         self.users_client.update_user(user['id'], email="changedUser@xyz.com")
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:set_user_enabled")
+                                 rule="identity:set_user_enabled",
+                                 admin_only=True)
     @decorators.idempotent_id('0f148510-63bf-11e6-1342-080044d0d9a1')
     def test_update_user_enabled(self):
         user = self._create_user()
@@ -48,7 +51,8 @@
         self.users_client.update_user_enabled(user['id'], enabled=True)
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:delete_user")
+                                 rule="identity:delete_user",
+                                 admin_only=True)
     @decorators.idempotent_id('0f148510-63bf-11e6-1342-080044d0d906')
     def test_delete_user(self):
         user = self._create_user()
@@ -57,14 +61,16 @@
         self.users_client.delete_user(user['id'])
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:get_users")
+                                 rule="identity:get_users",
+                                 admin_only=True)
     @decorators.idempotent_id('0f148510-63bf-11e6-1342-080044d0d907')
     def test_list_users(self):
         self.rbac_utils.switch_role(self, switchToRbacRole=True)
         self.users_client.list_users()
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:get_user")
+                                 rule="identity:get_user",
+                                 admin_only=True)
     @decorators.idempotent_id('0f148510-63bf-11e6-1342-080044d0d908')
     def test_show_user(self):
         user = self._create_user()
@@ -73,7 +79,8 @@
         self.users_client.show_user(user['id'])
 
     @rbac_rule_validation.action(service="keystone",
-                                 rule="identity:change_password")
+                                 rule="identity:change_password",
+                                 admin_only=True)
     @decorators.idempotent_id('0f148510-63bf-11e6-1342-080044d0d909')
     def test_update_user_password(self):
         user = self._create_user()
diff --git a/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py b/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
index 9fa0d98..705c7e7 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
@@ -71,7 +71,7 @@
     def test_expect_not_found_but_raises_forbidden(self, mock_auth, mock_log):
         decorator = rbac_rv.action(mock.sentinel.service,
                                    mock.sentinel.action,
-                                   404)
+                                   expected_error_code=404)
         mock_function = mock.Mock()
         mock_function.side_effect = exceptions.NotFound
         wrapper = decorator(mock_function)
@@ -138,7 +138,7 @@
     def test_expect_not_found_and_not_allowed(self, mock_auth, mock_log):
         decorator = rbac_rv.action(mock.sentinel.service,
                                    mock.sentinel.action,
-                                   404)
+                                   expected_error_code=404)
 
         mock_function = mock.Mock()
         mock_function.side_effect = exceptions.NotFound
@@ -222,3 +222,39 @@
         mock_log.error.assert_called_once_with("Please pass an expected error "
                                                "code. Currently supported "
                                                "codes: [403, 404]")
+
+    @mock.patch.object(rbac_rv, 'LOG', autospec=True)
+    def test_rbac_decorator_with_admin_only_and_have_permission(self,
+                                                                mock_log):
+        CONF.set_override('rbac_test_role', 'admin', group='rbac',
+                          enforce_type=True)
+        self.addCleanup(CONF.clear_override, 'rbac_test_role', group='rbac')
+
+        decorator = rbac_rv.action(mock.sentinel.service,
+                                   mock.sentinel.policy_rule,
+                                   admin_only=True)
+        wrapper = decorator(mock.Mock(side_effect=None))
+        wrapper(self.mock_args)
+
+        mock_log.info.assert_called_once_with(
+            "As admin_only is True, only admin role should be allowed to "
+            "perform the API. Skipping oslo.policy check for policy action "
+            "{0}.".format(mock.sentinel.policy_rule))
+
+    @mock.patch.object(rbac_rv, 'LOG', autospec=True)
+    def test_rbac_decorator_with_admin_only_and_lack_permission(self,
+                                                                mock_log):
+        CONF.set_override('rbac_test_role', 'Member', group='rbac',
+                          enforce_type=True)
+        self.addCleanup(CONF.clear_override, 'rbac_test_role', group='rbac')
+
+        decorator = rbac_rv.action(mock.sentinel.service,
+                                   mock.sentinel.policy_rule,
+                                   admin_only=True)
+        wrapper = decorator(mock.Mock(side_effect=exceptions.Forbidden))
+        wrapper(self.mock_args)
+
+        mock_log.info.assert_called_once_with(
+            "As admin_only is True, only admin role should be allowed to "
+            "perform the API. Skipping oslo.policy check for policy action "
+            "{0}.".format(mock.sentinel.policy_rule))