Bug 45842 - Simplify user options (posix, samba, kerberos)
Simplify user options (posix, samba, kerberos)
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: UMC - Users
UCS 4.2
Other Linux
: P5 normal (vote)
: UCS 4.3
Assigned To: Arvid Requate
Felix Botner
: interim-2
: 34481 44582 46262 (view as bug list)
Depends on: 45877
Blocks: 46116 46172 46174 46193 46211 46357 47377 49018
  Show dependency treegraph
 
Reported: 2017-12-11 14:28 CET by Florian Best
Modified: 2019-04-29 18:48 CEST (History)
8 users (show)

See Also:
What kind of report is it?: Development Internal
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional): API change, Cleanup, Usability
Max CVSS v3 score:


Attachments
print all possible option combinations (320 bytes, text/x-python)
2017-12-11 15:15 CET, Florian Best
Details
Change structuralObjectClass with OpenLDAP (681 bytes, text/plain)
2017-12-19 09:55 CET, Philipp Hahn
Details
ldif nonactive shared mailbox (760 bytes, text/x-ldif)
2017-12-21 09:16 CET, Erik Damrose
Details
ldif contact (556 bytes, text/x-ldif)
2017-12-21 09:16 CET, Erik Damrose
Details
create_all_users.py (2.00 KB, text/x-python)
2018-01-31 16:22 CET, Florian Best
Details
patch for group loading (4.03 KB, patch)
2018-02-19 08:54 CET, univention
Details | Diff
check if OC already exists, before adding it (985 bytes, patch)
2018-03-01 12:18 CET, Daniel Tröder
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Best univentionstaff 2017-12-11 14:28:14 CET
Maintaining multiple variants of users is too complex, has bugs and can be simplified.

Therefore we should:

1. unify the UDM options "samba", "posix", "kerberos" and remove them: They are always activated.
2. split the "ldap_pwd" option into a new UDM object type e.g. users/ldap.
3. create a new UDM object type for an address book entry.

This is an API change of UDM and we have to write a migration script which modifies the LDAP in a way that UDM can handle the new user format.
Comment 1 Florian Best univentionstaff 2017-12-11 15:15:07 CET
Created attachment 9314 [details]
print all possible option combinations
Comment 2 Florian Best univentionstaff 2017-12-12 13:24:19 CET
We have the following options, with the object classes:

Posix:
posixAccount, shadowAccount, person, univentionPWHistory

Samba:
sambaSamAccount, person, univentionPWHistory

Kerberos:
krb5Principal, krb5KDCEntry, person, univentionPWHistory

Mail:
shadowAccount, univentionMail, person, univentionPWHistory

Person:
organizationalPerson, inetOrgPerson, person, univentionPWHistory

PKI:
pkiUser, person, univentionPWHistory

LDAP_Pwd:
simpleSecurityObject, uidObject, person, univentionPWHistory
Comment 3 Florian Best univentionstaff 2017-12-12 13:27:52 CET
There are 120 possible combinations of different user types.

A migration scenario must have at least these constraints:
* All primary groups of a user must have the attribute sambaSid.
* The domain must have a Kerberos Realm: 'krb5RealmName' attribute must be on the LDAP base
Comment 4 Florian Best univentionstaff 2017-12-12 15:16:10 CET
To further simplify the user we had the idea to:

* always enable the "mail" option (objectclass univentionMail with only "uid" as MUST attribute)
→ we have to check if this is possible, e.g. that listener modules can handle the modified objects.
→ Are there conflicts with Apps, e.g. OX?

* always enable the "personal information" option (object classes organizationalPerson, inetOrgPerson)
→ This introduces only the possibility for some more attributes
→ It possibly simplifies extended attributes, which don't need to depend on the 'person' option anymore.
Comment 5 Florian Best univentionstaff 2017-12-12 19:38:38 CET
The ldap modlist also sets the object classes:
* automount (if a "homeShare" or "homeSharePath" is set)
* univentionPerson (if "umcProperty" or "bithday" is set)
* univentionSambaPrivileges (if 'sambaPrivileges' is set)
Comment 6 Florian Best univentionstaff 2017-12-13 14:14:49 CET
A migration script has been written:
https://git.knut.univention.de/univention/ucs/blob/fbest/45842-simplify-user-options/management/univention-directory-manager-modules/univention-migrate-users-to-ucs4.3

