Univention Bugzilla – Bug 46349
Value of userexpiry derived from shadowExpire depends on timezone
Last modified: 2022-02-18 09:57:24 CET
Created attachment 9400 [details] utc_userexpiry_to_shadowExpire_and_sambaKickoffTime_independent_of_timezone.patch users/user converts UTC userexpiry date (format "%Y-%m-%d") to shadowExpire and sambaKickoffTime but uses time.mktime for that, which expects a localtime structure. There are two possibilities to fix this: A) keep userexpiry displayed in UTC B) show userexpiry in localtime The attached patch follows the direction of A. For usability I would prefer the latter. I'll attach an untested patch for direction B too.
Created attachment 9401 [details] localtime_userexpiry_to_shadowExpire_and_sambaKickoffTime_independent_of_timezone.patch
I'd vote for storage of all dates in UTC and input+output in localtime. Do I understand this correctly? UDM currently stores: * userexpiry in UTC * shadowExpire including the TZ adjustment of the server running UDM * sambaKickoffTime including the TZ adjustment of the server running UDM The TZ should be set as a UCR-policy on the LDAP-root to ensure consistency of UDM operations throughout the domain. Does the s4 connector use UDM to read/write those attributes, or does it read/write it directly from/to LDAP?
> I'd vote for storage of all dates in UTC and input+output in localtime. Yes, that's what my second patch (Comment 1) proposes. > Do I understand this correctly? UDM currently stores: > * userexpiry in UTC userexpiry is a UDM property that is calculated from sambaKickoffTime (or shadowExpire if that's missing), that's where the error lies. > * shadowExpire including the TZ adjustment of the server running UDM No, it's unixtime (epoch seconds) converted to day granularity. That's pretty sane. > * sambaKickoffTime including the TZ adjustment of the server running UDM Nope, also unix epoch. Same situation as with shadowExpire vs userexpiry. > The TZ should be set as a UCR-policy on the LDAP-root to ensure consistency > of UDM operations throughout the domain. In disagree, at least this should be optional. If I have servers in timezone A and servers in timezone B then I may want to use the local time in the input/output. But maybe you are right, there could be administrative situations where you want to always see the values consistently in one timezone. So, yes, I agree that much: it should be configurable to some degree. > Does the s4 connector use UDM to read/write those attributes, or does it > read/write it directly from/to LDAP? AFAIK from the top of my head it uses UDM here. Only some special things like passwords and stuff are accessed directly in OpenLDAP. Here is an example from a UCS 4.3 system: ==================================================================== root@master10:~# echo "Pacific/Samoa" > /etc/timezone root@master10:~# dpkg-reconfigure -f noninteractive tzdata Current default time zone: 'Pacific/Samoa' Local time is now: Thu Nov 3 06:37:37 SST 2016. Universal Time is now: Thu Nov 3 17:37:37 UTC 2016. root@master10:~# udm users/user create --set username=user1 \ --set lastname=name1 \ --set password=univention \ --set userexpiry="2018-03-31" root@master10:~# udm users/user list --filter uid=user3 \ | sed -n 's/ userexpiry: //p' 2018-04-01 ==================================================================== There is also a suspicious "+ 1" in the _modlist_shadow_expire method that converts from userexpiry to shadowExpire.
(In reply to Arvid Requate from comment #3) > > Do I understand this correctly? UDM currently stores: > > * userexpiry in UTC > > userexpiry is a UDM property that is calculated from sambaKickoffTime (or > shadowExpire if that's missing), that's where the error lies. > > > * shadowExpire including the TZ adjustment of the server running UDM > > No, it's unixtime (epoch seconds) converted to day granularity. That's > pretty sane. If we want to support deactivating users at daybreak in different time zones, then day granularity is not enough. By using unixtime (or IMHO better ISO 8601 → YYYY-MM-DDTHH:MM:SS+HH:MM) we could support "the user can login in Europe/Berlin until the end of 2018-03-31", as well as "the user can login in Pacific/Samoa until the end of 2018-03-31" - in the same domain. > There is also a suspicious "+ 1" in the _modlist_shadow_expire method that > converts from userexpiry to shadowExpire. I have stumbled over this already a few times when writing tests for ucsschool...
Correction, my example of Comment 3: That is an artefact of Bug 41574. So I can't currently reproduce any actual issue apart from that fact the code looks fishy. And, as Daniel points out, there is a need to consider conceptual and usage issues.
I just run into this and it took me second to understand the problem. So a quick note to my feature self and maybe it helps somebody else. This bug means that if you set a userexpiry through udm, shadowExpire will be one day less on servers using UTC+1 (or a higher timezone) compared to servers using UTC+0 (or a lower timezone). As Arvid said, time.mktime uses localtime but the unix epoche always starts at 1970-01-01_00:00:00 UTC+0 E.g.: Server uses UTC+0: >>> time.mktime(time.strptime("1970-01-01", "%Y-%m-%d")) 0.0 Server uses UTC+1: >>> time.mktime(time.strptime("1970-01-01", "%Y-%m-%d")) -3600.0 The function calculating shadowExpire always rounds down, so you end up with one day less. I like Arvids solution using "calendar.timegm", shadowExpire is only exact to a day so this solution makes sense to me and would result in the same behavior for servers in all timezones.