Merge "Add a policy layer for membership APIs"

doc/source/policies.rst

@@ -72,6 +72,27 @@ The actions that may have a rule enforced on them are:
   * ``DELETE /v1/images/<IMAGE_ID>``
   * ``DELETE /v2/images/<IMAGE_ID>``
 
+* ``add_member`` - Add a membership to the member repo of an image
+
+  * ``POST /v2/images/<IMAGE_ID>/members``
+
+* ``get_members`` - List the members of an image
+
+  * ``GET /v1/images/<IMAGE_ID>/members``
+  * ``GET /v2/images/<IMAGE_ID>/members``
+
+* ``delete_member`` - Delete a membership of an image
+
+  * ``DELETE /v1/images/<IMAGE_ID>/members/<MEMBER_ID>``
+  * ``DELETE /v2/images/<IMAGE_ID>/members/<MEMBER_ID>``
+
+* ``modify_member`` - Create or update the membership of an image
+
+  * ``PUT /v1/images/<IMAGE_ID>/members/<MEMBER_ID>``
+  * ``PUT /v1/images/<IMAGE_ID>/members``
+  * ``POST /v2/images/<IMAGE_ID>/members``
+  * ``PUT /v2/images/<IMAGE_ID>/members/<MEMBER_ID>``
+
 * ``manage_image_cache`` - Allowed to use the image cache management API
 
 

glance/api/policy.py

@@ -211,6 +211,10 @@ class ImageProxy(glance.domain.proxy.Image):
         self.policy.enforce(self.context, 'download_image', {})
         return self.image.get_data(*args, **kwargs)
 
+    def get_member_repo(self, **kwargs):
+        member_repo = self.image.get_member_repo(**kwargs)
+        return ImageMemberRepoProxy(member_repo, self.context, self.policy)
+
 
 class ImageFactoryProxy(glance.domain.proxy.ImageFactory):
 
@@ -227,3 +231,40 @@ class ImageFactoryProxy(glance.domain.proxy.ImageFactory):
         if kwargs.get('visibility') == 'public':
             self.policy.enforce(self.context, 'publicize_image', {})
         return super(ImageFactoryProxy, self).new_image(**kwargs)
+
+
+class ImageMemberFactoryProxy(glance.domain.proxy.ImageMembershipFactory):
+
+    def __init__(self, member_factory, context, policy):
+        super(ImageMemberFactoryProxy, self).__init__(
+            member_factory,
+            image_proxy_class=ImageProxy,
+            image_proxy_kwargs={'context': context, 'policy': policy})
+
+
+class ImageMemberRepoProxy(glance.domain.proxy.Repo):
+
+    def __init__(self, member_repo, context, policy):
+        self.member_repo = member_repo
+        self.context = context
+        self.policy = policy
+
+    def add(self, member):
+        self.policy.enforce(self.context, 'add_member', {})
+        return self.member_repo.add(member)
+
+    def get(self, member_id):
+        self.policy.enforce(self.context, 'get_member', {})
+        return self.member_repo.get(member_id)
+
+    def save(self, member):
+        self.policy.enforce(self.context, 'modify_member', {})
+        return self.member_repo.save(member)
+
+    def list(self, *args, **kwargs):
+        self.policy.enforce(self.context, 'get_members', {})
+        return self.member_repo.list(*args, **kwargs)
+
+    def remove(self, member):
+        self.policy.enforce(self.context, 'delete_member', {})
+        return self.member_repo.remove(member)

glance/api/v1/members.py

@@ -17,6 +17,7 @@
 
 import webob.exc
 
+from glance.api import policy
 from glance.api.v1 import controller
 from glance.common import exception
 from glance.common import utils
@@ -29,10 +30,20 @@ LOG = logging.getLogger(__name__)
 
 class Controller(controller.BaseController):
 
