[Freeipa-devel] [PATCH] Password vault

Endi Sukma Dewata edewata at redhat.com
Wed Mar 11 14:12:45 UTC 2015


Thanks for the review. New patch attached to be applied on top of all 
previous patches. Please see comments below.

On 3/6/2015 3:53 PM, Jan Cholasta wrote:
> Patch 353:
>
> 1) Please follow PEP8 in new code.
>
> The pep8 tool reports these errors in existing files:
>
> ./ipalib/constants.py:98:80: E501 line too long (84 > 79 characters)
> ./ipalib/plugins/baseldap.py:1527:80: E501 line too long (81 > 79
> characters)
> ./ipalib/plugins/user.py:915:80: E501 line too long (80 > 79 characters)
>
> as well as many errors in the files this patch adds.

For some reason pylint keeps crashing during build so I cannot run it 
for all files. I'm fixing the errors that I can see. If you see other 
errors please let me know while I'm still trying to figure out the problem.

Is there an existing ticket for fixing PEP8 errors? Let's use that for 
fixing the errors in the existing code.

> 2) Pylint reports the following error:
>
> ipatests/test_xmlrpc/test_vault_plugin.py:153:
> [E0102(function-redefined), test_vault] class already defined line 27)

Fixed.

> 3) The container_vault config option should be renamed to
> container_vaultcontainer, as it is used in the vaultcontainer plugin,
> not the vault plugin.

It was named container_vault because it defines the DN for of the 
subtree that contains all vault-related entries. I moved the base_dn 
variable from vaultcontainer object to the vault object for clarity.

> 4) The vault object should be child of the vaultcontainer object.
>
> Not only is this correct from the object model perspective, but it would
> also make all the container_id hacks go away.

It's a bit difficult because it will affect how the container & vault 
ID's are represented on the CLI.

In the design the container ID would be a single value like this:

   $ ipa vault-add /services/server.example.com/HTTP

And if the vault ID is relative (without initial slash), it will be 
appended to the user's private container (i.e. /users/<username>/):

   $ ipa vault-add PrivateVault

The implementation is not complete yet. Currently it accepts this format:

   $ ipa vault-add <vault name> [--container <container ID>]

and I'm still planning to add this:

   $ ipa vault-add <vault ID>

If the vault must be a child of vaultcontainer, and the vaultcontainer 
must be a child of a vaultcontainer, does it mean the vault ID would 
have to be split into separate arguments like this?

   $ ipa vaultcontainer-add services server.example.com HTTP

If that's the case we'd lose the ability to specify a relative vault ID.

> 5) When specifying param flags, use set literals.
>
> This is especially wrong, because it's not a tuple, but a string in
> parentheses:
>
> +            flags=('virtual_attribute'),

Fixed.

> 6) The `container` param of vault should actually be an option in
> vault_* commands.
>
> Also it should be renamed to `container_id`, for consistency with
> vaultcontainer.

Fixed. It was actually made to be consistent with the 'parent' attribute 
in the vaultcontainer class. Now the 'parent' has been renamed to 
'parent_id' as well.

> 7) The `vault_id` param of vault should have "no_option" in flags, since
> it is output-only.

Fixed.

> 8) Don't translate docstrings where not necessary:
>
> +    def get_dn(self, *keys, **options):
> +        __doc__ = _("""
> +        Generates vault DN from vault ID.
> +        """)
>
> Only plugin modules and classes should have translated docstrings.

Fixed.

> 9) This looks wrong in vault.get_dn() and vaultcontainer.get_dn():
>
> +        name = None
> +        if keys:
> +            name = keys[0]
>
> Primary key of the object should always be set, so the if statement
> should not be there.

Fixed.

> Also, primary key of any given object is always last in "keys", so use
> keys[-1] instead of keys[0].

Fixed.

> 10) Use "self.api" instead of "api" to access the API in plugins.

Fixed.

> 11) No clever optimizations like this please:
>
> +        # vault DN cannot be the container base DN
> +        if len(dn) == len(api.Object.vaultcontainer.base_dn):
> +            raise ValueError('Invalid vault DN: %s' % dn)
>
> Compare the DNs by value instead.

Actually the DN values have already been compared in the code right 
above it:

     # make sure the DN is a vault DN
     if not dn.endswith(self.api.Object.vaultcontainer.base_dn):
         raise ValueError('Invalid vault DN: %s' % dn)

This code confirms that the incoming vault DN is within the vault 
subtree. After that, the DN length comparison above is just to make sure 
the incoming vault DN is not the root of the vault subtree itself. It 
doesn't need to compare the values again.

> 12) vault.split_id() is not used anywhere.

Removed.

> 13) Bytes is not base64-encoded data:
>
> +        Bytes('data?',
> +            cli_name='data',
> +            doc=_('Base-64 encoded binary data to archive'),
> +        ),
>
> It is base64-encoded in the CLI, but on the API level it is not. The doc
> should say just "Binary data to archive".

Fixed.

> 14) Use File instead of Str for input files:
>
> +        Str('in?',
> +            cli_name='in',
> +            doc=_('File containing data to archive'),
> +        ),

The File type doesn't work with binary files because it tries to decode 
the content.

> 15) Use MutuallyExclusiveError instead of ValidationError when there are
> mutually exclusive options specified.

Fixed.

> 16) You do way too much stuff in vault_add.forward(). Only code that
> must be done on the client needs to be there, i.e. handling of the
> "data", "text" and "in" options.
>
> The vault_archive call must be in vault_add.execute(), otherwise a) we
> will be making 2 RPC calls from the client and b) it won't be called at
> all when api.env.in_server is True.

This is done by design. The vault_add.forward() generates the salt and 
the keys. The vault_archive.forward() will encrypt the data. These 
operations have to be done on the client side to secure the transport of 
the data from the client through the server and finally to KRA. This 
mechanism prevents the server from looking at the unencrypted data.

The add & archive combination was added for convenience, not for 
optimization. This way you would be able to archive data into a new 
vault using a single command. Without this, you'd have to execute two 
separate commands: add & archive, which will result in 2 RPC calls anyway.

> 17) Why are vaultcontainer objects automatically created in vault_add?
>
> If you have to automatically create them, you also have to automatically
> delete them when the command fails. But that's a hassle, so I would just
> not create them automatically.

The vaultcontainer is created automatically to provide a private 
container (i.e. /users/<username>/) for the each user if they need it. 
Without this, the admin will have to create the container manually first 
before a user can create a vault, which would be an unreasonable 
requirement. If the vault_add fails, it's ok to leave the private 
container intact because it can be used again if the user tries to 
create a vault again later and it will not affect other users. If the 
user is deleted, the private container will be deleted too.

The code was fixed to create the container only if they are adding a 
vault/vault container into the user's private container. If they are 
adding into other container, the container must already exist.

> 18) Why are vaultcontainer objects automatically created in vault_find?
>
> This is just plain wrong and has to be removed, now.

The code was supposed to create the user's private container like in 
#17, but the behavior has been changed. If the container being searched 
is the user's private container, it will ignore the container not found 
error and return zero results as if the private container already 
exists. For other containers the container must already exist. For this 
to work I had to add a handle_not_found() into LDAPSearch so the plugins 
can customize the proper search response for the missing private container.

> 19) What is the reason behind all the json stuff in vault_transport_cert?
>
> vault_transport_cert.__json__() is exactly the same as
> Command.__json__() and hence redundant.

Removed. It should've been cleaned up.

> 20) Are vault_transport_cert, vault_archive and vault_retrieve supposed
> to be runnable by users? If not, add "NO_CLI = True" to the class
> definition.

Yes. These commands are available if the users want to retrieve the 
transport cert for other purposes, or archive/retrieve data to/from the 
vault as a whole instead of individual secrets.

> 21) vault_archive is not a retrieve operation, it should be based on
> LDAPUpdate instead of LDAPRetrieve. Or Command actually, since it does
> not do anything with LDAP. The same applies to vault_retrieve.

The vault_archive does not actually modify the LDAP entry because it 
stores the data in KRA. It is actually an LDAPRetrieve operation because 
it needs to get the vault info before it can perform the archival 
operation. Same thing with vault_retrieve.

> 22) vault_archive will break with binary data that is not UTF-8 encoded
> text.
>
> This is where it occurs:
>
> +        vault_data[u'data'] = unicode(data)
>
> Generally, don't use unicode() on str values and str() on unicode values
> directly, always use .decode() and .encode().

It needs to be a Unicode because json.dumps() doesn't work with binary 
data. Fixed by adding base-64 encoding.

> 23) Since vault containers are nested, the vaultcontainer object should
> be child of itself.
>
> There is no support for nested objects in the framework, but it
> shouldn't be too hard to do anyway.

See #4.

> 24) Instead of:
>
> +        while len(dn) > len(self.base_dn):
> +
> +            rdn = dn[0]
> ...
> +            dn = DN(*dn[1:])
>
> you can do:
>
> +        for rdn in dn[:-len(self.base_dn)]:
> ...

Fixed.

> 25) Why are parent vaultcontainer objects automatically created in
> vaultcontainer_add?

See #17.

