Univention Bugzilla – Bug 46349
Value of userexpiry derived from shadowExpire depends on timezone
Last modified: 2024-08-21 15:34:55 CEST
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.
Successful build Package: univention-directory-manager-modules Version: 15.0.27-6 Branch: 5.0-0 Scope: errata5.0-8 c3b15d161e Bug #46349: Don't let expiry date depend on timezone We now use calendar.timegm to calculate shadowExpire as it only really evaluated to the expiry date set by the Administrator, if the system is in a "positive" timezone relative to UTC. If you are on UTC or below, the expiry date was set to a day after the actual expire date. This was visible in our Openstack test, since they use timezone UTC.
<https://errata.software-univention.de/#/?erratum=5.0x1103>