+    def __init__(self):
+        self.policy = policy.Enforcer()
+
     def _check_can_access_image_members(self, context):
         if context.owner is None and not context.is_admin:
             raise webob.exc.HTTPUnauthorized(_("No authenticated user"))
 
+    def _enforce(self, req, action):
+        """Authorize an action against our policies"""
+        try:
+            self.policy.enforce(req.context, action, {})
+        except exception.Forbidden:
+            raise webob.exc.HTTPForbidden()
+
     def index(self, req, image_id):
         """
         Return a list of dictionaries indicating the members of the
@@ -47,6 +58,8 @@ class Controller(controller.BaseController):
                  'can_share': <SHARE_PERMISSION>, ...}, ...
             ]}
         """
+        self._enforce(req, 'get_members')
+
         try:
             members = registry.get_image_members(req.context, image_id)
         except exception.NotFound:
@@ -65,6 +78,7 @@ class Controller(controller.BaseController):
         Removes a membership from the image.
         """
         self._check_can_access_image_members(req.context)
+        self._enforce(req, 'delete_member')
 
         try:
             registry.delete_member(req.context, image_id, id)
@@ -99,6 +113,7 @@ class Controller(controller.BaseController):
         remain unchanged and new memberships default to False.
         """
         self._check_can_access_image_members(req.context)
+        self._enforce(req, 'modify_member')
 
         # Figure out can_share
         can_share = None