It has a --check parameter for a dry run which lists all affected users and their modifications.
It also has a --set-kerberos-realm parameter which sets the krb5RealmName on the LDAP Base.
Comment 7 Florian Best univentionstaff 2017-12-18 12:04:06 CET
(In reply to Florian Best from comment #4)
> To further simplify the user we had the idea to:
> * always enable the "personal information" option (object classes
> organizationalPerson, inetOrgPerson)
> → This introduces only the possibility for some more attributes
> → It possibly simplifies extended attributes, which don't need to depend on
> the 'person' option anymore.

I am unsure if this is possible. In a short test I could not modify the structural object class of a user:
univention.admin.uexceptions.ldapError:
Cannot modify object class: structural object class modification from 'person' to 'inetOrgPerson' not allowed
Comment 8 Philipp Hahn univentionstaff 2017-12-19 09:55:04 CET
Created attachment 9320 [details]
Change structuralObjectClass with OpenLDAP

(In reply to Florian Best from comment #7)
> I am unsure if this is possible. In a short test I could not modify the
> structural object class of a user:
> univention.admin.uexceptions.ldapError:
> Cannot modify object class: structural object class modification from
> 'person' to 'inetOrgPerson' not allowed

This is forbidden by LDAP.
But there was <https://tools.ietf.org/html/draft-zeilenga-ldap-managedit-00>, which has been been superseded by the "relay" extension: <https://tools.ietf.org/html/draft-zeilenga-ldap-relax-03>

# ~phahn/misc/LDAP/ldap_features 
C 2.16.840.1.113730.3.4.18       Proxy Authorization Control [RFC4370]
C 2.16.840.1.113730.3.4.2        ManageDsaIT [RFC3296]
C 1.3.6.1.4.1.4203.1.10.1        Subentries [RFC3672]
C 1.3.6.1.1.22                   LDAP Don't Use Copy Control [RFC6171]
C 1.2.840.113556.1.4.319         LDAP Simple Paged Results Control [RFC2696]
C 1.2.826.0.1.3344810.2.3        Matched Values Control [RFC3876]
C 1.3.6.1.1.13.2                 LDAP Post-read Control [RFC4527]
C 1.3.6.1.1.13.1                 LDAP Pre-read Control [RFC4527]
C 1.3.6.1.1.12                   Assertion Control [RFC4528]
E 1.3.6.1.4.1.1466.20037         StartTLS [RFC4511][RFC4513]
E 1.3.6.1.4.1.4203.1.11.1        Modify Password [RFC3062]
E 1.3.6.1.4.1.4203.1.11.3        Who am I? [RFC4532]
E 1.3.6.1.1.8                    Cancel Operation [RFC3909]
F 1.3.6.1.1.14                   Modify-Increment [RFC4525]
F 1.3.6.1.4.1.4203.1.5.1         All Op Attrs [RFC3673]
F 1.3.6.1.4.1.4203.1.5.2         OC AD Lists [RFC4529]
F 1.3.6.1.4.1.4203.1.5.3         True/False filters [RFC4526]
F 1.3.6.1.4.1.4203.1.5.4         Language Tag Options [RFC3866]
F 1.3.6.1.4.1.4203.1.5.5         Language Range Options [RFC3866]

Even 1.3.6.1.4.1.4203.666.5.12 is missing, but it still works with OpenLDAP-2.4.
Comment 9 Florian Best univentionstaff 2017-12-20 18:58:54 CET
(In reply to Philipp Hahn from comment #8)
> Created attachment 9320 [details]
> Change structuralObjectClass with OpenLDAP
Thank you, this works like a charm.
Comment 10 Florian Best univentionstaff 2017-12-20 19:52:27 CET
So far, the basis in UDM is implemented.
I adjusted all tests from ucs-test-udm-users. They are all passing.

TODO: write more tests, we could have a greater test coverage.

TODO: call the migration script in preup.sh. How to do this? The preup.sh is downloaded dynamically. The migration script is part of the UDM package in UCS 4.3.

TODO: test what happens in the S4 and AD connector during the migration of objects from UCS 4.2:
* object classes get added to/removed from user objects which were previously read-only after modification. Can the code handle it?

TODO: The UMC aspects need to be considered:

When opening the "users" module now in UMC we only see users/user types.
If we want to show also users/ldap (and the maybe following users/addressbook-entry) we have to rename the flavor "users/user" e.g. into "users/all" and create a new virtual UDM module for it.
This has several things which needs to be considered:
* All UCR variables which are referring to the users/user flavor needs to be rewritten
* The UMC ACL's needs to be migrated/modified to allow the flavor users/all instead of users/user
* The favorites (in UCR variables and on the users ldap objects) needs to be adjusted
* There are some links in the self-service app description which use the #module=users/user hash.
* Various hardcoded checks for users/user in the javascript.

TODO: Other questions:
Do we want to allow user templates for users/ldap?
→ I don't think so.

Should it be possible to create a report for users/ldap objects?
→ I don't think so.
Comment 11 Erik Damrose univentionstaff 2017-12-21 09:09:37 CET
A quick thought to consider: In Kopano there are also special user account types, for contacts (addressbook entries) and shared mailboxes. These are currently designed to have some user specific features, but do not and should not consume a UCS user licence. How do these objects look like after the migration.
Comment 12 Erik Damrose univentionstaff 2017-12-21 09:16:18 CET
Created attachment 9322 [details]
ldif nonactive shared mailbox

ldif nonactive shared mailbox
Comment 13 Erik Damrose univentionstaff 2017-12-21 09:16:32 CET
Created attachment 9323 [details]
ldif contact

ldif contact
Comment 14 Florian Best univentionstaff 2017-12-21 09:58:38 CET
(In reply to Erik Damrose from comment #11)
> A quick thought to consider: In Kopano there are also special user account
> types, for contacts (addressbook entries) and shared mailboxes. These are
> currently designed to have some user specific features, but do not and
> should not consume a UCS user licence. How do these objects look like after
> the migration.

Thanks! I added a check if univentionObjectType contains users/user (if it's set). Otherwise the objects aren't modified anymore.
Comment 15 Florian Best univentionstaff 2018-01-22 20:05:14 CET
*** Bug 34481 has been marked as a duplicate of this bug. ***
Comment 16 Florian Best univentionstaff 2018-01-24 01:15:30 CET
*** Bug 44582 has been marked as a duplicate of this bug. ***
Comment 17 Florian Best univentionstaff 2018-01-24 10:33:10 CET
In the current state the following tests are failing:
 01_base.47faillog_account-smb-krb.test
 10_ldap.50ppolicy_account_lockout.test
 60_umc.03_acls.test
 60_umc.07_expired_password.test (fails in 4.3-0 as well, Bug #46126)
 60_umc.105_change_expired_password_fail_reason.test (fails in 4.3-0 as well, Bug #46126)
 60_umc.23_udm_create_user.test
 60_umc.33_umc_users_user_unset_userexpiry.test
 60_umc.80_udm_license.test
 71_udm-settings.11_use_usertemplate.test
(and some more which aren't related I think).
Comment 18 Florian Best univentionstaff 2018-01-24 17:09:18 CET
(In reply to Florian Best from comment #17)
> In the current state the following tests are failing:
>  01_base.47faillog_account-smb-krb.test
OK, fails in UCS 4.3-0, too
>  10_ldap.50ppolicy_account_lockout.test
This is caused by Bug #45877. I will fix it soon.
>  60_umc.03_acls.test
TODO: cannot be reproduced, the log output doesn't offer enough information (authentication fails)
I added more debug output and switched to the python-lib. If it still fails, we will see better why.
>  60_umc.07_expired_password.test (fails in 4.3-0 as well, Bug #46126)
OK, fails in UCS 4.3-0, too
>  60_umc.105_change_expired_password_fail_reason.test (fails in 4.3-0 as
> well, Bug #46126)
OK, fails in UCS 4.3-0, too
>  60_umc.23_udm_create_user.test
fixed, was a typo
>  60_umc.33_umc_users_user_unset_userexpiry.test
fixed, userexpiry=None is now also sent to the frontend.
>  60_umc.80_udm_license.test
OK, the license on the System is reached because I added some user objects
>  71_udm-settings.11_use_usertemplate.test
fixed, kerberos flags were missing as the user has now also the kerberos option
> (and some more which aren't related I think).
Comment 19 Florian Best univentionstaff 2018-01-24 17:23:54 CET
The current changes have been merged into the UCS 4.3-0 branch:

changelog-4.3-0.xml
71e515b8592e | Bug #45842: Add Changelog entry

univention-management-console-module-diagnostic (4.0.0-4)
62aa67db9d0f | Bug #45842: Merge branch 'fbest/45842-simplify-user-options' into 4.3-0
3549c54e4f62 | Bug #45842: remove user options posix, samba, kerberos and merge them into one default option. split ldap_pwd option into users/ldap object type.

univention-appcenter (7.0.1-5)
62aa67db9d0f | Bug #45842: Merge branch 'fbest/45842-simplify-user-options' into 4.3-0
3549c54e4f62 | Bug #45842: remove user options posix, samba, kerberos and merge them into one default option. split ldap_pwd option into users/ldap object type.

ucs-test (8.0.15-1)
c24cc062d8f9 | Bug #45842: compare the whole dict at once in verify_udm_object
3466185a91aa | Bug #45842: fix timezone problem?
3aa17a07d596 | Bug #45842: fix certificate check

univention-directory-manager-modules (13.0.2-1)
62aa67db9d0f | Bug #45842: Merge branch 'fbest/45842-simplify-user-options' into 4.3-0
3549c54e4f62 | Bug #45842: remove user options posix, samba, kerberos and merge them into one default option. split ldap_pwd option into users/ldap object type.

univention-updater (13.0.1-6)
62aa67db9d0f | Bug #45842: Merge branch 'fbest/45842-simplify-user-options' into 4.3-0
3549c54e4f62 | Bug #45842: remove user options posix, samba, kerberos and merge them into one default option. split ldap_pwd option into users/ldap object type.

univention-management-console-module-udm (8.0.1-0)
62aa67db9d0f | Bug #45842: Merge branch 'fbest/45842-simplify-user-options' into 4.3-0
3549c54e4f62 | Bug #45842: remove user options posix, samba, kerberos and merge them into one default option. split ldap_pwd option into users/ldap object type.

ucs-test (8.0.16-10)
702aa5e391fe | Bug #45842: mark tests x-fail

NONE
e5102730f968 | Bug #45842: adjust app tutorial
3367e466b87c | Bug #45842: adjust documentation about user options
03776818bb7a | Bug #45842: temporarily disable adding of users because some tests fail due to license is reached
c09bd8550b48 | Bug #45842: reenable updater tests
721fdaec1e65 | Bug #45842: dont run unstable updater tests and longrunning docker tests
610bd029406e | Bug #45842: disable coverage testing
80aa24908963 | Bug #45842: fix execution of migration script
abb4420cf0d6 | Bug #45842: skip test which takes two hours
605001c1c620 | Bug #45842: enhance monkeypatching
844ec192ac20 | Bug #45842: copy monkeypatch.py correctly
644dc7a97a9b | Bug #45842: upgrade to fbest scope

ucs-test (8.0.16-5)
a93e754024d3 | Bug #45842: support list and strings for --append-option

univention-directory-manager-modules (13.0.1-1)
6b3892e6477a | Bug #45842: krb5KDCFlags is a bitmap, treat it like one to not destroy specific possible flags
582114ae8865 | Bug #45842: remove unused SVG files, which are part of package univention-management-console-module-udm
080f3b606816 | Revert "Bug #45842: add users/all flavor"
23ead0fbc9d2 | Bug #45842: ensure that the primary group is a samba group when modifying it
20480bcd54db | Bug #45842: copyright 2018
99d5c8d4f4f0 | Bug #45842: merge base and simpleLdap
b98d7b6d510d | Bug #45842: add user migration diagnose script
1ef3f194e34c | Bug #45842: don't use self.remove() in move() as it modifies the instance
5ff0f9df750f | Bug #45842: fix typo in migration script
c29aa83a1b10 | Bug #45842: object argument in replace_pattern might be a dict
04888b70fdd8 | Bug #45842: enhance debug messages
76a92a5e1747 | Bug #45842: execute migration script only on DC Master
1e360125cbd5 | Bug #45842: be more verbose, fix typo
be8ee24ca402 | Bug #45842: use univention.admin.modules.options for modules without defined options
15baa1108a77 | Bug #45842: only lock username if changed
92ad5dcb938c | Bug #45842: fix typos
d75ce5dc60cc | Bug #45842: install migration script
1756c9334b95 | Bug #45842: migrate primary group to samba group
aabe27d92d94 | Bug #45842: migrate option setting in admincli/adduser
6a439aa08316 | Bug #45842: migrate options of usertemplates
6779c2871b44 | Bug #45842: remove options from users/passwd
856726cd48fe | Bug #45842: add users/ldap to univentionAdminModules syntax class
845be671b38c | Bug #45842: do not crash if firstname="" and lastname=" "
5bf425a898de | Bug #45842: ignore copano-users in the migration
79f33e23d6cf | Bug #45842: add objectFlag extended attribute for users/ldap
fb9160b97fdb | Bug #45842: add users/all flavor
5c432c520a99 | Bug #45842: use relaxed LDAP control to modify structural object class of user
32af54e4cb0f | Bug #45842: if shadowLastChange is 0 then pwdChangeNextLogin is true
f8f09c295361 | Bug #45842: move kerberos password end block to the bottom
5c6fe95335e1 | Bug #45842: removed unused variable self.newPrimaryGroupDn and self.oldPrimaryGroupDn
d984d6e2d071 | Bug #45842: move sambaSID into modlist
5ef4c44a8459 | Bug #45842: move krb5PrincipalName into modlist
4cd10477eaad | Bug #45842: add icon for users/ldap objects
f3a883df191a | Bug #45842: add required attributes of person object class
f2a2fe40e90a | Bug #45842: fix updating of displayName
b028b7ba45fc | Bug #45842: enhance cancel() behavior
52c509a4bc51 | Bug #45842: fix typo when resolving exception name
0d5ecbe0d54d | Bug #45842: set object class only of not exists
6803964a8e2c | Bug #45842: fix import error of users.ldap
44e35ebf1128 | Bug #45842: replace deprecated has_key()
72822fd6e75d | Bug #45842: fix ucslint warnings
c61fe3b3fafd | Bug #45842: fix missing debhelper in postrm
98d10639969a | Bug #45842: rename baseConfig to ucr
66dcd52d344e | Bug #45842: replace has_key()
0b44cae1fe82 | Bug #45842: migration script cannot change structural object class
979b49a36882 | Bug #45842: split unmapping of "disabled" into functions
5a80f66392ae | Bug #45842: move _ldap_addlist to "default" option
08c60aec920e | Bug #45842: sambaAcctFlags do not need to be updated if they did not change, e.g. because only the password changed
513e8a194393 | Bug #45842: reduce logic of krb5PasswordEnd/sambaPwdLastSet / shadowLastChange
def5f1a98471 | Bug #45842: unify setting of krb5PasswordEnd in modlist
840bd1d89394 | Bug #45842: move krb5PasswordEnd and shadowLastChange logic into one common block
03b8b5f13767 | Bug #45842: simplify duplicated krb5KDCFlags logic
28a32304723d | Bug #45842: simplify password policy checks
257207c88217 | Bug #45842: remove code redundancy when loading password history policy object
eb5e998c0104 | Bug #45842: locks are already released in self.cancel()
0dee1ae34e06 | Bug #45842: merge __modlist() and __modlist2()
bf0072c5c18b | Bug #45842: split modlist into small functions
6a323047a7d9 | Bug #45842: unify setting of shadowMax
53bc091d40fb | Bug #45842: move self.modifypassword logic into the place where it is used
6aa4fcaecf8f | Bug #45842: add comment about ignore_license in modify()
1c652aeb8e8a | Bug #45842: removed unused syntax class
1fe380f51f69 | Bug #45842: fix various unescaped filter strings
189da82ca524 | Bug #45842: make it possible to escape filter expressions
8463a56c0fd7 | Bug #45842: fine tuning of users/user option constellation
98619300aaaf | Bug #45842: add object classes of the default option always
5c3db566f5cb | Bug #45842: account deactivation is not searchable anymore
1157064c18c4 | Bug #45842: use generic unmapped_lookup_filter()
8c644509c7be | Bug #45842: use generic unmapped_lookup_filter()
78cb306ed7fc | Bug #45842: lookup_filter() now can apply the map rewriting by itself
f954bf44821b | Bug #45842: adjust lookup() and identify() function
2d8b829b468e | Bug #45842: move password locking utilities into univention.admin.password module
4273c07c53ec | Bug #45842: use generic lookup() methods
e8e1887d00fc | Bug #45842: cleanup unsued imports
580776885a7a | Bug #45842: fix password mapping during create()
7b6abacef1e7 | Bug #45842: move locking of uid into common pre_ready method
7c561ea8d71c | Bug #45842: check prohibited usernames also when renaming objects
51c2bd3420e5 | Bug #45842: implement password changes for users/ldap
3e745f35d8b9 | Bug #45842: cleanup users/ldap skeleton
e9973ab2ce64 | Bug #45842: fork users/ldap from users/user
81e7fcfd43a4 | Bug #45842: add user migration script
f568d94f0a54 | Bug #45842: fix releasing of username lock if renaming user fails
2a0e2e5a6604 | Bug #45842: unify kerberos principal name generation
339351d85cf9 | Bug #45842: has_key() migration
c84a9839324b | Bug #45842: remove unnecessary univention.debug.function call
67a2ebb5fd5c | Bug #45842: add mapping.registerUnmapping()
2c7e75dd6a97 | Bug #45842: move unmap pwdChangeNextLogin into method
05a8fc7aadc1 | Bug #45842: move loading of groups into method
c63c3eb4909a | Bug #45842: move unmap mail forward copy into method
09bd0c526880 | Bug #45842: move unmap automount information into method
db0a45b39584 | Bug #45842: unmap password
e3955975caba | Bug #45842: cleanup
541e9313977c | Bug #45842: unmap locked
d82f44cdbd08 | Bug #45842: implement unmap functions
ce53ba87f413 | Bug #45842: prevent non-changes in the modlist
1322cc44f300 | Bug #45842: cancel() if _ldap_pre_ready() fails
0f19deeafddd | Bug #45842: fix removal of "automount" object class when attributes are removed
34af187e8408 | Bug #45842: remove dead code: univentionPWHistory is always set
29848eb946e9 | Bug #45842: move locking/releasing into simpleLDAP and unify mailPrimaryAdress locking
acdd131a02b1 | Bug #45842: enhance docstring
de702ae24ed6 | Bug #45842: remove dead impossible code
0fd855478ac4 | Bug #45842: add generic function to confirm locks after creation
0ac5177d5dc4 | Bug #45842: raise exception if no kerberos realm is set up
ab5fc06e0a53 | Bug #45842: move lock allocations into _ldap_pre_create()
d9d013961ff0 | Bug #45842: move lock allocations into _ldap_pre_create()
7ace1891a35e | Bug #45842: assign uidNumber property directly
99ad3c3dff46 | Bug #45842: let object classes automatically be added
92f613674fd1 | Bug #45842: automatically map firstname and lastname
852b9e7dc67c | Bug #45842: call self.cancel() in simpleLDAP if _ldap_addlist / _ldap_modlist fails
86f5e54b2e00 | Bug #45842: remove unnecessary if blocks
5e67a1e9b2ca | Bug #45842: fix error message if object cannot be created (due to invalid option combination) and --ignore_exists was given.
73952e847572 | Bug #45842: copy users/user
b60c0ef1332d | Bug #45842: test loading of user certificate

ucs-test (8.0.13-1)
d371f627a11e | Bug #45842: fix certificate check

univention-appcenter (7.0.1-4)
5abe9086e6cc | Bug #45842: adjust appcenter joinscript helper

ucs-test (8.0.19-1)
62aa67db9d0f | Bug #45842: Merge branch 'fbest/45842-simplify-user-options' into 4.3-0
3549c54e4f62 | Bug #45842: remove user options posix, samba, kerberos and merge them into one default option. split ldap_pwd option into users/ldap object type.

ucs-test (8.0.18-1)
2275d76d6fd8 | Bug #45842: tested user is now a kerberos user
9b926ca1fd83 | Revert "Bug #45842: mark tests x-fail"
2753a50be20d | Bug #45842: revert using users/all flavor
f6c8643f82ef | Bug #45842: fix tested object type of sys-idp-user in 82_saml/01_saml_user
fbf8e9113dac | Bug #45842: adjust test cases
9a19ddbeb769 | Bug #45842: fix test case
26c754d05fda | Bug #45842: fix test cases
3bee604a92a3 | Bug #45842: adjust test cases
9dd25293ba38 | Bug #45842: adjust test cases
b06e0b4a0e94 | Bug #45842: migrate user options in UMC tests
1f8d735e3bcf | Bug #45842: migrate user options in 71_udm-settings/11_use_usertemplate
0ccac40a08aa | Bug #45842: migrate user options in 60_umc/06_udm_non_ucr_policies
aaf4c008ce4f | Bug #45842: migrate user options in 49_nfs tests
179fb5579b28 | Bug #45842: migrate user options in 40_mail tests
61a5e5f0abbf | Bug #45842: mark test as fixed
86bd694bcf9c | Bug #45842: adjust test case for users/ldap account
c0512ec59d81 | Bug #45842: remove obsolete test
f0def9c3cdc1 | Bug #45842: make test case more verbose
6eed80f1eddc | Bug #45842: be more verbose
5bb572903bc9 | Bug #45842: fix encoding of data
2416d456c1cd | Bug #45842: adjust test case to accept None/empty in the response of userexpiry
43b83a02df44 | Bug #45842: fix typo in test case

ucs-test (8.0.10-2)
06ce433e6285 | Bug #45842: implement test_modlist_univention_person
3c8d68ad26c7 | Bug #45842: implement test_modlist_krb_principal
9e19d70efc67 | Bug #45842: implement test_modlist_display_name
3fecf0b90af6 | Bug #45842: implement test_modlist_gecos
fa7abbd136c8 | Bug #45842: implement test_modlist_cn
953fbf612f47 | Bug #45842: implement test_modlist_samba_privileges
b41126e983a9 | Bug #45842: implement test_kerberos_values_are_set
8d0fd2529ff9 | Bug #45842: implement test_modification_of_username
3d8480235ae4 | Bug #45842: implement test_prohibited_username_are_checked
b60c0ef1332d | Bug #45842: test loading of user certificate
4eb3b04b84fa | Bug #45842: test unmapping of automountInformation / homeShare / homeSharePath
da70aff3de28 | Bug #45842: add udm.verify_udm_object()
068a32139e27 | Bug #45842: test unmap of pwdChangeNextLogin in open()

univention-directory-manager-modules (13.0.0-4)
194e5c02c981 | Bug #45842: fix typo in german translation

ucs-test-tools (7.0.0-2)
62aa67db9d0f | Bug #45842: Merge branch 'fbest/45842-simplify-user-options' into 4.3-0
8cdea129f28b | Bug #45842: migrate create-32k-users-in-groups

changelog-3.2.xml
r45842 | added changelog entry for Bug #30811

univention-updater (13.0.1-5)
59ab8ce934df | Bug #45842: call migration script in postup.sh

univention-saml (5.0.2-3)
7694aa450db3 | Bug #45842: adjust creation of SAML sys-idp-user

ucs-test (8.0.8-1)
447873dde429 | Bug #45842: move verify_udm_object into univention.testing.udm

univention-management-console-module-diagnostic (4.0.0-3)
b98d7b6d510d | Bug #45842: add user migration diagnose script

univention-management-console-module-udm (8.0.0-2)
22c4ff78a943 | Bug #45842: add icon for users/ldap
080f3b606816 | Revert "Bug #45842: add users/all flavor"
1d3dc38a07c8 | Revert "Bug #45842: reloading of detail page is currently broken due to wrong object type detection"
643eb0e865b3 | Bug #45842: reloading of detail page is currently broken due to wrong object type detection
fb9160b97fdb | Bug #45842: add users/all flavor
98619300aaaf | Bug #45842: add object classes of the default option always

univention-saml (5.1.2-3)
62aa67db9d0f | Bug #45842: Merge branch 'fbest/45842-simplify-user-options' into 4.3-0
3549c54e4f62 | Bug #45842: remove user options posix, samba, kerberos and merge them into one default option. split ldap_pwd option into users/ldap object type.

ucs-test (8.0.11-1)
b80404404687 | Bug #45842: add 61_udm-users/100_test_users.py
Comment 20 Florian Best univentionstaff 2018-01-29 16:22:14 CET
Can we remove the script services/univention-samba/scripts/kerberize_user?
Comment 21 Florian Best univentionstaff 2018-01-29 16:22:47 CET
TODO: have a look at saml/univention-saml/simplesamlphp-modules/uldap/lib/Auth/Source/uLDAP.php
Comment 22 Florian Best univentionstaff 2018-01-29 16:24:46 CET
TODO: the migration script doesn't consider, that user might be disabled/locked. We should lock them e.g. in kerberos if that is the case.
Comment 23 Florian Best univentionstaff 2018-01-29 19:12:05 CET
(In reply to Florian Best from comment #22)
> TODO: the migration script doesn't consider, that user might be
> disabled/locked. We should lock them e.g. in kerberos if that is the case.

I modified the migration script to include all attributes not only the MUST attributes:

* 'sambaAcctFlags'
* 'krb5KDCFlags'
* 'krb5MaxLife'
* 'krb5MaxRenew'
* 'krb5ValidEnd'
* 'krb5Key' (calculated from sambaNTPassword)
* 'krb5PasswordEnd'
* 'userPassword' (set to {KINIT})
* 'shadowExpire'
* 'shadowLastChange'
* 'sambaNTPassword' (calculated from krb5Key)
* 'sambaAcctFlags'
* 'sambaPwdLastSet'
* 'sambaKickoffTime'

I don't know any way to set userPassword from any information in sambaNTPassword or krb5Key. Therefore it's set to "{KINIT}".

I don't know any way to set krb5Key from userPassword. So if the user has only shadowAccount/posixAccount but not sambaSamAccount, krb5Key will be empty.

I don't know any way to set sambaNTPassword from userPassword. So if the user has only shadowAccount/posixAccount but not krb5KDCEntry, sambaNTPassword will be empty.
Comment 24 Stefan Gohmann univentionstaff 2018-01-30 07:12:41 CET
(In reply to Florian Best from comment #23)
> (In reply to Florian Best from comment #22)
> > TODO: the migration script doesn't consider, that user might be
> > disabled/locked. We should lock them e.g. in kerberos if that is the case.
> 
> I modified the migration script to include all attributes not only the MUST
> attributes:
> 
> * 'sambaAcctFlags'
> * 'krb5KDCFlags'
> * 'krb5MaxLife'
> * 'krb5MaxRenew'
> * 'krb5ValidEnd'
> * 'krb5Key' (calculated from sambaNTPassword)
> * 'krb5PasswordEnd'
> * 'userPassword' (set to {KINIT})
> * 'shadowExpire'
> * 'shadowLastChange'
> * 'sambaNTPassword' (calculated from krb5Key)
> * 'sambaAcctFlags'
> * 'sambaPwdLastSet'
> * 'sambaKickoffTime'
> 
> I don't know any way to set userPassword from any information in
> sambaNTPassword or krb5Key. Therefore it's set to "{KINIT}".

Please use {K5KEY} if the user has the krb5Key attribute set.
Comment 25 Florian Best univentionstaff 2018-01-31 14:29:06 CET
The policy "policies/umc" can now be applied to users/ldap users as well. Are there any futher policies we should allow for users/ldap? What about "policies/pwhistory"?
Comment 26 Florian Best univentionstaff 2018-01-31 16:22:47 CET
Created attachment 9376 [details]
create_all_users.py

Attached is a script which creates user objects in UCS 4.2 with all possible combinations of options, disabled and locked.
It might be usefull for the QA of the migration script.
Comment 27 Florian Best univentionstaff 2018-01-31 17:33:21 CET
(In reply to Stefan Gohmann from comment #24)
> (In reply to Florian Best from comment #23)
> > I don't know any way to set userPassword from any information in
> > sambaNTPassword or krb5Key. Therefore it's set to "{KINIT}".
> 
> Please use {K5KEY} if the user has the krb5Key attribute set.

Okay, this has been changed.
Comment 28 Florian Best univentionstaff 2018-01-31 17:59:43 CET
Summary of changes:

* the user options posix, samba, kerberos have been unified into one invisible option called "default".
* the "default" option is now always evaluated/attached by UDM. Therefore no module needs to define a _ldap_addlist() anymore. In the future we might automatically build ldap-filters from it.
* PKI, mail, etc. aren't touched
* There is a migration script in /usr/share/univention-directory-manager-tools/univention-migrate-users-to-ucs4.3.
This script migrates user objects, user templates (if they specified any of the old options) and primary groups (if they aren't samba groups).
The script has a --check option, which only prints what would have been done (ldap modlist).
If you want, we can rename it into --dry-run.
The output could be more user friendly.

* A plugin for the diagnose module has been added which calls the script with --check and offers a "Migrate users" button.

* The migration script also adds the inetOrgPerson + organizationPerson object classes to every user. The "person" option in UDM still exists. I didn't have the time to remove it as well.

* A new UDM object type users/ldap has been implemented. The "ldap_pwd" option from users/user therefore has been removed.
We should probably backport Bug #46193 so that DC Backup Systems with UCS 4.2 don't have problems.

* For the address book feature there is Bug #46117.

* The test script 61_users/100_test_users.py has been implemented. It's a unit test for all functionality of users/user. It's not yet finished. There might be problems in jenkins due to different time zones? (We should fix Bug #36210!) I backported the test case to UCS 4.2-3 so that it shows that there are no real regressions when cleaning up the _ldap_modlist. The test script uses py.test which prints some binary things. That's why it is displayed strange in jenkins. Have a look at ucs-test.log to see it's real output.
Comment 29 Felix Botner univentionstaff 2018-02-07 11:02:42 CET
(1)

disabled handling in users/ldap seems to be broken, syntax for disabled is univention.admin.syntax.boolean (not univention.admin.syntax.disabled as in the users module), UDM gets True as value for disabled

import univention.admin.modules as modules
import univention.admin.uldap as uldap
import univention.admin.config as config

lo, position = uldap.getAdminConnection()
co = config.config()

modules.update() 
ldap = modules.get('users/ldap')
modules.init(lo, position, ldap)
for r in ldap.lookup(co, lo, "name=*"):
    r.open()
    print r['name']
    print r['disabled']

-> test3
-> True

i think that is also the reason for this udm traceback:

-> udm users/ldap list
Traceback (most recent call last):
  File "/usr/share/univention-directory-manager-tools/univention-cli-server", line 218, in doit
    output = univention.admincli.admin.doit(arglist)
  File "/usr/lib/pymodules/python2.7/univention/admincli/admin.py", line 398, in doit
    out = _doit(arglist)
  File "/usr/lib/pymodules/python2.7/univention/admincli/admin.py", line 1006, in _doit
    out.append('  %s: %s' % (_2utf8(key), _2utf8(s.tostring(value))))
  File "/usr/lib/pymodules/python2.7/univention/admincli/admin.py", line 245, in _2utf8
    return text.decode('iso-8859-1')
AttributeError: 'bool' object has no attribute 'decode'

besides, i created this test3 simple ldap account, why is he disabled
Comment 30 Felix Botner univentionstaff 2018-02-07 11:07:18 CET
(2)

users/ldap has four mandatory parameters

		username (*)                             User name
		description                              Description
		name (*)                                 Name
		lastname (*)                             Last name
		password (*)                             Password

why, name is not mandatory in users/user, instead firstname/lastname is used for cn in the users module, in users/ldap name is mapped to cnß
Comment 31 univention 2018-02-07 11:11:20 CET
(In reply to Felix Botner from comment #30)
> (2)
> 
> users/ldap has four mandatory parameters
> 
> 		username (*)                             User name
> 		description                              Description
> 		name (*)                                 Name
> 		lastname (*)                             Last name
> 		password (*)                             Password
> 
> why, name is not mandatory in users/user, instead firstname/lastname is used
> for cn in the users module, in users/ldap name is mapped to cnß

The "cn" attribute is required by the "person" object class. We could alternatively automatically generate set it to the "username" value.
Ee don't have a "firstname"/givenName in these object classes, so we can't use the logic from users/user.
Comment 32 univention 2018-02-07 11:14:16 CET
(In reply to Felix Botner from comment #29)
> (1)
> 
> disabled handling in users/ldap seems to be broken, syntax for disabled is
> univention.admin.syntax.boolean (not univention.admin.syntax.disabled as in
> the users module), UDM gets True as value for disabled
> 
> import univention.admin.modules as modules
> import univention.admin.uldap as uldap
> import univention.admin.config as config
> 
> lo, position = uldap.getAdminConnection()
> co = config.config()
> 
> modules.update() 
> ldap = modules.get('users/ldap')
> modules.init(lo, position, ldap)
> for r in ldap.lookup(co, lo, "name=*"):
>     r.open()
>     print r['name']
>     print r['disabled']
> 
> -> test3
> -> True
> 
> i think that is also the reason for this udm traceback:
> 
> -> udm users/ldap list
> Traceback (most recent call last):
>   File
> "/usr/share/univention-directory-manager-tools/univention-cli-server", line
> 218, in doit
>     output = univention.admincli.admin.doit(arglist)
>   File "/usr/lib/pymodules/python2.7/univention/admincli/admin.py", line
> 398, in doit
>     out = _doit(arglist)
>   File "/usr/lib/pymodules/python2.7/univention/admincli/admin.py", line
> 1006, in _doit
>     out.append('  %s: %s' % (_2utf8(key), _2utf8(s.tostring(value))))
>   File "/usr/lib/pymodules/python2.7/univention/admincli/admin.py", line
> 245, in _2utf8
>     return text.decode('iso-8859-1')
> AttributeError: 'bool' object has no attribute 'decode'
> 
> besides, i created this test3 simple ldap account, why is he disabled
The syntax class here is okay. It's a programmatical error. Please apply the following patch:

diff --git a/management/univention-directory-manager-modules/modules/univention/admin/handlers/users/ldap.py b/management/univention-directory-manager-modules/modules/univention/admin/handlers/users/ldap.py
index 9112671..a4bab8d 100644
--- a/management/univention-directory-manager-modules/modules/univention/admin/handlers/users/ldap.py
+++ b/management/univention-directory-manager-modules/modules/univention/admin/handlers/users/ldap.py
@@ -160,7 +160,7 @@ class object(univention.admin.handlers.simpleLdap):
        def open(self):
                super(object, self).open()
                if self.exists():
-                       self.info['disabled'] = univention.admin.password.is_locked(self['password'])
+                       self.info['disabled'] = '1' if univention.admin.password.is_locked(self['password']) else '0'
                self.save()
 
        def _ldap_pre_ready(self):
@@ -181,7 +181,7 @@ def _ldap_pre_ready(self):
                        # make a crypt password out of it
                        self['password'] = "{crypt}%s" % (univention.admin.password.crypt(self['password']),)
 
-               if self['disabled']:
+               if self['disabled'] == '1':
                        self['password'] = univention.admin.password.lock_password(self['password'])
                else:
                        self['password'] = univention.admin.password.unlock_password(self['password'])
Comment 33 univention 2018-02-07 11:21:10 CET
You can also remove the whole cancel() method from users/ldap as the same logic is part of simpleLDAP.
Comment 34 Felix Botner univentionstaff 2018-02-07 11:55:20 CET
(In reply to univention from comment #31)
> (In reply to Felix Botner from comment #30)
> > (2)
> > 
> > users/ldap has four mandatory parameters
> > 
> > 		username (*)                             User name
> > 		description                              Description
> > 		name (*)                                 Name
> > 		lastname (*)                             Last name
> > 		password (*)                             Password
> > 
> > why, name is not mandatory in users/user, instead firstname/lastname is used
> > for cn in the users module, in users/ldap name is mapped to cnß
> 
> The "cn" attribute is required by the "person" object class. We could
> alternatively automatically generate set it to the "username" value.
> Ee don't have a "firstname"/givenName in these object classes, so we can't
> use the logic from users/user.

yes, my opinion, remove name and lastname, use username for uid and cn
Comment 35 univention 2018-02-07 11:56:37 CET
(In reply to Felix Botner from comment #34)
> > The "cn" attribute is required by the "person" object class. We could
> > alternatively automatically generate set it to the "username" value.
> > Ee don't have a "firstname"/givenName in these object classes, so we can't
> > use the logic from users/user.
> 
> yes, my opinion, remove name and lastname, use username for uid and cn

lastname is required for the "sn" attribute.
Comment 36 Felix Botner univentionstaff 2018-02-07 12:07:14 CET
(3)

created a simple ldap account in 4.3 (no account deactivation)
opened the created object and account deactivation is checked?
ldapsearch on the object reveals
userPassword: {crypt}!$6$QumkkqKvqFpq8wH0$Tb4/YXYOlflecAeoZno5ChMq4f7z.At2mhE7cm.nvZANHewc2UaxGdvAMO3ZdS.1/eJsiuYCy4n96tGUNZTFL/

why is this account deactivated?
Comment 37 Felix Botner univentionstaff 2018-02-07 14:31:07 CET
(4) migration - also test directory/manager/samba3/legacy for the s4connector_present check (ucs school)
Comment 38 Felix Botner univentionstaff 2018-02-07 14:58:39 CET
(5) migration userPassword

the following object is 

cn: test1
objectClass: person
objectClass: univentionPWHistory
objectClass: univentionMail
objectClass: simpleSecurityObject
objectClass: top
objectClass: uidObject
objectClass: univentionSAMLEnabled
objectClass: univentionObject
description: test1
sn: test1
univentionObjectType: users/user
univentionMailUserQuota: 0
uid: test1
pwhistory: $6$DU3ENCRMQVLlsnU7$7...
userPassword:: e2NyeXB0fSQ2JF....

is changed to 


cn: test1
objectClass: person
objectClass: univentionPWHistory
objectClass: univentionMail
objectClass: top
objectClass: univentionSAMLEnabled
objectClass: univentionObject
objectClass: krb5KDCEntry
objectClass: inetOrgPerson
objectClass: sambaSamAccount
objectClass: organizationalPerson
objectClass: shadowAccount
objectClass: krb5Principal
objectClass: posixAccount
description: test1
sn: test1
univentionObjectType: users/user
univentionMailUserQuota: 0
uid: test1
pwhistory: $6$DU3ENCRMQVLl...
userPassword:: e2NyeXB0fSQ2JF...
userPassword:: e0tJTklUfQ== (KINIT)
krb5PrincipalName: test1@...
krb5KeyVersionNumber: 1
krb5KDCFlags: 126
krb5MaxLife: 86400
krb5MaxRenew: 604800
uidNumber: 284677
gidNumber: 5001
homeDirectory: /home/test1
sambaSID: S-1-5-21-40....
sambaAcctFlags: [U          ]

why is userPassword set twice => set userPassword only if it is NOT already set!

For objects without userPassword (only "Persönliche Informationen") userPassword ist set to KINIT, why? better KRB5?, i dont' know
Comment 39 Felix Botner univentionstaff 2018-02-07 15:06:32 CET
(6) migration - univentionObjectType

The migration script checks if univentionObjectType exists and if users/user is not set in univentionObjectType, then the migration for this object is skipped,

but objects without univentionObjectType are checked?! I would prefer to skip objects without univentionObjectType
Comment 40 Felix Botner univentionstaff 2018-02-07 16:34:01 CET
(7) migration - krb5PasswordEnd

i have a user with krb5PasswordEnd: 20060801090247Z and the script fails with

Traceback (most recent call last):
  File "/usr/share/univention-directory-manager-tools/univention-migrate-users-to-ucs4.3.o", line 415, in <module>
    Migration()
  File "/usr/share/univention-directory-manager-tools/univention-migrate-users-to-ucs4.3.o", line 71, in __init__
    self.migrate_users()
  File "/usr/share/univention-directory-manager-tools/univention-migrate-users-to-ucs4.3.o", line 165, in migrate_users
    password_set_date = time.strptime(user['krb5PasswordEnd'][0], "%Y%m%d000000Z")
  File "/usr/lib/python2.7/_strptime.py", line 478, in _strptime_time
    return _strptime(data_string, format)[0]
  File "/usr/lib/python2.7/_strptime.py", line 332, in _strptime
    (data_string, format))
ValueError: time data '20060801090247Z' does not match format '%Y%m%d000000Z'


so first, the parsing of krb5PasswordEnd seems to be broken (at least for this object)

second, better try expect for krb5PasswordEnd (as already done for sambaPwdLastSet shadowLastChange)

-				password_set_date = time.strptime(user['krb5PasswordEnd'][0], "%Y%m%d000000Z")
+				try:
+					password_set_date = time.strptime(user['krb5PasswordEnd'][0], "%Y%m%d000000Z")
+				except ValueError:
+					pass
Comment 41 Felix Botner univentionstaff 2018-02-07 16:44:47 CET
(8) migration - shadowLastChange krb5PasswordEnd

shadowLastChange is used for password_set_date, ok so far, but if the object has no kerberos attributes, krb5PasswordEnd is generated form this password_set_date.

So the kerberos end date is set to the shadowLastChange?
I think the "shadow password end" is shadowLastChange + shadowMax

e.g.

Account with 

  shadowMax: 7000
  shadowLastChange: 16260

gets

 ('krb5PasswordEnd', [], ['20140709000000Z']),

i think correct would be 20330907000000Z
Comment 42 univention 2018-02-07 17:08:57 CET
(In reply to Felix Botner from comment #37)
> (4) migration - also test directory/manager/samba3/legacy for the
> s4connector_present check (ucs school)
Hm, really? We are deprecating Samba3.

(In reply to Felix Botner from comment #38)
> (5) migration userPassword
> 
> the following object is 
> 
> cn: test1
> objectClass: person
> objectClass: univentionPWHistory
> objectClass: univentionMail
> objectClass: simpleSecurityObject
> objectClass: top
> objectClass: uidObject
> objectClass: univentionSAMLEnabled
> objectClass: univentionObject
> description: test1
> sn: test1
> univentionObjectType: users/user
> univentionMailUserQuota: 0
> uid: test1
> pwhistory: $6$DU3ENCRMQVLlsnU7$7...
> userPassword:: e2NyeXB0fSQ2JF....
> 
> is changed to 
> 
> 
> cn: test1
> objectClass: person
> objectClass: univentionPWHistory
> objectClass: univentionMail
> objectClass: top
> objectClass: univentionSAMLEnabled
> objectClass: univentionObject
> objectClass: krb5KDCEntry
> objectClass: inetOrgPerson
> objectClass: sambaSamAccount
> objectClass: organizationalPerson
> objectClass: shadowAccount
> objectClass: krb5Principal
> objectClass: posixAccount
> description: test1
> sn: test1
> univentionObjectType: users/user
> univentionMailUserQuota: 0
> uid: test1
> pwhistory: $6$DU3ENCRMQVLl...
> userPassword:: e2NyeXB0fSQ2JF...
> userPassword:: e0tJTklUfQ== (KINIT)
> krb5PrincipalName: test1@...
> krb5KeyVersionNumber: 1
> krb5KDCFlags: 126
> krb5MaxLife: 86400
> krb5MaxRenew: 604800
> uidNumber: 284677
> gidNumber: 5001
> homeDirectory: /home/test1
> sambaSID: S-1-5-21-40....
> sambaAcctFlags: [U          ]
> 
> why is userPassword set twice => set userPassword only if it is NOT already
> set!
Yes, this is an error. But I am unsure how this can happen. The code says:
230 »   »   »   if not has_shadow or not has_posix and not user.get('userPassword'):
231 »   »   »   »   ml.append(('userPassword', [], ['{K5KEY}' if user.get('krb5Key') else '{KINIT}']))

So there is a check not user.get('userPassword') which should not add any value if a password exists already.

> For objects without userPassword (only "Persönliche Informationen")
> userPassword ist set to KINIT, why? better KRB5?, i dont' know
Because they don't have an attribute krb5Key and then the {K5KEY} overlay doesn't work.

(In reply to Felix Botner from comment #39)
> (6) migration - univentionObjectType
> 
> The migration script checks if univentionObjectType exists and if users/user
> is not set in univentionObjectType, then the migration for this object is
> skipped,
> 
> but objects without univentionObjectType are checked?! I would prefer to
> skip objects without univentionObjectType
This was done on purpose. There might be users from the very past which don't have univentionObjectType yet. The skipping is there to ignore kopano users which have kopano/contact in the univentionObjectType.


(In reply to Felix Botner from comment #40)
> (7) migration - krb5PasswordEnd
> 
> i have a user with krb5PasswordEnd: 20060801090247Z and the script fails with
> 
> Traceback (most recent call last):
>   File
> "/usr/share/univention-directory-manager-tools/univention-migrate-users-to-
> ucs4.3.o", line 415, in <module>
>     Migration()
>   File
> "/usr/share/univention-directory-manager-tools/univention-migrate-users-to-
> ucs4.3.o", line 71, in __init__
>     self.migrate_users()
>   File
> "/usr/share/univention-directory-manager-tools/univention-migrate-users-to-
> ucs4.3.o", line 165, in migrate_users
>     password_set_date = time.strptime(user['krb5PasswordEnd'][0],
> "%Y%m%d000000Z")
>   File "/usr/lib/python2.7/_strptime.py", line 478, in _strptime_time
>     return _strptime(data_string, format)[0]
>   File "/usr/lib/python2.7/_strptime.py", line 332, in _strptime
>     (data_string, format))
> ValueError: time data '20060801090247Z' does not match format '%Y%m%d000000Z'
> 
> 
> so first, the parsing of krb5PasswordEnd seems to be broken (at least for
> this object)
oh yes, we should accept this format of course!

> second, better try expect for krb5PasswordEnd (as already done for
> sambaPwdLastSet shadowLastChange)
> 
> -				password_set_date = time.strptime(user['krb5PasswordEnd'][0],
> "%Y%m%d000000Z")
> +				try:
> +					password_set_date = time.strptime(user['krb5PasswordEnd'][0],
> "%Y%m%d000000Z")
> +				except ValueError:
> +					pass
No, I don't think it's necessary because the LDAP syntax ensures the correct syntax. On python side nothing can go wrong if we use the correct parsing.

(In reply to Felix Botner from comment #41)
> (8) migration - shadowLastChange krb5PasswordEnd
> 
> shadowLastChange is used for password_set_date, ok so far, but if the object
> has no kerberos attributes, krb5PasswordEnd is generated form this
> password_set_date.
> 
> So the kerberos end date is set to the shadowLastChange?
> I think the "shadow password end" is shadowLastChange + shadowMax
> 
> e.g.
> 
> Account with 
> 
>   shadowMax: 7000
>   shadowLastChange: 16260
> 
> gets
> 
>  ('krb5PasswordEnd', [], ['20140709000000Z']),
> 
> i think correct would be 20330907000000Z
Yes.
Comment 43 Arvid Requate univentionstaff 2018-02-07 17:27:36 CET
Sorry to interrupt your discussion, I've adjusted the users/ldap module:

* Applied Florian patch to  fix the handling of "disabled"
* Automatically initialize "lastname" and "name" from "username"
* Don't show "lastname" and "name" properties in layout
* Fix cli traceback when not supplying a password during create
* Show "locked" checkbox to unlock account (no unlockTime shown yet, sorry).
Comment 44 univention 2018-02-07 17:29:05 CET
I implemented some fixes. You can add them via:

git remote add fbest https://github.com/spaceone/univention-corporate-server
git fetch fbest
git checkout fbest/45842-patches
git diff 4.3-0..fbest/45842-patches
git cherry-pick $the_commit_hashs

Diff can be viewed with:
https://github.com/univention/univention-corporate-server/compare/4.3-0...spaceone:fbest/45842-patches
Comment 45 Felix Botner univentionstaff 2018-02-07 17:39:09 CET
(In reply to univention from comment #42)
> (In reply to Felix Botner from comment #37)
> > (4) migration - also test directory/manager/samba3/legacy for the
> > s4connector_present check (ucs school)
> Hm, really? We are deprecating Samba3.

this is what school uses to check for s4_present


> (In reply to Felix Botner from comment #38)
> > (5) migration userPassword
> > 
> > the following object is 
> > 
> > cn: test1
> > objectClass: person
> > objectClass: univentionPWHistory
> > objectClass: univentionMail
> > objectClass: simpleSecurityObject
> > objectClass: top
> > objectClass: uidObject
> > objectClass: univentionSAMLEnabled
> > objectClass: univentionObject
> > description: test1
> > sn: test1
> > univentionObjectType: users/user
> > univentionMailUserQuota: 0
> > uid: test1
> > pwhistory: $6$DU3ENCRMQVLlsnU7$7...
> > userPassword:: e2NyeXB0fSQ2JF....
> > 
> > is changed to 
> > 
> > 
> > cn: test1
> > objectClass: person
> > objectClass: univentionPWHistory
> > objectClass: univentionMail
> > objectClass: top
> > objectClass: univentionSAMLEnabled
> > objectClass: univentionObject
> > objectClass: krb5KDCEntry
> > objectClass: inetOrgPerson
> > objectClass: sambaSamAccount
> > objectClass: organizationalPerson
> > objectClass: shadowAccount
> > objectClass: krb5Principal
> > objectClass: posixAccount
> > description: test1
> > sn: test1
> > univentionObjectType: users/user
> > univentionMailUserQuota: 0
> > uid: test1
> > pwhistory: $6$DU3ENCRMQVLl...
> > userPassword:: e2NyeXB0fSQ2JF...
> > userPassword:: e0tJTklUfQ== (KINIT)
> > krb5PrincipalName: test1@...
> > krb5KeyVersionNumber: 1
> > krb5KDCFlags: 126
> > krb5MaxLife: 86400
> > krb5MaxRenew: 604800
> > uidNumber: 284677
> > gidNumber: 5001
> > homeDirectory: /home/test1
> > sambaSID: S-1-5-21-40....
> > sambaAcctFlags: [U          ]
> > 
> > why is userPassword set twice => set userPassword only if it is NOT already
> > set!
> Yes, this is an error. But I am unsure how this can happen. The code says:
> 230 »   »   »   if not has_shadow or not has_posix and not
> user.get('userPassword'):
> 231 »   »   »   »   ml.append(('userPassword', [], ['{K5KEY}' if
> user.get('krb5Key') else '{KINIT}']))
> 
> So there is a check not user.get('userPassword') which should not add any
> value if a password exists already.

i think we have to add some parentheses

>>> True or True and False
True
>>> True or True and True
True
>>> (True or True) and True
True
>>> (True or True) and False
False


> > For objects without userPassword (only "Persönliche Informationen")
> > userPassword ist set to KINIT, why? better KRB5?, i dont' know
> Because they don't have an attribute krb5Key and then the {K5KEY} overlay
> doesn't work.

But KINIT does not work either, there are no Krb5 Keys, KINIT is only for ad member mode


> (In reply to Felix Botner from comment #39)
> > (6) migration - univentionObjectType
> > 
> > The migration script checks if univentionObjectType exists and if users/user
> > is not set in univentionObjectType, then the migration for this object is
> > skipped,
> > 
> > but objects without univentionObjectType are checked?! I would prefer to
> > skip objects without univentionObjectType
> This was done on purpose. There might be users from the very past which
> don't have univentionObjectType yet. The skipping is there to ignore kopano
> users which have kopano/contact in the univentionObjectType.

OK, the old users/user ldap filter is used to find objects, so only users objects are considered, should be ok


> (In reply to Felix Botner from comment #40)
> > (7) migration - krb5PasswordEnd
> > 
> > i have a user with krb5PasswordEnd: 20060801090247Z and the script fails with
> > 
> > Traceback (most recent call last):
> >   File
> > "/usr/share/univention-directory-manager-tools/univention-migrate-users-to-
> > ucs4.3.o", line 415, in <module>
> >     Migration()
> >   File
> > "/usr/share/univention-directory-manager-tools/univention-migrate-users-to-
> > ucs4.3.o", line 71, in __init__
> >     self.migrate_users()
> >   File
> > "/usr/share/univention-directory-manager-tools/univention-migrate-users-to-
> > ucs4.3.o", line 165, in migrate_users
> >     password_set_date = time.strptime(user['krb5PasswordEnd'][0],
> > "%Y%m%d000000Z")
> >   File "/usr/lib/python2.7/_strptime.py", line 478, in _strptime_time
> >     return _strptime(data_string, format)[0]
> >   File "/usr/lib/python2.7/_strptime.py", line 332, in _strptime
> >     (data_string, format))
> > ValueError: time data '20060801090247Z' does not match format '%Y%m%d000000Z'
> > 
> > 
> > so first, the parsing of krb5PasswordEnd seems to be broken (at least for
> > this object)
> oh yes, we should accept this format of course!
> 
> > second, better try expect for krb5PasswordEnd (as already done for
> > sambaPwdLastSet shadowLastChange)
> > 
> > -				password_set_date = time.strptime(user['krb5PasswordEnd'][0],
> > "%Y%m%d000000Z")
> > +				try:
> > +					password_set_date = time.strptime(user['krb5PasswordEnd'][0],
> > "%Y%m%d000000Z")
> > +				except ValueError:
> > +					pass
> No, I don't think it's necessary because the LDAP syntax ensures the correct
> syntax. On python side nothing can go wrong if we use the correct parsing.

But is is already done for sambaPwdLastSet, we not here?
Comment 46 univention 2018-02-07 17:52:45 CET
(In reply to Felix Botner from comment #45)
> (In reply to univention from comment #42)
> > (In reply to Felix Botner from comment #37)
> > > (4) migration - also test directory/manager/samba3/legacy for the
> > > s4connector_present check (ucs school)
> > Hm, really? We are deprecating Samba3.
> 
> this is what school uses to check for s4_present
Okay, I added this in my patches.

 
> i think we have to add some parentheses
> 
> >>> True or True and False
> True
> >>> True or True and True
> True
> >>> (True or True) and True
> True
> >>> (True or True) and False
> False
Thanks, has been fixed in that branch, too!

> > > For objects without userPassword (only "Persönliche Informationen")
> > > userPassword ist set to KINIT, why? better KRB5?, i dont' know
> > Because they don't have an attribute krb5Key and then the {K5KEY} overlay
> > doesn't work.
> 
> But KINIT does not work either, there are no Krb5 Keys, KINIT is only for ad
> member mode
Oh yes, what to do then?

> > No, I don't think it's necessary because the LDAP syntax ensures the correct
> > syntax. On python side nothing can go wrong if we use the correct parsing.
> 
> But is is already done for sambaPwdLastSet, we not here?

Well, it's impossible to fail by definition of the LDAP schema. But I don't care, you can of course add such a check.
Comment 47 Arvid Requate univentionstaff 2018-02-07 19:30:30 CET
My 2 cent:

* If we have a userPassword, we should not change it and we should not append a second value. According to Comment 42 this should not happen, so we need to check that and fix the bug if it still happens.

* If we don't have userPassword but have krb5Key (possibly derived from sambaNTPassword) we should write "{K5KEY}" to it.
Comment 48 Felix Botner univentionstaff 2018-02-08 11:20:55 CET
(9) migration 

we need a way to overwrite the LDAP user filter

update43/udm/migration/user/filter='...'

univention-migrate-users-to-ucs4.3 should check for this variable and overwrite the default user filter
Comment 49 Jürn Brodersen univentionstaff 2018-02-08 15:05:43 CET
73014c26: Translations for 55_user_migration.py
Comment 50 Arvid Requate univentionstaff 2018-02-08 20:20:02 CET
Ok, I've adjusted the script considering Felix feedback.


For the record, because it bit me yesterday too:

time.mktime() and time.strftime() are not the inverse of time.gmtime(),
as https://docs.python.org/2/library/time.html explains calendar.timegm() should be used instead:

===========================================================================
>>> unixtime = 1234567890
>>> import time
>>> int(time.mktime(time.gmtime(unixtime))) - unixtime
-3600
>>>> int(time.strftime('%s', time.gmtime(unixtime))) - unixtime
-3600
>>> import calendar
>>> int(calendar.timegm(time.gmtime(unixtime))) - unixtime
0
===========================================================================


Also, I guess we should not use or set sambaKickoffTime because it seems to be a  dynamic attribute that is derived by Samba from:
* Password expiration (sambaPwdLastSet+sambaMaxPwdAge)
* The AD-Attribute accountExpires
* Some interpretation of (sambaLogonHours - sambaForceLogoff)

So I removed the corresponding code from the migration script.
Comment 51 Stefan Gohmann univentionstaff 2018-02-09 12:39:48 CET
I think it is a change of these changes, that a user creation fails if the given group doesn't exist. At least it works in UCS 4.2.

Anyway, I've adjusted the test case:


* 01_base/21ssh_motd: Since UCS 4.3, the user creation fails if the
  given group doesn't exist. Switch to domain admins in this test case
  (Bug #45842)

https://git.knut.univention.de/univention/ucs/commit/07f48f77264962f05cae170e3184816fdc9a239d
Comment 52 Felix Botner univentionstaff 2018-02-12 17:03:34 CET
NEW 10 - migration template update fails

templates are "migrated", but during a second run, the script again wants to migrate the templates ???

-> template:
userOptionsPreset: kerberos
userOptionsPreset: posix
userOptionsPreset: person

-> migrated to kerberos, posix

second run
-> Modifying  cn=Helpdesk template
[('userOptionsPreset', ['kerberos', 'posix'], ['kerberos', 'posix'])]


-> template:
userOptionsPreset: kerberos
userOptionsPreset: person
userOptionsPreset: samba
userOptionsPreset: posix
userOptionsPreset: mail

-> migrated to kerberos, samba, posix

second run
-> Modifying  cn=open-xchange groupware account
[('userOptionsPreset',
  ['samba', 'kerberos', 'posix'],
  ['kerberos', 'samba', 'posix'])]

-> template:
userOptionsPreset: person
userOptionsPreset: posix
userOptionsPreset: mail

-> migrated to posix

second run
-> Modifying  cn=OX ...
[('userOptionsPreset', ['posix'], ['posix'])]



OK - (4) migration - also test directory/manager/samba3/legacy for the....

OK - (5) why is userPassword set twice => set userPassword only if it is NOT alr

INVALID - (6) migration - univentionObjectType

OK - (7) migration - krb5PasswordEnd  traceback

OK -  (8) migration - shadowLastChange krb5PasswordEnd

OK - (9) migration we need a way to overwrite the LDAP user filter
Comment 53 Arvid Requate univentionstaff 2018-02-12 18:59:30 CET
> (10)

fixed via fd9a3cd61e, package rebuilt.
Comment 54 Felix Botner univentionstaff 2018-02-13 14:41:11 CET
(In reply to Arvid Requate from comment #53)
> > (10)
> 
> fixed via fd9a3cd61e, package rebuilt.

OK, but is this correct

Old
-> template:
userOptionsPreset: kerberos
userOptionsPreset: posix
userOptionsPreset: person

After migration
userOptionsPreset: kerberos
userOptionsPreset: posix

i thought we removed the kerberos posix options?
Comment 55 Felix Botner univentionstaff 2018-02-13 14:44:53 CET
(In reply to Felix Botner from comment #54)
> (In reply to Arvid Requate from comment #53)
> > > (10)
> > 
> > fixed via fd9a3cd61e, package rebuilt.
> 
> OK, but is this correct
> 
> Old
> -> template:
> userOptionsPreset: kerberos
> userOptionsPreset: posix
> userOptionsPreset: person
> 
> After migration
> userOptionsPreset: kerberos
> userOptionsPreset: posix
> 
> i thought we removed the kerberos posix options?

valid options with 4.3 are default, mail, pki and person

also the syntax for the settings/usertemplate.py property _options (univention.admin.syntax.optionsUsersUser) is 

class optionsUsersUser(select):
    choices = [
        ('person', _('Personal Information')),
        ('mail', _('Mail Account')),
    ]

is pki missing here?
Comment 56 Arvid Requate univentionstaff 2018-02-13 15:04:47 CET
Package rebuilt with patch for Comment 54:

Package: univention-directory-manager-modules
Version: 13.0.19-16A~4.3.0.201802131501
Branch: ucs_4.3-0

Comment 55: Yes, but it was not present before, so that would be a different bug.
Comment 57 Arvid Requate univentionstaff 2018-02-13 17:38:34 CET
Ok, as discussed, we've picked up Comment 4:

Decision: user accounts will always have 'univentionMail', 'organizationalPerson' and 'inetOrgPerson' objectClass. The previous user options 'mail' and 'person' are removed as they are non-optional defaults.

As a consequence, settings/usertemplate doesn't offer these options any longer either. Instead it offers 'pki' now, which was missing (see Comment 55).

Migration script adjusted.

Package: univention-directory-manager-modules
Version: 13.0.19-20A~4.3.0.201802131736
Branch: ucs_4.3-0
Comment 58 Arvid Requate univentionstaff 2018-02-13 17:51:05 CET
*** Bug 46262 has been marked as a duplicate of this bug. ***
Comment 59 Arvid Requate univentionstaff 2018-02-14 19:21:18 CET
I fixed a regression which caused new users not getting added to the univentionDefaultGroup any longer and I added a test case for that.

Commit 27a784c76b. Packages are rebuilt.
Comment 60 Felix Botner univentionstaff 2018-02-15 10:42:41 CET
(In reply to Arvid Requate from comment #59)
> I fixed a regression which caused new users not getting added to the
> univentionDefaultGroup any longer and I added a test case for that.
> 
> Commit 27a784c76b. Packages are rebuilt.

OK, default group is updated, test looks good
Comment 61 Arvid Requate univentionstaff 2018-02-15 12:51:38 CET
I had to adjust self.open() and self._load_groups() a bit more in an attempt to fix the issue reported in Bug 46118 Comment 7.
Comment 62 Felix Botner univentionstaff 2018-02-16 09:50:19 CET
group membership in group object is no longer removed if group is removed from user

-> udm users/user create --set username=test6 --set lastname=test1 --set password=univention

-> udm users/user modify --dn uid=test6,dc=fb,dc=bf --append groups="cn=Domain Admins,cn=groups,dc=fb,dc=bf"

-> udm users/user modify --dn uid=test6,dc=fb,dc=bf --remove groups="cn=Domain Admins,cn=groups,dc=fb,dc=bf"

-> univention-ldapsearch cn=Domain\ Admins uniqueMember memberUid
dn: cn=Domain Admins,cn=groups,dc=fb,dc=bf
memberUid: Administrator
memberUid: test4
memberUid: test5
memberUid: test6
uniqueMember: uid=Administrator,cn=users,dc=fb,dc=bf
uniqueMember: uid=test4,dc=fb,dc=bf
uniqueMember: uid=test5,dc=fb,dc=bf
uniqueMember: uid=test6,dc=fb,dc=bf

i guess this is also the reason for the failed 4.3 s4connector tests
Comment 63 Arvid Requate univentionstaff 2018-02-16 14:28:29 CET
Fixed in commit bf7048f423.
Testcase added to ucs-test.
Packages rebuilt.

I'm now going to light a candle and perform an ancient shamanic UDM ritual to pray for good riddance to these regressions.
Comment 64 univention 2018-02-19 08:54:44 CET
Created attachment 9397 [details]
patch for group loading

(In reply to Arvid Requate from comment #63)
> Fixed in commit bf7048f423.
> Testcase added to ucs-test.
> Packages rebuilt.
> 
> I'm now going to light a candle and perform an ancient shamanic UDM ritual
> to pray for good riddance to these regressions.
I suggest the attached patch for the group loading problem.
Comment 65 Arvid Requate univentionstaff 2018-02-19 09:33:55 CET
Thanks for posting the patch. From a quick glance I think the code is just an equivalent refactoring? I'll discuss with Felix since the bug is currently in resolved state.
Comment 66 univention 2018-02-19 10:33:03 CET
Yes, it's only refactoring which makes it more clear how to further continue development.
Comment 67 Felix Botner univentionstaff 2018-02-26 11:50:57 CET
FAIL - please update to changelog to refelct this

> Decision: user accounts will always have 'univentionMail', 
> 'organizationalPerson' and 'inetOrgPerson' objectClass. The previous user 
> options 'mail' and 'person' are removed as they are non-optional defaults.

and the contact object changes.


OK - ucs-test
OK - migration script
     - user/template migration
     - tested with several ldap dumps from real word setups
OK - master with 4.1, 4.2, 4.3 slave, old user object, migration, 
     ldap replication
OK - 4.3 master 4.2 backup with ad connector
     https://forge.univention.org/bugzilla/show_bug.cgi?id=46381 
     (pwdChangeNextLogin)
OK 4.3 master 4.2 slave with s4 connector
     https://forge.univention.org/bugzilla/show_bug.cgi?id=46387
     (pwdChangeNextLogin)
Comment 68 Arvid Requate univentionstaff 2018-02-26 13:44:11 CET
d9407557b2 | Changelog adjusted.
Comment 69 Felix Botner univentionstaff 2018-02-26 14:55:57 CET
OK
Comment 70 Daniel Tröder univentionstaff 2018-03-01 12:14:00 CET
Changing the birthday fails in a ucsschool environment:

http://jenkins.knut.univention.de:8080/job/UCSschool-4.3/job/Upgrade%20Singleserver/lastBuild/Config=s4,TestGroup=base1/testReport/junit/90_ucsschool/112_import_user_pyhooks/test/

or more simply:

# udm users/user modify --dn uid=aname2,cn=schueler,cn=users,ou=$OU,$ldap/base --set birthday=1987-06-05
LDAP Error: Type or value exists: modify/add: objectClass: value #0 already exists

The problem is in univention/admin/handlers/users/user.py:2274

ml.append(('objectClass', '', 'univentionPerson'))  # TODO: check if exists already
Comment 71 Daniel Tröder univentionstaff 2018-03-01 12:18:24 CET
Created attachment 9425 [details]
check if OC already exists, before adding it

patch fixes the problem
Comment 72 Arvid Requate univentionstaff 2018-03-01 13:08:35 CET
Patch applied and rebuilt:

Package: univention-directory-manager-modules
Version: 13.0.21-3A~4.3.0.201803011304
Branch: ucs_4.3-0
Comment 73 Felix Botner univentionstaff 2018-03-02 11:52:28 CET
(In reply to Arvid Requate from comment #72)
> Patch applied and rebuilt:
> 
> Package: univention-directory-manager-modules
> Version: 13.0.21-3A~4.3.0.201803011304
> Branch: ucs_4.3-0

OK, works

-> udm users/user modify --dn uid=Administrator,cn=users,dc=fb,dc=bf --set birthday=1987-06-07
Object modified: uid=Administrator,cn=users,dc=fb,dc=bf

-> udm users/user modify --dn uid=Administrator,cn=users,dc=fb,dc=bf --set birthday=1987-06-08
Object modified: uid=Administrator,cn=users,dc=fb,dc=bf
Comment 74 Stefan Gohmann univentionstaff 2018-03-14 14:38:04 CET
UCS 4.3 has been released:
 https://docs.software-univention.de/release-notes-4.3-0-en.html
 https://docs.software-univention.de/release-notes-4.3-0-de.html

If this error occurs again, please use "Clone This Bug".