- 
  Notifications
 You must be signed in to change notification settings 
- Fork 279
Description
Description of the problem
I am experiencing some weird and unexpected behavior for the /api/v4/users/{id} endpoint. If I am not mistaken similarly to #2381, POST to the normal /api/v4/users endpoint is not allowed to set the external id for entity due to "reasons" and it is instead possible to use PUT for creating a specific entity (and this is working well, although the id from the path is completely ignored).
According to the docs, the PUT method for endpoint /api/v4/users/{id} should be able to also update the entity, but that is not the case; instead an error is shown when the entity is already present. This prevents updating the team_id on the User entity over the API.
Your environment
DOMjudge 9.0.0/d8c018e4c
Steps to reproduce
- curl -X PUT /api/v4/users/whatever --json '{"id": "externalId", "name": "name", "username": "username", "roles": []}'
{"last_login_time":null,"last_api_login_time":null,"first_login_time":null,"team":null,"team_id":null,"roles":[],"type":null,"userid":84,"id":"externalId","username":"username","name":"name","email":null,"last_ip":null,"ip":null,"enabled":true}
- curl -X PUT /api/v4/users/whatever --json '{"id": "externalId", "name": "new name", "username": "username", "roles": []}'
{"code":400,"message":"User username already exists"}
Expected behaviour
I like how the groups endpoint for creating/updating Team categories from #2383 works. It allows to create a category, update the category and when called repeatedly with the same data no error is reported, which allows to replay import scripts easily without the need to delete the data first.
curl -X PUT /api/v4/contests/demo/groups/testgroup --json '{"id": "testgroup", "name": "Test group"}'
{"hidden":false,"categoryid":17,"id":"testgroup","icpc_id":null,"name":"Test group","sortorder":0,"color":null,"allow_self_registration":false}
curl -X PUT /api/v4/contests/demo/groups/testgroup --json '{"id": "testgroup", "name": "Test group"}'
{"hidden":false,"categoryid":17,"id":"testgroup","icpc_id":null,"name":"Test group","sortorder":0,"color":null,"allow_self_registration":false}
curl -X PUT /api/v4/contests/demo/groups/testgroup --json '{"id": "testgroup", "name": "New Test group"}'
{"hidden":false,"categoryid":17,"id":"testgroup","icpc_id":null,"name":"New Test group","sortorder":0,"color":null,"allow_self_registration":false}
Any other information that you want to share?
I briefly looked into the code and was able to find the problematic code. It probably should not ignore the id from path and check it against the id provided in the object like it is done in other update endpoints. Basically it is missing a check similar to 
domjudge/webapp/src/Controller/API/GroupController.php
Lines 157 to 159 in 795cc23
For the main part of the problem, it might be also a little bit problematic, that the username also needs to be unique so currently the entity basically has 3 different unique IDs (userid, id and username).
The error message itself is due to
domjudge/webapp/src/Controller/API/UserController.php
Lines 350 to 352 in 795cc23
I guess the condition there is missing a
!($addUser instanceof UpdateUser) check, but even then I was not able to make it work due to some Integrity constraint violation / Duplicate entries in the database. My knowledge of the code base is currently not enough to find out where a new entity is created instead of updated.