@@ -134,6 +149,7 @@ class Controller(controller.BaseController):
             ]}
         """
         self._check_can_access_image_members(req.context)
+        self._enforce(req, 'modify_member')
 
         try:
             registry.replace_members(req.context, image_id, body)

glance/gateway.py

@@ -44,8 +44,10 @@ class Gateway(object):
 
     def get_image_member_factory(self, context):
         image_factory = glance.domain.ImageMemberFactory()
+        policy_member_factory = policy.ImageMemberFactoryProxy(
+                image_factory, context, self.policy)
         authorized_image_factory = authorization.ImageMemberFactoryProxy(
-                                    image_factory, context)
+                                    policy_member_factory, context)
         return authorized_image_factory
 
     def get_repo(self, context):

glance/tests/unit/test_policy.py

@@ -60,6 +60,23 @@ class ImageFactoryStub(object):
         return 'new_image'
 
 
+class MemberRepoStub(object):
+    def add(self, *args, **kwargs):
+        return 'member_repo_add'
+
+    def get(self, *args, **kwargs):
+        return 'member_repo_get'
+
+    def save(self, *args, **kwargs):
+        return 'member_repo_save'
+
+    def list(self, *args, **kwargs):
+        return 'member_repo_list'
+
+    def remove(self, *args, **kwargs):
+        return 'member_repo_remove'
+
+
 class TestPolicyEnforcer(base.IsolatedUnitTest):
     def test_policy_file_default_rules_default_location(self):
         enforcer = glance.api.policy.Enforcer()
@@ -260,6 +277,69 @@ class TestImagePolicy(test_utils.BaseTestCase):
         self.assertRaises(exception.Forbidden, image.get_data)
 
 
+class TestMemberPolicy(test_utils.BaseTestCase):
+    def setUp(self):
+        self.policy = unit_test_utils.FakePolicyEnforcer()
+        self.member_repo = glance.api.policy.ImageMemberRepoProxy(
+            MemberRepoStub(), {}, self.policy)
+        super(TestMemberPolicy, self).setUp()
+
+    def test_add_member_not_allowed(self):
+        rules = {'add_member': False}
+        self.policy.set_rules(rules)
+        self.assertRaises(exception.Forbidden, self.member_repo.add, '')
+
+    def test_add_member_allowed(self):
+        rules = {'add_member': True}
+        self.policy.set_rules(rules)
+        output = self.member_repo.add('')
+        self.assertEqual(output, 'member_repo_add')
+
+    def test_get_member_not_allowed(self):
+        rules = {'get_member': False}
+        self.policy.set_rules(rules)
+        self.assertRaises(exception.Forbidden, self.member_repo.get, '')
+
+    def test_get_member_allowed(self):
+        rules = {'get_member': True}
+        self.policy.set_rules(rules)
+        output = self.member_repo.get('')
+        self.assertEqual(output, 'member_repo_get')
+
+    def test_modify_member_not_allowed(self):
+        rules = {'modify_member': False}
+        self.policy.set_rules(rules)
+        self.assertRaises(exception.Forbidden, self.member_repo.save, '')
+
+    def test_modify_member_allowed(self):
+        rules = {'modify_member': True}
+        self.policy.set_rules(rules)
+        output = self.member_repo.save('')
+        self.assertEqual(output, 'member_repo_save')
+
+    def test_get_members_not_allowed(self):
+        rules = {'get_members': False}
+        self.policy.set_rules(rules)
+        self.assertRaises(exception.Forbidden, self.member_repo.list, '')
+
+    def test_get_members_allowed(self):
+        rules = {'get_members': True}
+        self.policy.set_rules(rules)
+        output = self.member_repo.list('')
+        self.assertEqual(output, 'member_repo_list')
+
+    def test_delete_member_not_allowed(self):
+        rules = {'delete_member': False}
+        self.policy.set_rules(rules)
+        self.assertRaises(exception.Forbidden, self.member_repo.remove, '')
+
+    def test_delete_member_allowed(self):
+        rules = {'delete_member': True}
+        self.policy.set_rules(rules)
+        output = self.member_repo.remove('')
+        self.assertEqual(output, 'member_repo_remove')
+
+
 class TestContextPolicyEnforcer(base.IsolatedUnitTest):
     def _do_test_policy_influence_context_admin(self,
                                                 policy_admin_role,

glance/tests/unit/v1/test_api.py

@@ -1497,6 +1497,30 @@ class TestGlanceAPI(base.IsolatedUnitTest):
         num_members = len(memb_list['members'])
         self.assertEquals(num_members, 0)
 
+    def test_get_image_members_allowed_by_policy(self):
+        rules = {"get_members": '@'}
+        self.set_policy_rules(rules)
+
+        req = webob.Request.blank('/images/%s/members' % UUID2)
+        req.method = 'GET'
+
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, 200)
+
+        memb_list = json.loads(res.body)
+        num_members = len(memb_list['members'])
+        self.assertEquals(num_members, 0)
+
+    def test_get_image_members_forbidden_by_policy(self):
+        rules = {"get_members": '!'}
+        self.set_policy_rules(rules)
+
+        req = webob.Request.blank('/images/%s/members' % UUID2)
+        req.method = 'GET'
+
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, webob.exc.HTTPForbidden.code)
+
     def test_get_image_members_not_existing(self):
         """
         Tests proper exception is raised if attempt to get members of
@@ -1602,6 +1626,36 @@ class TestGlanceAPI(base.IsolatedUnitTest):
         res = req.get_response(self.api)
         self.assertEquals(res.status_int, 204)
 
