airship/deckhand
Code Issues Proposed changes

Add validation for empty documents inside multi-document payload

Browse Source 
This is to add a helper function to base controller resource class
for detecting whether a list of documents has any empty (None)
or non-object entries in it (since all documents should be
objects), resulting in a 400 Bad Request getting raised.
This is to prevent the following stacktrace from occurring:

File "/usr/local/lib/python3.5/dist-packages/falcon/api.py", line 244, in __call__
responder(req, resp, **params)
File "./deckhand/policy.py", line 104, in handler
return func(*args, **kwargs)
File "./deckhand/control/buckets.py", line 58, in on_put
documents, data_schemas, pre_validate=True)
File "./deckhand/engine/document_validation.py", line 387, in __init__
raw_document[prop] = document.get(prop)
AttributeError: 'NoneType' object has no attribute 'get'

Change-Id: I76fc9b0d7662358f8b26b5bddf1187e92d1554de
This commit is contained in:
Felipe Monteiro 2018-03-30 19:14:33 +01:00
parent 99e3064eda
commit 04ad3fa93b
8 changed files with 83 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
deckhand/control/base.py
View File

@ -12,10 +12,15 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import yaml
import falcon
from oslo_log import log as logging
from deckhand import context
LOG = logging.getLogger(__name__)
class BaseResource(object):
"""Base resource class for implementing API resources."""
@ -42,6 +47,45 @@ class BaseResource(object):
resp.headers['Allow'] = ','.join(allowed_methods)
resp.status = falcon.HTTP_200
def from_yaml(self, req, expect_list=True, allow_empty=False):
"""Reads and converts YAML-formatted request body into a dict or list
of dicts.
:param req: Falcon Request object.
:param expect_list: Whether to expect a list or an object.
:param allow_empty: Whether the request body can be empty.
:returns: List of dicts if ``expect_list`` is True or else a dict.
"""
raw_data = req.stream.read(req.content_length or 0)
if not allow_empty and not raw_data:
error_msg = ("The request body must not be empty.")
LOG.error(error_msg)
raise falcon.HTTPBadRequest(description=error_msg)
try:
if expect_list:
data = list(yaml.safe_load_all(raw_data))
else:
data = yaml.safe_load(raw_data)
except yaml.YAMLError as e:
error_msg = ("The request body must be properly formatted YAML. "
"Details: %s." % e)
LOG.error(error_msg)
raise falcon.HTTPBadRequest(description=error_msg)
if expect_list:
bad_entries = [str(i + 1) for i, x in enumerate(data)
if not x or not isinstance(x, dict)]
if bad_entries:
error_msg = (
"Expected a list of valid objects. Invalid entries "
"found at following indexes: %s." % ','.join(bad_entries))
LOG.error(error_msg)
raise falcon.HTTPBadRequest(description=error_msg)
return data
class DeckhandRequest(falcon.Request):
context_type = context.RequestContext

14
deckhand/control/buckets.py
View File

