On 11/04/2014 07:27 AM, Endi Sukma Dewata wrote:
On 10/28/2014 5:34 PM, Endi Sukma Dewata wrote:
The NSSConnection class has to be modified not to shutdown existing
database because some of the vault clients (e.g. vault-archive and
vault-retrieve) also use a database to encrypt/decrypt the secret.
The problem is described in more detail in this ticket:
https://fedorahosted.org/freeipa/ticket/4638
The changes to the NSSConnection in the first patch caused the
installation to fail. Attached is a new patch that uses the solution
proposed by jdennis.
New patch attached. It's now using the correct OID's for the schema. It
also has been rebased on top of #352-1 and #354.
New patch attached to fix the ticket URL. It depends on #352-2 and
#354-1.
New patch attached to fix vault/container ID problems and for some
cleanups.
The new schema can go to 60basev3.ldif, no need for a new file.
ipalib/plugins/user.py: Make the try: block as small as possible; maybe
something like this:
try:
vaultcontainer_id = ...
except errors.NotFound:
pass
else:
(vaultcontainer_name, vaultcontainer_parent_id) = ...
self.api.Command.vaultcontainer_del(...)
ipapython/dn.py: This change is not needed. If you have a sequence of
RNDs you can do `DN(*seq)`.
ipalib/plugins/vault.py:
Do not use star imports in new code; remove the line
from ipalib.plugins.baseldap import *
and use e.g. `baseldap.LDAPObject` instead of just `LDAPObject`.
Hopefully the only star-imported used are the base classes?
To make translators' jobs easier, split the help text in __doc__ into
strings that can be translated individually.
Replace every blank line by:
""") + _("""
The pattern and pattern_errmsg for the 'cn' param don't match. Which one
is right? Shouldn't a similar check be there for parent?
vaultcontainer.get_dn: Why put '/' at the end of container_id, if the
empty string is ignored later anyway?
Nitpick: In vaultcontainer.get_dn, prefer the if statement over the
if-else expressions; it's a bit longer but more readable.
In vaultcontainer.get_id, what's the purpose of the len() check? Did you
want dn.endswith() instead?
I sugggest writing doctests for the id manipulation methods (get_id,
get_private_id, ...) – it's not always obvious what exactly they do.
According to the design doc, container IDs shouldn't end in '/'. If you
did that I think the manipulation would be simpler, and it could be
shared with the vault equivalents.
vaultcontainer_add: You should use ldap backend directly. Calling
Commands is costly, most of the call is spent doing validation of what
here is already validated. You should add a function to recursively
create vault container using just the ldap client, and call that here
and in vault_add.
You can delete a container with children; is that expected?
vault_add should complain if it does not get exactly one of data/text/in.
What's the difference between vault_add and vault_archive? I don't see
vault_archive in the design.
It seems '/' is equivalent to '-' as far as KRA is concerned; should we
disallow '-' in container/vault names?
You can specify an absolute id by starting it with a slash, but only in
--parent and not in the name itself. I think this should be possible in
the name too.
You can't include slashes in the name, so you always need to specify the
prefix with --parent. I don't think there's a technical reason for this
limitation.
There are no tests.
--
Petr³
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