+    def test_replace_members_forbidden_by_policy(self):
+        rules = {"modify_member": '!'}
+        self.set_policy_rules(rules)
+        self.api = test_utils.FakeAuthMiddleware(router.API(self.mapper),
+                                                 is_admin=True)
+        fixture = [{'member_id': 'pattieblack', 'can_share': 'false'}]
+
+        req = webob.Request.blank('/images/%s/members' % UUID1)
+        req.method = 'PUT'
+        req.content_type = 'application/json'
+        req.body = json.dumps(dict(memberships=fixture))
+
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, webob.exc.HTTPForbidden.code)
+
+    def test_replace_members_allowed_by_policy(self):
+        rules = {"modify_member": '@'}
+        self.set_policy_rules(rules)
+        self.api = test_utils.FakeAuthMiddleware(router.API(self.mapper),
+                                                 is_admin=True)
+        fixture = [{'member_id': 'pattieblack', 'can_share': 'false'}]
+
+        req = webob.Request.blank('/images/%s/members' % UUID1)
+        req.method = 'PUT'
+        req.content_type = 'application/json'
+        req.body = json.dumps(dict(memberships=fixture))
+
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, webob.exc.HTTPNoContent.code)
+
     def test_add_member(self):
         """
         Tests adding image members raises right exception
@@ -1655,6 +1709,28 @@ class TestGlanceAPI(base.IsolatedUnitTest):
         res = req.get_response(self.api)
         self.assertEquals(res.status_int, 204)
 
+    def test_add_member_forbidden_by_policy(self):
+        rules = {"modify_member": '!'}
+        self.set_policy_rules(rules)
+        self.api = test_utils.FakeAuthMiddleware(router.API(self.mapper),
+                                                 is_admin=True)
+        req = webob.Request.blank('/images/%s/members/pattieblack' % UUID1)
+        req.method = 'PUT'
+
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, webob.exc.HTTPForbidden.code)
+
+    def test_add_member_allowed_by_policy(self):
+        rules = {"modify_member": '@'}
+        self.set_policy_rules(rules)
+        self.api = test_utils.FakeAuthMiddleware(router.API(self.mapper),
+                                                 is_admin=True)
+        req = webob.Request.blank('/images/%s/members/pattieblack' % UUID1)
+        req.method = 'PUT'
+
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, webob.exc.HTTPNoContent.code)
+
     def test_delete_member(self):
         """
         Tests deleting image members raises right exception
@@ -1718,6 +1794,32 @@ class TestGlanceAPI(base.IsolatedUnitTest):
         self.assertEquals(res.status_int, 404)
         self.assertTrue('Forbidden' in res.body)
 
+    def test_delete_member_allowed_by_policy(self):
+        rules = {"delete_member": '@', "modify_member": '@'}
+        self.set_policy_rules(rules)
+        self.api = test_utils.FakeAuthMiddleware(router.API(self.mapper),
+                                                 is_admin=True)
+        req = webob.Request.blank('/images/%s/members/pattieblack' % UUID2)
+        req.method = 'PUT'
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, webob.exc.HTTPNoContent.code)
+        req.method = 'DELETE'
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, webob.exc.HTTPNoContent.code)
+
+    def test_delete_member_forbidden_by_policy(self):
+        rules = {"delete_member": '!', "modify_member": '@'}
+        self.set_policy_rules(rules)
+        self.api = test_utils.FakeAuthMiddleware(router.API(self.mapper),
+                                                 is_admin=True)
+        req = webob.Request.blank('/images/%s/members/pattieblack' % UUID2)
+        req.method = 'PUT'
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, webob.exc.HTTPNoContent.code)
+        req.method = 'DELETE'
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, webob.exc.HTTPForbidden.code)
+
 
 class TestImageSerializer(base.IsolatedUnitTest):
     def setUp(self):

glance/tests/unit/v2/test_image_members_resource.py

@@ -173,6 +173,20 @@ class TestImageMembersController(test_utils.BaseTestCase):
         expected = set([TENANT1])
         self.assertEqual(actual, expected)
 
+    def test_index_allowed_by_get_members_policy(self):
+        rules = {"get_members": True}
+        self.policy.set_rules(rules)
+        request = unit_test_utils.get_fake_request()
+        output = self.controller.index(request, UUID2)
+        self.assertEqual(1, len(output['members']))
+
+    def test_index_forbidden_by_get_members_policy(self):
+        rules = {"get_members": False}
+        self.policy.set_rules(rules)
+        request = unit_test_utils.get_fake_request()
+        self.assertRaises(webob.exc.HTTPForbidden, self.controller.index,
+                          request, image_id=UUID2)
+
     def test_create(self):
         request = unit_test_utils.get_fake_request()
         image_id = UUID2
