Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
Martin Kosek wrote:
On Fri, 2012-03-02 at 20:01 +0100, Jan Cholasta wrote:
On 2.3.2012 19:43, Rob Crittenden wrote:
Martin Kosek wrote:
On Fri, 2012-03-02 at 11:40 -0500, Rob Crittenden wrote:
Jan Cholasta wrote:
On 1.3.2012 20:57, Rob Crittenden wrote:
Rob Crittenden wrote:
Jan Cholasta wrote:
On 17.1.2012 04:55, Rob Crittenden wrote:
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 -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
Patch 918:
1. LDAP generalized time allows you to omit minutes from time zone
differential, your code treats such values as invalid
2. IMO a better pattern could be used, such as
u'^([0-9]{2}){5,7}([.,][0-9]+)?([-+]([0-9]{2}){1,2}|Z)$'
3. 20120229000Z has malformed minutes, but the error message says
"Malformed seconds"
4. 20120229000+ has malformed minutes, but the error message
says
"Missing operator for differential or malformed time string"
5. 20120229+ is vali
Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
On Fri, 2012-03-02 at 20:01 +0100, Jan Cholasta wrote: > On 2.3.2012 19:43, Rob Crittenden wrote: > > Martin Kosek wrote: > >> On Fri, 2012-03-02 at 11:40 -0500, Rob Crittenden wrote: > >>> Jan Cholasta wrote: > On 1.3.2012 20:57, Rob Crittenden wrote: > > Rob Crittenden wrote: > >> Jan Cholasta wrote: > >>> On 17.1.2012 04:55, Rob Crittenden wrote: > 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 -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
Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
On 2.3.2012 19:43, Rob Crittenden wrote:
Martin Kosek wrote:
On Fri, 2012-03-02 at 11:40 -0500, Rob Crittenden wrote:
Jan Cholasta wrote:
On 1.3.2012 20:57, Rob Crittenden wrote:
Rob Crittenden wrote:
Jan Cholasta wrote:
On 17.1.2012 04:55, Rob Crittenden wrote:
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 -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
Patch 918:
1. LDAP generalized time allows you to omit minutes from time zone
differential, your code treats such values as invalid
2. IMO a better pattern could be used, such as
u'^([0-9]{2}){5,7}([.,][0-9]+)?([-+]([0-9]{2}){1,2}|Z)$'
3. 20120229000Z has malformed minutes, but the error message says
"Malformed seconds"
4. 20120229000+ has malformed minutes, but the error message
says
"Missing operator for differential or malformed time string"
5. 20120229+ is valid generalized time, but it causes
"Missing
operator for differential or malfo
Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
Martin Kosek wrote:
On Fri, 2012-03-02 at 11:40 -0500, Rob Crittenden wrote:
Jan Cholasta wrote:
On 1.3.2012 20:57, Rob Crittenden wrote:
Rob Crittenden wrote:
Jan Cholasta wrote:
On 17.1.2012 04:55, Rob Crittenden wrote:
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 -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
Patch 918:
1. LDAP generalized time allows you to omit minutes from time zone
differential, your code treats such values as invalid
2. IMO a better pattern could be used, such as
u'^([0-9]{2}){5,7}([.,][0-9]+)?([-+]([0-9]{2}){1,2}|Z)$'
3. 20120229000Z has malformed minutes, but the error message says
"Malformed seconds"
4. 20120229000+ has malformed minutes, but the error message says
"Missing operator for differential or malformed time string"
5. 20120229+ is valid generalized time, but it causes "Missing
operator for differential or malformed time string" error
6. Invalid month/
Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
On Fri, 2012-03-02 at 11:40 -0500, Rob Crittenden wrote: > Jan Cholasta wrote: > > On 1.3.2012 20:57, Rob Crittenden wrote: > >> Rob Crittenden wrote: > >>> Jan Cholasta wrote: > On 17.1.2012 04:55, Rob Crittenden wrote: > > 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 -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 reaso
Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
Jan Cholasta wrote:
On 1.3.2012 20:57, Rob Crittenden wrote:
Rob Crittenden wrote:
Jan Cholasta wrote:
On 17.1.2012 04:55, Rob Crittenden wrote:
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 -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
Patch 918:
1. LDAP generalized time allows you to omit minutes from time zone
differential, your code treats such values as invalid
2. IMO a better pattern could be used, such as
u'^([0-9]{2}){5,7}([.,][0-9]+)?([-+]([0-9]{2}){1,2}|Z)$'
3. 20120229000Z has malformed minutes, but the error message says
"Malformed seconds"
4. 20120229000+ has malformed minutes, but the error message says
"Missing operator for differential or malformed time string"
5. 20120229+ is valid generalized time, but it causes "Missing
operator for differential or malformed time string" error
6. Invalid month/day combinations (such as 20120231Z) are
treated as
valid
7. When + or - i
Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
Jan Cholasta wrote:
On 1.3.2012 20:57, Rob Crittenden wrote:
Rob Crittenden wrote:
Jan Cholasta wrote:
On 17.1.2012 04:55, Rob Crittenden wrote:
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 -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
Patch 918:
1. LDAP generalized time allows you to omit minutes from time zone
differential, your code treats such values as invalid
2. IMO a better pattern could be used, such as
u'^([0-9]{2}){5,7}([.,][0-9]+)?([-+]([0-9]{2}){1,2}|Z)$'
3. 20120229000Z has malformed minutes, but the error message says
"Malformed seconds"
4. 20120229000+ has malformed minutes, but the error message says
"Missing operator for differential or malformed time string"
5. 20120229+ is valid generalized time, but it causes "Missing
operator for differential or malformed time string" error
6. Invalid month/day combinations (such as 20120231Z) are
treated as
valid
7. When + or - i
Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
On 1.3.2012 20:57, Rob Crittenden wrote:
Rob Crittenden wrote:
Jan Cholasta wrote:
On 17.1.2012 04:55, Rob Crittenden wrote:
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 -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
Patch 918:
1. LDAP generalized time allows you to omit minutes from time zone
differential, your code treats such values as invalid
2. IMO a better pattern could be used, such as
u'^([0-9]{2}){5,7}([.,][0-9]+)?([-+]([0-9]{2}){1,2}|Z)$'
3. 20120229000Z has malformed minutes, but the error message says
"Malformed seconds"
4. 20120229000+ has malformed minutes, but the error message says
"Missing operator for differential or malformed time string"
5. 20120229+ is valid generalized time, but it causes "Missing
operator for differential or malformed time string" error
6. Invalid month/day combinations (such as 20120231Z) are treated as
valid
7. When + or - is missing, the error
Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
Rob Crittenden wrote:
Jan Cholasta wrote:
On 17.1.2012 04:55, Rob Crittenden wrote:
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 -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
Patch 918:
1. LDAP generalized time allows you to omit minutes from time zone
differential, your code treats such values as invalid
2. IMO a better pattern could be used, such as
u'^([0-9]{2}){5,7}([.,][0-9]+)?([-+]([0-9]{2}){1,2}|Z)$'
3. 20120229000Z has malformed minutes, but the error message says
"Malformed seconds"
4. 20120229000+ has malformed minutes, but the error message says
"Missing operator for differential or malformed time string"
5. 20120229+ is valid generalized time, but it causes "Missing
operator for differential or malformed time string" error
6. Invalid month/day combinations (such as 20120231Z) are treated as
valid
7. When + or - is missing, the error message says "Missing operator ..."
- IMO
Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
Jan Cholasta wrote:
On 17.1.2012 04:55, Rob Crittenden wrote:
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 -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
Patch 918:
1. LDAP generalized time allows you to omit minutes from time zone
differential, your code treats such values as invalid
2. IMO a better pattern could be used, such as
u'^([0-9]{2}){5,7}([.,][0-9]+)?([-+]([0-9]{2}){1,2}|Z)$'
3. 20120229000Z has malformed minutes, but the error message says
"Malformed seconds"
4. 20120229000+ has malformed minutes, but the error message says
"Missing operator for differential or malformed time string"
5. 20120229+ is valid generalized time, but it causes "Missing
operator for differential or malformed time string" error
6. Invalid month/day combinations (such as 20120231Z) are treated as
valid
7. When + or - is missing, the error message says "Missing operator ..."
- IMO a better term than "ope
Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
On 17.1.2012 04:55, Rob Crittenden wrote:
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 -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
Patch 918:
1. LDAP generalized time allows you to omit minutes from time zone
differential, your code treats such values as invalid
2. IMO a better pattern could be used, such as
u'^([0-9]{2}){5,7}([.,][0-9]+)?([-+]([0-9]{2}){1,2}|Z)$'
3. 20120229000Z has malformed minutes, but the error message says
"Malformed seconds"
4. 20120229000+ has malformed minutes, but the error message says
"Missing operator for differential or malformed time string"
5. 20120229+ is valid generalized time, but it causes "Missing
operator for differential or malformed time string" error
6. Invalid month/day combinations (such as 20120231Z) are treated as
valid
7. When + or - is missing, the error message says "Missing operator ..."
- IMO a better term than "operator" i
Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
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 -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 >From f9fa7231ea0c03e98687116d73eb28255fd80f46 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Fri, 13 Jan 2012 11:06:42 -0500 Subject: [PATCH] Add Generalized Time parameter type and unit test Per RFC 4517. https://fedorahosted.org/freeipa/ticket/1314 --- ipalib/__init__.py |2 +- ipalib/parameters.py | 133 ++ tests/test_ipalib/test_parameters.py | 49 + 3 files changed, 183 insertions(+), 1 deletions(-) diff --git a/ipalib/__init__.py b/ipalib/__init__.py index 29ba0bb..040d581 100644 --- a/ipalib/__init__.py +++ b/ipalib/__init__.py @@ -879,7 +879,7 @@ from frontend import Command, LocalOrRemote, Updater from frontend import Object, Method, Property from crud import Create, Retrieve, Update, Delete, Search from parameters imp
Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
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 -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 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
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 -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. 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. 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). Updated patches attached. rob >From f9fa7231ea0c03e98687116d73eb28255fd80f46 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Fri, 13 Jan 2012 11:06:42 -0500 Subject: [PATCH] Add Generalized Time parameter type and unit test Per RFC 4517. https://fedorahosted.org/freeipa/ticket/1314 --- ipalib/__init__.py |2 +- ipalib/parameters.py | 133 ++ tests/test_ipalib/test_parameters.py | 49 + 3 files changed, 183 insertions(+), 1 deletions(-) diff --git a/ipalib/__init__.py b/ipalib/__init__.py index 29ba0bb..040d581 100644 --- a/ipalib/__init__.py +++ b/ipalib/__init__.py @@ -879,7 +879,7 @@ from frontend import Command, LocalOrRemote, Updater from frontend import Object, Method, Property from crud import Create, Retrieve, Update, Delete, Search from parameters import DefaultFrom, Bool, Flag, Int, Float, Bytes, Str, IA5Str, Password -from parameters import BytesEnum, StrEnum, AccessTime, File +from parameters import BytesEnum, StrEnum, AccessTime, File, DateTime from errors import SkipPluginModule from text import _, ngettext, GettextFactory, NGettextFactory diff --git a/ipalib/param
Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
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 -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). 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)? 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. rob Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
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 -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? 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. 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. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
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). 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 -MM-DD HH:MM:SS) and use the first format that can be successfully used to parse the time string. rob Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
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. 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. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 918, 919 update sudo schema
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. 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. The sudo patch relies on my patch 916 or you'll have merge issues. rob Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
