Merge "Remove need to include admin in credentials in base classes"
diff --git a/README.rst b/README.rst
index f5b4c00..e983b60 100644
--- a/README.rst
+++ b/README.rst
@@ -5,174 +5,84 @@
 .. image:: http://governance.openstack.org/badges/patrole.svg
     :target: http://governance.openstack.org/reference/tags/index.html
 
-..
-
-=========================================
 Patrole - RBAC Integration Tempest Plugin
 =========================================
 
-Patrole is a tool for verifying that Role-Based Access Control is being
-correctly enforced.
+Patrole is a security validation tool for verifying that Role-Based Access
+Control is correctly configured and enforced in a system. It runs Tempest-based
+API tests using specified RBAC roles, thus allowing deployments to verify that
+only intended roles have access to those APIs.
 
-Patrole allows users to run API tests using specified RBAC roles. This allows
-deployments to verify that only intended roles have access to those APIs.
-This is critical to ensure security, especially in large deployments with
-custom roles.
-
-* Free software: Apache license
-* Documentation: https://docs.openstack.org/developer/patrole
-* Source: https://git.openstack.org/cgit/openstack/patrole
-* Bugs: https://bugs.launchpad.net/patrole
+Patrole currently offers testing for the following OpenStack services: Nova,
+Neutron, Glance, Cinder and Keystone.
 
 Features
-========
-Patrole offers RBAC testing for various OpenStack RBAC policies. It includes
-a decorator that wraps around tests which verifies that when the test calls the
-corresponding API endpoint, access is only granted for correct roles.
+--------
+* Validation of default policy definitions located in policy.json files.
+* Validation of in-code policy definitions.
+* Validation of custom policy file definitions that override default policy
+  definitions.
+* Built-in positive and negative testing. Positive and negative testing
+  are performed using the same tests and role-switching.
+* Valdation of custom roles as well as default OpenStack roles.
 
-Currently, Patrole supports policies contained in code and in policy.json files.
-If both exist, the policy actions in the policy.json are prioritized.
+.. note::
 
