Bug 52926 - unixHome is not set correctly by Kelvin
unixHome is not set correctly by Kelvin
Status: CLOSED FIXED
Product: UCS@school
Classification: Unclassified
Component: HTTP-API (Kelvin)
UCS@school 4.4
Other Linux
: P5 normal (vote)
: UCS@school 4.4 v9-errata
Assigned To: Joerg Baach
Ole Schwiegert
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2021-03-16 17:02 CET by Jan-Luca Kiok
Modified: 2021-09-14 08:44 CEST (History)
8 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 5: Major Usability: Impairs usability in key scenarios
Who will be affected by this bug?: 2: Will only affect a few installed domains
How will those affected feel about the bug?: 4: A User would return the product
User Pain: 0.229
Enterprise Customer affected?: Yes
School Customer affected?: Yes
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number: 2021080221000199
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 Jan-Luca Kiok univentionstaff 2021-03-16 17:02:24 CET
Similar to Bug 52668 (sambahome not set after creation), the unixHome is not set correctly for created users. Currently this affects users synced with the ID-Connector.
For example:

root@idc:~# udm users/user  list --filter uid=tobiasb2 | grep unixhome
  unixhome: /home/TSG/schueler/tobiasb2

root@traeger2:~# udm users/user  list --filter uid=tobiasb2 | grep unixhome
  unixhome: /home/tobiasb2
Comment 1 Marc Schwarz univentionstaff 2021-03-17 09:26:55 CET
just for completeness: 

the behaviour should not be: set the unixHome shall not be set in the leading system and synced through to the school authority systems.

the users unixHome shall be evaluated on school authority level according to their settings. this follows the exact same behaviour from Bug 52668 with the sambaHomeDrive.
Comment 2 Christina Scheinig univentionstaff 2021-08-19 14:24:54 CEST
A customer requested that, and it is blocking further test, with now imported users for the school year. 
This behaviour took them by surprise, that teachers and students get the wrong home. School start coming soon and so the homes have to be migrated later.
Comment 3 Jan-Luca Kiok univentionstaff 2021-08-23 09:14:52 CEST
Kind of a workaround: The unixhome can be made available as a mapped property and is set and evaluated correctly when supplied via CREATE, so if the configured unixhome is available on site of the ID-Connector it can be given this way.
Comment 5 Joerg Baach univentionstaff 2021-09-09 12:30:48 CEST
Reproduced bug
==============
create user on `https://<FQDN>/ucsschool/kelvin/v1/docs#/users/create_ucsschool_kelvin_v1_users__post`

with:

```json
{
  "name": "brokenuser",
  "school": "https://<FQDN>/ucsschool/kelvin/v1/schools/DEMOSCHOOL",
  "firstname": "broken",
  "lastname": "user",
  "birthday": "2021-09-06",
  "disabled": false,
  "email": "brokenuser@demoschool.de",
  "record_uid": "brokenuser",
  "roles": ["https://<FQDN>/ucsschool/kelvin/v1/roles/student"],
  "schools": [],
  "school_classes": {},
  "source_uid": "",
  "ucsschool_roles": [],
  "udm_properties": {},
  "password": "brokenuserbroken",
  "kelvin_password_hashes": null
}
```
Check at prompt

```bash
~# udm users/user  list --filter uid=brokenuser | grep unixhome
  unixhome: /home/brokenuser
```

Expected would be : /home/DEMOSCHOOL/schueler/brokenuser

Fix
===

Fix is in e6677f3d, in branch jbaach/52926_unixHome_is_not_set_correctly_by_kelvin

QA Tasks
========

- [ ] reproduce error
- [ ] delete user
- [ ] upgrade to new kelvin code
- [ ] recreate user 
- [ ] check that new user has correct unixhome
- [ ] check tests
Comment 6 Ole Schwiegert univentionstaff 2021-09-09 13:43:35 CEST
- [o] reproduce error
- [o] delete user
- [o] upgrade to new kelvin code
- [o] recreate user 
- [o] check that new user has correct unixhome
- [o] check tests

Changes work, fix the problem, tests+ new tests are still green

