Univention Bugzilla – Bug 41351
Prevent invalid combinations of UCS@school options
Last modified: 2016-10-04 13:24:51 CEST
A hook scripts should prevent that a UCS@school user is a student and a teacher at the same time. +++ This bug was initially created as a clone of Bug #41014 +++ Bug #41008 introduced object classes for user roles and an attribute which specifies the OUs the user belongs to. For each object class an extended option should be added to users/user. For the attribute ucsschoolSchool a extended attribute should be added. The attribute needs a dynamic syntax class which contains every UCS@school OU (objectClass=ucsschoolOrganizationalUnit) and present them as a dropdown.
There is a new
(In reply to Sönke Schwardt-Krummrich from comment #1) > There is a new ah really? cool.
I wanted to do this with a ditcontentrule instead of a hook. It would raise an LDAPError (OBJECT_CLASS_VIOLATION) if the user tries to set an invalid combination.
(In reply to Florian Best from comment #3) > I wanted to do this with a ditcontentrule instead of a hook. It would raise > an LDAPError (OBJECT_CLASS_VIOLATION) if the user tries to set an invalid > combination. But in this case, there is no possibility to present a "nice" error message to the user. Instead only "object class violation" is returned and presented in the popup. The hook is able to present a human readable error message that is also translated into several languages. For this scenario I still prefer the UDM hook.
(In reply to Sönke Schwardt-Krummrich from comment #4) > (In reply to Florian Best from comment #3) > > I wanted to do this with a ditcontentrule instead of a hook. It would raise > > an LDAPError (OBJECT_CLASS_VIOLATION) if the user tries to set an invalid > > combination. > > But in this case, there is no possibility to present a "nice" error message > to the user. Instead only "object class violation" is returned and presented > in the popup. The hook is able to present a human readable error message > that is also translated into several languages. > > For this scenario I still prefer the UDM hook. The error message which would be shown in UMC would be: LDAP Error: Object class violation: class 'ucsschoolTeacher' not allowed by content rule 'ucsschoolStudent'. We should add the ditcontentrule nevertheless to ensure LDAP consistency.
(In reply to Florian Best from comment #5) > The error message which would be shown in UMC would be: > LDAP Error: Object class violation: class 'ucsschoolTeacher' not allowed by > content rule 'ucsschoolStudent'. I do not think, that most end users are able/want to understand this message. > We should add the ditcontentrule nevertheless to ensure LDAP consistency. yes
Created attachment 7815 [details] ditcontentrules > We should add the ditcontentrule nevertheless to ensure LDAP consistency. I don't think that's possible. ditcontentrules can only be applied to STRUCTURAL object classes, not to AUXILIARY. I tried nonetheless (with the attached patch) and when running ucs_registerLDAPExtension I got in the listener.log: 19.07.16 09:49:19.205 LISTENER ( ERROR ) : ldap_extension: validation failed: 578ddb7f /var/lib/univention-ldap/local-schema/ucs-school-import.schema: line 45 ditcontentrule: Content Rule not for STRUCTURAL object class: "1.3.6.1.4.1.10176.4000.2.53.1" slapschema: bad configuration file! I'm implementing the hook for now.
(In reply to Daniel Tröder from comment #7) Oh yes, ditcontentrules can only be used on structural object classes.
Added a UDM hook that checks modifications for illegal option combinations. Code: 71093 YAML: 71094
REOPEN (also affects other bugs, but noticed here) 1) there are 2 directories in SVN containing UDM hooks: ./modules/univention/admin/hooks.d ./udm_hook 2) there are several hook directories below /usr/share/ucs-school-import/ /usr/share/ucs-school-import/pyhooks ← new python hooks /usr/share/ucs-school-import/scripts ← import_user & co /usr/share/ucs-school-import/schema ← contains LDAP schema and ???syntax.ucs-school-import.py??? /usr/share/ucs-school-import/configs ← new import config /usr/share/ucs-school-import/hooks ← old sh/bash hook scripts /usr/share/ucs-school-import/hooks.d ← contains schoolOU.py, schoolAdminGroup.py I think the last one directory is irritating to the customer and should be avoided. Why is there a new hooks.d/ if schema already contains UDM related stuff? 3) ucsschool_user-options_hook.py is not registered in LDAP and therefore without effect.
(In reply to Sönke Schwardt-Krummrich from comment #10) > 3) ucsschool_user-options_hook.py is not registered in LDAP and therefore > without effect. Small correction: the python file is installed to /usr/share/pyshared/univention/admin/hooks.d/ucsschool_user-options_hook.py but this way, the hook is not available on other systems that DC master. → please move the hook to /usr/share/ucs-school-import/schema → the hook and the mo-file has to be registered in LDAP via join script I will create a new bug for 1) + 2).
r71810: move files to /usr/share/ucs-school-import/schema and install UDM hook + msg catalog domain wide r71811: YAML
OK: hook is now in correct place and registered in LDAP 1) ucsschoolExam and ucsschoolStudent is allowed → REOPEN --- a/ucs-school-4.1r2/ucs-school-import/udm_hook/ucsschool_user-options_hook.py +++ b/ucs-school-4.1r2/ucs-school-import/udm_hook/ucsschool_user-options_hook.py @@ -43,9 +43,9 @@ _=translation.translate blacklisted_option_additions = { "ucsschoolAdministrator": ["ucsschoolExam", "ucsschoolStudent"], - "ucsschoolExam": ["ucsschoolAdministrator", "ucsschoolStaff", "ucsschoolStudent", "ucsschoolTeacher"], + "ucsschoolExam": ["ucsschoolAdministrator", "ucsschoolStaff", "ucsschoolTeacher"], "ucsschoolStaff": ["ucsschoolExam", "ucsschoolStudent"], - "ucsschoolStudent": ["ucsschoolExam", "ucsschoolAdministrator", "ucsschoolStaff", "ucsschoolTeacher"], + "ucsschoolStudent": ["ucsschoolAdministrator", "ucsschoolStaff", "ucsschoolTeacher"], "ucsschoolTeacher": ["ucsschoolExam", "ucsschoolStudent"], } 2) The hook does not catch invalid combinations during user account creation. Only the user account modification is checked. → REOPEN 3) The hook only checks if NEW options create an invalid combination. If the combination is already invalid, and the object is changed, no error message is thrown. The hook should check always all combinations. → REOPEN
(In reply to Sönke Schwardt-Krummrich from comment #11) > I will create a new bug for 1) + 2). I did: Bug 42066
r71981: * catch invalid option combinations at creation and modify time * allow ucsschoolExam and ucsschoolStudent together * check option combinations also, when no ucsschool options were modified * fix i18n
Please raise univention.admin.uexceptions.invalidOptions instead of valueError. I would also rename the hook script into "ucsschool_user_options.py".
done: r72008 If you have installed the previous version, after running univention-run-join-scripts --run-scripts --force 35ucs-school-import.inst, you may wish to run: rm -f /usr/lib/pymodules/python2.7/univention/admin/hooks.d/ucsschool_user-options_hook.py* /usr/share/locale/de/LC_MESSAGES/univention-admin-hooks-ucsschool_user-options_hook.mo /usr/share/pyshared/univention/admin/hooks.d/ucsschool_user-options_hook.py
r72516: added test for illegal option combinations: 90_ucsschool/109_invalid_option_combinations
r72596: add missing update code (jenkins 4.0 -> 4.1 revealed it)
(In reply to Daniel Tröder from comment #18) > r72516: added test for illegal option combinations: > 90_ucsschool/109_invalid_option_combinations Can you please not depend on the internals of the code: 20 import sys 21 sys.path.append("/usr/share/pyshared/univention/admin/hooks.d") 22 from ucsschool_user_options import blacklisted_option_combinations It's better to duplicate this, because: * it might be wrong in the code (somewhen) and causes the script to pass while there is an error * it makes changing the production code harder (e.g. renaming the hook or the variable causes the script to be broken)
agreed - r72666
Created attachment 8020 [details] patch Patch contains: * use the real option names / don't duplicate translation * don't log twice * remove unused "type = ..." member * only tell about the actual set options in the error message (first match wins as before)
r72675: removing duplicate strings and trans is nice. applied your patch, but kept "type = ...", because its in the ucs developer docs.
OK: invalid options are prevented OK: update OK: ucs-test OK: YAML
UCS@school 4.1 R2 v5 has been released. http://docs.software-univention.de/changelog-ucsschool-4.1R2v5-de.html If this error occurs again, please clone this bug.