Bug 42513 - import scripts change obj.info directly
import scripts change obj.info directly
Product: UCS@school
Classification: Unclassified
Component: Import scripts
UCS@school 4.1 R2
Other Linux
: P5 major (vote)
: UCS@school 4.1 R2 vXXX
Assigned To: Daniel Tröder
Florian Best
: interim-2
Depends on:
  Show dependency treegraph
Reported: 2016-09-27 14:57 CEST by Florian Best
Modified: 2016-11-10 16:00 CET (History)
1 user (show)

See Also:
What kind of report is it?: Development Internal
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Ticket number:
Bug group (optional):
Max CVSS v3 score:

patch (3.95 KB, patch)
2016-09-27 15:17 CEST, Florian Best
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Best univentionstaff 2016-09-27 14:57:45 CEST
The import scripts modifies obj.info directly:

751 »   »   udm_obj.info.update(self.udm_properties)

This is not allowed as this bypasses many consistency checks and syntax class validation. This might lead to broken objects (and maybe security issues).
Also errors for not existing properties aren't recognized.

Inconsistencies might be:
* property may not change
* property is required but not set
* property is not allowed to change on AD-objects
* it's not detected that a default value is overwritten, leading to followup-errors if the object is further used
* singlevalue properties might contain multiple entries
* property value has an invalid syntax
* property value with wrong encoding might be written into LDAP
Comment 1 Florian Best univentionstaff 2016-09-27 15:17:15 CEST
Created attachment 8038 [details]

Also multiple obj.modify() calls might cause side effects because the internal state is not updated after modifying.

Attached patch addresses these (untested).
Comment 2 Daniel Tröder univentionstaff 2016-09-28 13:32:40 CEST
This should be tested with
* 205_import-users_attribute_schemes
* 206_import-users_multivalue_attributes
* 210_import-users_extended_attribute
Comment 3 Daniel Tröder univentionstaff 2016-10-11 09:49:41 CEST
73042: the ImportUser code now uses the correct ucsschool.lib interface for setting UDM properties.
73043: check consistency also when creating users from within hooks
73064: use 'street' instead of 'organisation' ('o') for pyhook tests
73066: make exception message more informative
73070: add test to check if UDM syntax checks are applied to ImportUser.udm_properties

All import-related tests ran successfully in my test env:
- 90_ucsschool/34_import-users-legacy
- 90_ucsschool/34_import-users_via_cli
- 90_ucsschool/2??_import-users_*

Now 90_ucsschool/202_import-users_username_recordUID_UDM_property revealed an error that had before been masked:

UDMValueError: UDM properties could not be set. Bad value: 'Organisation: The value must not be longer than 64 characters.'

PyHook-tests were being done with 'organisation', which is string64. r73064 changes it to use 'street' (string).

In r73070 a test was added, that fails with the code before r73042 to check if UDM syntax checks are applied to ImportUser.udm_properties in create() and modify().
Comment 4 Daniel Tröder univentionstaff 2016-10-11 11:12:20 CEST
- rename: _check_consistency() → _prevent_mapped_attributes_in_udm_properties()
- move to common code path: _alter_udm_obj()
Comment 5 Florian Best univentionstaff 2016-10-11 11:14:42 CEST
OK: test with a hook script
OK: syntax checks on create()
OK: syntax checks on modify()
Comment 6 Daniel Tröder univentionstaff 2016-10-11 12:56:32 CEST
r73084: improved test to check that attributes mapped by the ucsschool.lib are prevented from being read from udm_properties
Comment 7 Sönke Schwardt-Krummrich univentionstaff 2016-11-10 16:00:46 CET
UCS@school 4.1 R2 v7 has been released.


If this error occurs again, please clone this bug.