Convert assignment.root_role config option to list of strings

The assigment.root_role config option was previously a single string
which specified a role name that was prohibited from being added as an
implied role. By default it was 'admin'. For greater flexibility we
now permit a list of role names that are prohibited from being
implied.

Summary of changes:

* Change assigment.root_role from cfg.StrOpt to cfg.ListOpt.
  ListOpt is preferred over MultiStrOpt because of config file formatting.
  Update help for option.

* Change assigment.root_role name to assignment.prohibited_implied_role

* Change test for implied role name from string equality to
  membership in list of strings.

* Expand ImpliedRolesTests.test_root_role_as_implied_role_forbidden()
  unit test to test 2 prohibited implied role names and 1 valid
  implied. role name.

Change-Id: Idfe14080e2f1ec1e89b85d8f5f00aad187f1fd22
Closes-Bug: #1541540
Signed-off-by: John Dennis <jdennis@redhat.com>

keystone/assignment/core.py

@@ -1459,7 +1459,7 @@ class RoleManager(manager.Manager):
     def create_implied_role(self, prior_role_id, implied_role_id):
         implied_role = self.driver.get_role(implied_role_id)
         self.driver.get_role(prior_role_id)
-        if implied_role['name'] == CONF.assignment.root_role:
+        if implied_role['name'] in CONF.assignment.prohibited_implied_role:
             raise exception.InvalidImpliedRole(role_id=implied_role_id)
         response = self.driver.create_implied_role(
             prior_role_id, implied_role_id)

keystone/common/config.py

@@ -405,10 +405,9 @@ FILE_OPTIONS = {
                         'keystone.assignment namespace. Only an SQL driver is '
                         'supplied.',
                    default='sql'),
-        cfg.StrOpt('root_role', default='admin',
-                   help='A role that is not allowed to be an implied '
-                   'role, as it is the root of role inference directed '
-                   'acyclic graph.'),
+        cfg.ListOpt('prohibited_implied_role', default=['admin'],
+                    help='A list of role names which are prohibited from '
+                         'being an implied role.'),
 
     ],
     'resource': [

keystone/tests/unit/test_v3_assignment.py

@@ -2568,16 +2568,48 @@ class ImpliedRolesTests(test_v3.RestfulTestCase, test_v3.AssignmentTestMixin,
         self._assert_effective_role_for_implied_has_prior_in_links(
             response, user, project, 1, 2)
 
-    def test_root_role_as_implied_role_forbidden(self):
-        self.config_fixture.config(group='assignment', root_role='root')
+    def _create_named_role(self, name):
+        role = unit.new_role_ref()
+        role['name'] = name
+        self.role_api.create_role(role['id'], role)
+        return role
 
-        root_role = unit.new_role_ref()
-        root_role['name'] = 'root'
-        self.role_api.create_role(root_role['id'], root_role)
-        prior = self._create_role()
-        url = '/roles/%s/implies/%s' % (prior['id'], root_role['id'])
+    def test_root_role_as_implied_role_forbidden(self):
+        """Create 2 roles that are prohibited from being an implied role.
+        Create 1 additional role which should be accepted as an implied
+        role. Assure the prohibited role names cannot be set as an implied
+        role. Assure the accepted role name which is not a member of the
+        prohibited implied role list can be successfully set an implied
+        role.
+        """
+        prohibited_name1 = 'root1'
+        prohibited_name2 = 'root2'
+        accepted_name1 = 'implied1'
+
+        prohibited_names = [prohibited_name1, prohibited_name2]
+        self.config_fixture.config(group='assignment',
+                                   prohibited_implied_role=prohibited_names)
+
+        prior_role = self._create_role()
+
+        prohibited_role1 = self._create_named_role(prohibited_name1)
+        url = '/roles/{prior_role_id}/implies/{implied_role_id}'.format(
+            prior_role_id=prior_role['id'],
+            implied_role_id=prohibited_role1['id'])
         self.put(url, expected_status=http_client.FORBIDDEN)
 
+        prohibited_role2 = self._create_named_role(prohibited_name2)
+        url = '/roles/{prior_role_id}/implies/{implied_role_id}'.format(
+            prior_role_id=prior_role['id'],
+            implied_role_id=prohibited_role2['id'])
+        self.put(url, expected_status=http_client.FORBIDDEN)
+
+        accepted_role1 = self._create_named_role(accepted_name1)
+        url = '/roles/{prior_role_id}/implies/{implied_role_id}'.format(
+            prior_role_id=prior_role['id'],
+            implied_role_id=accepted_role1['id'])
+        self.put(url, expected_status=http_client.CREATED)
+
 
 class DomainSpecificRoleTests(test_v3.RestfulTestCase, unit.TestCase):
     def setUp(self):

releasenotes/notes/implied-roles-026f401adc0f7fb6.yaml

@@ -7,5 +7,6 @@ features:
    of another. The rules are of the form ``prior_role`` implies
    ``implied_role``. At token generation time, user/group assignments of roles
    that have implied roles will be expanded to also include such roles in the
-   token. The expansion of implied roles is controlled by the ``root_role``
-   option in the ``[assignment]`` section of `keystone.conf`.
+   token. The expansion of implied roles is controlled by the
+   ``prohibited_implied_role`` option in the ``[assignment]``
+   section of `keystone.conf`.