@@ -182,6 +196,22 @@ class TestImageMembersController(test_utils.BaseTestCase):
         self.assertEqual(UUID2, output.image_id)
         self.assertEqual(TENANT3, output.member_id)
 
+    def test_create_allowed_by_add_policy(self):
+        rules = {"add_member": True}
+        self.policy.set_rules(rules)
+        request = unit_test_utils.get_fake_request()
+        output = self.controller.create(request, image_id=UUID2,
+                                        member_id=TENANT3)
+        self.assertEqual(UUID2, output.image_id)
+        self.assertEqual(TENANT3, output.member_id)
+
+    def test_create_forbidden_by_add_policy(self):
+        rules = {"add_member": False}
+        self.policy.set_rules(rules)
+        request = unit_test_utils.get_fake_request()
+        self.assertRaises(webob.exc.HTTPForbidden, self.controller.create,
+                          request, image_id=UUID2, member_id=TENANT3)
+
     def test_update_done_by_member(self):
         request = unit_test_utils.get_fake_request(tenant=TENANT4)
         image_id = UUID2
@@ -193,6 +223,25 @@ class TestImageMembersController(test_utils.BaseTestCase):
         self.assertEqual(TENANT4, output.member_id)
         self.assertEqual('accepted', output.status)
 
+    def test_update_done_by_member_forbidden_by_policy(self):
+        rules = {"modify_member": False}
+        self.policy.set_rules(rules)
+        request = unit_test_utils.get_fake_request(tenant=TENANT4)
+        self.assertRaises(webob.exc.HTTPForbidden, self.controller.update,
+                          request, image_id=UUID2, member_id=TENANT4,
+                          status='accepted')
+
+    def test_update_done_by_member_allowed_by_policy(self):
+        rules = {"modify_member": True}
+        self.policy.set_rules(rules)
+        request = unit_test_utils.get_fake_request(tenant=TENANT4)
+        output = self.controller.update(request, image_id=UUID2,
+                                        member_id=TENANT4,
+                                        status='accepted')
+        self.assertEqual(UUID2, output.image_id)
+        self.assertEqual(TENANT4, output.member_id)
+        self.assertEqual('accepted', output.status)
+
     def test_update_done_by_owner(self):
         request = unit_test_utils.get_fake_request(tenant=TENANT1)
         image_id = UUID2
@@ -246,6 +295,30 @@ class TestImageMembersController(test_utils.BaseTestCase):
         expected = set([TENANT4])
         self.assertEqual(actual, expected)
 
+    def test_delete_allowed_by_policies(self):
+        rules = {"get_member": True, "delete_member": True}
+        self.policy.set_rules(rules)
+        request = unit_test_utils.get_fake_request(tenant=TENANT1)
+        output = self.controller.delete(request, image_id=UUID2,
+                                        member_id=TENANT4)
+        request = unit_test_utils.get_fake_request()
+        output = self.controller.index(request, UUID2)
+        self.assertEqual(0, len(output['members']))
+
+    def test_delete_forbidden_by_get_member_policy(self):
+        rules = {"get_member": False}
+        self.policy.set_rules(rules)
+        request = unit_test_utils.get_fake_request(tenant=TENANT1)
+        self.assertRaises(webob.exc.HTTPForbidden, self.controller.delete,
+                          request, UUID2, TENANT4)
+
+    def test_delete_forbidden_by_delete_member_policy(self):
+        rules = {"delete_member": False}
+        self.policy.set_rules(rules)
+        request = unit_test_utils.get_fake_request(tenant=TENANT1)
+        self.assertRaises(webob.exc.HTTPForbidden, self.controller.delete,
+                          request, UUID2, TENANT4)
+
     def test_delete_private_image(self):
         request = unit_test_utils.get_fake_request()
         self.assertRaises(webob.exc.HTTPForbidden, self.controller.delete,