> 26) Instead of the delete_entry refactoring in baseldap and
> vaultcontainer_add, you can put this in vaultcontainer_add's pre_callback:
>
>      try:
>          ldap.get_entries(dn, scope=ldap.SCOPE_ONELEVEL, attrs_list=[])
>      except errors.NotFound:
>          pass
>      else:
>          if not options.get('force', False):
>              raise errors.NotAllowedOnNonLeaf()

I suppose you meant vaultcontainer_del. Fixed, but this will generate an 
additional search for each delete.

I'm leaving the changes baseldap because it may be useful later and it 
doesn't change the behavior of the current code.

> 27) Why are parent vaultcontainer objects automatically created in
> vaultcontainer_find?

See #18.

> 28) The vault and vaultcontainer plugins seem to be pretty similar, I
> think it would make sense to put common stuff in a base class and
> inherit vault and vaultcontainer from that.

I plan to refactor the common code later. Right now the focus is to get 
the functionality working correctly first.

> More later.
>
> Honza
>


-- 
Endi S. Dewata
-------------- next part --------------
>From 6c3d2ea06089a9e91741d57c412f6efbaee94a83 Mon Sep 17 00:00:00 2001
From: "Endi S. Dewata" <edewata at redhat.com>
Date: Mon, 9 Mar 2015 12:09:20 -0400
Subject: [PATCH] Vault improvements.

The vault plugins have been modified to clean up the code, to fix
some issues, and to improve error handling. The LDAPCreate and
LDAPSearch classes have been refactored to allow subclasses to
provide custom error handling. The test scripts have been updated
accordingly.

https://fedorahosted.org/freeipa/ticket/3872
---
 API.txt                                            |  50 ++--
 ipalib/plugins/baseldap.py                         |  35 +--
 ipalib/plugins/user.py                             |   6 +-
 ipalib/plugins/vault.py                            | 269 ++++++++++-----------
 ipalib/plugins/vaultcontainer.py                   | 224 +++++++++--------
 ipalib/plugins/vaultsecret.py                      | 108 ++++-----
 ipatests/test_xmlrpc/test_vault_plugin.py          |   2 +-
 ipatests/test_xmlrpc/test_vaultcontainer_plugin.py |  24 +-
 ipatests/test_xmlrpc/test_vaultsecret_plugin.py    |   2 +-
 9 files changed, 365 insertions(+), 355 deletions(-)

diff --git a/API.txt b/API.txt
index ffbffa78cde372d5c7027b758be58bf07caebbc6..3a741755ab3e15e0175599a16a090b04d46d6be8 100644
--- a/API.txt
+++ b/API.txt
@@ -4518,7 +4518,7 @@ args: 1,20,3
 arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
-option: Str('container', attribute=False, cli_name='container', multivalue=False, pattern='^[a-zA-Z0-9_.-/]+$', required=False)
+option: Str('container_id?', cli_name='container_id')
 option: Bytes('data?', cli_name='data')
 option: Str('description', attribute=True, cli_name='desc', multivalue=False, required=False)
 option: Str('escrow_public_key_file?', cli_name='escrow_public_key_file')
@@ -4543,7 +4543,7 @@ command: vault_add_member
 args: 1,7,3
 arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
@@ -4556,7 +4556,7 @@ command: vault_add_owner
 args: 1,7,3
 arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
@@ -4569,7 +4569,7 @@ command: vault_archive
 args: 1,15,3
 arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Bytes('data?', cli_name='data')
 option: Bytes('encryption_key?', cli_name='encryption_key')
 option: Str('in?', cli_name='in')
@@ -4589,7 +4589,7 @@ output: PrimaryKey('value', None, None)
 command: vault_del
 args: 1,3,3
 arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, multivalue=True, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Flag('continue', autofill=True, cli_name='continue', default=False)
 option: Str('version?', exclude='webui')
 output: Output('result', <type 'dict'>, None)
@@ -4600,7 +4600,7 @@ args: 1,15,4
 arg: Str('criteria?', noextrawhitespace=False)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Str('cn', attribute=True, autofill=False, cli_name='vault_name', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=False)
-option: Str('container', attribute=False, autofill=False, cli_name='container', multivalue=False, pattern='^[a-zA-Z0-9_.-/]+$', query=True, required=False)
+option: Str('container_id?', cli_name='container_id')
 option: Str('description', attribute=True, autofill=False, cli_name='desc', multivalue=False, query=True, required=False)
 option: Bytes('ipaescrowpublickey', attribute=True, autofill=False, cli_name='escrow_public_key', multivalue=False, query=True, required=False)
 option: Bytes('ipapublickey', attribute=True, autofill=False, cli_name='public_key', multivalue=False, query=True, required=False)
@@ -4622,7 +4622,7 @@ args: 1,15,3
 arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
-option: Str('container', attribute=False, autofill=False, cli_name='container', multivalue=False, pattern='^[a-zA-Z0-9_.-/]+$', required=False)
+option: Str('container_id?', cli_name='container_id')
 option: Str('delattr*', cli_name='delattr', exclude='webui')
 option: Str('description', attribute=True, autofill=False, cli_name='desc', multivalue=False, required=False)
 option: Bytes('ipaescrowpublickey', attribute=True, autofill=False, cli_name='escrow_public_key', multivalue=False, required=False)
@@ -4642,7 +4642,7 @@ command: vault_remove_member
 args: 1,7,3
 arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
@@ -4655,7 +4655,7 @@ command: vault_remove_owner
 args: 1,7,3
 arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
@@ -4668,7 +4668,7 @@ command: vault_retrieve
 args: 1,16,3
 arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Bytes('escrow_private_key?', cli_name='escrow_private_key')
 option: Str('escrow_private_key_file?', cli_name='escrow_private_key_file')
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
@@ -4690,7 +4690,7 @@ command: vault_show
 args: 1,6,3
 arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Flag('rights', autofill=True, default=False)
@@ -4711,7 +4711,7 @@ option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui
 option: Str('container_id', attribute=False, cli_name='container_id', multivalue=False, required=False)
 option: Str('description', attribute=True, cli_name='desc', multivalue=False, required=False)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
-option: Str('parent', attribute=False, cli_name='parent', multivalue=False, pattern='^[a-zA-Z0-9_.-/]+$', required=False)
+option: Str('parent_id?', cli_name='parent_id')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Str('setattr*', cli_name='setattr', exclude='webui')
 option: Str('version?', exclude='webui')
