From 78c9c3ad55f94212995cd1c7a46c30eccdcf499a Mon Sep 17 00:00:00 2001 From: "Kevin L. Mitchell" <kevin.mitchell@rackspace.com> Date: Wed, 20 Jul 2011 20:27:33 +0000 Subject: [PATCH 01/10] Add owner to database schema --- Authors | 1 + glance/registry/db/api.py | 3 +- .../db/migrate_repo/versions/007_add_owner.py | 82 +++++++++++++++++++ glance/registry/db/models.py | 1 + 4 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 glance/registry/db/migrate_repo/versions/007_add_owner.py diff --git a/Authors b/Authors index d1b2e1c355..9ab548c134 100644 --- a/Authors +++ b/Authors @@ -12,6 +12,7 @@ Jinwoo 'Joseph' Suh <jsuh@isi.edu> Josh Kearney <josh@jk0.org> Justin Shepherd <jshepher@rackspace.com> Ken Pepple <ken.pepple@gmail.com> +Kevin L. Mitchell <kevin.mitchell@rackspace.com> Matt Dietz <matt.dietz@rackspace.com> Monty Taylor <mordred@inaugust.com> Rick Clark <rick@openstack.org> diff --git a/glance/registry/db/api.py b/glance/registry/db/api.py index 694a7f2152..b189a9f4a2 100644 --- a/glance/registry/db/api.py +++ b/glance/registry/db/api.py @@ -45,7 +45,8 @@ BASE_MODEL_ATTRS = set(['id', 'created_at', 'updated_at', 'deleted_at', IMAGE_ATTRS = BASE_MODEL_ATTRS | set(['name', 'status', 'size', 'disk_format', 'container_format', - 'is_public', 'location', 'checksum']) + 'is_public', 'location', 'checksum', + 'owner']) CONTAINER_FORMATS = ['ami', 'ari', 'aki', 'bare', 'ovf'] DISK_FORMATS = ['ami', 'ari', 'aki', 'vhd', 'vmdk', 'raw', 'qcow2', 'vdi', diff --git a/glance/registry/db/migrate_repo/versions/007_add_owner.py b/glance/registry/db/migrate_repo/versions/007_add_owner.py new file mode 100644 index 0000000000..e860a3098a --- /dev/null +++ b/glance/registry/db/migrate_repo/versions/007_add_owner.py @@ -0,0 +1,82 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2011 OpenStack LLC. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from migrate.changeset import * +from sqlalchemy import * +from sqlalchemy.sql import and_, not_ + +from glance.registry.db.migrate_repo.schema import ( + Boolean, DateTime, BigInteger, Integer, String, + Text, from_migration_import) + + +def get_images_table(meta): + """ + Returns the Table object for the images table that corresponds to + the images table definition of this version. + """ + images = Table('images', meta, + Column('id', Integer(), primary_key=True, nullable=False), + Column('name', String(255)), + Column('disk_format', String(20)), + Column('container_format', String(20)), + Column('size', BigInteger()), + Column('status', String(30), nullable=False), + Column('is_public', Boolean(), nullable=False, default=False, + index=True), + Column('location', Text()), + Column('created_at', DateTime(), nullable=False), + Column('updated_at', DateTime()), + Column('deleted_at', DateTime()), + Column('deleted', Boolean(), nullable=False, default=False, + index=True), + Column('checksum', String(32)), + Column('owner', String(255)), + mysql_engine='InnoDB', + useexisting=True) + + return images + + +def get_image_properties_table(meta): + """ + No changes to the image properties table from 006... + """ + (get_image_properties_table,) = from_migration_import( + '006_key_to_name', ['get_image_properties_table']) + + image_properties = get_image_properties_table(meta) + return image_properties + + +def upgrade(migrate_engine): + meta = MetaData() + meta.bind = migrate_engine + + images = get_images_table(meta) + + owner = Column('owner', String(255)) + owner.create(images) + + +def downgrade(migrate_engine): + meta = MetaData() + meta.bind = migrate_engine + + images = get_images_table(meta) + + images.columns['owner'].drop() diff --git a/glance/registry/db/models.py b/glance/registry/db/models.py index 8507d16082..164acfd30f 100644 --- a/glance/registry/db/models.py +++ b/glance/registry/db/models.py @@ -105,6 +105,7 @@ class Image(BASE, ModelBase): is_public = Column(Boolean, nullable=False, default=False) location = Column(Text) checksum = Column(String(32)) + owner = Column(String(255)) class ImageProperty(BASE, ModelBase): From 52064a637859bd7a195e7f4c23acdd6515d08755 Mon Sep 17 00:00:00 2001 From: "Kevin L. Mitchell" <kevin.mitchell@rackspace.com> Date: Wed, 20 Jul 2011 22:53:44 +0000 Subject: [PATCH 02/10] Add in security context information --- etc/glance-api.conf | 5 ++++- etc/glance-registry.conf | 8 +++++++- glance/api/v1/images.py | 24 ++++++++++++++++-------- glance/client.py | 6 ++++-- glance/common/client.py | 6 +++++- glance/registry/__init__.py | 29 +++++++++++++++-------------- glance/registry/client.py | 5 +++-- glance/registry/server.py | 22 ++++++++++------------ 8 files changed, 64 insertions(+), 41 deletions(-) diff --git a/etc/glance-api.conf b/etc/glance-api.conf index 4551b10266..5aca496f6e 100644 --- a/etc/glance-api.conf +++ b/etc/glance-api.conf @@ -52,7 +52,7 @@ swift_store_container = glance swift_store_create_container_on_put = False [pipeline:glance-api] -pipeline = versionnegotiation apiv1app +pipeline = versionnegotiation context apiv1app [pipeline:versions] pipeline = versionsapp @@ -65,3 +65,6 @@ paste.app_factory = glance.api.v1:app_factory [filter:versionnegotiation] paste.filter_factory = glance.api.middleware.version_negotiation:filter_factory + +[filter:context] +paste.filter_factory = glance.common.context:filter_factory diff --git a/etc/glance-registry.conf b/etc/glance-registry.conf index 2e1f336209..56733cd414 100644 --- a/etc/glance-registry.conf +++ b/etc/glance-registry.conf @@ -29,5 +29,11 @@ sql_connection = sqlite:///glance.sqlite # before MySQL can drop the connection. sql_idle_timeout = 3600 -[app:glance-registry] +[pipeline:glance-registry] +pipeline = context registryapp + +[app:registryapp] paste.app_factory = glance.registry.server:app_factory + +[filter:context] +paste.filter_factory = glance.common.context:filter_factory diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py index f3a663bcca..0cc2f7466d 100644 --- a/glance/api/v1/images.py +++ b/glance/api/v1/images.py @@ -96,7 +96,8 @@ class Controller(object): """ params = self._get_query_params(req) try: - images = registry.get_images_list(self.options, **params) + images = registry.get_images_list(self.options, req.context, + **params) except exception.Invalid, e: raise HTTPBadRequest(explanation=str(e)) @@ -126,7 +127,8 @@ class Controller(object): """ params = self._get_query_params(req) try: - images = registry.get_images_detail(self.options, **params) + images = registry.get_images_detail(self.options, req.context, + **params) except exception.Invalid, e: raise HTTPBadRequest(explanation=str(e)) return dict(images=images) @@ -226,6 +228,7 @@ class Controller(object): try: image_meta = registry.add_image_metadata(self.options, + req.context, image_meta) return image_meta except exception.Duplicate: @@ -267,7 +270,7 @@ class Controller(object): image_id = image_meta['id'] logger.debug("Setting image %s to status 'saving'" % image_id) - registry.update_image_metadata(self.options, image_id, + registry.update_image_metadata(self.options, req.context, image_id, {'status': 'saving'}) try: logger.debug("Uploading image data for image %(image_id)s " @@ -294,7 +297,8 @@ class Controller(object): logger.debug("Updating image %(image_id)s data. " "Checksum set to %(checksum)s, size set " "to %(size)d" % locals()) - registry.update_image_metadata(self.options, image_id, + registry.update_image_metadata(self.options, req.context, + image_id, {'checksum': checksum, 'size': size}) @@ -325,6 +329,7 @@ class Controller(object): image_meta['location'] = location image_meta['status'] = 'active' return registry.update_image_metadata(self.options, + req.context, image_id, image_meta) @@ -336,6 +341,7 @@ class Controller(object): :param image_id: Opaque image identifier """ registry.update_image_metadata(self.options, + req.context, image_id, {'status': 'killed'}) @@ -432,8 +438,9 @@ class Controller(object): raise HTTPConflict("Cannot upload to an unqueued image") try: - image_meta = registry.update_image_metadata(self.options, id, - image_meta, True) + image_meta = registry.update_image_metadata(self.options, + req.context, id, + image_meta, True) if image_data is not None: image_meta = self._upload_and_activate(req, image_meta) except exception.Invalid, e: @@ -471,7 +478,7 @@ class Controller(object): "Continuing with deletion from registry." logger.error(msg % (image['location'],)) - registry.delete_image_metadata(self.options, id) + registry.delete_image_metadata(self.options, req.context, id) def get_image_meta_or_404(self, request, id): """ @@ -484,7 +491,8 @@ class Controller(object): :raises HTTPNotFound if image does not exist """ try: - return registry.get_image_metadata(self.options, id) + return registry.get_image_metadata(self.options, + request.context, id) except exception.NotFound: msg = "Image with identifier %s not found" % id logger.debug(msg) diff --git a/glance/client.py b/glance/client.py index af58cd533d..fc633a5c49 100644 --- a/glance/client.py +++ b/glance/client.py @@ -35,7 +35,8 @@ class V1Client(base_client.BaseClient): DEFAULT_PORT = 9292 - def __init__(self, host, port=None, use_ssl=False, doc_root="/v1"): + def __init__(self, host, port=None, use_ssl=False, doc_root="/v1", + auth_tok=None): """ Creates a new client to a Glance API service. @@ -43,10 +44,11 @@ class V1Client(base_client.BaseClient): :param port: The port where Glance resides (defaults to 9292) :param use_ssl: Should we use HTTPS? (defaults to False) :param doc_root: Prefix for all URLs we request from host + :param auth_tok: The auth token to pass to the server """ port = port or self.DEFAULT_PORT self.doc_root = doc_root - super(Client, self).__init__(host, port, use_ssl) + super(Client, self).__init__(host, port, use_ssl, auth_tok) def do_request(self, method, action, body=None, headers=None, params=None): action = "%s/%s" % (self.doc_root, action.lstrip("/")) diff --git a/glance/common/client.py b/glance/common/client.py index c20a616fa0..11bc159bca 100644 --- a/glance/common/client.py +++ b/glance/common/client.py @@ -41,17 +41,19 @@ class BaseClient(object): CHUNKSIZE = 65536 - def __init__(self, host, port, use_ssl): + def __init__(self, host, port, use_ssl, auth_tok): """ Creates a new client to some service. :param host: The host where service resides :param port: The port where service resides :param use_ssl: Should we use HTTPS? + :param auth_tok: The auth token to pass to the server """ self.host = host self.port = port self.use_ssl = use_ssl + self.auth_tok = auth_tok self.connection = None def get_connection_type(self): @@ -99,6 +101,8 @@ class BaseClient(object): try: connection_type = self.get_connection_type() headers = headers or {} + if 'x-auth-token' not in headers and self.auth_tok: + headers['x-auth-token'] = self.auth_tok c = connection_type(self.host, self.port) # Do a simple request or a chunked request, depending diff --git a/glance/registry/__init__.py b/glance/registry/__init__.py index 19e4cc57fe..f61d96de33 100644 --- a/glance/registry/__init__.py +++ b/glance/registry/__init__.py @@ -26,33 +26,33 @@ from glance.registry import client logger = logging.getLogger('glance.registry') -def get_registry_client(options): +def get_registry_client(options, cxt): host = options['registry_host'] port = int(options['registry_port']) - return client.RegistryClient(host, port) + return client.RegistryClient(host, port, auth_tok=cxt.auth_tok) -def get_images_list(options, **kwargs): - c = get_registry_client(options) +def get_images_list(options, context, **kwargs): + c = get_registry_client(options, context) return c.get_images(**kwargs) -def get_images_detail(options, **kwargs): - c = get_registry_client(options) +def get_images_detail(options, context, **kwargs): + c = get_registry_client(options, context) return c.get_images_detailed(**kwargs) -def get_image_metadata(options, image_id): - c = get_registry_client(options) +def get_image_metadata(options, context, image_id): + c = get_registry_client(options, context) return c.get_image(image_id) -def add_image_metadata(options, image_meta): +def add_image_metadata(options, context, image_meta): if options['debug']: logger.debug("Adding image metadata...") _debug_print_metadata(image_meta) - c = get_registry_client(options) + c = get_registry_client(options, context) new_image_meta = c.add_image(image_meta) if options['debug']: @@ -63,12 +63,13 @@ def add_image_metadata(options, image_meta): return new_image_meta -def update_image_metadata(options, image_id, image_meta, purge_props=False): +def update_image_metadata(options, context, image_id, image_meta, + purge_props=False): if options['debug']: logger.debug("Updating image metadata for image %s...", image_id) _debug_print_metadata(image_meta) - c = get_registry_client(options) + c = get_registry_client(options, context) new_image_meta = c.update_image(image_id, image_meta, purge_props) if options['debug']: @@ -79,9 +80,9 @@ def update_image_metadata(options, image_id, image_meta, purge_props=False): return new_image_meta -def delete_image_metadata(options, image_id): +def delete_image_metadata(options, context, image_id): logger.debug("Deleting image metadata for image %s...", image_id) - c = get_registry_client(options) + c = get_registry_client(options, context) return c.delete_image(image_id) diff --git a/glance/registry/client.py b/glance/registry/client.py index 3947c624b6..ee873e27f1 100644 --- a/glance/registry/client.py +++ b/glance/registry/client.py @@ -33,16 +33,17 @@ class RegistryClient(BaseClient): DEFAULT_PORT = 9191 - def __init__(self, host, port=None, use_ssl=False): + def __init__(self, host, port=None, use_ssl=False, auth_tok=None): """ Creates a new client to a Glance Registry service. :param host: The host where Glance resides :param port: The port where Glance resides (defaults to 9191) :param use_ssl: Should we use HTTPS? (defaults to False) + :param auth_tok: The auth token to pass to the server """ port = port or self.DEFAULT_PORT - super(RegistryClient, self).__init__(host, port, use_ssl) + super(RegistryClient, self).__init__(host, port, use_ssl, auth_tok) def get_images(self, **kwargs): """ diff --git a/glance/registry/server.py b/glance/registry/server.py index 9273d1ccb8..51413f0e0b 100644 --- a/glance/registry/server.py +++ b/glance/registry/server.py @@ -61,7 +61,7 @@ class Controller(object): Get images, wrapping in exception if necessary. """ try: - return db_api.image_get_all(None, **params) + return db_api.image_get_all(context, **params) except exception.NotFound, e: msg = "Invalid marker. Image could not be found." raise exc.HTTPBadRequest(explanation=msg) @@ -87,7 +87,7 @@ class Controller(object): } """ params = self._get_query_params(req) - images = self._get_images(None, **params) + images = self._get_images(req.context, **params) results = [] for image in images: @@ -111,7 +111,7 @@ class Controller(object): """ params = self._get_query_params(req) - images = self._get_images(None, **params) + images = self._get_images(req.context, **params) image_dicts = [make_image_dict(i) for i in images] return dict(images=image_dicts) @@ -223,7 +223,7 @@ class Controller(object): def show(self, req, id): """Return data about the given image id.""" try: - image = db_api.image_get(None, id) + image = db_api.image_get(req.context, id) except exception.NotFound: raise exc.HTTPNotFound() @@ -238,9 +238,8 @@ class Controller(object): :retval Returns 200 if delete was successful, a fault if not. """ - context = None try: - db_api.image_destroy(context, id) + db_api.image_destroy(req.context, id) except exception.NotFound: return exc.HTTPNotFound() @@ -260,9 +259,8 @@ class Controller(object): # Ensure the image has a status set image_data.setdefault('status', 'active') - context = None try: - image_data = db_api.image_create(context, image_data) + image_data = db_api.image_create(req.context, image_data) return dict(image=make_image_dict(image_data)) except exception.Duplicate: msg = ("Image with identifier %s already exists!" % id) @@ -286,15 +284,15 @@ class Controller(object): image_data = body['image'] purge_props = req.headers.get("X-Glance-Registry-Purge-Props", "false") - context = None try: logger.debug("Updating image %(id)s with metadata: %(image_data)r" % locals()) if purge_props == "true": - updated_image = db_api.image_update(context, id, image_data, - True) + updated_image = db_api.image_update(req.context, id, + image_data, True) else: - updated_image = db_api.image_update(context, id, image_data) + updated_image = db_api.image_update(req.context, id, + image_data) return dict(image=make_image_dict(updated_image)) except exception.Invalid, e: msg = ("Failed to update image metadata. " From cd9a799272515357da91c52372cc98c04ee30f25 Mon Sep 17 00:00:00 2001 From: "Kevin L. Mitchell" <kevin.mitchell@rackspace.com> Date: Thu, 21 Jul 2011 16:40:20 +0000 Subject: [PATCH 03/10] Make tests work again --- tests/functional/__init__.py | 13 +++++++++++-- tests/stubs.py | 7 +++++-- tests/unit/test_api.py | 8 +++++--- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/tests/functional/__init__.py b/tests/functional/__init__.py index 7490cc580d..4e158f52bc 100644 --- a/tests/functional/__init__.py +++ b/tests/functional/__init__.py @@ -149,7 +149,7 @@ registry_port = %(registry_port)s log_file = %(log_file)s [pipeline:glance-api] -pipeline = versionnegotiation apiv1app +pipeline = versionnegotiation context apiv1app [pipeline:versions] pipeline = versionsapp @@ -162,6 +162,9 @@ paste.app_factory = glance.api.v1:app_factory [filter:versionnegotiation] paste.filter_factory = glance.api.middleware.version_negotiation:filter_factory + +[filter:context] +paste.filter_factory = glance.common.context:filter_factory """ @@ -191,8 +194,14 @@ log_file = %(log_file)s sql_connection = %(sql_connection)s sql_idle_timeout = 3600 -[app:glance-registry] +[pipeline:glance-registry] +pipeline = context registryapp + +[app:registryapp] paste.app_factory = glance.registry.server:app_factory + +[filter:context] +paste.filter_factory = glance.common.context:filter_factory """ diff --git a/tests/stubs.py b/tests/stubs.py index d7e9e16ec2..8d0a9315ca 100644 --- a/tests/stubs.py +++ b/tests/stubs.py @@ -29,6 +29,7 @@ import stubout import webob import glance.common.client +from glance.common import context from glance.common import exception from glance.registry import server as rserver from glance.api import v1 as server @@ -172,7 +173,8 @@ def stub_out_registry_and_store_server(stubs): "sqlite://") options = {'sql_connection': sql_connection, 'verbose': VERBOSE, 'debug': DEBUG} - res = self.req.get_response(rserver.API(options)) + api = context.ContextMiddleware(rserver.API(options), options) + res = self.req.get_response(api) # httplib.Response has a read() method...fake it out def fake_reader(): @@ -223,7 +225,8 @@ def stub_out_registry_and_store_server(stubs): 'registry_port': '9191', 'default_store': 'file', 'filesystem_store_datadir': FAKE_FILESYSTEM_ROOTDIR} - res = self.req.get_response(server.API(options)) + api = context.ContextMiddleware(server.API(options), options) + res = self.req.get_response(api) # httplib.Response has a read() method...fake it out def fake_reader(): diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py index 1a82c1e033..f254d194d6 100644 --- a/tests/unit/test_api.py +++ b/tests/unit/test_api.py @@ -26,6 +26,7 @@ import stubout import webob from glance.api import v1 as server +from glance.common import context from glance.registry import server as rserver import glance.registry.db.api from tests import stubs @@ -41,8 +42,9 @@ class TestRegistryAPI(unittest.TestCase): stubs.stub_out_registry_and_store_server(self.stubs) stubs.stub_out_registry_db_image_api(self.stubs) stubs.stub_out_filesystem_backend() - self.api = rserver.API({'verbose': VERBOSE, - 'debug': DEBUG}) + options = {'verbose': VERBOSE, + 'debug': DEBUG} + self.api = context.ContextMiddleware(rserver.API(options), options) def tearDown(self): """Clear the test environment""" @@ -1458,7 +1460,7 @@ class TestGlanceAPI(unittest.TestCase): 'sql_connection': sql_connection, 'default_store': 'file', 'filesystem_store_datadir': stubs.FAKE_FILESYSTEM_ROOTDIR} - self.api = server.API(options) + self.api = context.ContextMiddleware(server.API(options), options) def tearDown(self): """Clear the test environment""" From 11b2f93dfea701c5cfdd3caee3a5819781e0c6b2 Mon Sep 17 00:00:00 2001 From: "Kevin L. Mitchell" <kevin.mitchell@rackspace.com> Date: Thu, 21 Jul 2011 16:40:44 +0000 Subject: [PATCH 04/10] How in the world did I manage to forget this? *sigh* --- glance/common/context.py | 66 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 glance/common/context.py diff --git a/glance/common/context.py b/glance/common/context.py new file mode 100644 index 0000000000..f60f6db5a8 --- /dev/null +++ b/glance/common/context.py @@ -0,0 +1,66 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2011 OpenStack LLC. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from glance.common import wsgi + + +class RequestContext(object): + """Security context and request information. + + Stores information about the security context under which the user + accesses the system, as well as additional request information. + + """ + + def __init__(self, auth_tok=None, user=None, tenant=None, is_admin=False, + read_only=False): + self.auth_tok = auth_tok + self.user = user + self.tenant = tenant + self.is_admin = is_admin + self.read_only = read_only + + def elevate(self): + """Return a version of this context with admin flag set.""" + return self.__class__(user, tenant, True) + + +class ContextMiddleware(wsgi.Middleware): + def __init__(self, app, options): + self.options = options + super(ContextMiddleware, self).__init__(app) + + def process_request(self, req): + """ + Extract any authentication information in the request and + construct an appropriate context from it. + """ + # Use the default empty context + req.context = RequestContext() + + +def filter_factory(global_conf, **local_conf): + """ + Factory method for paste.deploy + """ + conf = global_conf.copy() + conf.update(local_conf) + + def filter(app): + return ContextMiddleware(app, conf) + + return filter From ef2cde666c98847bd078c19f01ee80ab032dd838 Mon Sep 17 00:00:00 2001 From: "Kevin L. Mitchell" <kevin.mitchell@rackspace.com> Date: Thu, 21 Jul 2011 22:38:27 +0000 Subject: [PATCH 05/10] Link incoming context with image owner for authorization decisions --- glance/api/v1/images.py | 41 ++++++++++++++++++++++++++++++++++++++- glance/common/context.py | 25 +++++++++++++++++++++--- glance/registry/db/api.py | 15 +++++++++++++- glance/registry/server.py | 40 +++++++++++++++++++++++++++++++++++++- 4 files changed, 115 insertions(+), 6 deletions(-) diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py index 0cc2f7466d..9c24fe4f9e 100644 --- a/glance/api/v1/images.py +++ b/glance/api/v1/images.py @@ -27,7 +27,8 @@ import sys import webob from webob.exc import (HTTPNotFound, HTTPConflict, - HTTPBadRequest) + HTTPBadRequest, + HTTPForbidden) from glance.common import exception from glance.common import wsgi @@ -241,6 +242,11 @@ class Controller(object): for line in msg.split('\n'): logger.error(line) raise HTTPBadRequest(msg, request=req, content_type="text/plain") + except exception.NotAuthorized: + msg = "Not authorized to reserve image." + logger.error(msg) + raise HTTPForbidden(msg, request=req, + content_type="text/plain") def _upload(self, req, image_meta): """ @@ -310,6 +316,13 @@ class Controller(object): self._safe_kill(req, image_id) raise HTTPConflict(msg, request=req) + except exception.NotAuthorized, e: + msg = ("Unauthorized upload attempt: %s") % str(e) + logger.error(msg) + self._safe_kill(req, image_id) + raise HTTPForbidden(msg, request=req, + content_type='text/plain') + except Exception, e: msg = ("Error uploading image: %s") % str(e) logger.error(msg) @@ -410,6 +423,13 @@ class Controller(object): and the request body is not application/octet-stream image data. """ + # Is this a read-only context? + if req.context.read_only: + msg = "Read-only access" + logger.debug(msg) + raise HTTPForbidden(msg, request=req, + content_type="text/plain") + image_meta = self._reserve(req, image_meta) image_id = image_meta['id'] @@ -431,6 +451,13 @@ class Controller(object): :retval Returns the updated image information as a mapping """ + # Is this a read-only context? + if req.context.read_only: + msg = "Read-only access" + logger.debug(msg) + raise HTTPForbidden(msg, request=req, + content_type="text/plain") + orig_image_meta = self.get_image_meta_or_404(req, id) orig_status = orig_image_meta['status'] @@ -464,6 +491,13 @@ class Controller(object): :raises HttpNotAuthorized if image or any chunk is not deleteable by the requesting user """ + # Is this a read-only context? + if req.context.read_only: + msg = "Read-only access" + logger.debug(msg) + raise HTTPForbidden(msg, request=req, + content_type="text/plain") + image = self.get_image_meta_or_404(req, id) # The image's location field may be None in the case @@ -498,6 +532,11 @@ class Controller(object): logger.debug(msg) raise HTTPNotFound(msg, request=request, content_type='text/plain') + except exception.NotAuthorized: + msg = "Unauthorized image access" + logger.debug(msg) + raise HTTPForbidden(msg, request=request, + content_type='text/plain') def get_store_or_400(self, request, store_name): """ diff --git a/glance/common/context.py b/glance/common/context.py index f60f6db5a8..1ec47a7511 100644 --- a/glance/common/context.py +++ b/glance/common/context.py @@ -34,9 +34,27 @@ class RequestContext(object): self.is_admin = is_admin self.read_only = read_only + def image_visible(self, image): + """Return True if the image is visible in this context.""" + # Is admin == image visible + if self.is_admin: + return True + + # No owner == image visible + if image.owner is None: + return True + + # Image is_public == image visible + if image.is_public: + return True + + # Private image + return self.tenant and self.tenant == image.owner + def elevate(self): """Return a version of this context with admin flag set.""" - return self.__class__(user, tenant, True) + return self.__class__(self.auth_tok, self.user, self.tenant, + True, True) class ContextMiddleware(wsgi.Middleware): @@ -49,8 +67,9 @@ class ContextMiddleware(wsgi.Middleware): Extract any authentication information in the request and construct an appropriate context from it. """ - # Use the default empty context - req.context = RequestContext() + # Use the default empty context, with admin turned on for + # backwards compatibility + req.context = RequestContext(is_admin=True) def filter_factory(global_conf, **local_conf): diff --git a/glance/registry/db/api.py b/glance/registry/db/api.py index 77e8822a34..cbbe7b5931 100644 --- a/glance/registry/db/api.py +++ b/glance/registry/db/api.py @@ -121,7 +121,7 @@ def image_get(context, image_id, session=None): """Get an image or raise if it does not exist.""" session = session or get_session() try: - return session.query(models.Image).\ + image = session.query(models.Image).\ options(joinedload(models.Image.properties)).\ filter_by(deleted=_deleted(context)).\ filter_by(id=image_id).\ @@ -129,6 +129,12 @@ def image_get(context, image_id, session=None): except exc.NoResultFound: raise exception.NotFound("No image found with ID %s" % image_id) + # Make sure they can look at it + if not context.image_visible(image): + raise exception.NotAuthorized("Image not visible to you") + + return image + def image_get_all(context, filters=None, marker=None, limit=None, sort_key='created_at', sort_dir='desc'): @@ -169,6 +175,13 @@ def image_get_all(context, filters=None, marker=None, limit=None, query = query.filter(models.Image.size <= filters['size_max']) del filters['size_max'] + if 'is_public' in filters and filters['is_public'] is not None: + the_filter = models.Image.is_public == filters['is_public'] + if filters['is_public'] and context.tenant is not None: + the_filter = or_(the_filter, models.Image.owner == context.tenant) + query = query.filter(the_filter) + del filters['is_public'] + for (k, v) in filters.pop('properties', {}).items(): query = query.filter(models.Image.properties.any(name=k, value=v)) diff --git a/glance/registry/server.py b/glance/registry/server.py index 51413f0e0b..64a2bbb0a8 100644 --- a/glance/registry/server.py +++ b/glance/registry/server.py @@ -146,7 +146,11 @@ class Controller(object): filters = {} properties = {} - filters['is_public'] = self._get_is_public(req) + if req.context.is_admin: + # Only admin gets to look for non-public images + filters['is_public'] = self._get_is_public(req) + else: + filters['is_public'] = True for param in req.str_params: if param in SUPPORTED_FILTERS: filters[param] = req.str_params.get(param) @@ -226,6 +230,10 @@ class Controller(object): image = db_api.image_get(req.context, id) except exception.NotFound: raise exc.HTTPNotFound() + except exception.NotAuthorized: + # If it's private and doesn't belong to them, don't let on + # that it exists + raise exc.HTTPNotFound() return dict(image=make_image_dict(image)) @@ -238,10 +246,18 @@ class Controller(object): :retval Returns 200 if delete was successful, a fault if not. """ + # Is this a read-only context? + if req.context.read_only: + raise exc.HTTPForbidden() + try: db_api.image_destroy(req.context, id) except exception.NotFound: return exc.HTTPNotFound() + except exception.NotAuthorized: + # If it's private and doesn't belong to them, don't let on + # that it exists + raise exc.HTTPNotFound() def create(self, req, body): """ @@ -254,11 +270,19 @@ class Controller(object): which will include the newly-created image's internal id in the 'id' field """ + # Is this a read-only context? + if req.context.read_only: + raise exc.HTTPForbidden() + image_data = body['image'] # Ensure the image has a status set image_data.setdefault('status', 'active') + # Set up the image owner + if not req.context.is_admin or 'owner' not in image_data: + image_data['owner'] = req.context.tenant + try: image_data = db_api.image_create(req.context, image_data) return dict(image=make_image_dict(image_data)) @@ -281,8 +305,16 @@ class Controller(object): :retval Returns the updated image information as a mapping, """ + # Is this a read-only context? + if req.context.read_only: + raise exc.HTTPForbidden() + image_data = body['image'] + # Prohibit modification of 'owner' + if not req.context.is_admin and 'owner' in image_data: + del image_data['owner'] + purge_props = req.headers.get("X-Glance-Registry-Purge-Props", "false") try: logger.debug("Updating image %(id)s with metadata: %(image_data)r" @@ -303,6 +335,12 @@ class Controller(object): raise exc.HTTPNotFound(body='Image not found', request=req, content_type='text/plain') + except exception.NotAuthorized: + # If it's private and doesn't belong to them, don't let on + # that it exists + raise exc.HTTPNotFound(body='Image not found', + request=req, + content_type='text/plain') def create_resource(options): From 59f9b7c2b13350dfc4b57ae25397096929f24391 Mon Sep 17 00:00:00 2001 From: "Kevin L. Mitchell" <kevin.mitchell@rackspace.com> Date: Fri, 22 Jul 2011 19:02:08 +0000 Subject: [PATCH 06/10] We don't really need elevate()... --- glance/common/context.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/glance/common/context.py b/glance/common/context.py index 1ec47a7511..224c6e1bdf 100644 --- a/glance/common/context.py +++ b/glance/common/context.py @@ -49,12 +49,7 @@ class RequestContext(object): return True # Private image - return self.tenant and self.tenant == image.owner - - def elevate(self): - """Return a version of this context with admin flag set.""" - return self.__class__(self.auth_tok, self.user, self.tenant, - True, True) + return self.tenant is not None and self.tenant == image.owner class ContextMiddleware(wsgi.Middleware): From 51edd723968d9ae605d133a070a6b9f641f2a297 Mon Sep 17 00:00:00 2001 From: "Kevin L. Mitchell" <kevin.mitchell@rackspace.com> Date: Fri, 22 Jul 2011 19:02:48 +0000 Subject: [PATCH 07/10] Unit tests for the context's image_visible() routine --- tests/unit/test_context.py | 148 +++++++++++++++++++++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 tests/unit/test_context.py diff --git a/tests/unit/test_context.py b/tests/unit/test_context.py new file mode 100644 index 0000000000..98cf208a56 --- /dev/null +++ b/tests/unit/test_context.py @@ -0,0 +1,148 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2010-2011 OpenStack, LLC +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import unittest + +from glance.common import context + + +class FakeImage(object): + """ + Fake image for providing the image attributes needed for + TestContext. + """ + + def __init__(self, owner, is_public): + self.owner = owner + self.is_public = is_public + + +class TestContext(unittest.TestCase): + def do_visible(self, exp_res, img_owner, img_public, **kwargs): + """ + Perform a context test. Creates a (fake) image with the + specified owner and is_public attributes, then creates a + context with the given keyword arguments and expects exp_res + as the result of an image_visible() call on the context. + """ + + img = FakeImage(img_owner, img_public) + ctx = context.RequestContext(**kwargs) + + self.assertEqual(ctx.image_visible(img), exp_res) + + def test_empty_public(self): + """ + Tests that an empty context (with is_admin set to True) can + access an image with is_public set to True. + """ + self.do_visible(True, None, True, is_admin=True) + + def test_empty_public_owned(self): + """ + Tests that an empty context (with is_admin set to True) can + access an owned image with is_public set to True. + """ + self.do_visible(True, 'pattieblack', True, is_admin=True) + + def test_empty_private(self): + """ + Tests that an empty context (with is_admin set to True) can + access an image with is_public set to False. + """ + self.do_visible(True, None, False, is_admin=True) + + def test_empty_private_owned(self): + """ + Tests that an empty context (with is_admin set to True) can + access an owned image with is_public set to False. + """ + self.do_visible(True, 'pattieblack', False, is_admin=True) + + def test_anon_public(self): + """ + Tests that an anonymous context (with is_admin set to False) + can access an image with is_public set to True. + """ + self.do_visible(True, None, True) + + def test_anon_public_owned(self): + """ + Tests that an anonymous context (with is_admin set to False) + can access an owned image with is_public set to True. + """ + self.do_visible(True, 'pattieblack', True) + + def test_anon_private(self): + """ + Tests that an anonymous context (with is_admin set to False) + can access an unowned image with is_public set to False. + """ + self.do_visible(True, None, False) + + def test_anon_private_owned(self): + """ + Tests that an anonymous context (with is_admin set to False) + cannot access an owned image with is_public set to False. + """ + self.do_visible(False, 'pattieblack', False) + + def test_auth_public(self): + """ + Tests that an authenticated context (with is_admin set to + False) can access an image with is_public set to True. + """ + self.do_visible(True, None, True, tenant='froggy') + + def test_auth_public_unowned(self): + """ + Tests that an authenticated context (with is_admin set to + False) can access an image (which it does not own) with + is_public set to True. + """ + self.do_visible(True, 'pattieblack', True, tenant='froggy') + + def test_auth_public_owned(self): + """ + Tests that an authenticated context (with is_admin set to + False) can access an image (which it does own) with is_public + set to True. + """ + self.do_visible(True, 'pattieblack', True, tenant='pattieblack') + + def test_auth_private(self): + """ + Tests that an authenticated context (with is_admin set to + False) can access an image with is_public set to False. + """ + self.do_visible(True, None, False, tenant='froggy') + + def test_auth_private_unowned(self): + """ + Tests that an authenticated context (with is_admin set to + False) cannot access an image (which it does not own) with + is_public set to False. + """ + self.do_visible(False, 'pattieblack', False, tenant='froggy') + + def test_auth_private_owned(self): + """ + Tests that an authenticated context (with is_admin set to + False) can access an image (which it does own) with is_public + set to False. + """ + self.do_visible(True, 'pattieblack', False, tenant='pattieblack') From c2d591114f72d89707c5dad502882299a1014300 Mon Sep 17 00:00:00 2001 From: "Kevin L. Mitchell" <kevin.mitchell@rackspace.com> Date: Fri, 22 Jul 2011 19:51:52 +0000 Subject: [PATCH 08/10] Make a context property 'owner' that returns the tenant; this makes it possible to change the concept of ownership by using a different context object. --- glance/common/context.py | 7 ++++++- glance/registry/db/api.py | 4 ++-- glance/registry/server.py | 2 +- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/glance/common/context.py b/glance/common/context.py index 224c6e1bdf..d9c6262662 100644 --- a/glance/common/context.py +++ b/glance/common/context.py @@ -49,7 +49,12 @@ class RequestContext(object): return True # Private image - return self.tenant is not None and self.tenant == image.owner + return self.owner is not None and self.owner == image.owner + + @property + def owner(self): + """Return the owner to correlate with an image.""" + return self.tenant class ContextMiddleware(wsgi.Middleware): diff --git a/glance/registry/db/api.py b/glance/registry/db/api.py index cbbe7b5931..c716dfbeb1 100644 --- a/glance/registry/db/api.py +++ b/glance/registry/db/api.py @@ -177,8 +177,8 @@ def image_get_all(context, filters=None, marker=None, limit=None, if 'is_public' in filters and filters['is_public'] is not None: the_filter = models.Image.is_public == filters['is_public'] - if filters['is_public'] and context.tenant is not None: - the_filter = or_(the_filter, models.Image.owner == context.tenant) + if filters['is_public'] and context.owner is not None: + the_filter = or_(the_filter, models.Image.owner == context.owner) query = query.filter(the_filter) del filters['is_public'] diff --git a/glance/registry/server.py b/glance/registry/server.py index 64a2bbb0a8..3b4938b84e 100644 --- a/glance/registry/server.py +++ b/glance/registry/server.py @@ -281,7 +281,7 @@ class Controller(object): # Set up the image owner if not req.context.is_admin or 'owner' not in image_data: - image_data['owner'] = req.context.tenant + image_data['owner'] = req.context.owner try: image_data = db_api.image_create(req.context, image_data) From fba072aab4301f51b3078c34187a342ea04b7a94 Mon Sep 17 00:00:00 2001 From: "Kevin L. Mitchell" <kevin.mitchell@rackspace.com> Date: Fri, 22 Jul 2011 19:57:56 +0000 Subject: [PATCH 09/10] Allow plugging in alternate context classes so the owner property and the image_visible() method can be overridden. --- glance/common/context.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/glance/common/context.py b/glance/common/context.py index d9c6262662..c97fe05a7d 100644 --- a/glance/common/context.py +++ b/glance/common/context.py @@ -15,6 +15,7 @@ # License for the specific language governing permissions and limitations # under the License. +from glance.common import utils from glance.common import wsgi @@ -62,6 +63,18 @@ class ContextMiddleware(wsgi.Middleware): self.options = options super(ContextMiddleware, self).__init__(app) + def make_context(self, *args, **kwargs): + """ + Create a context with the given arguments. + """ + + # Determine the context class to use + ctxcls = RequestContext + if 'context_class' in self.options: + ctxcls = utils.import_class(self.options['context_class']) + + return ctxcls(*args, **kwargs) + def process_request(self, req): """ Extract any authentication information in the request and @@ -69,7 +82,7 @@ class ContextMiddleware(wsgi.Middleware): """ # Use the default empty context, with admin turned on for # backwards compatibility - req.context = RequestContext(is_admin=True) + req.context = self.make_context(is_admin=True) def filter_factory(global_conf, **local_conf): From c54c09048fc54ec7d5dce4f2e1709d0104c6991b Mon Sep 17 00:00:00 2001 From: "Kevin L. Mitchell" <kevin.mitchell@rackspace.com> Date: Fri, 22 Jul 2011 23:35:24 +0000 Subject: [PATCH 10/10] Take out extraneous comments; tune up doc string; rename image_visible() to is_image_visible(); log authorization failures --- glance/api/v1/images.py | 3 --- glance/common/context.py | 6 ++---- glance/registry/db/api.py | 2 +- glance/registry/server.py | 9 ++++++--- tests/unit/test_context.py | 4 ++-- 5 files changed, 11 insertions(+), 13 deletions(-) diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py index 9c24fe4f9e..db5b3bc822 100644 --- a/glance/api/v1/images.py +++ b/glance/api/v1/images.py @@ -423,7 +423,6 @@ class Controller(object): and the request body is not application/octet-stream image data. """ - # Is this a read-only context? if req.context.read_only: msg = "Read-only access" logger.debug(msg) @@ -451,7 +450,6 @@ class Controller(object): :retval Returns the updated image information as a mapping """ - # Is this a read-only context? if req.context.read_only: msg = "Read-only access" logger.debug(msg) @@ -491,7 +489,6 @@ class Controller(object): :raises HttpNotAuthorized if image or any chunk is not deleteable by the requesting user """ - # Is this a read-only context? if req.context.read_only: msg = "Read-only access" logger.debug(msg) diff --git a/glance/common/context.py b/glance/common/context.py index c97fe05a7d..d4ac8197ab 100644 --- a/glance/common/context.py +++ b/glance/common/context.py @@ -20,11 +20,9 @@ from glance.common import wsgi class RequestContext(object): - """Security context and request information. - + """ Stores information about the security context under which the user accesses the system, as well as additional request information. - """ def __init__(self, auth_tok=None, user=None, tenant=None, is_admin=False, @@ -35,7 +33,7 @@ class RequestContext(object): self.is_admin = is_admin self.read_only = read_only - def image_visible(self, image): + def is_image_visible(self, image): """Return True if the image is visible in this context.""" # Is admin == image visible if self.is_admin: diff --git a/glance/registry/db/api.py b/glance/registry/db/api.py index c716dfbeb1..434f4fa63e 100644 --- a/glance/registry/db/api.py +++ b/glance/registry/db/api.py @@ -130,7 +130,7 @@ def image_get(context, image_id, session=None): raise exception.NotFound("No image found with ID %s" % image_id) # Make sure they can look at it - if not context.image_visible(image): + if not context.is_image_visible(image): raise exception.NotAuthorized("Image not visible to you") return image diff --git a/glance/registry/server.py b/glance/registry/server.py index 3b4938b84e..fee1248554 100644 --- a/glance/registry/server.py +++ b/glance/registry/server.py @@ -233,6 +233,8 @@ class Controller(object): except exception.NotAuthorized: # If it's private and doesn't belong to them, don't let on # that it exists + logger.info("Access by %s to image %s denied" % + (req.context.user, id)) raise exc.HTTPNotFound() return dict(image=make_image_dict(image)) @@ -246,7 +248,6 @@ class Controller(object): :retval Returns 200 if delete was successful, a fault if not. """ - # Is this a read-only context? if req.context.read_only: raise exc.HTTPForbidden() @@ -257,6 +258,8 @@ class Controller(object): except exception.NotAuthorized: # If it's private and doesn't belong to them, don't let on # that it exists + logger.info("Access by %s to image %s denied" % + (req.context.user, id)) raise exc.HTTPNotFound() def create(self, req, body): @@ -270,7 +273,6 @@ class Controller(object): which will include the newly-created image's internal id in the 'id' field """ - # Is this a read-only context? if req.context.read_only: raise exc.HTTPForbidden() @@ -305,7 +307,6 @@ class Controller(object): :retval Returns the updated image information as a mapping, """ - # Is this a read-only context? if req.context.read_only: raise exc.HTTPForbidden() @@ -338,6 +339,8 @@ class Controller(object): except exception.NotAuthorized: # If it's private and doesn't belong to them, don't let on # that it exists + logger.info("Access by %s to image %s denied" % + (req.context.user, id)) raise exc.HTTPNotFound(body='Image not found', request=req, content_type='text/plain') diff --git a/tests/unit/test_context.py b/tests/unit/test_context.py index 98cf208a56..970393da59 100644 --- a/tests/unit/test_context.py +++ b/tests/unit/test_context.py @@ -37,13 +37,13 @@ class TestContext(unittest.TestCase): Perform a context test. Creates a (fake) image with the specified owner and is_public attributes, then creates a context with the given keyword arguments and expects exp_res - as the result of an image_visible() call on the context. + as the result of an is_image_visible() call on the context. """ img = FakeImage(img_owner, img_public) ctx = context.RequestContext(**kwargs) - self.assertEqual(ctx.image_visible(img), exp_res) + self.assertEqual(ctx.is_image_visible(img), exp_res) def test_empty_public(self): """