-Stable Interface
-----------------
-Patrole offers a stable interface that is guaranteed to be backwards compatible and
-can be directly consumed by other projects. Currently, rbac_exceptions.py and
-rbac_policy_parser.py are guaranteed to be stable.
+    Patrole does not yet support policy.yaml files, the new file format for
+    policy files in OpenStack.
+
+How It Works
+------------
+Patrole leverages ``oslo.policy`` (OpenStack's policy enforcement engine) to
+determine whether a given role is allowed to perform a policy action, given a
+specific role and OpenStack service. The output from ``oslo.policy`` (the
+expected result) and the actual result from test execution are compared to
+each other: if both results match, then the test passes; else it fails.
+
+* Documentation: https://docs.openstack.org/developer/patrole
+* Bugs: https://bugs.launchpad.net/patrole
+
+Quickstart
+----------
+Tempest is a prerequisite for running Patrole. If you do not have Tempest
+installed, please reference the official Tempest documentation for guidance.
+
+Assuming Tempest is installed, the simplest way to configure Patrole is:
+
+1. Open up the ``tempest.conf`` configuration file and include the following
+settings:
+
+.. code-block:: ini
+
+    [rbac]
+    enable_rbac = True
+    rbac_test_role = admin
+
+These settings tell Patrole to run RBAC tests using the "admin" role (which
+is the default admin role in OpenStack) to verify the default policy
+definitions used by OpenStack services. Specifying a different role
+for ``rbac_test_role`` will run Patrole tests against that role. For additional
+information about Patrole's configuration settings, please refer to
+:ref:`patrole-configuration` and :ref:`patrole-sampleconf` for a sample
+configuration file.
+
+2. You are now ready to run Patrole. To do so, you can use any testr-based test
+runner::
+
+    $ testr run patrole_tempest_plugin.tests.api
+
+or::
+
+    $ ostestr --regex '(?!.*\[.*\bslow\b.*\])(^patrole_tempest_plugin\.tests\.api)'
+
+It is also possible to run Patrole using tox::
+
+    tox -eall-plugin -- patrole_tempest_plugin.tests.api
 
 Release Versioning
 ------------------
-`Patrole Release Notes <https://docs.openstack.org/releasenotes/patrole/>`_ show
-what changes have been released.
+`Patrole Release Notes <https://docs.openstack.org/releasenotes/patrole/>`_
+shows which changes have been released for each version.
 
-.. _test-flows:
-
-Test Flows
-----------
-There are several possible test flows.
-
-If the ``rbac_test_role`` is allowed to access the endpoint:
-
-* The test passes if no 403 ``Forbidden`` or ``RbacActionFailed`` exception is raised.
-
-If the ``rbac_test_role`` is not allowed to access the endpoint:
-
-* If the endpoint returns a 403 `Forbidden` exception the test will pass.
-* If the endpoint returns successfully, then the test will fail with an
-  ``RbacOverPermission`` exception.
-* If the endpoint returns something other than a 403 ``Forbidden`` to indicate
-  that the role is not allowed, the test will raise an ``RbacActionFailed`` exception.
-
-.. note::
-
-    Certain services like Neutron *intentionally* raise a 404 instead of a 403
-    for security concerns. Patrole accomodates this behavior by anticipating
-    a 404 instead of a 403, using the ``expected_exception`` argument. For more
-    information about Neutron's policy enforcement, see:
-    `<https://docs.openstack.org/developer/neutron/devref/policy.html#request-authorization>`__.
-
-How It Works
-============
-Patrole leverages oslo_policy (OpenStack's policy enforcement engine) to
-determine whether a given role is allowed to perform a policy action given a
-specific rule and OpenStack service. This is done before test execution inside
-the ``rbac_rule_validation.action`` decorator. Then, inside the test, the API
-that does policy enforcement for the same rule is called. The outcome is
-compared against the result from oslo_policy and a pass or fail is determined
-as outlined above: `Test Flows`_.
-
-.. note::
-
-    Currently, Patrole does not support checking multiple rules against a single
-    API call. Even though some APIs enforce multiple rules (some indirectly),
-    it is increasingly difficult to maintain the tests if multiple policy
-    actions are expected to be called.
-
-Test Execution Workflow
------------------------
-The workflow is as follows:
-
-#. Each test uses the ``rbac_rule_validation.action`` decorator, like below: ::
-
-    @rbac_rule_validation.action(
-        service="nova",
-        rule="os_compute_api:servers:stop")
-    @decorators.idempotent_id('ab4a17d2-166f-4a6d-9944-f17baa576cf2')
-    def test_stop_server(self):
-        # Set the primary credential's role to "rbac_test_role".
-        self.rbac_utils.switch_role(self, toggle_rbac_role=True)
-        # Call the API that enforces the policy action specified by "rule".
-        self._test_stop_server()
-
-   The ``service`` attribute accepts an OpenStack service and the ``rule`` attribute
-   accepts a valid OpenStack policy action, like "os_compute_api:servers:stop".
-
-#. The ``rbac_rule_validation.action`` decorator passes these attributes,
-   along with user_id and project_id information derived from the primary
-   Tempest credential (``self.os.credentials.user_id`` and ``self.os.credentials.project_id``),
-   to the ``rbac_policy_parser``.
-
-#. The logic in ``rbac_policy_parser`` then passes all this information along
-   and the role in ``CONF.rbac.rbac_test_role`` to oslo_policy to determine whether
-   the ``rbac_test_role`` is authorized to perform the policy action for the given
-   service.
-
-#. After all of the logic above has executed inside the rbac decorator, the
-   test is executed. The test then sets up test-level resources, if necessary,
-   with **admin** credentials implicitly. This is accomplished through
-   ``rbac_utils.switch_role(toggle_rbac_role=False)``, which is done as part of
-   client setup (inside the call to ``rbac_utils.RbacUtils``): ::
-
-    @classmethod
-    def setup_clients(cls):
-        super(BaseV2ComputeRbacTest, cls).setup_clients()
-        cls.auth_provider = cls.os_primary.auth_provider
-        cls.rbac_utils = rbac_utils.RbacUtils(cls)
-        ...
-
-   This code has *already* executed when the test class is instantiated, because
-   it is located in the base rbac test class. Whenever ``cls.rbac_utils.switch_role``
-   is called, one of two behaviors are possible:
-
-    #. The primary credential's role is changed to admin if ``toggle_rbac_role=False``
-    #. The primary credential's role is changed to ``rbac_test_role`` if
-       ``toggle_rbac_role=True``
-
-   Thus, at the *beginning* of every test and during ``resource_setup`` and
-   ``resource_cleanup``, the primary credential has the admin role.
-
-#. After preliminary test-level setup is performed, like creating a server, a
-   second call to ``self.rbac_utils.switch_role`` is done: ::
-
-    self.rbac_utils.switch_role(cls, toggle_rbac_role=True)
-
-   Now the primary credential has the role specified by ``rbac_test_role``.
-
-#. The API endpoint in which policy enforcement of "os_compute_api:servers:stop"
-   is performed can now be called.
-
-   .. note:
-
-        To determine whether a policy action is enforced, refer to the relevant
-        controller code to make sure that the policy action is indeed enforced.
-
-#. Now that a call is made to "stop_server" with the primary credentials having
-   the role specified by ``rbac_test_role``, either the nova contoller will allow
-   or disallow the action to be performed. Since the "stop_server" policy action in
-   nova is defined as "base.RULE_ADMIN_OR_OWNER", the API will most likely
-   return a successful status code. For more information about this policy action,
-   see `<https://github.com/openstack/nova/blob/master/nova/policies/servers.py>`__.
-
-#. As mentioned above, the result from the API call and the result from oslo_policy
-   are compared for consistency.
-
-#. Finally, after the test has executed, but before ``tearDown`` or ``resource_cleanup``
-   is called, ``self.rbac_utils.switch_role(cls, toggle_rbac_role=False)`` is
-   called, so that the primary credential yet again has admin permissions for
-   test clean up. This call is always performed in the "finally" block inside
-   the ``rbac_rule_validation`` decorator.
-
-.. warning::
-
-    Failure to call ``self.rbac_utils.switch_role(cls, toggle_rbac_role=True)``
-    inside a test with the ``rbac_rule_validation`` decorator applied results
-    in a ``RbacResourceSetupFailed`` being raised, causing the test to fail.
+Patrole's release versioning follows Tempest's conventions. Like Tempest,
+Patrole is branchless and uses versioning instead.
diff --git a/doc/source/configuration.rst b/doc/source/configuration.rst
new file mode 100644
index 0000000..550d2ce
--- /dev/null
+++ b/doc/source/configuration.rst
@@ -0,0 +1,73 @@
+.. _patrole-configuration:
+
+Patrole Configuration Guide
+===========================
+
+Patrole can be customized by updating Tempest's ``tempest.conf`` configuration
+file. All Patrole-specific configuration options should be included under
+the ``rbac`` group.
+
+RBAC Test Role
+--------------
+
+The RBAC test role governs which role is used when running Patrole tests. For
+example, setting ``rbac_test_role`` to "admin" will execute all RBAC tests
+using admin credentials. Changing the ``rbac_test_role`` value will `override`
+Tempest's primary credentials to use that role.
+
+This implies that, if ``rbac_test_role`` is "admin", regardless of the Tempest
+credentials used by a client, the client will be calling APIs using the admin
+role. That is, ``self.os_primary.servers_client`` will run as though it were
+``self.os_admin.servers_client``.
+
+Similarly, setting ``rbac_test_role`` to a non-admin role results in Tempest's
+primary credentials being overriden by the role specified by
+``rbac_test_role``.
+
+.. note::
+
+    Only the role of the primary Tempest credentials ("os_primary") is
+    modified. The ``user_id`` and ``project_id`` remain unchanged.
+
+Enable RBAC
+-----------
+
+Given the value of ``enable_rbac``, enables or disables Patrole tests. If
+``enable_rbac`` is ``False``, then Patrole tests are skipped.
+
+Strict Policy Check
+-------------------
+
+Currently, many services define their "default" rule to be "anyone allowed".
+If a policy action is not explicitly defined in a policy file, then
+``oslo.policy`` will fall back to the "default" rule. This implies that if
+there's a typo in a policy action specified in a Patrole test, ``oslo.policy``
+can report that the ``rbac_test_role`` will be able to perform the
+non-existent policy action. For a testing framework, this is undesirable
+behavior.
+
+Hence, ``strict_policy_check``, if ``True``, will throw an error in the event
+that a non-existent or bogus policy action is passed to a Patrole test. If
+``False``, however, a ``self.skipException`` will be raised.
+
+Custom Policy Files
+-------------------
+
+Patrole supports testing custom policy file definitions, along with default
+policy definitions. Default policy definitions are used if custom file
+definitions are not specified. If both are specified, the custom policy
+definition takes precedence (that is, replaces the default definition,
+as this is the default behavior in OpenStack).
+
+The ``custom_policy_files`` option allows a user to specify a comma-separated
+list of custom policy file locations that are on the same host as Patrole.
+Each policy file must include the name of the service that is being tested:
+for example, if "compute" tests are executed, then Patrole will use the first
+policy file contained in ``custom_policy_files`` that contains the "nova"
+keyword.
+
+.. note::
+
+    Patrole currently does not support policy files located on a host different
+    than the one on which it is running.
+..
diff --git a/doc/source/index.rst b/doc/source/index.rst
index 35e8439..f58ee7f 100644
--- a/doc/source/index.rst
+++ b/doc/source/index.rst
@@ -1,24 +1,30 @@
-.. patrole documentation master file, created by
-   sphinx-quickstart on Tue Jul  9 22:26:36 2013.
-   You can adapt this file completely to your liking, but it should at least
-   contain the root `toctree` directive.
+========================================
+Patrole: Tempest Plugin for RBAC Testing
+========================================
 
-Patrole - an OpenStack Tempest Plugin for RBAC Testing
-========================================================
+User's Guide
+============
 
-Contents:
+Patrole Configuration Guide
+---------------------------
 
 .. toctree::
    :maxdepth: 2
 
-   readme
    installation
+   configuration
    usage
+   sampleconf
+
+Developer's Guide
+=================
+
+.. toctree::
+   :maxdepth: 2
+
    contributing
 
 Indices and tables
 ==================
 
-* :ref:`genindex`
-* :ref:`modindex`
 * :ref:`search`
diff --git a/doc/source/installation.rst b/doc/source/installation.rst
index 31f94f4..c244152 100644
--- a/doc/source/installation.rst
+++ b/doc/source/installation.rst
@@ -1,9 +1,11 @@
-============
-Installation
-============
+.. _patrole-installation:
 
-Installation Information
-========================
+==========================
+Patrole Installation Guide
+==========================
+
+Manual Installation Information
+===============================
 
 At the command line::
 
@@ -30,45 +32,3 @@
     ...
 
     enable_plugin patrole git://git.openstack.org/openstack/patrole
-
-Configuration Information
-=========================
-
-tempest.conf
-++++++++++++
-
-To run the RBAC tempest api test, you have to make the following changes to
-the tempest.conf file.
-
-#. ``auth`` section updates ::
-
-    # Allows test cases to create/destroy projects and users. This option
-    # requires that OpenStack Identity API admin credentials are known. If
-    # false, isolated test cases and parallel execution, can still be
-    # achieved configuring a list of test accounts (boolean value)
-    use_dynamic_credentials = True
-
-#. ``rbac`` section updates ::
-
-    # The role that you want the RBAC tests to use for RBAC testing
-    # This needs to be edited to run the test as a different role.
-    rbac_test_role = _member_
-
-    # Enables RBAC Tempest tests if set to True. Otherwise, they are
-    # skipped.
-    enable_rbac = True
-
-    # If set to true, tests throw a RbacParsingException for policies
-    # not found in the policy.json. Otherwise, they throw a
-    # skipException.
-    strict_policy_check = False
-
-    # The following config options set the location of the service's
-    # policy file. For services that have their policy in code (e.g.,
-    # Nova), this would be the location of a custom policy.json, if
-    # one exists.
-    cinder_policy_file = /etc/cinder/policy.json
-    glance_policy_file = /etc/glance/policy.json
-    keystone_policy_file = /etc/keystone/policy.json
-    neutron_policy_file = /etc/neutron/policy.json
-    nova_policy_file = /etc/nova/policy.json
diff --git a/doc/source/readme.rst b/doc/source/readme.rst
deleted file mode 100644
index a6210d3..0000000
--- a/doc/source/readme.rst
+++ /dev/null
@@ -1 +0,0 @@
-.. include:: ../../README.rst
diff --git a/doc/source/sampleconf.rst b/doc/source/sampleconf.rst
new file mode 100644
index 0000000..c262f2d
--- /dev/null
+++ b/doc/source/sampleconf.rst
@@ -0,0 +1,51 @@
+.. _patrole-sampleconf:
+
+Sample Configuration File
+==========================
+
+The following is a sample Patrole configuration for adaptation and use.
+
+.. code-block:: ini
+
+    [rbac]
+
+    # The role that you want the RBAC tests to use for RBAC testing
+    # This needs to be edited to run the test as a different role.
+    rbac_test_role = Member
+
+    # Enables RBAC Tempest tests if set to True. Otherwise, they are
+    # skipped.
+    enable_rbac = True
+
+    # If set to True, tests throw a RbacParsingException for policies
+    # not found in the policy file. Otherwise, they throw a skipException.
+    strict_policy_check = False
+
+    # List of the paths to search for policy files. Each policy path assumes that
+    # the service name is included in the path once. Also assumes Patrole is on the
+    # same host as the policy files. The paths should be ordered by precedence,
+    # with high-priority paths before low-priority paths. The first path that is
+    # found to contain the service's policy file will be used.
+    custom_policy_files = /etc/nova/policy.json,/etc/neutron/policy.json
+
+    # This option determines whether Patrole should run against a
+    # `custom_requirements_file` which defines RBAC requirements. The
+    # purpose of setting this flag to True is to verify that RBAC policy
+    # is in accordance to requirements. The idea is that the
+    # `custom_requirements_file` perfectly defines what the RBAC requirements
+    # are.
+    test_custom_requirements = False
+
+    File path of the yaml file that defines your RBAC requirements. This
+    # file must be located on the same host that Patrole runs on. The yaml
+    # file should be written as follows:
+    custom_requirements_file = patrole/requirements.txt
+
+    # DEPRECATED: The following config options set the location of the service's
+    # policy file. For services that have their policy in code (e.g., Nova),
+    # this would be the location of a custom policy.json, if one exists.
+    cinder_policy_file = /etc/cinder/policy.json
+    glance_policy_file = /etc/glance/policy.json
+    keystone_policy_file = /etc/keystone/policy.json
+    neutron_policy_file = /etc/neutron/policy.json
+    nova_policy_file = /etc/nova/policy.json
diff --git a/doc/source/usage.rst b/doc/source/usage.rst
index d2570bc..7470e9e 100644
--- a/doc/source/usage.rst
+++ b/doc/source/usage.rst
@@ -1,4 +1,4 @@
-..
+.. _patrole-usage:
 
 ========
 Usage
diff --git a/patrole_tempest_plugin/tests/api/compute/rbac_base.py b/patrole_tempest_plugin/tests/api/compute/rbac_base.py
index aa148ea..7e9a402 100644
--- a/patrole_tempest_plugin/tests/api/compute/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/compute/rbac_base.py
@@ -28,7 +28,7 @@
         super(BaseV2ComputeRbacTest, cls).skip_checks()
         if not CONF.rbac.enable_rbac:
             raise cls.skipException(
-                '%s skipped as RBAC flag not enabled' % cls.__name__)
+                '%s skipped as RBAC testing not enabled' % cls.__name__)
 
     @classmethod
     def setup_clients(cls):
diff --git a/patrole_tempest_plugin/tests/unit/test_rbac_policy_parser.py b/patrole_tempest_plugin/tests/unit/test_rbac_policy_parser.py
index e3a4429..95e6bff 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_policy_parser.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_policy_parser.py
@@ -40,8 +40,9 @@
 
     def setUp(self):
         super(RbacPolicyTest, self).setUp()
-        mock_admin_mgr = self.patchobject(rbac_policy_parser, 'credentials')
-        mock_admin_mgr.AdminManager().identity_services_v3_client.\
+        self.mock_admin_mgr = self.patchobject(
+            rbac_policy_parser, 'credentials')
+        self.mock_admin_mgr.AdminManager().identity_services_v3_client.\
             list_services.return_value = self.services
 
         current_directory = os.path.dirname(os.path.realpath(__file__))
@@ -57,14 +58,19 @@
         self.tenant_policy_file = os.path.join(current_directory,
                                                'resources',
                                                'tenant_rbac_policy.json')
+        self.conf_policy_path = os.path.join(
+            current_directory, 'resources', '%s.json')
 
         CONF.set_override(
-            'custom_policy_files',
-            [os.path.join(current_directory, 'resources', '%s.json')],
-            group='rbac')
+            'custom_policy_files', [self.conf_policy_path], group='rbac')
         self.addCleanup(CONF.clear_override, 'custom_policy_files',
                         group='rbac')
 
