[Freeipa-devel] [PATCH] 1112 Add service constraint delegation plugin

[Martin Basti]

On 20/05/15 18:02, Rob Crittenden wrote:
> Rob Crittenden wrote:
>> Rob Crittenden wrote:
>>> Add a plugin to manage service delegations, like the one allowing the
>>> HTTP service to obtain an ldap service ticket on behalf of the user.
>>>
>>> This does not include impersonation targets, so one cannot yet limit by
>>> user what tickets can be obtained.
>>>
>>> There is also no referential integrity for the memberPrincipal 
>>> attribute
>>> since it is a string and not a DN. I don't see a way around this that
>>> isn't either clunky or requires a 389-ds plugin, both of which are
>>> overkill in this case IMHO.
>>>
>>> If you wonder why all the overrides it's because all of this is stored
>>> in the same container, and membership-like functions are used for a
>>> non-DN attribute (memberPrincipal).
>>>
>>> I used Alexander's patch in the ticket as a jumping off point.
>>
>> Removed a couple of hardcoded domain/realm elements in the tests.
>
> I must be getting rustly. Forgot to include ACIs. Added now.
>
> rob
>
>
>
Thank you.

I haven't finished review yet, but I have few notes in case you will 
modify the patch.

Please fix following issues:

1) Patch needs rebase, VERSION conflict

2)
+            pattern='^[a-zA-Z0-9_.][ a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.-]?$',
+            pattern_errmsg='may only include letters, numbers, _, -, ., 
and a space inside',
+            maxlength=255,

If I count correctly, regexp allows only 254 characters, not 255, and 
this regexp also allows the space at the end of the string.

IMHO '^[a-zA-Z0-9_.]([ a-zA-Z0-9_.-]*[a-zA-Z0-9_.-])?$' would work.

3)
There are many PEP8 errors, can you fix some of them,?

4)
Please use
except Exception as e: to be compatible with python 3

5)
For new files we stared using shorter license header.
#
# Copyright (C) 2015  FreeIPA Contributors see COPYING for license
#


-- 
Martin Basti

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150527/886c72ab/attachment.htm>