Univention Bugzilla – Bug 52147
script that verifies the consistency of UCS@school LDAP objects
Last modified: 2021-03-30 12:29:05 CEST
Create a CLI script that verifies the consistency of all UCS@school LDAP objects. The CLI should be extensible, so that in a first iteration only user objects are checked. (See https://help.univention.com/t/how-a-ucs-school-user-should-look-like/15630 for rules for a user object.) Clone bugs from this one for further objects to check (school classes, workgroups, shares, OUs). The functions should be implemented in a shared Python module, so that they can be reused by other software. Maybe they should even become part of the ucsschool.lib.models that they check. In a separate bug, create a system diagnose module, that calls the functions of this script. The script should offer the possibility to verify the consistency of a single UCS@school LDAP object, passed by DN.
CLI script is good. Maybe a diagnosis module would be a good idea. There are already some checks which may be extended.
(In reply to Daniel Tröder from comment #0) > In a separate bug, create a system diagnose module, that calls the functions > of this script.
The script should offer an option to check only objects below a given school (OU).
*** Bug 46738 has been marked as a duplicate of this bug. ***
Added script on feature branch troehmey/bug52147_object_consistency_script: 46dc80a3d Bug #52147: added check for user in classes 639165b22 Bug #52147: added shares check c575ccb10 Bug #52147: added group check, improved user role check 4388e5e33 Bug #52147: wrote script for checking consistency of all users It can be executed without any parameters for checking all LDAP objects. With --school the checks will only be executed for the given schoool. With --userdn the user check is only applied for the given dn. At first it checks all users if it matches all requirements specified here: https://help.univention.com/t/how-a-ucs-school-user-should-look-like/15630 That includes checking... - if the ObjectClass and ucsschoolRole is correctly set - if the user is member of all required groups - is the user is not member in a group where it does not belong to - if users in a school class have a corresponding OU in the school property set Afterwards it checks all groups for these requirements: - for each school these groups must exist: - cn=Domain Users $OU,ou=$OU,$LDAP-BASE - groups which are specified in https://docs.software-univention.de/ucsschool-handbuch-4.4.html#structure:ldap:ou Finally it checks all share object for the following requirements: - for each class-share a corresponding class must exist - for each school a corresponding Marktplatz must exist
The firstname and lastname attributes must be set for a school user. I have added this to the help article. * Please also verify those attributes for the user objects. * encoding comment should be utf-8 * comment is copy and paste error: # UCS@school lib" # module: Create role specific shares also such a description should be a docstring on the module level * The copyright notice starts with 2014, but the script is from this year. * sys and traceback imported but unused. Please use flake8 or similar to check your code. # flake8 --isolated --max-line-length=105 ucs-school-object-consistency ucs-school-object-consistency:35:1: F401 'sys' imported but unused ucs-school-object-consistency:36:1: F401 'traceback' imported but unused ucs-school-object-consistency:109:5: F841 local variable 'admins_regex' is assigned to but never used ucs-school-object-consistency:156:9: E722 do not use bare 'except' ucs-school-object-consistency:190:20: E713 test for membership should be 'not in' ucs-school-object-consistency:302:5: F841 local variable 'ldap_base' is assigned to but never used ucs-school-object-consistency:315:5: F841 local variable 'issues' is assigned to but never used * Please don't use OptionParser. Use Argparse or Click. * The help texts for the options of the script don't explain a) what effect using them has and b) what allowed values are * In main() a UCR instance is created but not used. * The variable named "arg_binddn" doesn't match its content. a) an argument does not need to be prefixed with "arg(ument)" - it is already an argument; b) a "bind DN" is the name of an object (usually a user object) in LDAP that is used to authenticate, when opening a connection to LDAP. What you really have here is the DN of a OU or school below which the LDAP search will be done. For LDAP searches that is called the "base". So the variable name should reflect that. * /unallowed/disallowed/ As a structure for the software I suggest to have a check_user() function, that takes a single DN and verifies that user account. Then have another function that collects all DNs that should be checked (possibly only 1 from the command line) and call that function. (The regex should then be compiled at import time at the top of the module or in a class, so that it isn't done inside a loop.)
Fixed mentioned issues with commit 762e57be8 Bug #52147: restructured code; enable giving multiple users as argument on branch troehmey/bug52147_object_consistency_script Now more than one user can be specified as command line arguments.
Great changes! Still a few things in the code review: * # This module check the constistency of USC@school users, shares and groups → """ → This module checks the consistency of UCS@school users, shares and groups → """ * "The container %s does not exist." % marktplatz_share → "The 'Marktplatz' share of school %r does not exist." % (ou,) * The group regex are good, but they can be made more strict (more validation) like this: -------------------------------------------------- self.teachers_regex = re.compile( r"cn={}-(?P<ou>[^,]+?),cn=groups,ou=(?P=ou),{}".format(container_teachers, ldap_base), flags=re.IGNORECASE, ) -------------------------------------------------- This will validate that the value for "ou" is the same in both the cn and the ou part. As a side effect the value of "ou" will be accessible like this: m := self.teachers_regex.match(dn): ou = m.groupdict()["ou"] * string formatting: in »"... %s" % (var)« the parenthesis are either redundant or better: it must be a tuple: »(var,)« * LDAP filters must be escaped, when receiving external input (cmdline): ---------------------------------------- from ldap.filter import filter_format lo.search(filter=filter_format('...(ucsschoolSchool=%s)...', (school,)) ---------------------------------------- * change the signature of the following function: --------------------------------------------------- def check_allowed_membership(self, group_dn, **kwargs): classes = {"students": False, "teachers": False, "staff": False, "teachers_and_staff": False} for cls in classes: if cls in kwargs: classes[cls] = kwargs[cls] ... if self.students_regex.match(group_dn) and not classes["students"]: ------------------------ to ------------------------ def check_allowed_membership(self, group_dn, students=False, teachers=False, staff=False, teachers_and_staff=False): ... if self.students_regex.match(group_dn) and not students: --------------------------------------------------- * Also add a docstring that explains what the function does (what it validates), and not the possible values of the arguments. If you want to describe argument values, please use static type annotations (see https://mypy.readthedocs.io/en/latest/cheat_sheet.html): # type: (str, Optional[bool], Optional[bool], Optional[bool], Optional[bool]) -> List[str] Regarding imports for "typing", see other school code. * check_user() shouldn't return a dict containing a single key/value pair with the DN it got passed as argument. The caller knows it. It can simply return the list of issues. * change the signature: --------------------------------------------------- def check_user(self, user_from_ldap): dn, attrs = user_from_ldap ... return problematic_objects ... for user in users_from_ldap: user_problematic_objects.update(user_check.check_user(user)) ---------------------- to --------------------------- def check_user(self, dn, attrs): ... return issues ... for dn, attrs in users_from_ldap: user_problematic_objects[dn] = user_check.check_user(dn, attrs) --------------------------------------------------- * line 180: returning {} means a WrongObjectType exception is OK * line 184: this can never happen. all LDAP objects have an objectClass. furthermore that case would be handled in line 187. * line 284: except for "cn=Domain Users $OU,..." all DNs in "mandatory_groups" are containers - not groups. The variable name suggests otherwise. →→→ Please create a separate function »check_containers()« that verifies this list (except for "Domain Users $OU"). The containers are documented in "Struktur einer UCS@school-OU" (https://docs.software-univention.de/ucsschool-handbuch-4.4.html#structure:ldap:ou). →→→ Add the groups documented in "Weitere UCS@school-Objekte" (https://docs.software-univention.de/ucsschool-handbuch-4.4.html#structure:ldap:global) to the list instead ($OU-DC-Edukativnetz, .., $OU-Klassenarbeit, admins-$OU). →→→ Also test the global groups (not per OU) documented in the same manual chapter: DC-Edukativnetz, .. * line 300: lo.searchDn(base=search_base.schoolDN) is a list of all DNs in a OU. * lines 300-303: simply look for the existence of those exact DNs in "mandatory_groups" with try: lo.searchDn(base=mandatory_group) except NO_SUCH_OBJECT: issues.append("...") * line 261 and 305: -------------------------- for issue in issues: problematic_objects.setdefault(search_base.schoolDN, []).append(issue) ------------- to ----------- problematic_objects[search_base.schoolDN] = issues -------------------------- * line 333: "container" → "share" * line 338: brackets not needed in »(dn, attrs)«
Applied fixes suggested in comment #8 with 170a34a8b Bug #52147: seperate container and group check; several fixes on branch troehmey/bug52147_object_consistency_script
String formatting with '%' is done with a tuple on the right side. -------------------------------------------- issues.append("Admin should not be in a students group %s" % (group_dn),) -------------- should be ------------------- issues.append("Admin should not be in a students group %s" % (group_dn,)) --------------------------------------------
-------------------------------------------- uid=exam-urs,cn=examusers,ou=Gym21,dc=uni,dc=dtr - User does not have UCS@School Role student:school:Gym21 -------------------------------------------- root@m20:/sync/ucs-school-metapackage# univention-ldapsearch -LLL uid=exam-urs ucsschoolRole dn: uid=exam-urs,cn=examusers,ou=Gym21,dc=uni,dc=dtr ucsschoolRole: exam_user:exam:exam07-Gym21 ucsschoolRole: exam_user:school:Gym21 -------------------------------------------- the ucsschool_role for exam users is "ucsschool.lib.roles.role_exam_user" → "exam_user". If an exam-user exists, its role should include "exam_user:school:*". '*', because the OU is the one in which the exam is taking place. It can be any of the schools the user is a member of.
I think the last sentence was confusing… If exam-users "(&(objectClass=ucsschoolStudent)(objectClass=ucsschoolExam))" are encountered they should have at least one role: -------------------------------------------- ucsschoolRole: exam_user:school:<one of the users schools> -------------------------------------------- Currently they should have one or more: -------------------------------------------- ucsschoolRole: exam_user:exam:<exam-id> -------------------------------------------- But that can be ignored, as exam-users will in the future be "recycled", and will thus exist without a running exam.
Please add a ucs-test that * creates an OU * deletes/modifies objects in it * runs the script * verifies in the scripts captured output, that all errors are caught BTW: I noticed, thanks to this script, that the test-OUs that we create in our test suite lack the mandatory container cn=examusers,ou=$OU,$ldap_base.
Fixed issues mentioned in comment #10 and comment #11 with commit 23f3ca0c8 Bug #52147: use format() function; handle exam users correctly A ucs-test as suggested in comment #13 is still to do.
OK: code changes OK: manual tests I have squash all commits into 1 and merged it into '4.4': [4.4] c5019858b Bug #52147: script to check the consistency of user, group, container and share objects Please build the package and write the advisory.
The script is not part of the Debian package ucs-school-umc-diagnostic (1.0.0-16A~4.4.0.202012142106).
Added the script to package with 41873d6b6 Bug #52147: update yaml c23801833 Bug #52147: add consistency script to debian package; changelog Successful build: Package: ucs-school-umc-diagnostic Version: 1.0.0-17A~4.4.0.202012151423 Branch: ucs_4.4-0 Scope: ucs-school-4.4
OK: script is installed as /usr/share/ucs-school-umc-diagnostic/scripts/ucs-school-object-consistency
Script is not automatically executed as diagnose module, so the package can be released.
The script expects users of the role "teacher and staff" to be in a group "lehrer und mitarbeiter-$OU" which doesn't exist. It complains about them being in "lehrer-$OU" and "mitarbeiter-$OU", but that is correct. ----------------------------------------------------------------------- The following USER PROBLEMS were found: uid=lehrmitarb,cn=lehrer und mitarbeiter,cn=users,ou=Gym21,dc=uni,dc=dtr - Not member of group cn=lehrer und mitarbeiter-gym21,cn=groups,ou=Gym21,dc=uni,dc=dtr - Disallowed member of teachers group cn=lehrer-gym21,cn=groups,ou=Gym21,dc=uni,dc=dtr. - Disallowed member of staff group cn=mitarbeiter-gym21,cn=groups,ou=Gym21,dc=uni,dc=dtr. ----------------------------------------------------------------------- Users of the role "teacher and staff" have all the properties of both teachers and staff, including their groups, ucsschool_roles and options/objectClasses.
Fixed the issue from comment #20 with commit 9ec7ca76b Bug #52147: fixed teacher_and_staff group check
A user with the role "teacher and staff" must be in BOTH "lehrer-$OU" and "mitarbeiter-$OU". ----------------------------------------------------------------------- uid=lehrmitarb,cn=lehrer und mitarbeiter,cn=users,ou=Gym21,dc=uni,dc=dtr - Disallowed member of staff group cn=mitarbeiter-gym21,cn=groups,ou=Gym21,dc=uni,dc=dtr. - Disallowed member of teachers group cn=lehrer-gym21,cn=groups,ou=Gym21,dc=uni,dc=dtr.
I'm sorry, I somehow missed that during my testing. This commit should fix this: c004d3145 Bug #52147: Fixed error in consistency script
Thanks, looks good now. I'll wait for Bug 52500 before I verify this.
I wrote a help article (I'm open for improvements) on school classes and work groups, which can be taken as a reference when the consistency reveals some problems: https://help.univention.com/t/ucs-school-work-groups-and-school-classes/16925 This article together with the following links may help the customer and developers with the troubleshooting: https://help.univention.com/t/how-a-ucs-school-user-should-look-like/15630 https://docs.software-univention.de/ucsschool-handbuch-4.4.html#structure:ldap:global
(In reply to Toni Röhmeyer from comment #25) > I wrote a help article (I'm open for improvements) on school classes and > work groups, which can be taken as a reference when the consistency reveals > some problems: > https://help.univention.com/t/ucs-school-work-groups-and-school-classes/16925 Thank you. I have redacted it. Please review my changes and improve on them where you see fit.
OK: code changes I merged and build it in 4.4, advisories.
Failed jenkins test should be fixed with: da19c8950 Bug #52147: updated version 4a82ecb7f Bug #52147: fixed consistency ucs-test (on 4.4) https://jenkins.knut.univention.de:8181/job/UCSschool-4.4/job/Install%20Singleserver/808/Config=s4,TestGroup=import1,UCSRelease=public/testReport/junit/90_ucsschool/77_object_consistency_script/master201/ successful builds: Package: ucs-school-lib Version: 12.2.17A~4.4.0.202103231509 Branch: ucs_4.4-0 Scope: ucs-school-4.4 Package: ucs-test-ucsschool Version: 6.0.200A~4.4.0.202103231524 Branch: ucs_4.4-0 Scope: ucs-school-4.4
UCS@school 4.4 v9 has been released. https://docs.software-univention.de/changelog-ucsschool-4.4v9-de.html If this error occurs again, please clone this bug.