[Freeipa-devel] [PATCH] Password vault

Endi Sukma Dewata edewata at redhat.com
Thu Mar 12 19:27:57 UTC 2015


On 3/11/2015 9:12 PM, Endi Sukma Dewata wrote:
> Thanks for the review. New patch attached to be applied on top of all
> previous patches. Please see comments below.

New patch #362-1 attached replacing #362. It fixed some issues in 
handle_not_found().

-- 
Endi S. Dewata
-------------- next part --------------
>From 6741138b647427cd3448ff72369dfc6fa21354aa 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                            | 273 ++++++++++-----------
 ipalib/plugins/vaultcontainer.py                   | 226 +++++++++--------
 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, 371 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..d47067758186601365e5924f5d13c7ab51ba66e5 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,41 @@ 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'))
+
+        raise errors.NotFound(
+            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 +627,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 +641,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 +655,33 @@ 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
+        raise errors.NotFound(
+            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 +697,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 +715,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 +728,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 +746,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 +760,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 +807,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 +815,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 +853,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 +901,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 +958,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 +980,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 +1011,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 +1019,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 +1061,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 +1121,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 +1178,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 +1208,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 +1342,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 +1352,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 +1387,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 +1409,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 +1431,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 +1450,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..27cb6fff3479335943bae59340c9afc773dfc004 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,34 @@ 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
+        raise errors.NotFound(
+            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 +394,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 +413,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 +435,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 +457,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 +477,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