Bug 52725 - Wrong status code sent for "move object" by UDM REST API
Wrong status code sent for "move object" by UDM REST API
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: UDM - REST API
UCS 4.4
Other Linux
: P5 normal (vote)
: UCS 4.4-7-errata
Assigned To: Daniel Tröder
Florian Best
:
Depends on:
Blocks: 52769
  Show dependency treegraph
 
Reported: 2021-01-29 18:32 CET by Daniel Tröder
Modified: 2021-02-11 12:08 CET (History)
1 user (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?: 1: Will affect a very few installed domains
How will those affected feel about the bug?: 3: A User would likely not purchase the product
User Pain: 0.051
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:
best: Patch_Available+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Tröder univentionstaff 2021-01-29 18:32:10 CET
The UDM REST API replies with HTTP 201 to a PUT request with a previously retrieved object where only the position was changed: a move operation.

200 GET /udm/users/user/uid=paul.pineda,cn=users,dc=uni,dc=dtr (0.0.0.0) 14.87ms
201 PUT /udm/users/user/uid=paul.pineda,cn=users,dc=uni,dc=dtr (0.0.0.0) 7.12ms

This breaks the autogenerated REST client for the UDM REST API client, as the OpenAPI schema says it would reply with HTTP 200.
Comment 1 Daniel Tröder univentionstaff 2021-01-29 18:34:11 CET
My guess is line 2567:

ObjectsMove.post():
    ..
    self.set_status(201)
Comment 2 Daniel Tröder univentionstaff 2021-01-29 18:40:03 CET
Ah no. It is line 2945 in Object.move().

3700 lines of undocumented code is really hard to maintain.
Comment 3 Florian Best univentionstaff 2021-01-29 18:52:48 CET
201 is correct here.
We need to make the schema conditionally then.
Comment 4 Florian Best univentionstaff 2021-01-29 19:02:51 CET
Patch:
diff --git management/univention-directory-manager-rest/src/univention/admin/rest/module.py management/univention-directory-manager-rest/src/univention/admin/rest/module.py
index 1820787793..0a04d1fefa 100755
--- management/univention-directory-manager-rest/src/univention/admin/rest/module.py
+++ management/univention-directory-manager-rest/src/univention/admin/rest/module.py
@@ -1389,6 +1389,11 @@ class OpenAPI(Resource):
                                                        "content": content_schema(),
                                                        "headers": global_response_headers(),
                                                },
+                                               "201": {
+                                                       "description": "Success (on move and rename)",
+                                                       "content": content_schema(),
+                                                       "headers": global_response_headers(),
+                                               },
                                                "404": {
                                                        "description": "Object not found",
                                                        "headers": global_response_headers(),
Comment 5 Daniel Tröder univentionstaff 2021-01-29 21:33:27 CET
(In reply to Florian Best from comment #4)
> Patch:
Works for me.
The OpenAPI client is now generated expecting for PUT for example:

response_types_map = {
    201: "UsersUser",  # <-- was missing before
    200: "UsersUser",
    404: None,
}
Comment 6 Daniel Tröder univentionstaff 2021-02-03 15:33:37 CET
the patch was applied and the package build:

[4.4-7 1153b61d9f] Bug #52725: add HTTP 201 as possible response code for modify operation to OpenAPI schema (when moving)
[4.4-7 004abb2adf] Bug #52725: advisory

univention-directory-manager-rest (9.0.16-11)
Comment 7 Daniel Tröder univentionstaff 2021-02-03 15:56:59 CET
Merge request to 5.0-0: https://git.knut.univention.de/univention/ucs/-/merge_requests/65
Comment 8 Florian Best univentionstaff 2021-02-05 14:18:54 CET
OK: merge to UCS 5.0
OK: status code is correctly set in schema