openstack/barbican
Code Issues Proposed changes

Readability-related changes to secret store functions

Browse Source 
This CR introduces minor changes with the aim to improve and clean the
code a little bit. So unnecessary comments were removed and some
functions renamed to reflect more context.

The need of the spec in the secret_store function in plugin.resources
was removed in favor of always getting a valid secret model.

On the other hand, there was an unnecessary check in the store_secret
function in the resources file. It was never the case (and there
shouldn't be a case) where the "spec" is null. So that check was
erased.

Change-Id: I73f59bdcc59ea213402f08862b87b48db8146ca0
This commit is contained in:
Juan Antonio Osorio Robles
2015-04-10 14:13:48 +03:00
parent 66fc6d7a1c
commit 3563aef382
6 changed files with 95 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
barbican/api/controllers/secrets.py
View File

@@ -23,6 +23,7 @@ from barbican.common import resources as res
from barbican.common import utils
from barbican.common import validators
from barbican import i18n as u
from barbican.model import models
from barbican.model import repositories as repo
from barbican.plugin import resources as plugin
from barbican.plugin import util as putil
@@ -175,7 +176,6 @@ class SecretController(controllers.ACLMixin):
@controllers.enforce_content_types(['application/octet-stream',
'text/plain'])
def on_put(self, external_project_id, **kwargs):
if (not pecan.request.content_type or
pecan.request.content_type == 'application/json'):
pecan.abort(
@@ -199,10 +199,13 @@ class SecretController(controllers.ACLMixin):
content_type = pecan.request.content_type
content_encoding = pecan.request.headers.get('Content-Encoding')
plugin.store_secret(payload, content_type,
content_encoding, self.secret.to_dict_fields(),
self.secret, project_model,
transport_key_id=transport_key_id)
plugin.store_secret(
unencrypted_raw=payload,
content_type_raw=content_type,
content_encoding=content_encoding,
secret_model=self.secret,
project_model=project_model,
transport_key_id=transport_key_id)
LOG.info(u._LI('Updated secret for project: %s'), external_project_id)
@index.when(method='DELETE')
@@ -310,12 +313,15 @@ class SecretsController(controllers.ACLMixin):
if ctxt: # in authenticated pipleline case, always use auth token user
data['creator_id'] = ctxt.user
secret_model = models.Secret(data)
new_secret, transport_key_model = plugin.store_secret(
data.get('payload'),
data.get('payload_content_type',
'application/octet-stream'),
data.get('payload_content_encoding'),
data, None, project,
unencrypted_raw=data.get('payload'),
content_type_raw=data.get('payload_content_type',
'application/octet-stream'),
content_encoding=data.get('payload_content_encoding'),
secret_model=secret_model,
project_model=project,
transport_key_needed=transport_key_needed,
transport_key_id=data.get('transport_key_id'))

114
barbican/plugin/resources.py
View File

@@ -62,26 +62,17 @@ def _get_plugin_name_and_transport_key(transport_key_id):
def store_secret(unencrypted_raw, content_type_raw, content_encoding,
spec, secret_model, project_model,
secret_model, project_model,
transport_key_needed=False,
transport_key_id=None):
"""Store a provided secret into secure backend."""
# Create a secret model is one isn't provided.
# Note: For one-step secret stores, the model is not provided. For
# two-step secrets, the secret entity is already created and should then
# be passed into this function.
if not secret_model:
secret_model = models.Secret(spec)
elif _secret_already_has_stored_data(secret_model):
if _secret_already_has_stored_data(secret_model):
raise ValueError('Secret already has encrypted data stored for it.')
# Create a KeySpec to find a plugin that will support storing the secret
key_spec = None
if spec:
key_spec = secret_store.KeySpec(alg=spec.get('algorithm'),
bit_length=spec.get('bit_length'),
mode=spec.get('mode'))
key_spec = secret_store.KeySpec(alg=secret_model.algorithm,
bit_length=secret_model.bit_length,
mode=secret_model.mode)
# If there is no secret data to store, then just create Secret entity and
# leave. A subsequent call to this method should provide both the Secret
@@ -89,35 +80,31 @@ def store_secret(unencrypted_raw, content_type_raw, content_encoding,
if not unencrypted_raw:
key_model = _get_transport_key_model(key_spec, transport_key_needed)
_save_secret(secret_model, project_model)
_save_secret_in_repo(secret_model, project_model)
return secret_model, key_model
plugin_name, transport_key = _get_plugin_name_and_transport_key(
transport_key_id)
# Locate a suitable plugin to store the secret.
plugin_manager = secret_store.get_manager()
store_plugin = plugin_manager.get_plugin_store(
key_spec=key_spec, plugin_name=plugin_name)
# Normalize inputs prior to storage.
unencrypted, content_type = tr.normalize_before_encryption(
unencrypted_raw, content_type_raw, content_encoding,
secret_model.secret_type, enforce_text_only=True)
# Store the secret securely.
plugin_manager = secret_store.get_manager()
store_plugin = plugin_manager.get_plugin_store(key_spec=key_spec,
plugin_name=plugin_name)
secret_dto = secret_store.SecretDTO(type=secret_model.secret_type,
secret=unencrypted,
key_spec=key_spec,
content_type=content_type,
transport_key=transport_key)
secret_metadata = _store_secret(
store_plugin, secret_dto, secret_model, project_model)
# Save secret and metadata.
_save_secret(secret_model, project_model)
_save_secret_metadata(secret_model, secret_metadata, store_plugin,
content_type)
secret_metadata = _store_secret_using_plugin(store_plugin, secret_dto,
secret_model, project_model)
_save_secret_in_repo(secret_model, project_model)
_save_secret_metadata_in_repo(secret_model, secret_metadata, store_plugin,
content_type)
return secret_model, None
@@ -183,9 +170,9 @@ def generate_secret(spec, content_type, project_model):
generate_plugin, key_spec, secret_model, project_model, content_type)
# Save secret and metadata.
_save_secret(secret_model, project_model)
_save_secret_metadata(secret_model, secret_metadata, generate_plugin,
content_type)
_save_secret_in_repo(secret_model, project_model)
_save_secret_metadata_in_repo(secret_model, secret_metadata,
generate_plugin, content_type)
return secret_model
@@ -211,7 +198,6 @@ def generate_asymmetric_secret(spec, content_type, project_model):
passphrase_type = secret_store.SecretType.PASSPHRASE
passphrase_secret_model['secret_type'] = passphrase_type
# Generate the secret.
asymmetric_meta_dto = _generate_asymmetric_key(
generate_plugin,
key_spec,
@@ -222,31 +208,30 @@ def generate_asymmetric_secret(spec, content_type, project_model):
content_type
)
# Save secret and metadata.
_save_secret(private_secret_model, project_model)
_save_secret_metadata(private_secret_model,
asymmetric_meta_dto.private_key_meta,
generate_plugin,
content_type)
_save_secret_in_repo(private_secret_model, project_model)
_save_secret_metadata_in_repo(private_secret_model,
asymmetric_meta_dto.private_key_meta,
generate_plugin,
content_type)
_save_secret(public_secret_model, project_model)
_save_secret_metadata(public_secret_model,
asymmetric_meta_dto.public_key_meta,
generate_plugin,
content_type)
_save_secret_in_repo(public_secret_model, project_model)
_save_secret_metadata_in_repo(public_secret_model,
asymmetric_meta_dto.public_key_meta,
generate_plugin,
content_type)
if spec.get('passphrase'):
_save_secret(passphrase_secret_model, project_model)
_save_secret_metadata(passphrase_secret_model,
asymmetric_meta_dto.passphrase_meta,
generate_plugin,
content_type)
if passphrase_secret_model:
_save_secret_in_repo(passphrase_secret_model, project_model)
_save_secret_metadata_in_repo(passphrase_secret_model,
asymmetric_meta_dto.passphrase_meta,
generate_plugin,
content_type)
# Now create container
container_model = _save_container(spec, project_model,
private_secret_model,
public_secret_model,
passphrase_secret_model)
container_model = _create_container_for_asymmetric_secret(spec,
project_model)
_save_asymmetric_secret_in_repo(
container_model, private_secret_model, public_secret_model,
passphrase_secret_model)
return container_model
@@ -273,7 +258,8 @@ def delete_secret(secret_model, project_id):
external_project_id=project_id)
def _store_secret(store_plugin, secret_dto, secret_model, project_model):
def _store_secret_using_plugin(store_plugin, secret_dto, secret_model,
project_model):
if isinstance(store_plugin, store_crypto.StoreCryptoAdapterPlugin):
context = store_crypto.StoreCryptoContext(
project_model,
@@ -337,22 +323,21 @@ def _get_secret_meta(secret_model):
return {}
def _save_secret_metadata(secret_model, secret_metadata,
store_plugin, content_type):
def _save_secret_metadata_in_repo(secret_model, secret_metadata,
store_plugin, content_type):
"""Add secret metadata to a secret."""
if not secret_metadata:
secret_metadata = {}
secret_metadata['plugin_name'] = utils.generate_fullname_for(store_plugin)
secret_metadata['content_type'] = content_type
secret_meta_repo = repos.get_secret_meta_repository()
secret_meta_repo.save(secret_metadata, secret_model)
def _save_secret(secret_model, project_model):
def _save_secret_in_repo(secret_model, project_model):
"""Save a Secret entity."""
secret_repo = repos.get_secret_repository()
@@ -377,15 +362,19 @@ def _secret_already_has_stored_data(secret_model):
return secret_model.encrypted_data or secret_model.secret_store_metadata
def _save_container(spec, project_model, private_secret_model,
public_secret_model, passphrase_secret_model):
def _create_container_for_asymmetric_secret(spec, project_model):
container_model = models.Container()
container_model.name = spec.get('name')
container_model.type = spec.get('algorithm', '').lower()
container_model.status = models.States.ACTIVE
container_model.project_id = project_model.id
container_model.creator_id = spec.get('creator_id')
return container_model
def _save_asymmetric_secret_in_repo(container_model, private_secret_model,
public_secret_model,
passphrase_secret_model):
container_repo = repos.get_container_repository()
container_repo.create_from(container_model)
@@ -399,12 +388,11 @@ def _save_container(spec, project_model, private_secret_model,
public_secret_model,
container_model)
if spec.get('passphrase'):
if passphrase_secret_model:
# create container_secret for passphrase
_create_container_secret_association('private_key_passphrase',
passphrase_secret_model,
container_model)
return container_model
def _create_container_secret_association(assoc_name, secret_model,

6
barbican/tasks/certificate_resources.py
View File

@@ -387,8 +387,7 @@ def _save_secrets(result, project_model):
unencrypted_raw=result.certificate,
content_type_raw='application/pkix-cert',
content_encoding='base64',
spec={},
secret_model=None,
secret_model=models.Secret(),
project_model=project_model)
# save the certificate chain as a secret.
@@ -397,8 +396,7 @@ def _save_secrets(result, project_model):
unencrypted_raw=result.intermediates,
content_type_raw='application/pkix-cert',
content_encoding='base64',
spec={},
secret_model=None,
secret_model=models.Secret(),
project_model=project_model
)
else:

11
barbican/tests/api/controllers/test_secrets.py
View File

@@ -148,12 +148,11 @@ class WhenTestingSecretsResource(utils.BarbicanAPIBaseTestCase):
self.assertEqual(201, resp.status_int)
# We're interested in the transport key values
mocked_store.assert_called_once_with(
'not-encrypted',
'text/plain',
None,
mock.ANY,
None,
mock.ANY,
unencrypted_raw='not-encrypted',
content_type_raw='text/plain',
content_encoding=None,
secret_model=mock.ANY,
project_model=mock.ANY,
transport_key_id=transport_key_id,
transport_key_needed=False
)

21
barbican/tests/api/test_resources.py
View File

@@ -617,11 +617,11 @@ class WhenGettingPuttingOrDeletingSecretUsingSecretResource(FunctionalTest):
self.assertEqual(resp.status_int, 204)
mock_store_secret.assert_called_once_with(
'plain text',
'text/plain', None,
self.secret.to_dict_fields(),
self.secret,
self.project,
unencrypted_raw='plain text',
content_type_raw='text/plain',
content_encoding=None,
secret_model=self.secret,
project_model=self.project,
transport_key_id=self.transport_key_id
)
@@ -643,12 +643,11 @@ class WhenGettingPuttingOrDeletingSecretUsingSecretResource(FunctionalTest):
self.assertEqual(resp.status_int, 204)
mock_store_secret.assert_called_once_with(
'plain text',
'application/octet-stream',
None,
self.secret.to_dict_fields(),
self.secret,
self.project,
unencrypted_raw='plain text',
content_type_raw='application/octet-stream',
content_encoding=None,
secret_model=self.secret,
project_model=self.project,
transport_key_id=self.transport_key_id
)

23
barbican/tests/plugin/test_resource.py
View File

@@ -92,12 +92,11 @@ class WhenTestingPluginResource(testtools.TestCase,
secret = base64.b64encode('ABCDEFABCDEFABCDEFABCDEF')
self.plugin_resource.store_secret(
secret,
self.content_type,
'base64',
spec,
None,
self.project_model)
unencrypted_raw=secret,
content_type_raw=self.content_type,
content_encoding='base64',
secret_model=models.Secret(spec),
project_model=self.project_model)
dto = self.moc_plugin.store_secret.call_args_list[0][0][0]
self.assertEqual("symmetric", dto.type)
@@ -155,12 +154,12 @@ class WhenTestingPluginResource(testtools.TestCase,
'secret_type': 'symmetric'}
self.plugin_resource.store_secret(
base64.b64encode(raw_secret),
self.content_type,
'base64',
spec,
None,
self.project_model)
unencrypted_raw=base64.b64encode(raw_secret),
content_type_raw=self.content_type,
content_encoding='base64',
secret_model=models.Secret(spec),
project_model=self.project_model)
secret = self.plugin_resource.get_secret(
'application/octet-stream',
models.Secret(spec),