Bug 51494 - diagnostic module to check uniqueness of recordUID and sourceUID
diagnostic module to check uniqueness of recordUID and sourceUID
Status: CLOSED FIXED
Product: UCS@school
Classification: Unclassified
Component: General
UCS@school 4.4
Other Linux
: P5 normal (vote)
: UCS@school 4.4 v5-errata
Assigned To: Toni Röhmeyer
Tobias Wenzel
:
Depends on: 51542
Blocks:
  Show dependency treegraph
 
Reported: 2020-06-15 12:33 CEST by Daniel Tröder
Modified: 2020-07-30 13:15 CEST (History)
0 users

See Also:
What kind of report is it?: Feature Request
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):
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Tröder univentionstaff 2020-06-15 12:33:43 CEST
In a UCS@school domain that uses the UCS@school import, all users that should be considered for imports must have a unique recordUID-sourceUID combination.

Read the recordUID and sourceUID attributes of all users and verify that their combinations are all different.

Users without a sourceUID are not considered by the UCS@school import and must be ignored by the check.
Comment 1 Toni Röhmeyer univentionstaff 2020-06-16 22:06:56 CEST
Added diagnosis module "906_ucsschool_uid_uniqueness.py"
on branch "troehmey/bug51494_diagnostic_uid_uniqueness"
with commits:

commit 3694826e73ae3b84b904a8f6babb60d9fc9dab2e
Bug #51494: applied fix in if condition

commit 083fd489bec2bc3d8af8a502d9875038f94d6d6e
Bug #51494: added new diagnosis module uid_uniqueness

Feature was tested by manually setting the recordUID and sourceUID to the same as an already existing user via 
$udm users/user modify --dn <user_dn> --set ucsschoolRecordUID=<already_existing_uid>
Comment 2 Tobias Wenzel univentionstaff 2020-06-18 16:31:25 CEST
QA -> REOPEN 

The code works as expected for one user. I would like to see warnings for all false students. Have a look at the problematic_objects dict in the 904 diagnostic module and collect the objects like that. 

Your filter is not false, but I think this is more clear, since you access both:
"(&(ucsschoolSourceUID=*)(ucsschoolRecordUID=*))"

Use source_uid = attrs[UCSSCHOOLSOURCEUID][0] to make it more readable.

"Each user registered on the LDAP must be clearly identifiable by a unique combination of a sourceUID and recordUID."
-> I would suggest you simply use this:
In a UCS@school domain that uses the UCS@school import, all users that should be considered for imports must have a unique recordUID-sourceUID combination.


 "If multiple users have the same combination of those UID's, users may not be found or wron_d_ user objects could get modified."
Comment 3 Toni Röhmeyer univentionstaff 2020-06-20 12:16:56 CEST
Applied and tested suggested fixes with commit:

commit b2336f99783fedf898b8a36aea29b43460a189b9
Bug #51494: added problematic objects collection


Thanks for the hints!
Comment 4 Daniel Tröder univentionstaff 2020-06-22 08:50:26 CEST
I disagree with the inclusion of (ucsschoolRecordUID=*) to the filter.
If two users have the same source_uid and no record_uid, it is still a clash, as that makes it the same tuple.

It is also an error to have a source_uid set, but no record_uid. I created a separate feature request for a separate diagnostic module for that: Bug #51542
Comment 5 Daniel Tröder univentionstaff 2020-06-22 09:05:30 CEST
I revert my REOPEN to a dependency on Bug #51542.
If the other diagnostic module catches all instances of source_uid=* with record_uid=None, then this module can just skip them.
Comment 6 Tobias Wenzel univentionstaff 2020-06-25 11:50:31 CEST
Thanks for your open eyes! The suggested additional diagnose module, see bug 51494 comment 4, has already been released Bug #51542. 

QA -> all ok -> REOPEN for merge&build
=> make sure the pre-commits are installed

Code -> looks good
Messages -> updated, as suggested -> ok
Functionality -> still working
Comment 7 Toni Röhmeyer univentionstaff 2020-06-25 12:39:56 CEST
Feature branch merged into 4.4 with commits

commit 4afbfa5e35e7b732efd8cf51301ad5ee46912be7
Bug #51494: added yaml for ucs-school-umc-diagnostic

commit 4876ba36443c38945c6fa673cf4f9c7706d3ea9d
Bug #51494: added changelog entry

commit 217f4b5437adf2728a97255e944546ce19ceb84f
Merge: ea6e5e961 2b5610174
Bug #51494: Merge branch 'troehmey/bug51494_diagnostic_uid_uniqueness' into 4.4



Successful build:
Package: ucs-school-umc-diagnostic
Version: 1.0.0-12A~4.4.0.202006251224
Branch: ucs_4.4-0
Scope: ucs-school-4.4
Comment 8 Tobias Wenzel univentionstaff 2020-06-25 14:26:44 CEST
QA -> all ok -> VERIFY

Changelog -> Ok
YAML -> Ok
Functionality -> Works like before -> ok
No merge conflicts -> ok
Comment 9 Tobias Wenzel univentionstaff 2020-07-30 13:15:17 CEST
UCS@school 4.4 v5 has been released (errata update to the release).

http://docs.software-univention.de/changelog-ucsschool-4.4v5-de.html#changelog:ucsschool:2020-07-30

If this error occurs again, please clone this bug.