Bug 53139 - Diagnose Check admin accounts gives misleading output
Diagnose Check admin accounts gives misleading output
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-04-23 11:55 CEST by Christina Scheinig
Modified: 2021-07-01 12:08 CEST (History)
3 users (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?: 3: Will affect average number of installed domains
How will those affected feel about the bug?: 1: Nuisance – not a big deal but noticeable
User Pain: 0.051
Enterprise Customer affected?:
School Customer affected?: Yes
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number: 2021041521000332
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 Christina Scheinig univentionstaff 2021-04-23 11:55:08 CEST
UCS@school Administrators are configured with an objectclass 'ucsschoolAdministrator'.
An administrator of a school should be a member of the respective admins-school group.

The following problems were found:

  uid=orsine.gisker,cn=lehrer,cn=users,ou=one,dc=schein,dc=qa
   - is registered as admin but no member of the following schools: ['one']

-----
 univention-ldapsearch -LLL uid=orsine.gisker memberof ucsschoolRole ucsschoolSchool objectClass
dn: uid=orsine.gisker,cn=lehrer,cn=users,ou=one,dc=schein,dc=qa
ucsschoolRole: teacher:school:one
ucsschoolSchool: one
memberOf: cn=Domain Users one,cn=groups,ou=one,dc=schein,dc=qa
memberOf: cn=one-1a,cn=klassen,cn=schueler,cn=groups,ou=one,dc=schein,dc=qa
memberOf: cn=one-1b,cn=klassen,cn=schueler,cn=groups,ou=one,dc=schein,dc=qa
memberOf: cn=lehrer-one,cn=groups,ou=one,dc=schein,dc=qa
objectClass: krb5KDCEntry
objectClass: ucsschoolAdministrator
objectClass: organizationalPerson
objectClass: automount
objectClass: top
objectClass: inetOrgPerson
objectClass: krb5Principal
objectClass: person
objectClass: ucsschoolTeacher
objectClass: univentionMail
objectClass: univentionObject
objectClass: ucsschoolType
objectClass: univentionPWHistory
objectClass: shadowAccount
objectClass: sambaSamAccount
objectClass: posixAccount

The school is set properly but the group is missing. The Module should show the missing group, like the consistency check does.
Comment 1 Daniel Tröder univentionstaff 2021-04-26 08:12:02 CEST
Is this the desired change?

- is registered as admin but no member of the following schools: ['one']
+ is registered as admin but no member of the following groups: ['admins-one']
Comment 2 Christina Scheinig univentionstaff 2021-04-28 15:57:13 CEST
(In reply to Daniel Tröder from comment #1)
> Is this the desired change?
> 
> - is registered as admin but no member of the following schools: ['one']
> + is registered as admin but no member of the following groups:
> ['admins-one']

Yes, or even more better is the output of the consistency check giving the dn of the group. But 'admins-one' would do the job. 
- Not member of group cn=admins-one,cn=ouadmins,cn=groups,dc=schein,dc=qa

Also a little bit inconsequential is that this check, does not complain about the role, although the check tests the admin account.
Comment 3 Toni Röhmeyer univentionstaff 2021-04-30 14:29:18 CEST
I committed a patch to troehmey/bug53139_check_admin_accounts_output with:
6b637710b Bug #53139: improved output for admin group membership check

The output message was improved as suggested in the bug description.

I tested the behavior by removing an admin from its admin-ou group and then executing the diagnostic module.
Comment 4 Tobias Wenzel univentionstaff 2021-05-10 16:15:02 CEST
QA → reopen

try:
    attr["uniqueMember"]
except KeyError:
    # this group has no members
    attr["uniqueMember"] = []

→ attr.get("uniqueMember", [])

if not admin["dn"] in attr["uniqueMember"]:
    problematic_objects.setdefault(admin["dn"], []).append(
        _("is registered as admin but no member of the following groups: {}".format(dn))
    )
→ This does not seem to make sense for admins in multiple ous. Collect the dns of the missing groups and do something like this after the group-loop:

if missing_group_dns:
            problematic_objects.setdefault(admin["dn"], []).append(
                _("is registered as admin but no member of the following groups: {}".format(missing_group_dns))
            )


if not admin["schools"]:
→ move to outer loop (for admin in admins)


Functionality


before fix:

  uid=h.schlemmer,cn=lehrer,cn=users,ou=DEMOSCHOOL,dc=dc-we,dc=intranet
   - is registered as admin but no member of the following schools: ['DEMOSCHOOL']

after fix:

  uid=demo_admin,cn=lehrer,cn=users,ou=DEMOSCHOOL,dc=dc-we,dc=intranet
   - is registered as admin but no member of the following groups: cn=admins-demoschool,cn=ouadmins,cn=groups,dc=dc-we,dc=intranet
Comment 5 Toni Röhmeyer univentionstaff 2021-06-04 12:25:57 CEST
I applied the suggestions from comment #4 with

40c52e649 Bug #53139: fixup, improved printing
Comment 6 Toni Röhmeyer univentionstaff 2021-06-08 15:59:48 CEST
Fixup:
44e99bc82 Bug #53139: check if admin in wrong admins-ou group
c1fd882bd Bug #53139: fixup search missing groups by schoolRole

With the previous patch, teachers which are admin at a school but not at all schools were not handled correctly.
Those are not expected to be in an admin group at every school just because their objectClass is set to "ucsschoolAdministrator"

The requirements for those are the following:

1.  objectClass = ucsschoolAdministrator
2.  must be admin at at least one of his schools
2.1 must be member of cn=admin-ou
2.2 ucsschoolRole must be corresponding entry
3.  must not be in any other cn=admin-ou group (of other school)


The correct group memberships are now derived from the ucsschoolRole.
Example:
school_admin:school:DEMOSCHOOL -> this admin should be member of cn=admins-demoschool

Also wrong group memberships now get detected.
Comment 7 Toni Röhmeyer univentionstaff 2021-06-08 16:59:21 CEST
Merged to 4.4 with

1e644f072 Bug #53139: added advisory
ed4a1dcb3 Bug #53139: added changelog entry
63709a5d0 Bug #53139: Merge branch 'troehmey/bug53139_check_admin_accounts_output' into 4.4
89e42dd43 Bug #53139: check if admin in wrong admins-ou group
408a9c499 Bug #53139: improved output for admin group membership check


Successful build:

Package: ucs-school-umc-diagnostic
Version: 1.0.0-23A~4.4.0.202106081652
Branch: ucs_4.4-0
Scope: ucs-school-4.4
Comment 8 Tobias Wenzel univentionstaff 2021-06-09 08:49:07 CEST
QA: all OK

changelog OK
advisory OK
merge OK
jenkins happy
Comment 9 Toni Röhmeyer univentionstaff 2021-06-09 11:16:40 CEST
Create merge request to 5.0 with
troehmey/5.0/53139_check_admin_accounts_output

https://git.knut.univention.de/univention/ucsschool/-/merge_requests/23
Comment 10 Tobias Wenzel univentionstaff 2021-07-01 12:08:42 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.