Dne 21.10.2014 v 13:10 Martin Kosek napsal(a):
On 10/21/2014 12:41 PM, Alexander Bokovoy wrote:
On Tue, 21 Oct 2014, Alexander Bokovoy wrote:
On Tue, 21 Oct 2014, Martin Kosek wrote:
On 10/21/2014 08:41 AM, Alexander Bokovoy wrote:
On Tue, 21 Oct 2014, Martin Kosek wrote:
On 10/20/2014 08:25 PM, Alexander Bokovoy wrote:
Hi!

This patch is for ipa-4-1 branch to enable uniqueness plugin for uid
attribute for entries with objectclass posixAccount.

We don't have uid uniqueness enforced in FreeIPA < 4.1 yet but for
posixAccounts it worked due to our design of a flat tree: as uid
attribute is
part of the DN, renaming user entries
enforces uniqueness as MODRDN will fail if entry with the same uid
already exists.

However, it is not enough for ID views -- we should be able to allow
ID view overrides for the same uid across multiple views and we should
be able to protect uid uniqueness more generally too.

Implementation is done via update plugin that checks for existing uid
uniqueness plugin and if it is missing, it will be added. If plugin
exists, its configuration will be updated.

I haven't added update specific to git master where staging subtree is
added but I'll do that after FreeIPA 4.1 release as in 4.1 we don't yet
have the staging subtree. Currently master has broken setup for uid
uniqueness plugin that doesn't actually work anyway so it will be easier
to add upgrade over properly configured entry.

https://fedorahosted.org/freeipa/ticket/4636

Hi Alexander,

Thanks for the patch! However, I am personally not very confident with
merging
it right before 4.1 release, I thought it will be a simple update definition
while this is a complex upgrade script which needs to be properly tested.

I would rather wait for 4.1.x, especially given it does not block any 4.1
major
feature in any way.
I disagree on it for multiple reasons and one of them is that 'a simple
update definition' is not right here. Attribute uniqueness plugin
supports three different types of setting its own arguments. These types
aren't mixable, you have to do switch from one to another. That's why
update plugin is the correct approach here.

The update plugin I've wrote is very simple by itself.

Ok, reviewing the patch now. I found 2 issues:

1) It is missing in plugins' Makefile.am so it won't be built properly

2) Instead of
+        if len(entries) == 0:
use
+        if entries:
You mean 'if not entries:'? Will fix.


3) I think we should force "uid uniqueness" plugin creation in all cases so
that we can expect it is there in future and for example do simple updates
for it.
It does enforce it creating already. The only case that is not handled
right now is the case of already existing multiple uid uniqueness
plugin instances which I explicitly want to handle in git master and
then merge back to 4.1 as a backport -- we only have any chance to
encounter this case with current git master.

So if existing active UID uniqueness is there, should we just remove uid from
it's list of watched attributes, give warning and add our own controlled
plugin?
No. There could be multiple uid uniqueness plugin instances, looking for
different subtrees, potentially with different top level object classes
and different object class filtering.

I am just trying to get rid of clashes with custom user configuration.
I understand that, I'll handle it in git master version. Right now we
didn't set up any of uid uniqueness and chances touching any custom
configuration like that are low, looking into my experience with
existing deployments.


4) Because of 3), when I enabled "attribute uniqueness" plugin before upgrade,
it did a strange franken-update and now I have an update plugin with DN
"cn=attribute uniqueness,cn=plugins,cn=config" but "cn: uid uniqueness: inside.
Good catch. I'll fix this part.

(BTW these concerns is where my simplicity expectations came from ;-)
Note that now you realize the whole deal is not that simple as you
thought. ;)

New patch attached.
Missed Makefile.am part.


The
+        if not entries:
path works looks ok.

However, I am still concerned about the situation when you detect existing UID
plugin.

You basically create a new LDAP entry with the same DN and then try to
overwrite the original one. I do not think this is how the LDAP framework
works. Petr/Honza, is it?

It will work, but updating the old entry object and calling update_entry on it would be nicer.

The difference is that with the current approach, the generated modlist will contain MOD_REPLACE for every attribute in the entry, whereas updating the original entry will result in a modlist with MOD_ADD/MOD_DELETE only for the attributes which were actually modified.

--
Jan Cholasta

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to