Bug 56699 - Could not find object of type 'role' with URL ...
Summary: Could not find object of type 'role' with URL ...
Status: CLOSED FIXED
Alias: None
Product: UCS@school
Classification: Unclassified
Component: HTTP-API (Kelvin)
Version: UCS@school 5.0
Hardware: Other Linux
: P5 normal
Target Milestone: ---
Assignee: Johannes Königer
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-10-04 20:53 CEST by Jürn Brodersen
Modified: 2023-10-23 18:16 CEST (History)
3 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 4: Minor Usability: Impairs usability in secondary scenarios
Who will be affected by this bug?: 3: Will affect average number of installed domains
How will those affected feel about the bug?: 2: A Pain – users won’t like this once they notice it
User Pain: 0.137
Enterprise Customer affected?:
School Customer affected?: Yes
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number: 20230901510002, 2023090121000168
Bug group (optional):
Customer ID: 01427
Max CVSS v3 score:


Attachments
Script to reproduce (2.79 KB, application/x-shellscript)
2023-10-04 20:53 CEST, Jürn Brodersen
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jürn Brodersen univentionstaff 2023-10-04 20:53:27 CEST
Created attachment 11133 [details]
Script to reproduce

Due to a cache issue you can get a 404 for roles even if the role is correct:

```
Could not find object of type 'role' with URL 'http://primary.school.test/ucsschool/kelvin/v1/roles/teacher'.
```

`request.url_for` uses the `HOST` header from the request to calculate an absolute url which we cache.
https://git.knut.univention.de/univention/components/ucsschool-kelvin-rest-api/-/blob/main/kelvin-api/ucsschool/kelvin/urls.py?ref_type=heads#L50

Our cache key is missing the `HOST` header. So if you use the IP of the system first it gets cached. If you later use the FQDN, your request fails because the cached url is using the IP.

https://git.knut.univention.de/univention/components/ucsschool-kelvin-rest-api/-/blob/main/kelvin-api/ucsschool/kelvin/urls.py?ref_type=heads#L85

We should add the `HOST` header to the cache key.


Notes:
Do we we even care if the absolute url is correct? Would anything break if we only check the path?
Why is this cached at all? Is `request.url_for` that slow?
Comment 2 Johannes Königer univentionstaff 2023-10-05 15:56:25 CEST
(In reply to Jürn Brodersen from comment #0)

> Why is this cached at all? Is `request.url_for` that slow?

I cannot remember exact numbers, but yes, that function is pretty slow. It iterates over all available routes further down the call chain to find a match: https://github.com/encode/starlette/blob/e11a7d8471e7b4d859ac4cc33db2ae5263463a35/starlette/routing.py#L668

And we use it or now `cached_url_for` pretty excessively throughout the routes, I think it makes sense to cache that.
Comment 4 Johannes Königer univentionstaff 2023-10-18 14:16:11 CEST
The fix has been published with Kelvin version 1.9.0.