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

[Jan Cholasta]

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

-- 
Jan Cholasta