Bug 53211 - ucs-school-object-consistency checks all objects even when --user_dn is given
ucs-school-object-consistency checks all objects even when --user_dn is given
Status: CLOSED FIXED
Product: UCS@school
Classification: Unclassified
Component: UMC - System diagnostic
UCS@school 4.4
Other Linux
: P5 normal (vote)
: UCS@school 4.4 v9-errata
Assigned To: Toni Röhmeyer
Tobias Wenzel
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2021-05-03 16:50 CEST by Daniel Tröder
Modified: 2021-07-01 12:09 CEST (History)
1 user (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 3: Simply Wrong: The implementation doesn't match the docu
Who will be affected by this bug?: 1: Will affect a very few installed domains
How will those affected feel about the bug?: 1: Nuisance – not a big deal but noticeable
User Pain: 0.017
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 2021-05-03 16:50:14 CEST
The ucs-school-object-consistency script has an option "--user_dn" which it says only checks given user(s). But it doesn't stop after checking them, it keep checking all schools.

=========================================================================

# /usr/share/ucs-school-umc-diagnostic/scripts/ucs-school-object-consistency --help

usage: ucs-school-object-consistency [-h] [--school SCHOOL]
                                     [--user_dn USER_DN [USER_DN ...]]

UCS@School Object Consistency Check

optional arguments:
  -h, --help            show this help message and exit
  --school SCHOOL       When specified, only objects of this school get checked.
                        Give the desired school abbreviation.
  --user_dn USER_DN [USER_DN ...]
                        When specified, only the given user(s) gets checked.
                        Give the desired DN(s) of the user(s) seperated by spaces.

=========================================================================

/usr/share/ucs-school-umc-diagnostic/scripts/ucs-school-object-consistency --user_dn uid=demo_student,cn=schueler,cn=users,ou=DEMOSCHOOL,dc=uni,dc=dtr


The following GROUP PROBLEMS were found:

  ou=TestOU01,dc=uni,dc=dtr
  - Mandatory group cn=OUTestOU01-Klassenarbeit,cn=ucsschool,cn=groups,dc=uni,dc=dtr

[..]

=========================================================================

I think a "if not school: return" at line 81 would fix this, but havent' tested it.
Comment 1 Toni Röhmeyer univentionstaff 2021-05-05 10:23:15 CEST
I supplied a patch on branch troehmey/bug53211_fix_consistency_script_param
with commit:
73622d56c Bug #53211: dont do other checks when user_dn is specified

Other checks than the user check are executed after the following conditions:
if school or not user_dn:
    ...

Leads to the following behavior:
no parameters -> everything gets checked
--school -> check everything for this school
-- user_dn -> only this user gets checked
-- school --user_dn -> check this user and execute all checks for given school
Comment 2 Tobias Wenzel univentionstaff 2021-05-10 15:44:22 CEST
> I think a "if not school: return" at line 81 would fix this, but havent' tested it.
This is before the print_problematic_objects calls.



QA → reopen

Code:

if school or not user_dn
→ add at least a comment. Because school=None in check_*, this is confusing to read.
e.g. "If no school is passed, all UCS@school objects are checked."


Functionality:

Before the fix

The following USER PROBLEMS were found:

  uid=demo_staff,cn=mitarbeiter,cn=users,ou=DEMOSCHOOL,dc=dc-we,dc=intranet
  - Not member of group cn=mitarbeiter-demoschool,cn=groups,ou=DEMOSCHOOL,dc=dc-we,dc=intranet

The following GROUP PROBLEMS were found:

  ou=DEMOSCHOOL2,dc=dc-we,dc=intranet
  - Mandatory group cn=OUDEMOSCHOOL2-Klassenarbeit,cn=ucsschool,cn=groups,dc=dc-we,dc=intranet does not exist.

After the fix


The following USER PROBLEMS were found:

  uid=demo_staff,cn=mitarbeiter,cn=users,ou=DEMOSCHOOL,dc=dc-we,dc=intranet
  - Not member of group cn=mitarbeiter-demoschool,cn=groups,ou=DEMOSCHOOL,dc=dc-we,dc=intranet


-- school --user_dn → check this user and execute all checks for given school

/usr/share/ucs-school-umc-diagnostic/scripts/ucs-school-object-consistency --user_dn uid=demo_staff,cn=mitarbeiter,cn=users,ou=DEMOSCHOOL,dc=dc-we,dc=intranet --school DEMOSCHOOL

I got all broken users of DEMOSCHOOL, but not the groups.


If think using user_dn and school should not be allowed (e.g. via https://docs.python.org/2.7/library/argparse.html#mutual-exclusion) or the code has to be changed to handle a list of user_dn and only return those being inside --school DEMOSCHOOL.  We should talk about this.
Comment 3 Toni Röhmeyer univentionstaff 2021-05-25 20:42:04 CEST
I like the idea with the mutually exclusive group.
I implemented that with this commit:
940de61ef Bug #53211: add mutually exclusive group in argument parser

behavior now:

no parameters -> everything gets checked
--school -> check everything for this school
-- user_dn -> only this user gets checked
-- school --user_dn -> won't run; arguments exclude each other


I do not think it is a common use-case to check one particular user and a complete school in one consistency-script run.
Therefore, this (new) behavior seems fine by me.
Comment 4 Tobias Wenzel univentionstaff 2021-06-01 16:40:40 CEST
QA → reopen


/usr/share/ucs-school-umc-diagnostic/scripts/ucs-school-object-consistency --user_dn uid=demo_staff,cn=mitarbeiter,cn=users,ou=DEMOSCHOOL,dc=dc-we,dc=intranet --school DEMOSCHOOL
usage: ucs-school-object-consistency [-h]
                                     [--school SCHOOL | --user_dn USER_DN [USER_DN ...]]
ucs-school-object-consistency: error: argument --school: not allowed with argument --user_d
→ OK




I create DEMOSCHOOL-2 in a multi-server-env and got the following output:

/usr/share/ucs-school-umc-diagnostic/scripts/ucs-school-object-consistency --school DEMOSCHOOL-2


The following GROUP PROBLEMS were found:

  ou=DEMOSCHOOL-2,dc=dc-we,dc=intranet
  - Mandatory group cn=Domain Users DEMOSCHOOL-2,cn=groups,ou=DEMOSCHOOL-2,dc=dc-we,dc=intranet does not exist.
  - Mandatory group cn=OUDEMOSCHOOL-2-DC-Edukativnetz,cn=ucsschool,cn=groups,dc=dc-we,dc=intranet does not exist.
 ....

but not   

uid=h.schlemmer,cn=schueler,cn=users,ou=DEMOSCHOOL2,dc=dc-we,dc=intranet
  - Not member of group cn=schueler-demoschool2,cn=groups,ou=DEMOSCHOOL2,dc=dc-we,dc=intranet


also, when

/usr/share/ucs-school-umc-diagnostic/scripts/ucs-school-object-consistency 
-> broken users & groups -> OK
-> the rest, e.g. containers, shares etc. are missing
Comment 5 Toni Röhmeyer univentionstaff 2021-06-03 15:16:28 CEST
You probably specified a non-existing school.
I tried to reproduced the problem and stumbled over the following:

Via UMC I created a new school with the name DEMOSCHOOL-2.
A school abbreviation DEMOSCHOOL2 was automatically generated.

The script expects the school abbreviation. However, when giving a school which does not exist, the script still checks all the properties.
-> Many errors get displayed but not user errors, because users in a school "DEMOSCHOOL-2" simply cannot be found, since the ucsschoolSchool attribute is "DEMOSCHOOL2".

I committed a fix, so that the user gets warned when the given school-abbreviation does not exist.

b03179502 (HEAD -> troehmey/bug53211_fix_consistency_script_param, origin/troehmey/bug53211_fix_consistency_script_param) Bug #53211: warn user if given school does not exist


Was this causing your problem?
Comment 6 Toni Röhmeyer univentionstaff 2021-06-04 10:23:00 CEST
As communicated, I applied a patch so that the school existence is checked via school-lib function instead of an ldap-search:

8dfc43fa2 Bug #53211: use school-lib to check school existence
Comment 7 Toni Röhmeyer univentionstaff 2021-06-08 14:13:34 CEST
Merged to 4.4 with

commit 44fbf16293e8b908c7e9cac1243c960af7856db8 
Author: Toni Röhmeyer <roehmeyer@univention.de>
Date:   Tue Jun 8 14:11:56 2021 +0200

    Bug #53211: added advisories

commit 00e4b144a033fc3411ed61307c40e6d87494d914
Author: Toni Röhmeyer <roehmeyer@univention.de>
Date:   Tue Jun 8 13:59:16 2021 +0200

    Bug #53211: add changelog entries

commit 58a3083514eb1ec4c2c6d3023a3f813f15b09033
Merge: bfe51462a a62499a56
Author: Toni Röhmeyer <roehmeyer@univention.de>
Date:   Tue Jun 8 13:53:05 2021 +0200

    Bug #53211: Merge branch 'troehmey/bug53211_fix_consistency_script_param' into 4.4

commit a62499a563d672b15a70f774333354f317025292 (troehmey/bug53211_fix_consistency_script_param)
Author: Toni Röhmeyer <roehmeyer@univention.de>
Date:   Wed May 5 10:13:26 2021 +0200

    Bug #53211: dont do other checks when user_dn is specified
    
    Bug #53211: warn user if given school does not exist


Successful builds:

Package: ucs-school-lib
Version: 12.2.32A~4.4.0.202106081359
Branch: ucs_4.4-0
Scope: ucs-school-4.4

Package: ucs-school-umc-diagnostic
Version: 1.0.0-22A~4.4.0.202106081405
Branch: ucs_4.4-0
Scope: ucs-school-4.4
Comment 8 Tobias Wenzel univentionstaff 2021-06-08 14:42:38 CEST
QA

merge OK
changelog OK
yaml OK

wait for jenkins to verify
Comment 9 Tobias Wenzel univentionstaff 2021-06-09 08:50:39 CEST
Jenkins is happy -> verify
Comment 10 Toni Röhmeyer univentionstaff 2021-06-09 11:15:40 CEST
Create request to merge into 5.0 with
troehmey/5.0/53211_fix_consistency_script_param

https://git.knut.univention.de/univention/ucsschool/-/merge_requests/22
Comment 11 Tobias Wenzel univentionstaff 2021-07-01 12:09:17 CEST
Errata updates for UCS@school 4.4 v9 have been released.

https://docs.software-univention.de/changelog-ucsschool-4.4v9-de.html

If this error occurs again, please clone this bug.