Univention Bugzilla – Bug 52659
[Kelvin API] allow changing user roles
Last modified: 2021-02-23 09:29:56 CET
The Kelvin should support changing the roles of users.
Allowed user role changes have been documented: [feature/kelvin bf1858574] Bug #52659: document user role change ============= ============= ========= Old New Allowed ============= ============= ========= staff student True staff teacher True staff staff+teacher True staff+teacher staff True staff+teacher student True staff+teacher teacher True student staff False student staff+teacher False student teacher False teacher staff True teacher staff+teacher True teacher student True ============= ============= ========= All transitions are allowed, except for students, they may never be changed. Is this really desired?
Code for this bug will now be commited to branch "dtroeder/52659_kelvin_change_user_roles". The documentation for role changes has been updated: All transitions are now allowed with one exception: * any→student: The transition is not allowed if the user is also a school administrator. A restriction for a change exist: * any→student: Students must be member of *exactly one* school class. When changing the ``roles`` attribute, the user must already have *one* ``school_class`` entry or a new value for ``school_class`` must be sent in the same request. A note was added: any→staff: Staff users have no school classes. The ``school_class`` attribute will be cleared automatically by the Kelvin API. ------ 324b8f213 Bug #52659: update role change documentation
The feature has been implemented in git branch dtroeder/52659_kelvin_change_user_roles: 929e0bc65 Bug #52659: document user role change d05b33250 Bug #52659: user role converter lib functions and tests 9e76e1a2f Bug #52659: import user role converter lib functions and tests 8799de8cc fixup! Bug #52659: user role converter lib functions and tests f2d1299e0 Bug #52659: add support for changing the user role in Kelvin and test 201456a23 Bug #52659: update documentation 4c0b95887 fixup! Bug #52659: add support for changing the user role in Kelvin and test The role change is done in the ucsschool lib. A small wrapper for ImportUser objects has been added, as that is what's used in Kelvin. Both PATCH und PUT methods support changing the users role. Tests have been added for the code in the ucsschool lib, the u@s-import and Kelvin.
The rule that a student must be in a school class is now enforced and tested. The rule that a school admin must not be converted to a student is now enforced and tested. 614baee33 fixup! Bug #52659: document user role change d1d825d49 fixup! Bug #52659: user role converter lib functions and tests b60fb8345 fixup! Bug #52659: import user role converter lib functions and tests a26f2b885 fixup! fixup! Bug #52659: add support for changing the user role in Kelvin and test
The rule that a student must be in a school class was not exact. The rule is actually that is must be one class per school, in which the user is. The check and the tests have been adapted: 3b5da8637 fixup! Bug #52659: user role converter lib functions and tests 42edafd13 fixup! Bug #52659: import user role converter lib functions and tests dbb05436c fixup! Bug #52659: add support for changing the user role in Kelvin and test
I squashed all those fixup commits to simplify the QA. Please reset to f87531f76 and then pull.
Commits from Bug 52150, Bug 52659 and Bug 52727 have been merged into git branch dtroeder/kelvin_release_1.3.0 and a Docker image was built from it. docker-upload.software-univention.de/ucsschool-kelvin-rest-api bed4fa4043ba 407MB A new Kelvin app version "1.3.0" is in the appcenter now.
Code has been merged into "feature/kelvin". A new Docker image was build and the Kelvin app version 1.3.0 can now be installed form the test appcenter.
QA → Reopen, minor mistakes merge → OK app version 1.3.0 → is installed from test appcenter changelog → OK Documentation kelvin/docs/resource-users.rst See section ``Changing roles`` below about changing a users → user's roles Home directories of UCS\@school users are located on school servers in a directory structure containing the users → user's .... Code In user.py groups_lower = {grp.lower() for grp in groups} groups.update(grp for grp in add_groups if grp.lower() not in groups_lower) → is this a performance thing? If not, this can be rewritten as groups.update(grp for grp in add_groups) other than that the code looks → OK Functionalities: with patch & put UCS: 4.4-7 errata893 Installed: cups=2.2.1 samba4=4.10 squid=3.5 ucsschool=4.4 v8 ucsschool-kelvin-rest-api=1.3.0 ============= ============= ========= Old New Allowed ============= ============= ========= staff student True staff teacher True staff staff+teacher True staff+teacher staff True staff+teacher student True staff+teacher teacher True student staff True (looses classes) student staff+teacher True student teacher True teacher staff True teacher staff+teacher True teacher student True exam_user any False (covered by test) * user any False (covered by test) without class staff student False staff+teacher student False teacher student False (patch is possible, if teacher still has a class. This is OK.) any → student transition is not allowed if the user is also a school administrator ⇒ Conversion to 'ImportStudent' is not allowed for school administrator. Patch student with same role but without classes → Returns user with class without any message. I think this is OK. * in the Swagger UI I get an error 500 and e.g. udm_rest_client.exceptions.ModifyError: Unprocessable Entity: {'options': 'Invalid combination of options. UCS@school Teacher cannot be activated together with UCS@school Examuser.' → this is expected behaviour since exam_user are not supported.
Thank you for the review. user»'«s have been fixed: [feature/kelvin 1587d0cc0] Bug #52659: fix orthography (In reply to Tobias Wenzel from comment #9) > groups_lower = {grp.lower() for grp in groups} > groups.update(grp for grp in add_groups if grp.lower() not in groups_lower) > > → is this a performance thing? If not, this can be rewritten as It is not performance related. The case of OUs in DNs is not consistent and DN-comparison must be implemented case insensitive. It is implemented in a way that the case of the existing group DNs is preserved.
(In reply to Daniel Tröder from comment #10) > (In reply to Tobias Wenzel from comment #9) > > groups_lower = {grp.lower() for grp in groups} > > groups.update(grp for grp in add_groups if grp.lower() not in groups_lower) > > > > → is this a performance thing? If not, this can be rewritten as > It is not performance related. The case of OUs in DNs is not consistent and > DN-comparison must be implemented case insensitive. It is implemented in a > way that the case of the existing group DNs is preserved. → Now I see it :) I forgot to mention: tests are OK and passing on VM and jenkins. All OK → Verify
A Kelvin API app with the changes in this bug has been published.