[Freeipa-devel] [PATCHES] Fix bug where List parameters were always cloned with keywords parsed from name. + Remove unused reference to old LDAP backend in join plugin. + Make delegation plugin consistent with plugin2 and use new Crud methods. + Fix DS ACI parsing. + Add ACI plugin port to new LDAP backend.

[Rob Crittenden]

Pavel Zuna wrote:
> Patch 0001: Fix bug where List parameters were always cloned with 
> keywords parsed from name.
> 
> For example, required List options were always cloned as required in 
> crud.Search.
> 
> 
> Patch 0002: Remove unused reference to old LDAP backend in join plugin.
> 
> 
> Patch 0003: Make delegation plugin consistent with plugin2 and use new 
> Crud methods.
> 
> The delegation plugin doesn't do anything at this point, but I think we 
> should get rid of the old Crud methods.
> 
> 
> Patch 0004: Fix DS ACI parsing.
> 
> Parsing of ACI strings was faulty and didn't want to make friends with 
> unicode.
> 
> 
> Patch 0005: Add ACI plugin port to new LDAP backend.
> 
> It's more of a complete rewrite actually. I dropped the showall command, 
> use aci2-find with no parameters instead.
> 
> 
> Pavel

As a separate issue to the team, we have all been doing multiple patches 
per e-mail lately. I've found this to be difficult to review from time 
to time, particularly when one part of the patch gets nacked but the 
rest are ok (but perhaps related to the nack patch). Do we want to go 
back to 1-patch-per-e-mail?

Patch 1-4 look fine. Pushed.

I think the delegation plugin is going to go away though. It just does 
the "grant write on these attributes for group A to group B". The aci 
plugin can do much more (and is more dangerous too).

Patch 5 nack.

I think we really need a ListEnum but that's another story :-)

Instead of normalize I think this should use a validate function for the 
permission types and not continue if unknown values are used. This 
silently drops unknown values so could have unknown side-effects.

You're missing the dn argument when setting the target filter:

+    if 'memberof' in kw:
+        (dn, entry_attrs) = api.Command['group2_show'](kw['memberof'])
+        a.set_target_filter('memberOf=%s')

The reason I didn't implement mod is that we don't currently have a way 
to say "remove this thing". So if you had an ACI that limited access by 
attributes there is no way to remove that. I suppose something is better 
than teasing with a command that is not implemented at all though.

I'm still not sure I want to ship this with as a general-use plugin. 
I've used it to create the current set of ACIs but it is not for the 
faint of heart, that's for sure.

rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3245 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20090602/0b8df2a2/attachment.bin>