@ -12,8 +12,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import yaml
import falcon
from oslo_log import log as logging
import six
@ -34,18 +32,10 @@ class BucketsResource(api_base.BaseResource):
"""API resource for realizing CRUD operations for buckets."""
view_builder = document_view.ViewBuilder()
secrets_mgr = secrets_manager.SecretsManager()
@policy.authorize('deckhand:create_cleartext_documents')
def on_put(self, req, resp, bucket_name=None):
document_data = req.stream.read(req.content_length or 0)
try:
documents = list(yaml.safe_load_all(document_data))
except yaml.YAMLError as e:
error_msg = ("Could not parse the document into YAML data. "
"Details: %s." % e)
LOG.error(error_msg)
raise falcon.HTTPBadRequest(description=six.text_type(e))
documents = self.from_yaml(req, expect_list=True, allow_empty=True)
# NOTE: Must validate documents before doing policy enforcement,
# because we expect certain formatting of the documents while doing
@ -86,7 +76,7 @@ class BucketsResource(api_base.BaseResource):
for document in secret_documents:
# TODO(fmontei): Move all of this to document validation directly.
if document['metadata'].get('storagePolicy') == 'encrypted':
secret_data = self.secrets_mgr.create(document)
secret_data = secrets_manager.SecretsManager.create(document)
document['data'] = secret_data
def _create_revision_documents(self, bucket_name, documents,

12
deckhand/control/revision_tags.py
View File

@ -12,8 +12,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import yaml
import falcon
from oslo_log import log as logging
@ -32,15 +30,7 @@ class RevisionTagsResource(api_base.BaseResource):
@policy.authorize('deckhand:create_tag')
def on_post(self, req, resp, revision_id, tag=None):
"""Creates a revision tag."""
body = req.stream.read(req.content_length or 0)
try:
tag_data = yaml.safe_load(body)
except yaml.YAMLError as e:
error_msg = ("Could not parse the request body into YAML data. "
"Details: %s." % e)
LOG.error(error_msg)
raise falcon.HTTPBadRequest(description=e)
tag_data = self.from_yaml(req, expect_list=False, allow_empty=True)
try:
resp_tag = db_api.revision_tag_create(revision_id, tag, tag_data)

18
deckhand/control/validations.py
View File

@ -12,11 +12,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import yaml
import falcon
from oslo_log import log as logging
import six
from deckhand.control import base as api_base
from deckhand.control.views import validation as validation_view
@ -34,19 +31,8 @@ class ValidationsResource(api_base.BaseResource):
@policy.authorize('deckhand:create_validation')
def on_post(self, req, resp, revision_id, validation_name):
validation_data = req.stream.read(req.content_length or 0)
try:
validation_data = yaml.safe_load(validation_data)
except yaml.YAMLError as e:
error_msg = ("Could not parse the validation into YAML data. "
"Details: %s." % e)
LOG.error(error_msg)
raise falcon.HTTPBadRequest(description=six.text_type(e))
if not validation_data:
error_msg = 'Validation payload must be provided.'
LOG.error(error_msg)
raise falcon.HTTPBadRequest(description=error_msg)
validation_data = self.from_yaml(
req, expect_list=False, allow_empty=False)
if not all([validation_data.get(x) for x in ('status', 'validator')]):
error_msg = 'Validation payload must contain keys: %s.' % (

31
deckhand/tests/unit/control/test_buckets_controller.py
View File

@ -17,7 +17,7 @@ import yaml
import mock
from oslo_config import cfg
from deckhand.control import buckets
from deckhand.engine import secrets_manager
from deckhand import factories
from deckhand.tests import test_utils
from deckhand.tests.unit.control import base as test_base
@ -100,7 +100,7 @@ class TestBucketsController(test_base.BaseControllerTest):
secrets_factory = factories.DocumentSecretFactory()
payload = [secrets_factory.gen_test('Certificate', 'encrypted')]
with mock.patch.object(buckets.BucketsResource, 'secrets_mgr',
with mock.patch.object(secrets_manager, 'SecretsManager',
autospec=True) as mock_secrets_mgr:
mock_secrets_mgr.create.return_value = payload[0]['data']
_do_test(payload)
@ -115,7 +115,7 @@ class TestBucketsController(test_base.BaseControllerTest):
data_schema_factory = factories.DataSchemaFactory()
data_schema = data_schema_factory.gen_test(document['schema'], {})
with mock.patch.object(buckets.BucketsResource, 'secrets_mgr',
with mock.patch.object(secrets_manager, 'SecretsManager',
autospec=True) as mock_secrets_mgr:
mock_secrets_mgr.create.return_value = document['data']
_do_test([document, data_schema])
@ -217,6 +217,31 @@ class TestBucketsControllerNegative(test_base.BaseControllerTest):
body=yaml.safe_dump_all([payload]))
self.assertEqual(409, resp.status_code)
def test_bucket_with_empty_document_raises_bad_request(self):
rules = {'deckhand:create_cleartext_documents': '@'}
self.policy.set_rules(rules)
# Verify that 400 is returned for empty entries.
bucket_name = test_utils.rand_name('bucket')
resp = self.app.simulate_put(
'/api/v1.0/buckets/%s/documents' % bucket_name,
headers={'Content-Type': 'application/x-yaml'},
body=yaml.safe_dump_all([None, '', {}]))
self.assertEqual(400, resp.status_code)
self.assertRegex(
resp.text,
r'.*Invalid entries found at following indexes:\n.*1,2,3.')
# Verify that 400 is returned for non-object entries.
resp = self.app.simulate_put(
'/api/v1.0/buckets/%s/documents' % bucket_name,
headers={'Content-Type': 'application/x-yaml'},
body=yaml.safe_dump_all([{'foo': 'bar'}, 'foo', 5]))
self.assertEqual(400, resp.status_code)
self.assertRegex(
resp.text,
r'.*Invalid entries found at following indexes:\n.*2,3.')
class TestBucketsControllerNegativeRBAC(test_base.BaseControllerTest):
"""Test suite for validating negative RBAC scenarios for bucket

4
deckhand/tests/unit/control/test_rendered_documents_controller.py
View File

@ -16,8 +16,8 @@ import yaml
import mock
from deckhand.control import buckets
from deckhand.control import revision_documents
from deckhand.engine import secrets_manager
from deckhand import errors
from deckhand import factories
from deckhand.tests.unit.control import base as test_base
@ -276,7 +276,7 @@ class TestRenderedDocumentsControllerNegativeRBAC(
'encrypted')
payload = [layering_policy, encrypted_document]
with mock.patch.object(buckets.BucketsResource, 'secrets_mgr',
with mock.patch.object(secrets_manager, 'SecretsManager',
autospec=True) as mock_secrets_mgr:
mock_secrets_mgr.create.return_value = payload[0]['data']
resp = self.app.simulate_put(

4
deckhand/tests/unit/control/test_revision_documents_controller.py
View File

@ -16,7 +16,7 @@ import yaml
import mock
from deckhand.control import buckets
from deckhand.engine import secrets_manager
from deckhand import factories
from deckhand.tests.unit.control import base as test_base
@ -62,7 +62,7 @@ class TestRevisionDocumentsControllerNegativeRBAC(
# Create a document for a bucket.
secrets_factory = factories.DocumentSecretFactory()
payload = [secrets_factory.gen_test('Certificate', 'encrypted')]
with mock.patch.object(buckets.BucketsResource, 'secrets_mgr',
with mock.patch.object(secrets_manager, 'SecretsManager',
autospec=True) as mock_secrets_mgr:
mock_secrets_mgr.create.return_value = payload[0]['data']
resp = self.app.simulate_put(

4
deckhand/tests/unit/control/test_revisions_rollback_controller.py
View File

@ -16,7 +16,7 @@ import yaml
import mock
from deckhand.control import buckets
from deckhand.engine import secrets_manager
from deckhand import factories
from deckhand.tests.unit.control import base as test_base
@ -59,7 +59,7 @@ class TestRevisionsRollbackControllerNegativeRBAC(
secrets_factory = factories.DocumentSecretFactory()
payload = [secrets_factory.gen_test('Certificate', 'encrypted')]
with mock.patch.object(buckets.BucketsResource, 'secrets_mgr',
with mock.patch.object(secrets_manager, 'SecretsManager',
autospec=True) as mock_secrets_mgr:
mock_secrets_mgr.create.return_value = payload[0]['data']
resp = self.app.simulate_put(