Implement system admin role in protocol API

This commit introduces the system admin role to the protocol API,
making it consistent with other system-admin policy definitions.

Subsequent patches will build on this work to expose more
functionality to domain and project users:

 - domain user test coverage
 - project user test coverage

Change-Id: I9384e0fdd95545f1afef65a5e97e8513b709f150
Closes-Bug: 1804523
Related-Bug: 1806762

keystone/common/policies/protocol.py

@@ -23,6 +23,18 @@ deprecated_list_protocols = policy.DeprecatedRule(
     name=base.IDENTITY % 'list_protocols',
     check_str=base.RULE_ADMIN_REQUIRED
 )
+deprecated_update_protocol = policy.DeprecatedRule(
+    name=base.IDENTITY % 'update_protocol',
+    check_str=base.RULE_ADMIN_REQUIRED
+)
+deprecated_create_protocol = policy.DeprecatedRule(
+    name=base.IDENTITY % 'create_protocol',
+    check_str=base.RULE_ADMIN_REQUIRED
+)
+deprecated_delete_protocol = policy.DeprecatedRule(
+    name=base.IDENTITY % 'delete_protocol',
+    check_str=base.RULE_ADMIN_REQUIRED
+)
 
 DEPRECATED_REASON = """
 As of the Stein release, the federated protocol API now understands default
@@ -35,7 +47,7 @@ relying on overrides in your deployment for the protocol API.
 protocol_policies = [
     policy.DocumentedRuleDefault(
         name=base.IDENTITY % 'create_protocol',
-        check_str=base.RULE_ADMIN_REQUIRED,
+        check_str=base.SYSTEM_ADMIN,
         # FIXME(lbragstad): Once it is possible to add complete federated
         # identity without having to modify system configuration files, like
         # Apache, this should include 'project' in scope_types.
@@ -43,15 +55,21 @@ protocol_policies = [
         description='Create federated protocol.',
         operations=[{'path': ('/v3/OS-FEDERATION/identity_providers/{idp_id}/'
                               'protocols/{protocol_id}'),
-                     'method': 'PUT'}]),
+                     'method': 'PUT'}],
+        deprecated_rule=deprecated_create_protocol,
+        deprecated_reason=DEPRECATED_REASON,
+        deprecated_since=versionutils.deprecated.STEIN),
     policy.DocumentedRuleDefault(
         name=base.IDENTITY % 'update_protocol',
-        check_str=base.RULE_ADMIN_REQUIRED,
+        check_str=base.SYSTEM_ADMIN,
         scope_types=['system'],
         description='Update federated protocol.',
         operations=[{'path': ('/v3/OS-FEDERATION/identity_providers/{idp_id}/'
                               'protocols/{protocol_id}'),
-                     'method': 'PATCH'}]),
+                     'method': 'PATCH'}],
+        deprecated_rule=deprecated_update_protocol,
+        deprecated_reason=DEPRECATED_REASON,
+        deprecated_since=versionutils.deprecated.STEIN),
     policy.DocumentedRuleDefault(
         name=base.IDENTITY % 'get_protocol',
         check_str=base.SYSTEM_READER,
@@ -76,12 +94,15 @@ protocol_policies = [
         deprecated_since=versionutils.deprecated.STEIN),
     policy.DocumentedRuleDefault(
         name=base.IDENTITY % 'delete_protocol',
-        check_str=base.RULE_ADMIN_REQUIRED,
+        check_str=base.SYSTEM_ADMIN,
         scope_types=['system'],
         description='Delete federated protocol.',
         operations=[{'path': ('/v3/OS-FEDERATION/identity_providers/{idp_id}/'
                               'protocols/{protocol_id}'),
-                     'method': 'DELETE'}])
+                     'method': 'DELETE'}],
+        deprecated_rule=deprecated_delete_protocol,
+        deprecated_reason=DEPRECATED_REASON,
+        deprecated_since=versionutils.deprecated.STEIN)
 ]
 
 

keystone/tests/unit/protection/v3/test_protocols.py

@@ -43,6 +43,32 @@ class _CommonUtilities(object):
         return (protocol, mapping, identity_provider)
 
 
+class _SystemUserProtocolTests(object):
+    """Common default functionality for all system users."""
+
+    def test_user_can_list_protocols(self):
+        protocol, mapping, identity_provider = self._create_protocol_and_deps()
+
+        with self.test_client() as c:
+            path = (
+                '/v3/OS-FEDERATION/identity_providers/%s/protocols' %
+                identity_provider['id']
+            )
+            r = c.get(path, headers=self.headers)
+            self.assertEqual(1, len(r.json['protocols']))
+            self.assertEqual(protocol['id'], r.json['protocols'][0]['id'])
+
+    def test_user_can_get_a_protocol(self):
+        protocol, mapping, identity_provider = self._create_protocol_and_deps()
+
+        with self.test_client() as c:
+            path = (
+                '/v3/OS-FEDERATION/identity_providers/%s/protocols/%s' %
+                (identity_provider['id'], protocol['id'])
+            )
+            c.get(path, headers=self.headers)
+
+
 class _SystemReaderAndMemberProtocolTests(object):
 
     def test_user_cannot_create_protocols(self):
@@ -86,28 +112,6 @@ class _SystemReaderAndMemberProtocolTests(object):
                 expected_status_code=http_client.FORBIDDEN
             )
 
-    def test_user_can_list_protocols(self):
-        protocol, mapping, identity_provider = self._create_protocol_and_deps()
-
-        with self.test_client() as c:
-            path = (
-                '/v3/OS-FEDERATION/identity_providers/%s/protocols' %
-                identity_provider['id']
-            )
-            r = c.get(path, headers=self.headers)
-            self.assertEqual(1, len(r.json['protocols']))
-            self.assertEqual(protocol['id'], r.json['protocols'][0]['id'])
-
-    def test_user_can_get_a_protocol(self):
-        protocol, mapping, identity_provider = self._create_protocol_and_deps()
-
-        with self.test_client() as c:
-            path = (
-                '/v3/OS-FEDERATION/identity_providers/%s/protocols/%s' %
-                (identity_provider['id'], protocol['id'])
-            )
-            c.get(path, headers=self.headers)
-
     def test_user_cannot_delete_protocol(self):
         protocol, mapping, identity_provider = self._create_protocol_and_deps()
 
@@ -125,6 +129,7 @@ class _SystemReaderAndMemberProtocolTests(object):
 class SystemReaderTests(base_classes.TestCaseWithBootstrap,
                         common_auth.AuthTestMixin,
                         _CommonUtilities,
+                        _SystemUserProtocolTests,
                         _SystemReaderAndMemberProtocolTests):
 
     def setUp(self):
@@ -159,6 +164,7 @@ class SystemReaderTests(base_classes.TestCaseWithBootstrap,
 class SystemMemberTests(base_classes.TestCaseWithBootstrap,
                         common_auth.AuthTestMixin,
                         _CommonUtilities,
+                        _SystemUserProtocolTests,
                         _SystemReaderAndMemberProtocolTests):
 
     def setUp(self):
@@ -188,3 +194,79 @@ class SystemMemberTests(base_classes.TestCaseWithBootstrap,
             r = c.post('/v3/auth/tokens', json=auth)
             self.token_id = r.headers['X-Subject-Token']
             self.headers = {'X-Auth-Token': self.token_id}
+
+
+class SystemAdminTests(base_classes.TestCaseWithBootstrap,
+                       common_auth.AuthTestMixin,
+                       _CommonUtilities,
+                       _SystemUserProtocolTests):
+
+    def setUp(self):
+        super(SystemAdminTests, self).setUp()
+        self.loadapp()
+        self.useFixture(ksfixtures.Policy(self.config_fixture))
+        self.config_fixture.config(group='oslo_policy', enforce_scope=True)
+
+        # Reuse the system administrator account created during
+        # ``keystone-manage bootstrap``
+        self.user_id = self.bootstrapper.admin_user_id
+        auth = self.build_authentication_request(
+            user_id=self.user_id,
+            password=self.bootstrapper.admin_password,
+            system=True
+        )
+
+        # Grab a token using the persona we're testing and prepare headers
+        # for requests we'll be making in the tests.
+        with self.test_client() as c:
+            r = c.post('/v3/auth/tokens', json=auth)
+            self.token_id = r.headers['X-Subject-Token']
+            self.headers = {'X-Auth-Token': self.token_id}
+
+    def test_user_can_create_protocols(self):
+        identity_provider = unit.new_identity_provider_ref()
+        identity_provider = PROVIDERS.federation_api.create_idp(
+            identity_provider['id'], identity_provider
+        )
+
+        mapping = PROVIDERS.federation_api.create_mapping(
+            uuid.uuid4().hex, unit.new_mapping_ref()
+        )
+
+        protocol_id = 'saml2'
+        create = {'protocol': {'mapping_id': mapping['id']}}
+
+        with self.test_client() as c:
+            path = (
+                '/v3/OS-FEDERATION/identity_providers/%s/protocols/%s' %
+                (identity_provider['id'], protocol_id)
+            )
+            c.put(
+                path, json=create, headers=self.headers,
+                expected_status_code=http_client.CREATED
+            )
+
+    def test_user_can_update_protocols(self):
+        protocol, mapping, identity_provider = self._create_protocol_and_deps()
+
+        new_mapping = PROVIDERS.federation_api.create_mapping(
+            uuid.uuid4().hex, unit.new_mapping_ref()
+        )
+
+        update = {'protocol': {'mapping_id': new_mapping['id']}}
+        with self.test_client() as c:
+            path = (
+                '/v3/OS-FEDERATION/identity_providers/%s/protocols/%s' %
+                (identity_provider['id'], protocol['id'])
+            )
+            c.patch(path, json=update, headers=self.headers)
+
+    def test_user_can_delete_protocol(self):
+        protocol, mapping, identity_provider = self._create_protocol_and_deps()
+
+        with self.test_client() as c:
+            path = (
+                '/v3/OS-FEDERATION/identity_providers/%s/protocols/%s' %
+                (identity_provider['id'], protocol['id'])
+            )
+            c.delete(path, headers=self.headers)

releasenotes/notes/bug-1804523-d1768909b13b167e.yaml

@@ -0,0 +1,32 @@
+---
+features:
+  - |
+    [`bug 1804523 <https://bugs.launchpad.net/keystone/+bug/1804523>`_]
+    The federated protocol API now supports the ``admin``, ``member``,
+    and ``reader`` default roles.
+upgrade:
+  - |
+    [`bug 1804523 <https://bugs.launchpad.net/keystone/+bug/1804523>`_]
+    The federated protocol API uses new default policies that
+    make it more accessible to end users and administrators. Please consider
+    these new defaults if your deployment overrides federated protocol
+    policies.
+deprecations:
+  - |
+    [`bug 1804523 <https://bugs.launchpad.net/keystone/+bug/1804523>`_]
+    The federated protocol policies have been deprecated. The
+    ``identity:get_protocol`` and ``identity:list_protocols`` now use
+    ``role:reader and system_scope:all`` instead of
+    ``rule:admin_required``. The ``identity:create_protocol``,
+    ``identity:update_protocol``, and ``identity:delete_protocol``
+    policies now use ``role:admin and system_scope:all`` instead of
+    ``rule:admin_required``. These new defaults automatically account
+    for system-scope and support a read-only role, making it easier
+    for system administrators to delegate subsets of responsibility
+    without compromising security. Please consider these new defaults
+    if your deployment overrides the federated protocol policies.
+security:
+  - |
+    [`bug 1804523 <https://bugs.launchpad.net/keystone/+bug/1804523>`_]
+    The federated protocol API now uses system-scope and default
+    roles to provide better accessibility to users in a secure way.