[Freeipa-devel] [PATCH] 918, 919 update sudo schema

[Rob Crittenden]

Jan Cholasta wrote:
> Dne 13.1.2012 17:39, Rob Crittenden napsal(a):
>> Jan Cholasta wrote:
>>> Dne 14.12.2011 16:21, Rob Crittenden napsal(a):
>>>> Jan Cholasta wrote:
>>>>> Dne 14.12.2011 15:23, Rob Crittenden napsal(a):
>>>>>> Jan Cholasta wrote:
>>>>>>> Dne 14.12.2011 05:20, Rob Crittenden napsal(a):
>>>>>>>> The sudo schema now defines sudoOrder, sudoNotBefore and
>>>>>>>> sudoNotAfter
>>>>>>>> but these weren't available in the sudorule plugin.
>>>>>>>>
>>>>>>>> I've added support for these. sudoOrder enforces uniqueness because
>>>>>>>> duplicates are undefined.
>>>>>>>>
>>>>>>>> I also added support for a GeneralizedTime parameter type. This is
>>>>>>>> similar to the existing AccessTime parameter but it only handles a
>>>>>>>> single time value.
>>>>>>>
>>>>>>> You should parse the date/time part of the value with
>>>>>>> time.strptime(timestr, '%Y%m%d%H%M%S') instead of doing it manually,
>>>>>>> that way you'll get most of the validation for free.
>>>>>>
>>>>>> Yes but it gives a crappy error message, just saying that some
>>>>>> data is
>>>>>> left over not what is wrong.
>>>>>
>>>>> IMHO having a separate error message for every field in the time
>>>>> string
>>>>> (like you do in the patch) is an overkill, simple "invalid time"
>>>>> and/or
>>>>> "unknown time format" should suffice (we don't have errors like
>>>>> "invalid
>>>>> 3rd octet" for IP adresses either).
>>>>
>>>> Well, the work is done, hard to go back on a better error message.
>>>>
>>>>>>
>>>>>>> Also, it would be nice to be able to enter the value in more
>>>>>>> user-friendly format (e.g. "2011-12-14 13:01:25 +0100") and
>>>>>>> normalize
>>>>>>> that to LDAP generalized time.
>>>>>>
>>>>>> When dealing with time there are so many ways to input and display
>>>>>> the
>>>>>> same values this becomes difficult.
>>>>>>
>>>>>> I'd expect that the times for these two attributes will be relatively
>>>>>> simple and I somehow doubt users are going to want seconds, leap
>>>>>> seconds
>>>>>> or fractions, but we'll need to consider how to do it for future
>>>>>> consistency (otherwise we could have a case where time is entered in
>>>>>> one
>>>>>> format for some attributes and another for others).
>>>>>>
>>>>>> If we input in a nice way we need to output in the same way.
>>>>>
>>>>> We could make the preferred input/output time format
>>>>> user-configurable,
>>>>> defaulting to current locale time format. This format would be used
>>>>> for
>>>>> output. For input, we could go over a list of formats (first the
>>>>> user-configured format, then current locale format, then a handful of
>>>>> "standard" formats like YYYY-MM-DD HH:MM:SS) and use the first format
>>>>> that can be successfully used to parse the time string.
>>>>
>>>> See how far you get into the rabbit hole with even this simple format?
>>>
>>> I don't mind, as long as it is the right thing to do (IMHO) :)
>>>
>>> Anyway, I think this could be done on the client side, so we might use
>>> your patch without changes. However, I would prefer if the parameter
>>> class was more generic, so we could use it (hypothetically) to store
>>> time in some other way than LDAP generalized time attribute (at least
>>> name it DateTime please).
>>>
>>
>> Ok, I'm fine with that.
>
> Thanks.
>
>>
>>>>
>>>> The LDAP GeneralizedTime needs to be either in GMT or include a
>>>> differential. This gets us into the territory where the client could be
>>>> in a different timezone than the server which leads us to why we
>>>> dropped
>>>> AccessTime in the first place.
>>>
>>> Speaking of time zones, the differential alone is not a sufficient time
>>> zone description, as it doesn't account for DST. Is there a way to store
>>> time in LDAP with full time zone name (just in case it's needed sometime
>>> in future)?
>>
>> There is no way to store DST in LDAP (probably for good reason). Oddly
>> enough the older LDAP v3 RFC (2252) strongly recommends using only GMT
>> but the RFC that obsoletes it (4517) does not include this.
>
> Thanks for the info.
>
>>
>>>
>>>> So I'd like the user to supply the
>>>> timezone themselves so I don't have to guess (wrongly) and let them
>>>> worry about differing timezones.
>>>
>>> We don't have to guess, IIRC there is a way to get the local timezone
>>> differential in both Python and JavaScript, so the client could supply
>>> it automatically if necessary.
>>
>> I was thinking more about non-IPA clients (like sudo and notBefore).
>
> I think this can still be done at least in CLI, but it could be done in
> a separate patch.
>
>>
>> Updated patches attached.
>>
>> rob
>
> Patch 919 doesn't cleanly apply on current master (neither does 916 BTW).
>
> Honza
>

Rebased patch (and 916 too, separately).

rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-rcrit-918-1-generalizedtime.patch
Type: text/x-diff
Size: 9145 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120116/9c74d4e8/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-rcrit-919-2-sudo.patch
Type: text/x-diff
Size: 15846 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120116/9c74d4e8/attachment-0001.bin>