Univention Bugzilla – Bug 45577
allow import with usernames longer than 15 characters
Last modified: 2017-12-21 12:23:00 CET
The import restricts the length of usernames to 20 characters. It then removes 5 for the "exam-" prefix and another 3 for numbers for duplicate names. The result is an effective maximum length of 12 for usernames. At least for teachers and staff the removal of 5 for the "exam-" prefix is not necessary. 1) Read the maximum allowed username length from a UCRV, with a default of 20. 2) Lower the max length by len("exam-")=5 only for students. 3) Make the lowering of the length for students optional for those customers not using the exam mode.
Changes are in branch dtroeder/45577_username_length: * manual * code * ucs-test (1 adapted, 1 new) I accidentally merged the branch to 4.2, but reverted it immediately: - Commit e78934f6: Merge branch 'dtroeder/45577_username_length' into 4.2 - Commit 3b2a81ec: Revert "Bug #45577: Merge branch 'dtroeder/45577_username_length' into 4.2"
* Please rename the config key "length" into "maxlength". +<!-- username:no_exam_users --> +» » » » » » » <row> +» » » » » » » » <entry morerows="1"><varname>username: \ no_exam_users</varname></entry> +» » » » » » » » <entry>Die maximale Länge des Benutzernamens von Schülern nicht um die Länge des Klassenarbeitsmodus-Präfixes (Standard <literal>"exam-"</literal>: 5) verkürzen. +» » » » » » » » » Diese Option darf nur eingeschaltet werden, wenn sicher ist, dass der Klassenarbeitsmodus <emphasis>nicht</emphasis> verwendet wird. +» » » » » » » » » Schüler mit einem Benutzernamen der länger als 15 Zeichen ist, werden sich bei der Verwendung des Klassenarbeitsmodus nicht einloggen können!</entry> +» » » » » » » </row> → I am not sure if we should support/document this scenario: "Diese Option darf nur eingeschaltet werden, wenn sicher ist, dass der Klassenarbeitsmodus <emphasis>nicht</emphasis> verwendet wird." Otherwise i'm fine with the changes, you can merge.
8340915b: make maximum username length configurable 848045fe: advisory http://jenkins.knut.univention.de:8080/job/UCSschool%204.2/job/Manual/30/artifact/webroot/ucsschool-import-handbuch-4.2.pdf ucs-school-import 15.0.3-8 ucs-test-ucsschool 4.0.4-41
There is a little inconsistency. The max-length is reduced by 3 if counter variables are used in the scheme. Then it is from code perspective possible that the cut_pos is negative which results in name[:cut_pos] == 'foobarbaz' instead of ''. 'foobarbaz' is longer than the allowed maximum length. You will get a traceback then: Entry #2: Username 'ubald.boeckma' is longer than allowed. Traceback (most recent call last): File "/usr/lib/pymodules/python2.7/ucsschool/importer/mass_import/user_import.py", line 119, in create_and_modify_users user = self.determine_add_modify_action(imported_user) File "/usr/lib/pymodules/python2.7/ucsschool/importer/mass_import/user_import.py", line 210, in determine_add_modify_action imported_user.prepare_all(new_user=True) File "/usr/lib/pymodules/python2.7/ucsschool/importer/models/import_user.py", line 377, in prepare_all self.run_checks(check_username=new_user) File "/usr/lib/pymodules/python2.7/ucsschool/importer/models/import_user.py", line 744, in run_checks raise UsernameToLong("Username '{}' is longer than allowed.".format(self.name), entry_count=self.entry_count, import_user=self) UsernameToLong: Username 'ubald.boeckma' is longer than allowed. Instead of a technical more correct traceback: Entry #0: No username in 'ilda.richer[counter2]'. Traceback (most recent call last): File "/usr/lib/pymodules/python2.7/ucsschool/importer/mass_import/user_import.py", line 119, in create_and_modify_users user = self.determine_add_modify_action(imported_user) File "/usr/lib/pymodules/python2.7/ucsschool/importer/mass_import/user_import.py", line 210, in determine_add_modify_action imported_user.prepare_all(new_user=True) File "/usr/lib/pymodules/python2.7/ucsschool/importer/models/import_user.py", line 376, in prepare_all self.prepare_attributes(new_user) File "/usr/lib/pymodules/python2.7/ucsschool/importer/models/import_user.py", line 389, in prepare_attributes self.make_username() File "/usr/lib/pymodules/python2.7/ucsschool/importer/models/import_user.py", line 654, in make_username self.name = self.username_handler.format_name(self.name) File "/usr/lib/pymodules/python2.7/ucsschool/importer/utils/username_handler.py", line 366, in format_name raise EmptyFormatResultError("No {} in {!r}.".format(self.attribute_name, name), name, name) EmptyFormatResultError: No username in 'ilda.richer[counter2]'. A patch for this would be: diff --git a/ucs-school-import/modules/ucsschool/importer/utils/username_handler.py b/ucs-school-import/modules/ucsschool/importer/utils/username_handler.py index e87f874..b1b65af 100644 --- a/ucs-school-import/modules/ucsschool/importer/utils/username_handler.py +++ b/ucs-school-import/modules/ucsschool/importer/utils/username_handler.py @@ -338,7 +338,7 @@ class UsernameHandler(object): » » match = self.replacement_variable_pattern.search(name) » » if match: » » » func = self.counter_variable_to_function[match.group().upper()] -» » » cut_pos = max_length - PATTERN_FUNC_MAXLENGTH +» » » cut_pos = max(0, max_length - PATTERN_FUNC_MAXLENGTH) » » » # it's not allowed to have two [COUNTER] patterns » » » if len(self.replacement_variable_pattern.findall(name)) >= 2:
If you now configure a maximum limit for students then the limit is not decreased by len('exam-'). Is this correct behavior? For example I configured a maxlength of 25 and imported a user with a counter variable. The user is named 'giannifoobar.jungerman' (len 22), which is more than the samba maximum of 20 - len('exam-').
OK: The limits are ignored if the user already exists (but has a longer name than the configured maximum) and is not renamed. (I think a test case for this might be good).
Maybe an explicit error message might help, too. -» » » cut_pos = max_length - PATTERN_FUNC_MAXLENGTH +» » » cut_pos = max(0, max_length - PATTERN_FUNC_MAXLENGTH) +» » » if not cut_pos: +» » » » raise RuntimeError('The specified maximum length %d is too low. Please fix the configuration.' % (max_length,))
The configuration key username:no_exam_users was not implemented. [4.2 e1eb5ace] Bug #45577: handle no_exam_users switch [4.2 aca63c31] Bug #45577: advisory I don't think it is necessary to write code to handle users that manage to configure username:max_length <= 3. "username" in the UCRV ucsschool/import/generate/user/attributes/no-overwrite prevents renames. ucs-school-import 15.0.3-10A~4.2.0.201711281530
comment #5 is still unfixed. I would reverse the new complicated configuration: no_exam_users=false → exam_users=true. The samba maximum of 15(?!) characters with no_exam_users=False is ignored. Now we have something like 30 - len('exam-') = 25 which is higher than the samba maximum leading to it's not possible to write an exam.
(In reply to Florian Best from comment #9) > comment #5 is still unfixed. It is, see below. > I would reverse the new complicated configuration: > no_exam_users=false → exam_users=true. ACK. [4.2 1bda12a9] Bug #45577: remove double negation in configuration > The samba maximum of 15(?!) characters with no_exam_users=False is ignored. > Now we have something like 30 - len('exam-') = 25 which is higher than the > samba maximum leading to it's not possible to write an exam. Have you actually tested this? The defaults are 15 for students (with exam_users=true) and 20 for other roles. Test: $ cp /usr/share/ucs-school-import/configs/user_import_defaults.json /var/lib/ucs-school-import/configs/user_import.json $ python -c 'from ucsschool.importer.utils.shell import *; print("teacher={} student={}".format(ImportTeacher().username_max_length, ImportStudent().username_max_length))' [..] teacher=20 student=15 [4.2 c37c4b8b] Bug #45577: advisory update I enhanced the ucs-test with a case for username:exam_users=False: [4.2 2922aad8] Bug #45577: test username:exam_users switch ucs-school-import 15.0.3-12 ucs-test-ucsschool 4.0.4-43
(In reply to Daniel Tröder from comment #10) > (In reply to Florian Best from comment #9) > > comment #5 is still unfixed. > It is, see below. No, it's not, see the following: > > The samba maximum of 15(?!) characters with no_exam_users=False is ignored. > > Now we have something like 30 - len('exam-') = 25 which is higher than the > > samba maximum leading to it's not possible to write an exam. > Have you actually tested this? > The defaults are 15 for students (with exam_users=true) and 20 for other > roles. Samba has a maximum username length of 20 (sorry not 15). So if the importer allows to create usernames with 16-chars (which it now does!) and the new configuration exam_users=true is set, then the created exam user cannot login into windows because the username is e.g. len('exam-abcdefghijk') == 16. I still think the name of the configuration should be adjusted: username.exam_user cannot be understood intuitive. Why not naming it by the function which it does?: username.shorten_by_exam_user_prefix_length = true.
(In reply to Florian Best from comment #11) > I still think the name of the configuration should be adjusted: > username.exam_user cannot be understood intuitive. > Why not naming it by the function which it does?: > username.shorten_by_exam_user_prefix_length = true. I am also wondering if we need this configuration at all?
(In reply to Florian Best from comment #11) > Samba has a maximum username length of 20 (sorry not 15). So if the importer > allows to create usernames with 16-chars (which it now does!) Yes - that is intentional! If gives the user the choice to change the configuration from its default to something that fits its requirements. The default still allows student names only with 15 chars. If you change your settings from the default, you must read the documentation. The point of this bug was to allow longer usernames than 15 chars even for students. That they won't be able to use the exam mode is known and expicitely documented. The option to disable the shorting of students usernames is a requirement. (See the bugs description.) The meaning of username:exam_users is documented, a longer variable name is imho not necessary. > I am also wondering if we need this configuration at all? It was an explicit wish from ProfS. ----- But I think that in the end both username:exam_users and username.shorten_by_exam_user_prefix_length are somehow anti-intuitive: Either you want to have exam-users or not. * If you want to use them, then lowering by 5 from 21 doesn't make sense. * If you don't want them, lowering isn't necessary at all. So I suggest: 1) If exam users are desired, there is no way to raise the username length above 15 (or rather 20-len(UCRV, "exam-")): --- if username:exam_users == True: max_username_length(student) = max(15, username:max_length:student/default) else: max_username_length(student) = max(0, username:max_length:student/default) --- Other roles are not effected by username:exam_users: --- max_username_length(others) = max(0, username:max_length:other/default) --- 2) When the configuration is read from the JSON files (and username:exam_users == True), the value of username:max_length:student will be lowered if necessary to fit username:max_length:student - len(UCRV, "exam-") <= 15 That will be done right at the start of the import script, so that it is reflected in the configuration that is dumped to the logfile/stdout. A warning message in the middle of the import will be drowned be the rest of the log. 3) The default configuration will carry a value of "15" for username:max_length:student and "20" for username:max_length:default. So when a user changes the value to something above the default of 15, he will see in the log that his change is ignored. Then he'll be forced to set username:exam_users=False and will thus explicitly consent to not using the exam mode. IMHO this is transparent and explicit.
- max_username_length(student) = max(15, username:max_length:student/default) + max_username_length(student) = min(15, username:max_length:student/default)
[4.2 3275bbd1] Bug #45577: check configuration sanity and remove special handling of students username length [4.2 fd2b76e5] Bug #45577: advisory update Package: ucs-school-import Version: 15.0.3-13A~4.2.0.201712051456 [4.2 c1ead349] Bug #45577: adapt test Package: ucs-test-ucsschool Version: 4.0.4-44A~4.2.0.201712051532
The limitation of students usernames to 15 characters now works and a traceback is shown if the configuration uses higher limits. If we configure exam=false then the maximum length from the configuration is now used. This introduces another problem now: When importing teachers or students with exam=false configuration the maximum username length of 20 is not effective anymore. A user with a longer name than 20 characters cannot login into Samba/Windows. The greatest problem is that we cannot change this in the future anymore. If there are user objects with length > 20 then the import will not shorten the username anymore. This would lead to permanently broken user objects.
The implementation should only cover requirements 1 and 2 of > 1) Read the maximum allowed username length from a UCRV, with a default of 20. > 2) Lower the max length by len("exam-")=5 only for students. > 3) Make the lowering of the length for students optional for those customers not using the exam mode. The implementation of requirement 3 is not desired because it will make the product much more complicated and might force customers into very expensive migrations if they later on decide to start using the exam mode.
The hard limits of 15 for students and 20 for other roles were implemented. ("Exam-"user length is always remove for students.) The test now also checks the error condition (if the configuration is bad). [4.2 d6a2addb] Bug #45577: username length hard limit is 20, for students 15, test also for configuration errors [4.2 4e2b07fb] Bug #45577: advisory ucs-school-import 15.0.3-14
As discussed: * The hardcoded default of 15 is wrong. It must be 20 - len(exam_prefix) * The except KeyError has no affect anymore.
[4.2 c681df7c] Bug #45577: remove unnecessary try-except, don't hard code default for student username length [4.2 f249cf73] Bug #45577: advisory update ucs-school-import (15.0.3-15)
ucr set ucsschool/ldap/default/userprefix/exam="Arbeits-" config: u'username': {u'max_length': {u'default': 20}}, input: "Schulen","Benutzertyp","Vorname","Nachname","Klassen","Beschreibung","Telefon" "oldschool","student","Giannifoobar","Jungermannfoobarbaz","oldschool-1a","A student.","+12-574-296006" Created ImportStudent: 3 ['giannifoobar7', 'regnier.hopp7', 'yannes.betti7'] >>> len('giannifoobar7') + len('Arbeits-') 21
The test case ./225_import-users_username_length passes.
dn: cn=giannifoobar.jungerman,cn=unique-usernames,cn=ucsschool,cn=univention,d c=school,dc=local objectClass: ucsschoolUsername cn: giannifoobar.jungerman ucsschoolUsernameNextNumber: 3 dn: cn=giannifoobar,cn=unique-usernames,cn=ucsschool,cn=univention,dc=school,d c=local objectClass: ucsschoolUsername cn: giannifoobar ucsschoolUsernameNextNumber: 8 dn: cn=giannifoobar.jung,cn=unique-usernames,cn=ucsschool,cn=univention,dc=sch ool,dc=local objectClass: ucsschoolUsername ucsschoolUsernameNextNumber: 2 cn: giannifoobar.jung dn: cn=giannifoobar.jungermannfoob,cn=unique-usernames,cn=ucsschool,cn=univent ion,dc=school,dc=local objectClass: ucsschoolUsername ucsschoolUsernameNextNumber: 2 cn: giannifoobar.jungermannfoob "scheme": { "email": "<firstname>[0].<lastname>@<maildomain>", "recordUID": "<email>", "username": { "default": "<:umlauts><firstname>[0].<lastname><:lower>[COUNTER2]" } },
517d058d: Bug #45577: honor default username length ucs-school-import (15.0.3-17)
(In reply to Daniel Tröder from comment #24) > 517d058d: Bug #45577: honor default username length > > ucs-school-import (15.0.3-17) Hm, I am still getting incorrect results with the same config: CSV: "oldschool","student","Giannifoobar","Jungermannfoobarbaz","oldschool-1a","A student.","+12-574-296006" config: "username": { "max_length": { "default": 20 } } # ucr get ucsschool/ldap/default/userprefix/exam Arbeits2- result: >>> len('giannifoobar8') 13 >>> len('giannifoobar8') + len('Arbeits2-') 22 ii ucs-school-import 15.0.3-17A~4.2.0.201712111138
To cleanse the LDAP from old unique username objects run: from ucsschool.importer.utils.username_handler import UsernameHandler UsernameHandler(20, False).get_storage_backend().purge() Added unit test that modifies the UCRV ucsschool/ldap/default/userprefix/exam and iterates over different firstname, lastname input lengths and username:max_length:{default,student}. We should maybe deactivate it later, as it runs for a long time. 3e0e8a98: Bug #45577: add unit test for username length b4a609f2: Bug #45577: remove debug leftover ucs-test-ucsschool (4.0.4-47)
There is still the same bug, the test case does not cover it. I debugged it with an interactive PDB shell and found the reason. Here is a fix: diff --git a/ucs-school-import/modules/ucsschool/importer/models/import_user.py b/ucs-school-import/modules/ucsschool/importer/models/import_user.py index c2a357c..56644b0 100644 --- a/ucs-school-import/modules/ucsschool/importer/models/import_user.py +++ b/ucs-school-import/modules/ucsschool/importer/models/import_user.py @@ -916,7 +916,7 @@ class ImportUser(User): try: res = self.config['username']['max_length'][self.role_sting] except KeyError: - res = self.default_username_max_length + res = self._default_username_max_length return max(0, res) If the configuration key for "student" is removed from the config then the hardcoded max limit of 15 was used (self.default_username_max_length) instead of the reduced 20 - exam_prefix (self._default_username_max_length).
Good catch - thank for the debugging! [4.2 38a1a3e0] Bug #45577: fix typo [4.2 2602a5ce] Bug #45577: update advisory ucs-school-import (15.0.3-18)
Now it's fine!
[4.2 72f72ac0] Bug #45577: adapt tests to changes ucs-school-import (15.0.3-19) ucs-test-ucsschool (4.0.4-49)
UCS@school 4.2 v6 has been released. http://docs.software-univention.de/changelog-ucsschool-4.2v6-de.html If this error occurs again, please clone this bug.