openstack/deb-glance
Code Issues Proposed changes

Allow admin's to modify image members.

Browse Source 
Updates the v1 Glance API and Registry controllers so that admins
can modify image membership regardless of image ownership.

The motivation for this change is to be able to test image membership
when noauth middleware (UnauthenticatedContextMiddleware or
FakeAuthMiddleware) is being used and includes the following changes:

 * Added is_admin option to FakeAuthMiddleware.

 * Refactors the access checks into a common function called
   _check_can_access_image_members.

 * API unit tests which require is_admin=False for noauth were also
   refactored to use an improved FakeAuthMiddleware which supports
   is_admin = False.

 * Update RegistryClient to use get_status_code int for 204 status code
   comparisons. Fixes issue where incorrect status code was returned.

 * Updated members unit tests to support these changes.

Fixes LP Bug #1021740.

Change-Id: I9d0e4679f9c0cb37f6df7c6f90460c22c37f3fbd
This commit is contained in:
Dan Prince 2012-07-03 10:41:02 -04:00
parent 33ed25f892
commit c1e9986353
6 changed files with 62 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
glance/api/v1/members.py
View File

@ -30,6 +30,10 @@ LOG = logging.getLogger(__name__)
class Controller(object):
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 index(self, req, image_id):
"""
Return a list of dictionaries indicating the members of the
@ -61,8 +65,7 @@ class Controller(object):
"""
Removes a membership from the image.
"""
if req.context.owner is None:
raise webob.exc.HTTPUnauthorized(_("No authenticated user"))
self._check_can_access_image_members(req.context)
try:
registry.delete_member(req.context, image_id, id)
@ -95,8 +98,7 @@ class Controller(object):
set accordingly. If it is not provided, existing memberships
remain unchanged and new memberships default to False.
"""
if req.context.owner is None:
raise webob.exc.HTTPUnauthorized(_("No authenticated user"))
self._check_can_access_image_members(req.context)
# Figure out can_share
can_share = None
@ -130,8 +132,7 @@ class Controller(object):
["can_share": [True|False]]}, ...
]}
"""
if req.context.owner is None:
raise webob.exc.HTTPUnauthorized(_("No authenticated user"))
self._check_can_access_image_members(req.context)
try:
registry.replace_members(req.context, image_id, body)

13
glance/registry/api/v1/members.py
View File

@ -30,6 +30,10 @@ LOG = logging.getLogger(__name__)
class Controller(object):
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 __init__(self):
self.db_api = glance.db.get_api()
self.db_api.configure_db()
@ -68,8 +72,7 @@ class Controller(object):
["can_share": [True|False]]}, ...
]}
"""
if req.context.owner is None:
raise webob.exc.HTTPUnauthorized(_("No authenticated user"))
self._check_can_access_image_members(req.context)
# Make sure the image exists
session = self.db_api.get_session()
@ -175,8 +178,7 @@ class Controller(object):
set accordingly. If it is not provided, existing memberships
remain unchanged and new memberships default to False.
"""
if req.context.owner is None:
raise webob.exc.HTTPUnauthorized(_("No authenticated user"))
self._check_can_access_image_members(req.context)
# Make sure the image exists
try:
@ -231,8 +233,7 @@ class Controller(object):
"""
Removes a membership from the image.
"""
if req.context.owner is None:
raise webob.exc.HTTPUnauthorized(_("No authenticated user"))
self._check_can_access_image_members(req.context)
# Make sure the image exists
try:

6
glance/registry/client.py
View File

@ -182,7 +182,7 @@ class RegistryClient(BaseClient):
res = self.do_request("PUT", "/images/%s/members" % image_id,
body, headers)
return res.status == 204
return self.get_status_code(res) == 204
def add_member(self, image_id, member_id, can_share=None):
"""Adds to Registry's information about image membership"""
@ -195,10 +195,10 @@ class RegistryClient(BaseClient):
res = self.do_request("PUT", "/images/%s/members/%s" %
(image_id, member_id), body, headers)
return res.status == 204
return self.get_status_code(res) == 204
def delete_member(self, image_id, member_id):
"""Deletes Registry's information about image membership"""
res = self.do_request("DELETE", "/images/%s/members/%s" %
(image_id, member_id))
return res.status == 204
return self.get_status_code(res) == 204

38
glance/tests/unit/test_clients.py
View File

@ -1174,21 +1174,16 @@ class TestRegistryClient(base.IsolatedUnitTest):
num_members = len(memb_list)
self.assertEquals(num_members, 0)
def test_replace_members(self):
def test_add_replace_members(self):
"""Tests replacing image members"""
self.assertRaises(exception.NotAuthenticated,
self.client.replace_members, UUID2,
dict(member_id='pattieblack'))
self.assertTrue(self.client.add_member(UUID2, 'pattieblack'))
self.assertTrue(self.client.replace_members(UUID2,
dict(member_id='pattieblack2')))
def test_add_member(self):
"""Tests adding image members"""
self.assertRaises(exception.NotAuthenticated,
self.client.add_member, UUID2, 'pattieblack')
def test_delete_member(self):
def test_add_delete_member(self):
"""Tests deleting image members"""
self.assertRaises(exception.NotAuthenticated,
self.client.delete_member, UUID2, 'pattieblack')
self.client.add_member(UUID2, 'pattieblack')
self.assertTrue(self.client.delete_member(UUID2, 'pattieblack'))
class TestClient(base.IsolatedUnitTest):
@ -2073,21 +2068,16 @@ class TestClient(base.IsolatedUnitTest):
num_members = len(memb_list)
self.assertEquals(num_members, 0)
def test_replace_members(self):
def test_add_replace_members(self):
"""Tests replacing image members"""
self.assertRaises(exception.NotAuthenticated,
self.client.replace_members, UUID2,
dict(member_id='pattieblack'))
self.assertTrue(self.client.add_member(UUID2, 'pattieblack'))
self.assertTrue(self.client.replace_members(UUID2,
dict(member_id='pattieblack2')))
def test_add_member(self):
"""Tests adding image members"""
self.assertRaises(exception.NotAuthenticated,
self.client.add_member, UUID2, 'pattieblack')
def test_delete_member(self):
def test_add_delete_member(self):
"""Tests deleting image members"""
self.assertRaises(exception.NotAuthenticated,
self.client.delete_member, UUID2, 'pattieblack')
self.assertTrue(self.client.add_member(UUID2, 'pattieblack'))
self.assertTrue(self.client.delete_member(UUID2, 'pattieblack'))
class TestConfigureClientFromURL(test_utils.BaseTestCase):