@@ -4724,7 +4724,7 @@ arg: Str('cn', attribute=True, cli_name='container_name', maxlength=255, multiva
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
-option: Str('parent?', cli_name='parent')
+option: Str('parent_id?', cli_name='parent_id')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Str('user*', alwaysask=True, cli_name='users', csv=True)
 option: Str('version?', exclude='webui')
@@ -4737,7 +4737,7 @@ arg: Str('cn', attribute=True, cli_name='container_name', maxlength=255, multiva
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
-option: Str('parent?', cli_name='parent')
+option: Str('parent_id?', cli_name='parent_id')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Str('user*', alwaysask=True, cli_name='users', csv=True)
 option: Str('version?', exclude='webui')
@@ -4749,7 +4749,7 @@ args: 1,4,3
 arg: Str('cn', attribute=True, cli_name='container_name', maxlength=255, multivalue=True, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 option: Flag('continue', autofill=True, cli_name='continue', default=False)
 option: Flag('force?', autofill=True, default=False)
-option: Str('parent?', cli_name='parent')
+option: Str('parent_id?', cli_name='parent_id')
 option: Str('version?', exclude='webui')
 output: Output('result', <type 'dict'>, None)
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
@@ -4762,7 +4762,7 @@ option: Str('cn', attribute=True, autofill=False, cli_name='container_name', max
 option: Str('container_id', attribute=False, autofill=False, cli_name='container_id', multivalue=False, query=True, required=False)
 option: Str('description', attribute=True, autofill=False, cli_name='desc', multivalue=False, query=True, required=False)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
-option: Str('parent', attribute=False, autofill=False, cli_name='parent', multivalue=False, pattern='^[a-zA-Z0-9_.-/]+$', query=True, required=False)
+option: Str('parent_id?', cli_name='parent_id')
 option: Flag('pkey_only?', autofill=True, default=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Int('sizelimit?', autofill=False, minvalue=0)
@@ -4781,7 +4781,7 @@ option: Str('container_id', attribute=False, autofill=False, cli_name='container
 option: Str('delattr*', cli_name='delattr', exclude='webui')
 option: Str('description', attribute=True, autofill=False, cli_name='desc', multivalue=False, required=False)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
-option: Str('parent', attribute=False, autofill=False, cli_name='parent', multivalue=False, pattern='^[a-zA-Z0-9_.-/]+$', required=False)
+option: Str('parent_id?', cli_name='parent_id')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Flag('rights', autofill=True, default=False)
 option: Str('setattr*', cli_name='setattr', exclude='webui')
@@ -4795,7 +4795,7 @@ arg: Str('cn', attribute=True, cli_name='container_name', maxlength=255, multiva
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
-option: Str('parent?', cli_name='parent')
+option: Str('parent_id?', cli_name='parent_id')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Str('user*', alwaysask=True, cli_name='users', csv=True)
 option: Str('version?', exclude='webui')
@@ -4808,7 +4808,7 @@ arg: Str('cn', attribute=True, cli_name='container_name', maxlength=255, multiva
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
-option: Str('parent?', cli_name='parent')
+option: Str('parent_id?', cli_name='parent_id')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Str('user*', alwaysask=True, cli_name='users', csv=True)
 option: Str('version?', exclude='webui')
@@ -4820,7 +4820,7 @@ args: 1,6,3
 arg: Str('cn', attribute=True, cli_name='container_name', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
-option: Str('parent?', cli_name='parent')
+option: Str('parent_id?', cli_name='parent_id')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Flag('rights', autofill=True, default=False)
 option: Str('version?', exclude='webui')
@@ -4832,7 +4832,7 @@ args: 2,12,3
 arg: Str('vaultcn', cli_name='vault', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 arg: Str('secret_name', attribute=True, cli_name='secret', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Bytes('data?', cli_name='data')
 option: Str('description?', cli_name='desc')
 option: Str('in?', cli_name='in')
@@ -4851,7 +4851,7 @@ args: 2,8,3
 arg: Str('vaultcn', cli_name='vault', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 arg: Str('secret_name', attribute=True, cli_name='secret', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Str('password?', cli_name='password')
 option: Str('password_file?', cli_name='password_file')
 option: Bytes('private_key?', cli_name='private_key')
@@ -4866,7 +4866,7 @@ args: 2,12,4
 arg: Str('vaultcn', cli_name='vault', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 arg: Str('criteria?', noextrawhitespace=False)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Bytes('data', attribute=True, autofill=False, cli_name='data', multivalue=False, query=True, required=False)
 option: Str('description', attribute=True, autofill=False, cli_name='desc', multivalue=False, query=True, required=False)
 option: Str('password?', cli_name='password')
@@ -4886,7 +4886,7 @@ args: 2,12,3
 arg: Str('vaultcn', cli_name='vault', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 arg: Str('secret_name', attribute=True, cli_name='secret', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Bytes('data?', cli_name='data')
 option: Str('description?', cli_name='desc')
 option: Str('in?', cli_name='in')
@@ -4905,7 +4905,7 @@ args: 2,11,3
 arg: Str('vaultcn', cli_name='vault', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 arg: Str('secret_name', attribute=True, cli_name='secret', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Str('out?', cli_name='out')
 option: Str('password?', cli_name='password')
 option: Str('password_file?', cli_name='password_file')
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index d693709ac1ba7ddb3c559199c199039b6f8bd9ac..fceaf95f42bef5fa71cbedeb291bd68d2919bc5a 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -1152,19 +1152,7 @@ class LDAPCreate(BaseLDAPCommand, crud.Create):
         try:
             self._exc_wrapper(keys, options, ldap.add_entry)(entry_attrs)
         except errors.NotFound:
-            parent = self.obj.parent_object
-            if parent:
-                raise errors.NotFound(
-                    reason=self.obj.parent_not_found_msg % {
-                        'parent': keys[-2],
-                        'oname': self.api.Object[parent].object_name,
-                    }
-                )
-            raise errors.NotFound(
-                reason=self.obj.container_not_found_msg % {
-                    'container': self.obj.container_dn,
-                }
-            )
+            self.handle_not_found(*keys, **options)
         except errors.DuplicateEntry:
             self.obj.handle_duplicate_entry(*keys)
 
@@ -1213,6 +1201,21 @@ class LDAPCreate(BaseLDAPCommand, crud.Create):
     def exc_callback(self, keys, options, exc, call_func, *call_args, **call_kwargs):
         raise exc
 
+    def handle_not_found(self, *args, **options):
+        parent = self.obj.parent_object
+        if parent:
+            raise errors.NotFound(
+                reason=self.obj.parent_not_found_msg % {
+                    'parent': args[-2],
+                    'oname': self.api.Object[parent].object_name,
+                }
+            )
+        raise errors.NotFound(
+            reason=self.obj.container_not_found_msg % {
+                'container': self.obj.container_dn,
+            }
+        )
+
     def interactive_prompt_callback(self, kw):
         return
 
@@ -2000,7 +2003,8 @@ class LDAPSearch(BaseLDAPCommand, crud.Search):
         except errors.EmptyResult:
             (entries, truncated) = ([], False)
         except errors.NotFound:
-            self.api.Object[self.obj.parent_object].handle_not_found(*args[:-1])
+            self.handle_not_found(*args, **options)
+            (entries, truncated) = ([], False)
 
         for callback in self.get_callbacks('post'):
             truncated = callback(self, ldap, entries, truncated, *args, **options)
@@ -2026,6 +2030,9 @@ class LDAPSearch(BaseLDAPCommand, crud.Search):
             truncated=truncated,
         )
 
+    def handle_not_found(self, *args, **options):
+        self.api.Object[self.obj.parent_object].handle_not_found(*args[:-1])
+
     def pre_callback(self, ldap, filters, attrs_list, base_dn, scope, *args, **options):
         assert isinstance(base_dn, DN)
         return (filters, base_dn, scope)
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index 70b237dc102f46ab62e10aab0250aa496dad60c6..f8ee3db05b5a36cf4ebb4261f8535932b834b1bf 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -903,10 +903,12 @@ class user_del(LDAPDelete):
 
         # Delete user's private vault container.
         vaultcontainer_id = self.api.Object.vaultcontainer.get_private_id(owner)
-        (vaultcontainer_name, vaultcontainer_parent_id) = self.api.Object.vaultcontainer.split_id(vaultcontainer_id)
+        (vaultcontainer_name, vaultcontainer_parent_id) =\
+            self.api.Object.vaultcontainer.split_id(vaultcontainer_id)
 
         try:
-            self.api.Command.vaultcontainer_del(vaultcontainer_name, parent=vaultcontainer_parent_id)
+            self.api.Command.vaultcontainer_del(
+                vaultcontainer_name, parent_id=vaultcontainer_parent_id)
         except errors.NotFound:
             pass
 
diff --git a/ipalib/plugins/vault.py b/ipalib/plugins/vault.py
index 69c12aaf1c8503a345e115fb1a660aed8f85fb17..0b12491a8fb140b889b7686486c794238c7ed48e 100644
--- a/ipalib/plugins/vault.py
+++ b/ipalib/plugins/vault.py
@@ -41,8 +41,8 @@ from ipalib.frontend import Command
 from ipalib import api, errors
 from ipalib import Str, Bytes, Flag
 from ipalib.plugable import Registry
-from ipalib.plugins.baseldap import LDAPObject, LDAPCreate, LDAPDelete, LDAPSearch, LDAPUpdate, LDAPRetrieve,\
-                                    LDAPAddMember, LDAPRemoveMember
+from ipalib.plugins.baseldap import LDAPObject, LDAPCreate, LDAPDelete, LDAPSearch, LDAPUpdate,\
+                                    LDAPRetrieve, LDAPAddMember, LDAPRemoveMember
 from ipalib.request import context
 from ipalib.plugins.user import split_principal
 from ipalib import _, ngettext
@@ -61,7 +61,7 @@ EXAMPLES:
    ipa vault-find
 """) + _("""
  List shared vaults:
-   ipa vault-find --container /shared
+   ipa vault-find --container-id /shared
 """) + _("""
  Add a standard vault:
    ipa vault-add MyVault
@@ -135,6 +135,8 @@ class vault(LDAPObject):
     Vault object.
     """)
 
+    base_dn = DN(api.env.container_vault, api.env.basedn)
+
     object_name = _('vault')
     object_name_plural = _('vaults')
 
@@ -173,19 +175,11 @@ class vault(LDAPObject):
             pattern_errmsg='may only include letters, numbers, _, ., and -',
             maxlength=255,
         ),
-        Str('container?',
-            cli_name='container',
-            label=_('Container'),
-            doc=_('Container'),
-            flags=('virtual_attribute'),
-            pattern='^[a-zA-Z0-9_.-/]+$',
-            pattern_errmsg='may only include letters, numbers, _, ., -, and /',
-        ),
         Str('vault_id?',
             cli_name='vault_id',
             label=_('Vault ID'),
             doc=_('Vault ID'),
-            flags=('virtual_attribute'),
+            flags={'no_option', 'virtual_attribute'},
         ),
         Str('description?',
             cli_name='desc',
@@ -217,22 +211,16 @@ class vault(LDAPObject):
     )
 
     def get_dn(self, *keys, **options):
-        __doc__ = _("""
+        """
         Generates vault DN from vault ID.
-        """)
+        """
 
         # get vault ID from parameters
-        name = None
-        if keys:
-            name = keys[0]
+        name = keys[-1]
+        container_id = self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
+        vault_id = container_id + name
 
-        container_id = api.Object.vaultcontainer.normalize_id(options.get('container'))
-
-        vault_id = container_id
-        if name:
-            vault_id = container_id + name
-
-        dn = api.Object.vaultcontainer.base_dn
+        dn = self.base_dn
 
         # for each name in the ID, prepend the base DN
         for name in vault_id.split(u'/'):
@@ -242,45 +230,30 @@ class vault(LDAPObject):
         return dn
 
     def get_id(self, dn):
-        __doc__ = _("""
+        """
         Generates vault ID from vault DN.
-        """)
+        """
 
         # make sure the DN is a vault DN
-        if not dn.endswith(api.Object.vaultcontainer.base_dn):
+        if not dn.endswith(self.base_dn):
             raise ValueError('Invalid vault DN: %s' % dn)
 
         # vault DN cannot be the container base DN
-        if len(dn) == len(api.Object.vaultcontainer.base_dn):
+        if len(dn) == len(self.base_dn):
             raise ValueError('Invalid vault DN: %s' % dn)
 
         # construct the vault ID from the bottom up
         id = u''
-        while len(dn) > len(api.Object.vaultcontainer.base_dn):
-
-            rdn = dn[0]
+        for rdn in dn[:-len(self.base_dn)]:
             name = rdn['cn']
             id =  u'/' + name + id
 
-            dn = DN(*dn[1:])
-
         return id
 
-    def split_id(self, id):
-        __doc__ = _("""
-        Splits a vault ID into (vault name, container ID) tuple.
-        """)
-
-        # split ID into container ID and vault name
-        parts = id.rsplit(u'/', 1)
-
-        # return vault name and container ID
-        return (parts[1], parts[0] + u'/')
-
     def get_kra_id(self, id):
-        __doc__ = _("""
+        """
         Generates a client key ID to store/retrieve data in KRA.
-        """)
+        """
         return 'ipa:' + id
 
     def generate_symmetric_key(self, password, salt):
@@ -354,9 +327,13 @@ class vault_add(LDAPCreate):
     __doc__ = _('Create a new vault.')
 
     takes_options = LDAPCreate.takes_options + (
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
+        ),
         Bytes('data?',
             cli_name='data',
-            doc=_('Base-64 encoded binary data to archive'),
+            doc=_('Binary data to archive'),
         ),
         Str('text?',
             cli_name='text',
@@ -389,7 +366,7 @@ class vault_add(LDAPCreate):
     def forward(self, *args, **options):
 
         vault_name = args[0]
-        container_id = api.Object.vaultcontainer.normalize_id(options.get('container'))
+        container_id = self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
 
         vault_type = options.get('ipavaulttype')
         data = options.get('data')
@@ -420,18 +397,14 @@ class vault_add(LDAPCreate):
 
         # get data
         if data:
-            if text:
-                raise errors.ValidationError(name='text',
-                    error=_('Input data already specified'))
-
-            if input_file:
-                raise errors.ValidationError(name='input_file',
-                    error=_('Input data already specified'))
+            if text or input_file:
+                raise errors.MutuallyExclusiveError(
+                    reason=_('Input data specified multiple times'))
 
         elif text:
             if input_file:
-                raise errors.ValidationError(name='input_file',
-                    error=_('Input data already specified'))
+                raise errors.MutuallyExclusiveError(
+                    reason=_('Input data specified multiple times'))
 
             data = text.encode('utf-8')
 
@@ -564,9 +537,9 @@ class vault_add(LDAPCreate):
         response = super(vault_add, self).forward(*args, **options)
 
         # archive initial data
-        api.Command.vault_archive(
+        self.api.Command.vault_archive(
             vault_name,
-            container=container_id,
+            container_id=container_id,
             data=data,
             password=password,
             encryption_key=encryption_key)
@@ -576,13 +549,21 @@ class vault_add(LDAPCreate):
     def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         assert isinstance(dn, DN)
 
+        container_id = self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
+
+        # set owner
         principal = getattr(context, 'principal')
         (username, realm) = split_principal(principal)
-        owner_dn = self.api.Object['user'].get_dn(username)
+        owner_dn = self.api.Object.user.get_dn(username)
         entry_attrs['owner'] = owner_dn
 
-        container_dn = DN(*dn[1:])
-        api.Object.vaultcontainer.create_entry(container_dn, owner=owner_dn)
+        # container is user's private container, create container
+        if container_id == self.api.Object.vaultcontainer.get_private_id():
+            try:
+                self.api.Object.vaultcontainer.create_entry(
+                    DN(*dn[1:]), owner=owner_dn)
+            except errors.DuplicateEntry:
+                pass
 
         return dn
 
@@ -593,31 +574,39 @@ class vault_add(LDAPCreate):
 
         return dn
 
+    def handle_not_found(self, *args, **options):
+
+        container_id = self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
+
+        reason=self.obj.parent_not_found_msg % {
+            'parent': container_id,
+            'oname': self.api.Object.vaultcontainer.object_name,
+        }
 
 @register()
 class vault_del(LDAPDelete):
     __doc__ = _('Delete a vault.')
 
-    msg_summary = _('Deleted vault "%(value)s"')
-
     takes_options = LDAPDelete.takes_options + (
-        Str('container?',
-            cli_name='container',
-            doc=_('Container'),
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
         ),
     )
 
+    msg_summary = _('Deleted vault "%(value)s"')
+
     def post_callback(self, ldap, dn, *keys, **options):
         assert isinstance(dn, DN)
 
         vault_id = self.obj.get_id(dn)
 
-        kra_client = api.Backend.kra.get_client()
+        kra_client = self.api.Backend.kra.get_client()
 
         kra_account = pki.account.AccountClient(kra_client.connection)
         kra_account.login()
 
-        client_key_id = self.api.Object.vault.get_kra_id(vault_id)
+        client_key_id = self.obj.get_kra_id(vault_id)
 
         # deactivate vault record in KRA
         response = kra_client.keys.list_keys(client_key_id, pki.key.KeyClient.KEY_STATUS_ACTIVE)
@@ -636,6 +625,13 @@ class vault_del(LDAPDelete):
 class vault_find(LDAPSearch):
     __doc__ = _('Search for vaults.')
 
+    takes_options = LDAPSearch.takes_options + (
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
+        ),
+    )
+
     msg_summary = ngettext(
         '%(count)d vault matched', '%(count)d vaults matched', 0
     )
@@ -643,14 +639,9 @@ class vault_find(LDAPSearch):
     def pre_callback(self, ldap, filter, attrs_list, base_dn, scope, *keys, **options):
         assert isinstance(base_dn, DN)
 
-        principal = getattr(context, 'principal')
-        (username, realm) = split_principal(principal)
-        owner_dn = self.api.Object['user'].get_dn(username)
-
-        container_id = self.Object.vaultcontainer.normalize_id(options.get('container'))
-        base_dn = self.Object.vaultcontainer.get_dn(parent=container_id)
-
-        api.Object.vaultcontainer.create_entry(base_dn, owner=owner_dn)
+        container_id = self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
+        (name, parent_id) = self.api.Object.vaultcontainer.split_id(container_id)
+        base_dn = self.api.Object.vaultcontainer.get_dn(name, parent_id=parent_id)
 
         return (filter, base_dn, scope)
 
@@ -662,11 +653,31 @@ class vault_find(LDAPSearch):
 
         return truncated
 
+    def handle_not_found(self, *args, **options):
+
+        container_id = self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
+
+        # vault container is user's private container, ignore
+        if container_id == self.api.Object.vaultcontainer.get_private_id():
+            return
+
+        # otherwise, raise an error
+        reason=self.obj.parent_not_found_msg % {
+            'parent': container_id,
+            'oname': self.api.Object.vaultcontainer.object_name,
+        }
 
 @register()
 class vault_mod(LDAPUpdate):
     __doc__ = _('Modify a vault.')
 
+    takes_options = LDAPUpdate.takes_options + (
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
+        ),
+    )
+
     msg_summary = _('Modified vault "%(value)s"')
 
     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
@@ -682,9 +693,9 @@ class vault_show(LDAPRetrieve):
     __doc__ = _('Display information about a vault.')
 
     takes_options = LDAPRetrieve.takes_options + (
-        Str('container?',
-            cli_name='container',
-            doc=_('Container'),
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
         ),
     )
 
@@ -700,11 +711,6 @@ class vault_show(LDAPRetrieve):
 class vault_transport_cert(Command):
     __doc__ = _('Retrieve vault transport certificate.')
 
-    # list of attributes we want exported to JSON
-    json_friendly_attributes = (
-        'takes_args',
-    )
-
     takes_options = (
         Str('out?',
             cli_name='out',
@@ -718,13 +724,6 @@ class vault_transport_cert(Command):
         ),
     )
 
-    def __json__(self):
-        json_dict = dict(
-            (a, getattr(self, a)) for a in self.json_friendly_attributes
-        )
-        json_dict['takes_options'] = list(self.get_json_options())
-        return json_dict
-
     def forward(self, *args, **options):
 
         file = options.get('out')
@@ -743,7 +742,7 @@ class vault_transport_cert(Command):
 
     def execute(self, *args, **options):
 
-        kra_client = api.Backend.kra.get_client()
+        kra_client = self.api.Backend.kra.get_client()
         transport_cert = kra_client.system_certs.get_transport_cert()
         return {
             'result': {
@@ -757,13 +756,13 @@ class vault_archive(LDAPRetrieve):
     __doc__ = _('Archive data into a vault.')
 
     takes_options = LDAPRetrieve.takes_options + (
-        Str('container?',
-            cli_name='container',
-            doc=_('Container'),
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
         ),
         Bytes('data?',
             cli_name='data',
-            doc=_('Base-64 encoded binary data to archive'),
+            doc=_('Binary data to archive'),
         ),
         Str('text?',
             cli_name='text',
@@ -804,7 +803,7 @@ class vault_archive(LDAPRetrieve):
     def forward(self, *args, **options):
 
         vault_name = args[0]
-        container_id = api.Object.vaultcontainer.normalize_id(options.get('container'))
+        container_id = self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
 
         vault_type = 'standard'
         salt = None
@@ -812,7 +811,7 @@ class vault_archive(LDAPRetrieve):
         escrow_public_key = None
 
         # retrieve vault info
-        response = api.Command.vault_show(vault_name, container=container_id)
+        response = self.api.Command.vault_show(vault_name, container_id=container_id)
         result = response['result']
 
         if result.has_key('ipavaulttype'):
@@ -850,18 +849,14 @@ class vault_archive(LDAPRetrieve):
 
         # get data
         if data:
-            if text:
-                raise errors.ValidationError(name='text',
-                    error=_('Input data already specified'))
-
-            if input_file:
-                raise errors.ValidationError(name='input_file',
-                    error=_('Input data already specified'))
+            if text or input_file:
+                raise errors.MutuallyExclusiveError(
+                    reason=_('Input data specified multiple times'))
 
         elif text:
             if input_file:
-                raise errors.ValidationError(name='input_file',
-                    error=_('Input data already specified'))
+                raise errors.MutuallyExclusiveError(
+                    reason=_('Input data specified multiple times'))
 
             data = text.encode('utf-8')
 
@@ -902,9 +897,9 @@ class vault_archive(LDAPRetrieve):
                     password = unicode(getpass.getpass('Password: '))
 
                 try:
-                    api.Command.vault_retrieve(
+                    self.api.Command.vault_retrieve(
                         vault_name,
-                        container=container_id,
+                        container_id=container_id,
                         password=password)
 
                 except errors.NotFound:
@@ -959,7 +954,7 @@ class vault_archive(LDAPRetrieve):
         (file, filename) = tempfile.mkstemp()
         os.close(file)
         try:
-            api.Command.vault_transport_cert(out=unicode(filename))
+            self.api.Command.vault_transport_cert(out=unicode(filename))
             transport_cert_der = nss.read_der_from_file(filename, True)
             nss_transport_cert = nss.Certificate(transport_cert_der)
 
@@ -981,7 +976,7 @@ class vault_archive(LDAPRetrieve):
         options['nonce'] = unicode(base64.b64encode(nonce))
 
         vault_data = {}
-        vault_data[u'data'] = unicode(data)
+        vault_data[u'data'] = unicode(base64.b64encode(data))
 
         if encrypted_key:
             vault_data[u'encrypted_key'] = unicode(base64.b64encode(encrypted_key))
@@ -1012,7 +1007,7 @@ class vault_archive(LDAPRetrieve):
 
         principal = getattr(context, 'principal')
         (username, realm) = split_principal(principal)
-        user_dn = self.api.Object['user'].get_dn(username)
+        user_dn = self.api.Object.user.get_dn(username)
 
         if user_dn not in owners and user_dn not in members:
             raise errors.ACIError(info=_("Insufficient access to vault '%s'.") % vault_id)
@@ -1020,12 +1015,12 @@ class vault_archive(LDAPRetrieve):
         entry_attrs['vault_id'] = vault_id
 
         # connect to KRA
-        kra_client = api.Backend.kra.get_client()
+        kra_client = self.api.Backend.kra.get_client()
 
         kra_account = pki.account.AccountClient(kra_client.connection)
         kra_account.login()
 
-        client_key_id = self.api.Object.vault.get_kra_id(vault_id)
+        client_key_id = self.obj.get_kra_id(vault_id)
 
         # deactivate existing vault record in KRA
         response = kra_client.keys.list_keys(
@@ -1062,9 +1057,9 @@ class vault_retrieve(LDAPRetrieve):
     __doc__ = _('Retrieve a data from a vault.')
 
     takes_options = LDAPRetrieve.takes_options + (
-        Str('container?',
-            cli_name='container',
-            doc=_('Container'),
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
         ),
         Flag('show_text?',
             doc=_('Show text data'),
@@ -1122,13 +1117,13 @@ class vault_retrieve(LDAPRetrieve):
     def forward(self, *args, **options):
 
         vault_name = args[0]
-        container_id = api.Object.vaultcontainer.normalize_id(options.get('container'))
+        container_id = self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
 
         vault_type = 'standard'
         salt = None
 
         # retrieve vault info
-        response = api.Command.vault_show(vault_name, container=container_id)
+        response = self.api.Command.vault_show(vault_name, container_id=container_id)
         result = response['result']
 
         if result.has_key('ipavaulttype'):
@@ -1179,7 +1174,7 @@ class vault_retrieve(LDAPRetrieve):
         (file, filename) = tempfile.mkstemp()
         os.close(file)
         try:
-            api.Command.vault_transport_cert(out=unicode(filename))
+            self.api.Command.vault_transport_cert(out=unicode(filename))
             transport_cert_der = nss.read_der_from_file(filename, True)
             nss_transport_cert = nss.Certificate(transport_cert_der)
 
@@ -1209,7 +1204,7 @@ class vault_retrieve(LDAPRetrieve):
             nonce_iv=nonce)
 
         vault_data = json.loads(json_vault_data)
-        data = str(vault_data[u'data'])
+        data = base64.b64decode(str(vault_data[u'data']))
 
         encrypted_key = None
 
@@ -1343,7 +1338,7 @@ class vault_retrieve(LDAPRetrieve):
 
         principal = getattr(context, 'principal')
         (username, realm) = split_principal(principal)
-        user_dn = self.api.Object['user'].get_dn(username)
+        user_dn = self.api.Object.user.get_dn(username)
 
         if user_dn not in owners and user_dn not in members:
             raise errors.ACIError(info=_("Insufficient access to vault '%s'.") % vault_id)
@@ -1353,12 +1348,12 @@ class vault_retrieve(LDAPRetrieve):
         wrapped_session_key = base64.b64decode(options['session_key'])
 
         # connect to KRA
-        kra_client = api.Backend.kra.get_client()
+        kra_client = self.api.Backend.kra.get_client()
 
         kra_account = pki.account.AccountClient(kra_client.connection)
         kra_account.login()
 
-        client_key_id = self.api.Object.vault.get_kra_id(vault_id)
+        client_key_id = self.obj.get_kra_id(vault_id)
 
         # find vault record in KRA
         response = kra_client.keys.list_keys(
@@ -1388,9 +1383,9 @@ class vault_add_owner(LDAPAddMember):
     __doc__ = _('Add owners to a vault.')
 
     takes_options = LDAPAddMember.takes_options + (
-        Str('container?',
-            cli_name='container',
-            doc=_('Container'),
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
         ),
     )
 
@@ -1410,9 +1405,9 @@ class vault_remove_owner(LDAPRemoveMember):
     __doc__ = _('Remove owners from a vault.')
 
     takes_options = LDAPRemoveMember.takes_options + (
-        Str('container?',
-            cli_name='container',
-            doc=_('Container'),
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
         ),
     )
 
@@ -1432,9 +1427,9 @@ class vault_add_member(LDAPAddMember):
     __doc__ = _('Add members to a vault.')
 
     takes_options = LDAPAddMember.takes_options + (
-        Str('container?',
-            cli_name='container',
-            doc=_('Container'),
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
         ),
     )
 
@@ -1451,9 +1446,9 @@ class vault_remove_member(LDAPRemoveMember):
     __doc__ = _('Remove members from a vault.')
 
     takes_options = LDAPRemoveMember.takes_options + (
-        Str('container?',
-            cli_name='container',
-            doc=_('Container'),
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
         ),
     )
 
diff --git a/ipalib/plugins/vaultcontainer.py b/ipalib/plugins/vaultcontainer.py
index 881bbea7886e06356489cd49cc32f2a6a6460a5e..e0dc728362247f7cde252e21a54fa5c23761eb49 100644
--- a/ipalib/plugins/vaultcontainer.py
+++ b/ipalib/plugins/vaultcontainer.py
@@ -40,7 +40,7 @@ EXAMPLES:
    ipa vaultcontainer-find
 """) + _("""
  List top-level vault containers:
-   ipa vaultcontainer-find --parent /
+   ipa vaultcontainer-find --parent-id /
 """) + _("""
  Add a vault container:
    ipa vaultcontainer-add MyContainer
@@ -75,7 +75,6 @@ class vaultcontainer(LDAPObject):
     Vault container object.
     """)
 
-    base_dn = DN(api.env.container_vault, api.env.basedn)
     object_name = _('vault container')
     object_name_plural = _('vault containers')
 
@@ -109,19 +108,11 @@ class vaultcontainer(LDAPObject):
             pattern_errmsg='may only include letters, numbers, _, ., and -',
             maxlength=255,
         ),
-        Str('parent?',
-            cli_name='parent',
-            label=_('Parent'),
-            doc=_('Parent container'),
-            flags=('virtual_attribute'),
-            pattern='^[a-zA-Z0-9_.-/]+$',
-            pattern_errmsg='may only include letters, numbers, _, ., -, and /',
-        ),
         Str('container_id?',
             cli_name='container_id',
             label=_('Container ID'),
             doc=_('Container ID'),
-            flags=('virtual_attribute'),
+            flags={'no_option', 'virtual_attribute'},
         ),
         Str('description?',
             cli_name='desc',
@@ -131,22 +122,19 @@ class vaultcontainer(LDAPObject):
     )
 
     def get_dn(self, *keys, **options):
-        __doc__ = _("""
+        """
         Generates vault container DN from container ID.
-        """)
+        """
 
         # get container ID from parameters
-        name = None
-        if keys:
-            name = keys[0]
-
-        parent_id = api.Object.vaultcontainer.normalize_id(options.get('parent'))
+        name = keys[-1]
+        parent_id = self.normalize_id(options.get('parent_id'))
 
         container_id = parent_id
         if name:
             container_id = parent_id + name + u'/'
 
-        dn = self.base_dn
+        dn = self.api.Object.vault.base_dn
 
         # for each name in the ID, prepend the base DN
         for name in container_id.split(u'/'):
@@ -156,30 +144,26 @@ class vaultcontainer(LDAPObject):
         return dn
 
     def get_id(self, dn):
-        __doc__ = _("""
+        """
         Generates container ID from container DN.
-        """)
+        """
 
         # make sure the DN is a container DN
-        if not dn.endswith(self.base_dn):
+        if not dn.endswith(self.api.Object.vault.base_dn):
             raise ValueError('Invalid container DN: %s' % dn)
 
         # construct container ID from the bottom up
         id = u'/'
-        while len(dn) > len(self.base_dn):
-
-            rdn = dn[0]
+        for rdn in dn[:-len(self.api.Object.vault.base_dn)]:
             name = rdn['cn']
             id =  u'/' + name + id
 
-            dn = DN(*dn[1:])
-
         return id
 
     def get_private_id(self, username=None):
-        __doc__ = _("""
+        """
         Returns user's private container ID (i.e. /users/<username>/).
-        """)
+        """
 
         if not username:
             principal = getattr(context, 'principal')
@@ -188,9 +172,9 @@ class vaultcontainer(LDAPObject):
         return u'/users/' + username + u'/'
 
     def normalize_id(self, id):
-        __doc__ = _("""
+        """
         Normalizes container ID.
-        """)
+        """
 
         # if ID is empty, return user's private container ID
         if not id:
@@ -208,13 +192,13 @@ class vaultcontainer(LDAPObject):
         return self.get_private_id() + id
 
     def split_id(self, id):
-        __doc__ = _("""
+        """
         Splits a normalized container ID into (container name, parent ID) tuple.
-        """)
+        """
 
         # handle root ID
         if id == u'/':
-            return (None, None)
+            return (None, u'/')
 
         # split ID into parent ID, container name, and empty string
         parts = id.rsplit(u'/', 2)
@@ -223,23 +207,10 @@ class vaultcontainer(LDAPObject):
         return (parts[1], parts[0] + u'/')
 
     def create_entry(self, dn, owner=None):
-        __doc__ = _("""
+        """
         Creates a container entry and its parents.
-        """)
+        """
 
-        # if entry already exists, return
-        try:
-            self.backend.get_entry(dn)
-            return
-
-        except errors.NotFound:
-            pass
-
-        # otherwise, create parent entry first
-        parent_dn = DN(*dn[1:])
-        self.create_entry(parent_dn, owner=owner)
-
-        # then create the entry itself
         rdn = dn[0]
         entry = self.backend.make_entry(
             dn,
@@ -248,6 +219,20 @@ class vaultcontainer(LDAPObject):
                 'cn': rdn['cn'],
                 'owner': owner
             })
+
+        # if entry can be added return
+        try:
+            self.backend.add_entry(entry)
+            return
+
+        except errors.NotFound:
+            pass
+
+        # otherwise, create parent entry first
+        parent_dn = DN(*dn[1:])
+        self.create_entry(parent_dn, owner=owner)
+
+        # then create the entry again
         self.backend.add_entry(entry)
 
 
@@ -255,18 +240,33 @@ class vaultcontainer(LDAPObject):
 class vaultcontainer_add(LDAPCreate):
     __doc__ = _('Create a new vault container.')
 
+    takes_options = LDAPCreate.takes_options + (
+        Str('parent_id?',
+            cli_name='parent_id',
+            doc=_('Parent ID'),
+        ),
+    )
+
     msg_summary = _('Added vault container "%(value)s"')
 
     def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         assert isinstance(dn, DN)
 
+        parent_id = self.obj.normalize_id(options.get('parent_id'))
+
+        # set owner
         principal = getattr(context, 'principal')
         (username, realm) = split_principal(principal)
-        owner_dn = self.api.Object['user'].get_dn(username)
+        owner_dn = self.api.Object.user.get_dn(username)
         entry_attrs['owner'] = owner_dn
 
-        parent_dn = DN(*dn[1:])
-        self.obj.create_entry(parent_dn, owner=owner_dn)
+        # parent is user's private container, create parent
+        if parent_id == self.obj.get_private_id():
+            try:
+                self.obj.create_entry(
+                    DN(*dn[1:]), owner=owner_dn)
+            except errors.DuplicateEntry:
+                pass
 
         return dn
 
@@ -277,17 +277,26 @@ class vaultcontainer_add(LDAPCreate):
 
         return dn
 
+    def handle_not_found(self, *args, **options):
+
+        parent_id = self.obj.normalize_id(options.get('parent_id'))
+
+        raise errors.NotFound(
+            reason=self.obj.parent_not_found_msg % {
+                'parent': parent_id,
+                'oname': self.obj.object_name,
+            }
+        )
+
 
 @register()
 class vaultcontainer_del(LDAPDelete):
     __doc__ = _('Delete a vault container.')
 
-    msg_summary = _('Deleted vault container "%(value)s"')
-
     takes_options = LDAPDelete.takes_options + (
-        Str('parent?',
-            cli_name='parent',
-            doc=_('Parent container'),
+        Str('parent_id?',
+            cli_name='parent_id',
+            doc=_('Parent ID'),
         ),
         Flag('force?',
             doc=_('Force deletion'),
@@ -295,42 +304,33 @@ class vaultcontainer_del(LDAPDelete):
         ),
     )
 
-    def delete_entry(self, pkey, *keys, **options):
-        __doc__ = _("""
-        Overwrites the base method to control deleting subtree with force option.
-        """)
+    msg_summary = _('Deleted vault container "%(value)s"')
 
-        ldap = self.obj.backend
-        nkeys = keys[:-1] + (pkey, )
-        dn = self.obj.get_dn(*nkeys, **options)
+    def pre_callback(self, ldap, dn, *keys, **options):
         assert isinstance(dn, DN)
 
-        for callback in self.get_callbacks('pre'):
-            dn = callback(self, ldap, dn, *nkeys, **options)
-            assert isinstance(dn, DN)
-
         try:
-            self._exc_wrapper(nkeys, options, ldap.delete_entry)(dn)
+            ldap.get_entries(dn, scope=ldap.SCOPE_ONELEVEL, attrs_list=[])
         except errors.NotFound:
-            self.obj.handle_not_found(*nkeys)
-        except errors.NotAllowedOnNonLeaf:
-            # this entry is not a leaf entry
-            # if forced, delete all child nodes
-            if options.get('force'):
-                self.delete_subtree(dn, *nkeys, **options)
-            else:
-                raise
+            pass
+        else:
+            if not options.get('force', False):
+                raise errors.NotAllowedOnNonLeaf()
 
-        for callback in self.get_callbacks('post'):
-            result = callback(self, ldap, dn, *nkeys, **options)
-
-        return result
+        return dn
 
 
 @register()
 class vaultcontainer_find(LDAPSearch):
     __doc__ = _('Search for vault containers.')
 
+    takes_options = LDAPSearch.takes_options + (
+        Str('parent_id?',
+            cli_name='parent_id',
+            doc=_('Parent ID'),
+        ),
+    )
+
     msg_summary = ngettext(
         '%(count)d vault container matched', '%(count)d vault containers matched', 0
     )
@@ -338,14 +338,9 @@ class vaultcontainer_find(LDAPSearch):
     def pre_callback(self, ldap, filter, attrs_list, base_dn, scope, *keys, **options):
         assert isinstance(base_dn, DN)
 
-        principal = getattr(context, 'principal')
-        (username, realm) = split_principal(principal)
-        owner_dn = self.api.Object['user'].get_dn(username)
-
-        parent_id = self.obj.normalize_id(options.get('parent'))
-        base_dn = self.obj.get_dn(parent=parent_id)
-
-        self.obj.create_entry(base_dn, owner=owner_dn)
+        parent_id = self.obj.normalize_id(options.get('parent_id'))
+        (name, grandparent_id) = self.obj.split_id(parent_id)
+        base_dn = self.obj.get_dn(name, parent_id=grandparent_id)
 
         return (filter, base_dn, scope)
 
@@ -356,11 +351,32 @@ class vaultcontainer_find(LDAPSearch):
 
         return truncated
 
+    def handle_not_found(self, *args, **options):
+
+        parent_id = self.obj.normalize_id(options.get('parent_id'))
+
+        # parent is user's private container, ignore
+        if parent_id == self.obj.get_private_id():
+            return
+
+        # otherwise, raise an error
+        reason=self.obj.parent_not_found_msg % {
+            'parent': parent_id,
+            'oname': self.obj.object_name,
+        }
+
 
 @register()
 class vaultcontainer_mod(LDAPUpdate):
     __doc__ = _('Modify a vault container.')
 
+    takes_options = LDAPUpdate.takes_options + (
+        Str('parent_id?',
+            cli_name='parent_id',
+            doc=_('Parent ID'),
+        ),
+    )
+
     msg_summary = _('Modified vault container "%(value)s"')
 
     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
@@ -376,9 +392,9 @@ class vaultcontainer_show(LDAPRetrieve):
     __doc__ = _('Display information about a vault container.')
 
     takes_options = LDAPRetrieve.takes_options + (
-        Str('parent?',
-            cli_name='parent',
-            doc=_('Parent container'),
+        Str('parent_id?',
+            cli_name='parent_id',
+            doc=_('Parent ID'),
         ),
     )
 
@@ -395,9 +411,9 @@ class vaultcontainer_add_owner(LDAPAddMember):
     __doc__ = _('Add owners to a vault container.')
 
     takes_options = LDAPAddMember.takes_options + (
-        Str('parent?',
-            cli_name='parent',
-            doc=_('Parent container'),
+        Str('parent_id?',
+            cli_name='parent_id',
+            doc=_('Parent ID'),
         ),
     )
 
@@ -417,9 +433,9 @@ class vaultcontainer_remove_owner(LDAPRemoveMember):
     __doc__ = _('Remove owners from a vault container.')
 
     takes_options = LDAPRemoveMember.takes_options + (
-        Str('parent?',
-            cli_name='parent',
-            doc=_('Parent container'),
+        Str('parent_id?',
+            cli_name='parent_id',
+            doc=_('Parent ID'),
         ),
     )
 
@@ -439,9 +455,9 @@ class vaultcontainer_add_member(LDAPAddMember):
     __doc__ = _('Add members to a vault container.')
 
     takes_options = LDAPAddMember.takes_options + (
-        Str('parent?',
-            cli_name='parent',
-            doc=_('Parent container'),
+        Str('parent_id?',
+            cli_name='parent_id',
+            doc=_('Parent ID'),
         ),
     )
 
@@ -459,9 +475,9 @@ class vaultcontainer_remove_member(LDAPRemoveMember):
 
 
     takes_options = LDAPRemoveMember.takes_options + (
-        Str('parent?',
-            cli_name='parent',
-            doc=_('Parent container'),
+        Str('parent_id?',
+            cli_name='parent_id',
+            doc=_('Parent ID'),
         ),
     )
 
diff --git a/ipalib/plugins/vaultsecret.py b/ipalib/plugins/vaultsecret.py
index b0896155a5af13013f13f2b3ae586da2a8832c30..7f44f0816b571dac718fa02edfd187fe8666565e 100644
--- a/ipalib/plugins/vaultsecret.py
+++ b/ipalib/plugins/vaultsecret.py
@@ -93,7 +93,7 @@ class vaultsecret(LDAPObject):
         Bytes('data?',
             cli_name='data',
             label=_('Data'),
-            doc=_('Base-64 encoded binary secret data'),
+            doc=_('Binary secret data'),
         ),
     )
 
@@ -103,8 +103,8 @@ class vaultsecret_add(LDAPRetrieve):
     __doc__ = _('Add a new vault secret.')
 
     takes_options = (
-        Str('container?',
-            cli_name='container',
+        Str('container_id?',
+            cli_name='container_id',
             doc=_('Container ID'),
         ),
         Str('description?',
@@ -113,7 +113,7 @@ class vaultsecret_add(LDAPRetrieve):
         ),
         Bytes('data?',
             cli_name='data',
-            doc=_('Base-64 encoded binary secret data'),
+            doc=_('Binary secret data'),
         ),
         Str('text?',
             cli_name='text',
@@ -147,13 +147,13 @@ class vaultsecret_add(LDAPRetrieve):
 
         vault_name = args[0]
         secret_name = args[1]
-        container_id = api.Object.vaultcontainer.normalize_id(options.get('container'))
+        container_id = self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
 
         vault_type = 'standard'
         salt = None
 
         # retrieve vault info
-        response = api.Command.vault_show(vault_name, container=container_id)
+        response = self.api.Command.vault_show(vault_name, container_id=container_id)
         result = response['result']
 
         if result.has_key('ipavaulttype'):
@@ -256,9 +256,9 @@ class vaultsecret_add(LDAPRetrieve):
                 error=_('Invalid vault type'))
 
         # retrieve secrets
-        response = api.Command.vault_retrieve(
+        response = self.api.Command.vault_retrieve(
             vault_name,
-            container=container_id,
+            container_id=container_id,
             password=password,
             private_key=private_key)
 
@@ -276,18 +276,14 @@ class vaultsecret_add(LDAPRetrieve):
 
         # get data
         if data:
-            if text:
-                raise errors.ValidationError(name='text',
-                    error=_('Input data already specified'))
-
-            if input_file:
-                raise errors.ValidationError(name='input_file',
-                    error=_('Input data already specified'))
+            if text or input_file:
+                raise errors.MutuallyExclusiveError(
+                    reason=_('Input data specified multiple times'))
 
         elif text:
             if input_file:
-                raise errors.ValidationError(name='input_file',
-                    error=_('Input data already specified'))
+                raise errors.MutuallyExclusiveError(
+                    reason=_('Input data specified multiple times'))
 
             data = text.encode('utf-8')
 
@@ -315,9 +311,9 @@ class vaultsecret_add(LDAPRetrieve):
 
         # rearchive secrets
         vault_data = json.dumps(json_data)
-        response = api.Command.vault_archive(
+        response = self.api.Command.vault_archive(
             vault_name,
-            container=container_id,
+            container_id=container_id,
             data=vault_data,
             password=password)
 
@@ -338,8 +334,8 @@ class vaultsecret_del(LDAPRetrieve):
     __doc__ = _('Delete a vault secret.')
 
     takes_options = (
-        Str('container?',
-            cli_name='container',
+        Str('container_id?',
+            cli_name='container_id',
             doc=_('Container ID'),
         ),
         Str('password?',
@@ -366,13 +362,13 @@ class vaultsecret_del(LDAPRetrieve):
 
         vault_name = args[0]
         secret_name = args[1]
-        container_id = api.Object.vaultcontainer.normalize_id(options.get('container'))
+        container_id = self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
 
         vault_type = 'standard'
         salt = None
 
         # retrieve vault info
-        response = api.Command.vault_show(vault_name, container=container_id)
+        response = self.api.Command.vault_show(vault_name, container_id=container_id)
         result = response['result']
 
         if result.has_key('ipavaulttype'):
@@ -463,9 +459,9 @@ class vaultsecret_del(LDAPRetrieve):
                 error=_('Invalid vault type'))
 
         # retrieve secrets
-        response = api.Command.vault_retrieve(
+        response = self.api.Command.vault_retrieve(
             vault_name,
-            container=container_id,
+            container_id=container_id,
             password=password,
             private_key=private_key)
 
@@ -496,9 +492,9 @@ class vaultsecret_del(LDAPRetrieve):
 
         # rearchive secrets
         vault_data = json.dumps(json_data)
-        response = api.Command.vault_archive(
+        response = self.api.Command.vault_archive(
             vault_name,
-            container=container_id,
+            container_id=container_id,
             data=vault_data,
             password=password)
 
@@ -515,13 +511,11 @@ class vaultsecret_del(LDAPRetrieve):
 
 @register()
 class vaultsecret_find(LDAPSearch):
-    __doc__ = _("""
-    Search for vault secrets.
-    """)
+    __doc__ = _('Search for vault secrets.')
 
     takes_options = (
-        Str('container?',
-            cli_name='container',
+        Str('container_id?',
+            cli_name='container_id',
             doc=_('Container ID'),
         ),
         Str('password?',
@@ -545,13 +539,13 @@ class vaultsecret_find(LDAPSearch):
     def forward(self, *args, **options):
 
         vault_name = args[0]
-        container_id = api.Object.vaultcontainer.normalize_id(options.get('container'))
+        container_id = self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
 
         vault_type = 'standard'
         salt = None
 
         # retrieve vault info
-        response = api.Command.vault_show(vault_name, container=container_id)
+        response = self.api.Command.vault_show(vault_name, container_id=container_id)
         result = response['result']
 
         if result.has_key('ipavaulttype'):
@@ -641,9 +635,9 @@ class vaultsecret_find(LDAPSearch):
             raise errors.ValidationError(name='vault_type',
                 error=_('Invalid vault type'))
 
-        response = api.Command.vault_retrieve(
+        response = self.api.Command.vault_retrieve(
             vault_name,
-            container=container_id,
+            container_id=container_id,
             password=password,
             private_key=private_key)
 
@@ -678,8 +672,8 @@ class vaultsecret_mod(LDAPRetrieve):
     __doc__ = _('Modify a vault secret.')
 
     takes_options = (
-        Str('container?',
-            cli_name='container',
+        Str('container_id?',
+            cli_name='container_id',
             doc=_('Container ID'),
         ),
         Str('description?',
@@ -722,13 +716,13 @@ class vaultsecret_mod(LDAPRetrieve):
 
         vault_name = args[0]
         secret_name = args[1]
-        container_id = api.Object.vaultcontainer.normalize_id(options.get('container'))
+        container_id = self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
 
         vault_type = 'standard'
         salt = None
 
         # retrieve vault info
-        response = api.Command.vault_show(vault_name, container=container_id)
+        response = self.api.Command.vault_show(vault_name, container_id=container_id)
         result = response['result']
 
         if result.has_key('ipavaulttype'):
@@ -830,18 +824,14 @@ class vaultsecret_mod(LDAPRetrieve):
 
         # get data
         if data:
-            if text:
-                raise errors.ValidationError(name='text',
-                    error=_('Input data already specified'))
-
-            if input_file:
-                raise errors.ValidationError(name='input_file',
-                    error=_('Input data already specified'))
+            if text or input_file:
+                raise errors.MutuallyExclusiveError(
+                    reason=_('Input data specified multiple times'))
 
         elif text:
             if input_file:
-                raise errors.ValidationError(name='input_file',
-                    error=_('Input data already specified'))
+                raise errors.MutuallyExclusiveError(
+                    reason=_('Input data specified multiple times'))
 
             data = text.encode('utf-8')
 
@@ -853,9 +843,9 @@ class vaultsecret_mod(LDAPRetrieve):
             pass
 
         # retrieve secrets
-        response = api.Command.vault_retrieve(
+        response = self.api.Command.vault_retrieve(
             vault_name,
-            container=container_id,
+            container_id=container_id,
             password=password,
             private_key=private_key)
 
@@ -889,9 +879,9 @@ class vaultsecret_mod(LDAPRetrieve):
 
         # rearchive secrets
         vault_data = json.dumps(json_data)
-        response = api.Command.vault_archive(
+        response = self.api.Command.vault_archive(
             vault_name,
-            container=container_id,
+            container_id=container_id,
             data=vault_data,
             password=password)
 
@@ -912,8 +902,8 @@ class vaultsecret_show(LDAPRetrieve):
     __doc__ = _('Display information about a vault secret.')
 
     takes_options = (
-        Str('container?',
-            cli_name='container',
+        Str('container_id?',
+            cli_name='container_id',
             doc=_('Container ID'),
         ),
         Flag('show_text?',
@@ -956,13 +946,13 @@ class vaultsecret_show(LDAPRetrieve):
 
         vault_name = args[0]
         secret_name = args[1]
-        container_id = api.Object.vaultcontainer.normalize_id(options.get('container'))
+        container_id = self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
 
         vault_type = 'standard'
         salt = None
 
         # retrieve vault info
-        response = api.Command.vault_show(vault_name, container=container_id)
+        response = self.api.Command.vault_show(vault_name, container_id=container_id)
         result = response['result']
 
         if result.has_key('ipavaulttype'):
@@ -1062,9 +1052,9 @@ class vaultsecret_show(LDAPRetrieve):
                 error=_('Invalid vault type'))
 
         # retrieve secrets
-        response = api.Command.vault_retrieve(
+        response = self.api.Command.vault_retrieve(
             vault_name,
-            container=container_id,
+            container_id=container_id,
             password=password,
             private_key=private_key)
 
diff --git a/ipatests/test_xmlrpc/test_vault_plugin.py b/ipatests/test_xmlrpc/test_vault_plugin.py
index 04f56ecafd3a141b39a0e32f1725258582b6b141..f3a280b40d5b6972e8755f63d46013cadaa68334 100644
--- a/ipatests/test_xmlrpc/test_vault_plugin.py
+++ b/ipatests/test_xmlrpc/test_vault_plugin.py
@@ -150,7 +150,7 @@ MszdQuc/FTSJ2DYsIwx7qq5c8mtargOjWRgZU22IgY9PKeIcitQjqw==
 -----END RSA PRIVATE KEY-----
 """
 
-class test_vault(Declarative):
+class test_vault_plugin(Declarative):
 
     cleanup_commands = [
         ('vault_del', [test_vault], {'continue': True}),
diff --git a/ipatests/test_xmlrpc/test_vaultcontainer_plugin.py b/ipatests/test_xmlrpc/test_vaultcontainer_plugin.py
index b99f9f68b8a480908602503d726205eeec36c2d2..22e13769df19b40cd39a144df662bae8bbf53d9e 100644
--- a/ipatests/test_xmlrpc/test_vaultcontainer_plugin.py
+++ b/ipatests/test_xmlrpc/test_vaultcontainer_plugin.py
@@ -32,12 +32,12 @@ base_container = u'base_container'
 child_container = u'child_container'
 grandchild_container = u'grandchild_container'
 
-class test_vault(Declarative):
+class test_vaultcontainer_plugin(Declarative):
 
     cleanup_commands = [
         ('vaultcontainer_del', [private_container], {'continue': True}),
-        ('vaultcontainer_del', [shared_container], {'parent': u'/shared/', 'continue': True}),
-        ('vaultcontainer_del', [service_container], {'parent': u'/services/', 'continue': True}),
+        ('vaultcontainer_del', [shared_container], {'parent_id': u'/shared/', 'continue': True}),
+        ('vaultcontainer_del', [service_container], {'parent_id': u'/services/', 'continue': True}),
         ('vaultcontainer_del', [base_container], {'force': True, 'continue': True}),
     ]
 
@@ -49,7 +49,7 @@ class test_vault(Declarative):
                 'vaultcontainer_find',
                 [],
                 {
-                    'parent': u'/',
+                    'parent_id': u'/',
                 },
             ),
             'expected': {
@@ -202,7 +202,7 @@ class test_vault(Declarative):
                 'vaultcontainer_add',
                 [shared_container],
                 {
-                    'parent': u'/shared/',
+                    'parent_id': u'/shared/',
                 },
             ),
             'expected': {
@@ -224,7 +224,7 @@ class test_vault(Declarative):
                 'vaultcontainer_find',
                 [],
                 {
-                    'parent': u'/shared/',
+                    'parent_id': u'/shared/',
                 },
             ),
             'expected': {
@@ -247,7 +247,7 @@ class test_vault(Declarative):
                 'vaultcontainer_show',
                 [shared_container],
                 {
-                    'parent': u'/shared/',
+                    'parent_id': u'/shared/',
                 },
             ),
             'expected': {
@@ -268,7 +268,7 @@ class test_vault(Declarative):
                 'vaultcontainer_mod',
                 [shared_container],
                 {
-                    'parent': u'/shared/',
+                    'parent_id': u'/shared/',
                     'description': u'shared container',
                 },
             ),
@@ -290,7 +290,7 @@ class test_vault(Declarative):
                 'vaultcontainer_del',
                 [shared_container],
                 {
-                    'parent': u'/shared/',
+                    'parent_id': u'/shared/',
                 },
             ),
             'expected': {
@@ -308,7 +308,7 @@ class test_vault(Declarative):
                 'vaultcontainer_add',
                 [service_container],
                 {
-                    'parent': u'/services/',
+                    'parent_id': u'/services/',
                 },
             ),
             'expected': {
@@ -349,7 +349,7 @@ class test_vault(Declarative):
                 'vaultcontainer_add',
                 [child_container],
                 {
-                    'parent': base_container,
+                    'parent_id': base_container,
                 },
             ),
             'expected': {
@@ -371,7 +371,7 @@ class test_vault(Declarative):
                 'vaultcontainer_add',
                 [grandchild_container],
                 {
-                    'parent': base_container + u'/' + child_container,
+                    'parent_id': base_container + u'/' + child_container,
                 },
             ),
             'expected': {
diff --git a/ipatests/test_xmlrpc/test_vaultsecret_plugin.py b/ipatests/test_xmlrpc/test_vaultsecret_plugin.py
index 68f0fb0d7085be512939e397ea49abbcf3ca3c7b..cbfd231633e7c3c000e57d52d85b83f44f71df3c 100644
--- a/ipatests/test_xmlrpc/test_vaultsecret_plugin.py
+++ b/ipatests/test_xmlrpc/test_vaultsecret_plugin.py
@@ -29,7 +29,7 @@ test_vaultsecret = u'test_vaultsecret'
 binary_data = '\x01\x02\x03\x04'
 text_data = u'secret'
 
-class test_vault(Declarative):
+class test_vaultsecret_plugin(Declarative):
 
     cleanup_commands = [
         ('vault_del', [test_vault], {'continue': True}),
-- 
1.9.0



More information about the Freeipa-devel mailing list