Dne 10.6.2015 v 08:13 Martin Kosek napsal(a):
On 06/09/2015 11:13 PM, Endi Sukma Dewata wrote:
Please take a look at the attached patch to add symmetric & asymmetric vaults.
Some comments about the patch:
I think it would be better to use a new attribute type which inherits
from ipaPublicKey (ipaVaultPublicKey?) rather than ipaPublicKey directly
for assymetric vault public keys, so that assymetric public key and
escrow public key are on the same level and you can still use
ipaPublicKey to refer to either one:
ipaPublicKey
ipaVaultPublicKey
ipaEscrowPublicKey
( 2.16.840.1.113730.3.8.18.2.? NAME 'ipaVaultPublicKey' DESC
'Assymetric vault public key as DER-encoded SubjectPublicKeyInfo (RFC
5280)' SUP ipaPublicKey EQUALITY octetStringMatch SYNTAX
1.3.6.1.4.1.1466.115.121.1.40 X-ORIGIN 'IPA v4.2' )
( 2.16.840.1.113730.3.8.18.2.3 NAME 'ipaEscrowPublicKey' DESC 'IPA
escrow public key as DER-encoded SubjectPublicKeyInfo (RFC 5280)' SUP
ipaPublicKey EQUALITY octetStringMatch SYNTAX
1.3.6.1.4.1.1466.115.121.1.40 X-ORIGIN 'IPA v4.2' )
1. The vault_add was split into a client-side vault_add and server-side
vault_add_internal since the parameters are different (i.e. public key file and
future escrow-related params). Since vault_add inherits from Local all
non-primary-key attributes have to be added explicitly.
The split is not really necessary, since the only difference is the
public_key_file option, which exists only because of the lack of proper
file support in the framework. This is a different situation from
vault_{archive,retrieve}, which has two different sets of options on
client and server side. Escrow adds only ipaescrowpublickey and
escrow_public_key_file, right? If yes, we can safely keep the command in
a single piece.
2. Since the vault_archive_internal inherits from Update, it accepts all non
primary-key attributes automatically. This is incorrect since we don't want to
update these parameters during archival. Can this behavior be overridden?
Inherit from PKQuery instead (don't forget to add "has_output =
output.standard_entry").
BTW the correct solution would be to have a separate object and commands
for vault data (e.g. vaultdata object, vault_archive -> vaultdata_mod,
vault_retrieve -> vauldata_show), then we wouldn't have to deal with
mixing vault attributes with vault data and could use proper crud base
classes.
Just for the record, this changes API, right? It would be better to have this
in Alpha planned for this week. Not a blocker for Alpha though, we can give
warning that the internal API may change before GA.
--
Jan Cholasta
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