21
glance/tests/unit/v1/test_api.py
View File

@ -93,9 +93,8 @@ class TestRegistryAPI(base.IsolatedUnitTest):
def setUp(self):
"""Establish a clean test environment"""
super(TestRegistryAPI, self).setUp()
mapper = routes.Mapper()
self.api = (
context.UnauthenticatedContextMiddleware(rserver.API(mapper)))
self.mapper = routes.Mapper()
self.api = test_utils.FakeAuthMiddleware(rserver.API(self.mapper))
self.FIXTURES = [
{'id': UUID1,
'name': 'fake image #1',
@ -1928,6 +1927,8 @@ class TestRegistryAPI(base.IsolatedUnitTest):
"""
Tests replacing image members raises right exception
"""
self.api = test_utils.FakeAuthMiddleware(rserver.API(self.mapper),
is_admin=False)
fixture = dict(member_id='pattieblack')
req = webob.Request.blank('/images/%s/members' % UUID2)
@ -1942,6 +1943,8 @@ class TestRegistryAPI(base.IsolatedUnitTest):
"""
Tests adding image members raises right exception
"""
self.api = test_utils.FakeAuthMiddleware(rserver.API(self.mapper),
is_admin=False)
req = webob.Request.blank('/images/%s/members/pattieblack' % UUID2)
req.method = 'PUT'
@ -1952,6 +1955,8 @@ class TestRegistryAPI(base.IsolatedUnitTest):
"""
Tests deleting image members raises right exception
"""
self.api = test_utils.FakeAuthMiddleware(rserver.API(self.mapper),
is_admin=False)
req = webob.Request.blank('/images/%s/members/pattieblack' % UUID2)
req.method = 'DELETE'
@ -1963,8 +1968,8 @@ class TestGlanceAPI(base.IsolatedUnitTest):
def setUp(self):
"""Establish a clean test environment"""
super(TestGlanceAPI, self).setUp()
mapper = routes.Mapper()
self.api = context.UnauthenticatedContextMiddleware(router.API(mapper))
self.mapper = routes.Mapper()
self.api = test_utils.FakeAuthMiddleware(router.API(self.mapper))
self.FIXTURES = [
{'id': UUID1,
'name': 'fake image #1',
@ -2959,6 +2964,8 @@ class TestGlanceAPI(base.IsolatedUnitTest):
"""
Tests replacing image members raises right exception
"""
self.api = test_utils.FakeAuthMiddleware(router.API(self.mapper),
is_admin=False)
fixture = dict(member_id='pattieblack')
req = webob.Request.blank('/images/%s/members' % UUID2)
@ -2973,6 +2980,8 @@ class TestGlanceAPI(base.IsolatedUnitTest):
"""
Tests adding image members raises right exception
"""
self.api = test_utils.FakeAuthMiddleware(router.API(self.mapper),
is_admin=False)
req = webob.Request.blank('/images/%s/members/pattieblack' % UUID2)
req.method = 'PUT'
@ -2983,6 +2992,8 @@ class TestGlanceAPI(base.IsolatedUnitTest):
"""
Tests deleting image members raises right exception
"""
self.api = test_utils.FakeAuthMiddleware(router.API(self.mapper),
is_admin=False)
req = webob.Request.blank('/images/%s/members/pattieblack' % UUID2)
req.method = 'DELETE'

16
glance/tests/utils.py
View File

@ -28,6 +28,7 @@ import unittest
import nose.plugins.skip
from glance.common import config
from glance.common import context
from glance.common import utils
from glance.common import wsgi
from glance.openstack.common import cfg
@ -341,14 +342,27 @@ def minimal_add_command(port, name, suffix='', public=True):
class FakeAuthMiddleware(wsgi.Middleware):
def __init__(self, app):
def __init__(self, app, is_admin=True):
super(FakeAuthMiddleware, self).__init__(app)
self.is_admin = is_admin
def process_request(self, req):
auth_tok = req.headers.get('X-Auth-Token')
user = None
tenant = None
roles = []
if auth_tok:
user, tenant, role = auth_tok.split(':')
roles = [role]
req.headers['X-User-Id'] = user
req.headers['X-Tenant-Id'] = tenant
req.headers['X-Roles'] = role
req.headers['X-Identity-Status'] = 'Confirmed'
kwargs = {
'user': user,
'tenant': tenant,
'roles': roles,
'is_admin': self.is_admin,
}
req.context = context.RequestContext(**kwargs)