Bug 52895 - Kelvin API cannot handle invalid URLs for "school" attribute
Kelvin API cannot handle invalid URLs for "school" attribute
Status: CLOSED FIXED
Product: UCS@school
Classification: Unclassified
Component: HTTP-API (Kelvin)
UCS@school 4.4
Other Linux
: P5 normal (vote)
: ---
Assigned To: Alexander Steffen
Felix Botner
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2021-03-11 14:51 CET by Daniel Tröder
Modified: 2022-06-24 08:38 CEST (History)
4 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 3: Simply Wrong: The implementation doesn't match the docu
Who will be affected by this bug?: 2: Will only affect a few 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.069
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
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 Daniel Tröder univentionstaff 2021-03-11 14:51:04 CET
When sending POST {"name": "...", "description": "...", "school": "..."} to the Kelvin API, the value for "school" must be a URL.

If it is not (valid), then HTTP 500 will happen. Traceback:

--------------------------------------------------------------------------
2021-03-11 14:49:19 ERROR Exception in ASGI application
Traceback (most recent call last):
  File "/usr/lib/python3.8/site-packages/uvicorn/protocols/http/h11_impl.py", line 396, in run_asgi
    result = await app(self.scope, self.receive, self.send)
  File "/usr/lib/python3.8/site-packages/uvicorn/middleware/proxy_headers.py", line 45, in __call__
    return await self.app(scope, receive, send)
  File "/usr/lib/python3.8/site-packages/fastapi/applications.py", line 199, in __call__
    await super().__call__(scope, receive, send)
  File "/usr/lib/python3.8/site-packages/starlette/applications.py", line 111, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/usr/lib/python3.8/site-packages/starlette/middleware/errors.py", line 181, in __call__
    raise exc from None
  File "/usr/lib/python3.8/site-packages/starlette/middleware/errors.py", line 159, in __call__
    await self.app(scope, receive, _send)
  File "/usr/lib/python3.8/site-packages/starlette/exceptions.py", line 82, in __call__
    raise exc from None
  File "/usr/lib/python3.8/site-packages/starlette/exceptions.py", line 71, in __call__
    await self.app(scope, receive, sender)
  File "/usr/lib/python3.8/site-packages/starlette/routing.py", line 566, in __call__
    await route.handle(scope, receive, send)
  File "/usr/lib/python3.8/site-packages/starlette/routing.py", line 227, in handle
    await self.app(scope, receive, send)
  File "/usr/lib/python3.8/site-packages/starlette/routing.py", line 41, in app
    response = await func(request)
  File "/usr/lib/python3.8/site-packages/fastapi/routing.py", line 191, in app
    solved_result = await solve_dependencies(
  File "/usr/lib/python3.8/site-packages/fastapi/dependencies/utils.py", line 576, in solve_dependencies
    ) = await request_body_to_args(  # body_params checked above
  File "/usr/lib/python3.8/site-packages/fastapi/dependencies/utils.py", line 702, in request_body_to_args
    v_, errors_ = field.validate(value, values, loc=loc)
  File "/usr/lib/python3.8/site-packages/pydantic/fields.py", line 723, in validate
    v, errors = self._validate_singleton(v, values, loc, cls)
  File "/usr/lib/python3.8/site-packages/pydantic/fields.py", line 906, in _validate_singleton
    return self._apply_validators(v, values, loc, cls, self.validators)
  File "/usr/lib/python3.8/site-packages/pydantic/fields.py", line 913, in _apply_validators
    v = validator(cls, v, values, self, self.model_config)
  File "/usr/lib/python3.8/site-packages/pydantic/class_validators.py", line 310, in <lambda>
    return lambda cls, v, values, field, config: validator(v)
  File "/usr/lib/python3.8/site-packages/pydantic/main.py", line 729, in validate
    return cls(**value)
  File "/usr/lib/python3.8/site-packages/pydantic/main.py", line 398, in __init__
    values, fields_set, validation_error = validate_model(__pydantic_self__.__class__, data)
  File "/usr/lib/python3.8/site-packages/pydantic/main.py", line 1060, in validate_model
    values = validator(cls_, values)
  File "/kelvin/kelvin-api/ucsschool/kelvin/routers/school_class.py", line 76, in check_name2
    school = values["school"].split("/")[-1]
KeyError: 'school'
--------------------------------------------------------------------------

The input data handling should be robust.
The expected result should be HTTP_422 UNPROCESSABLE ENTITY for bad input data.
Comment 1 Alexander Steffen univentionstaff 2022-05-24 11:06:33 CEST
I've checked all routes provided by the Kelvin API (Version 1.5.4).
Whenever a request requires a school URL and a non valid URL was provided, Kelvin answered with 422 or ucsschool.kelvin.client.exceptions.NoObject which is already properly handled.
However, if a string was given that Kelvin was not able to identify as URL, the response was 500.
The changes in Bug 54739 clearly specify that URLs are needed. IMHO the handling of a literal string does not have to be fixed in order to
respond with 422 instead of 500.

POST TOKEN --> OK
GET CHANGELOG --> OK
GET README --> OK
SEARCH CLASSES --> HTTP 422 / 500
POST CLASSES --> HTTP 422 / 500
GET CLASSES --> NoObjectException
PUT CLASSES --> NoObjectException
PATCH CLASSES --> NoObjectException
DELETE CLASSES --> NoObjectException
SEARCH ROLES --> OK
GET ROLES --> OK
SEARCH SCHOOLS --> OK
POST SCHOOLS --> This did not run for obvious reasons. (Just here for completeness)

GET SCHOOLS --> NoObjectException
SEARCH USERS --> OK
POST USERS --> HTTP 422 / 500
GET USERS --> OK
PUT USERS --> NoObjectException
DELETE USERS --> NoObjectException
PATCH USERS -->  HTTP 422 / 500

Conclusion: works for me, no fix needed
Comment 4 Tobias Wenzel univentionstaff 2022-06-23 13:13:32 CEST
Setting this to verified: issue is waiting for release.
Comment 5 Tobias Wenzel univentionstaff 2022-06-24 08:38:03 CEST
Released with 1.5.5 -> closing this issue.