Please merge into the feature/kelvin branch and make sure to extend the changelog and appcenter update texts for the upcoming 1.5.0 (due to Bug #53744):

appcenter/README_UPDATE_DE
appcenter/README_UPDATE_EN

Something like

<h2>v1.5.0 (2021-09-10)</h2>
<ul>
    <li>
         Unixhomes are now set correctly for users. (Bug #52926)
    </li>
</ul>

kelvin-api/changelog.rst

Something like

v1.5.0 (2021-09-10)
...................
* Unixhomes are now set correctly for users. (Bug #52926)


Update versions (to 1.5.0) in:
kelvin-api/VERSION.txt
doc/kelvin/docs/conf.py


No need to prepare any docker images or apps in the test app center which was done for Bug #53744 already.
Comment 7 Joerg Baach univentionstaff 2021-09-09 15:09:30 CEST
merged in 63a7601c
updated kevlin version to 1.5.0, updated READMEs and changelog in 873b7d58
Comment 8 Ole Schwiegert univentionstaff 2021-09-10 06:54:30 CEST
Very good. Bug VERIFIED. Kelvin Release will happen with Bug #53744
Comment 9 Florian Best univentionstaff 2021-09-10 11:17:28 CEST
It is often helpful to describe in bugzilla what the problem was.
So far I understand that students weren't evaluated in the logic to fill the homedir. Now they are treated like pupils.
Comment 10 Ole Schwiegert univentionstaff 2021-09-10 14:23:53 CEST
Kelvin REST API 1.5.0 has been released.

https://appcenter.software-univention.de/univention-repository/4.4/maintained/component/ucsschool-kelvin-rest-api_20210902124852/README_UPDATE_DE

If this error occurs again, please clone this bug.
Comment 11 Florian Best univentionstaff 2021-09-12 00:10:22 CEST
Shouldn't the patch also be included into the regular UCS@school 4.4 code?
Also: shouldn't the code be formatted with isort and black?

I am experiencing a similar error when moving a user from one to another school: the home directory did not change.
Comment 12 Daniel Tröder univentionstaff 2021-09-13 08:11:23 CEST
(In reply to Florian Best from comment #11)
> Shouldn't the patch also be included into the regular UCS@school 4.4 code?
Roles are detected differently on the host and in Kelvin. So the problem does not occur there. My first instinct was to also put the patch there, but then I thought it'd be better to catch a programming error, than to handle it.

> Also: shouldn't the code be formatted with isort and black?
It was. You can run "pre-commit run -a" to verify it.

> I am experiencing a similar error when moving a user from one to another
> school: the home directory did not change.
That is on purpose. Changing the "unixhome" property means moving the actual files and folders on various school servers in the domain. A lot of problems can happen there. So this process is left to the user to do manually.
Comment 13 Florian Best univentionstaff 2021-09-13 09:35:17 CEST
(In reply to Daniel Tröder from comment #12)
> > Also: shouldn't the code be formatted with isort and black?
> It was. You can run "pre-commit run -a" to verify it.

git:63a7601c1dffa0b9b245bd75890c648940b05663 is obviously not PEP8 compatible, but was fixed by Ole in 77402ec0b7eb3ca7b1a3023010f7ea6cba44955d.
Comment 14 Ole Schwiegert univentionstaff 2021-09-13 10:10:54 CEST
Yeah, Joerg did not have the pre-commit hooks installed, and I did not see it in the QA

I fixed that in a spearate commit during my work on the same code at the same time, which sadly got squashed into another Bug during the merge.

Did not go optimal, its in my list of QA steps now to always verify Code formatting.
Comment 15 Florian Best univentionstaff 2021-09-13 17:20:32 CEST
(In reply to Daniel Tröder from comment #12)
> (In reply to Florian Best from comment #11)
> > Shouldn't the patch also be included into the regular UCS@school 4.4 code?
> Roles are detected differently on the host and in Kelvin. So the problem
> does not occur there. My first instinct was to also put the patch there, but
> then I thought it'd be better to catch a programming error, than to handle
> it.
OK, thanks.

> > I am experiencing a similar error when moving a user from one to another
> > school: the home directory did not change.
> That is on purpose. Changing the "unixhome" property means moving the actual
> files and folders on various school servers in the domain. A lot of problems
> can happen there. So this process is left to the user to do manually.
This does not seem to be on purpose. At least not in the SiSiPi import. A test case in UCS@school 4.4 proves that the home directory attribute of an existing use is changed after a school change. 
Please have a look at Bug #53783 for more details.
Comment 16 Daniel Tröder univentionstaff 2021-09-14 08:38:39 CEST
(In reply to Florian Best from comment #15)
> (In reply to Daniel Tröder from comment #12)
> > (In reply to Florian Best from comment #11)
> > > I am experiencing a similar error when moving a user from one to another
> > > school: the home directory did not change.
> > That is on purpose. Changing the "unixhome" property means moving the actual
> > files and folders on various school servers in the domain. A lot of problems
> > can happen there. So this process is left to the user to do manually.
> This does not seem to be on purpose. At least not in the SiSiPi import. A
> test case in UCS@school 4.4 proves that the home directory attribute of an
> existing use is changed after a school change. 
> Please have a look at Bug #53783 for more details.
I'll have to investigate... The SiSoPi import is a special case, because a user is always in exactly one school.
I will the PM about the desired behavior for unixhome.
Comment 17 Daniel Tröder univentionstaff 2021-09-14 08:44:32 CEST
I just looked at the ucsschool.lib.models.user code, and it seems I was wrong. In do_school_change() clearly the unixhome is modified. So I guess that is the standard. That means that not changing it, when a user changes its primary school, is a bug.