+        # Guarantee a blank slate for each test.
+        for attr in ('available_services', 'policy_files'):
+            if attr in dir(rbac_policy_parser.RbacPolicyParser):
+                delattr(rbac_policy_parser.RbacPolicyParser, attr)
+
     def _get_fake_policy_rule(self, name, rule):
         fake_rule = mock.Mock(check=rule)
         fake_rule.name = name
@@ -427,3 +433,60 @@
             .format('tenant_rbac_policy', [CONF.rbac.custom_policy_files[0]
                                            % 'tenant_rbac_policy']))
         self.assertIn(expected_error, str(e))
+
+    def test_discover_policy_files(self):
+        policy_parser = rbac_policy_parser.RbacPolicyParser(
+            None, None, 'tenant_rbac_policy')
+
+        # Ensure that "policy_files" is set at class and instance levels.
+        self.assertIn('policy_files',
+                      dir(rbac_policy_parser.RbacPolicyParser))
+        self.assertIn('policy_files', dir(policy_parser))
+        self.assertIn('tenant_rbac_policy', policy_parser.policy_files)
+        self.assertEqual(self.conf_policy_path % 'tenant_rbac_policy',
+                         policy_parser.policy_files['tenant_rbac_policy'])
+
+    @mock.patch.object(rbac_policy_parser, 'policy', autospec=True)
+    @mock.patch.object(rbac_policy_parser.RbacPolicyParser, '_get_policy_data',
+                       autospec=True)
+    @mock.patch.object(rbac_policy_parser, 'os', autospec=True)
+    def test_discover_policy_files_with_many_invalid_one_valid(self, m_os, *a):
+        # Only the 3rd path is valid.
+        m_os.path.isfile.side_effect = [False, False, True, False]
+
+        # Ensure the outer for loop runs only once in `discover_policy_files`.
+        self.mock_admin_mgr.AdminManager().identity_services_v3_client.\
+            list_services.return_value = {
+                'services': [{'name': 'test_service'}]}
+
+        # The expected policy will be 'baz/test_service'.
+        CONF.set_override(
+            'custom_policy_files', ['foo/%s', 'bar/%s', 'baz/%s'],
+            group='rbac')
+
+        policy_parser = rbac_policy_parser.RbacPolicyParser(
+            None, None, 'test_service')
+
+        # Ensure that "policy_files" is set at class and instance levels.
+        self.assertIn('policy_files',
+                      dir(rbac_policy_parser.RbacPolicyParser))
+        self.assertIn('policy_files', dir(policy_parser))
+        self.assertIn('test_service', policy_parser.policy_files)
+        self.assertEqual('baz/test_service',
+                         policy_parser.policy_files['test_service'])
+
+    def test_discover_policy_files_with_no_valid_files(self):
+        expected_error = ("Policy file for test_service service neither found "
+                          "in code nor at %s." %
+                          [self.conf_policy_path % 'test_service'])
+
+        e = self.assertRaises(rbac_exceptions.RbacParsingException,
+                              rbac_policy_parser.RbacPolicyParser,
+                              None, None, 'test_service')
+        self.assertIn(expected_error, str(e))
+
+        self.assertIn('policy_files',
+                      dir(rbac_policy_parser.RbacPolicyParser))
+        self.assertNotIn(
+            'test_service',
+            rbac_policy_parser.RbacPolicyParser.policy_files.keys())