Patch #31399
closed
make /my/account endpoint accessible through API
Added by Jens Krämer over 5 years ago. Updated over 5 years ago.
Description
This would allow a user to update their account info through an external app. Currently admin privileges are required to change i.e. a user's name through the /users API.
Files
| 0001-enables-API-access-to-my-account-for-updating-user-a.patch (7.1 KB) 0001-enables-API-access-to-my-account-for-updating-user-a.patch | Jens Krämer, 2019-05-20 05:34 | ||
| 0002-changes-my-account-html-form-to-put.patch (3.02 KB) 0002-changes-my-account-html-form-to-put.patch | Jens Krämer, 2019-06-11 08:55 | ||
| 0003-lets-sudo-mode-handle-PUT-on-my-account-makes-tests-.patch (3.34 KB) 0003-lets-sudo-mode-handle-PUT-on-my-account-makes-tests-.patch | Jens Krämer, 2019-06-14 09:44 |
Related issues
Updated by Go MAEDA over 5 years ago
- Target version set to Candidate for next major release
Updated by Go MAEDA over 5 years ago
- Target version changed from Candidate for next major release to 4.1.0
Setting the target version to 4.1.0.
Updated by Go MAEDA over 5 years ago
- Status changed from New to Needs feedback
- Assignee set to Jens Krämer
I have tested the patch and found that the endpoint behaves the same for both POST and PUT requests. In other words, POST updates the account instead of creating an account.
IMHO, Redmine should not respond to POST API requests. Since users think that POST requests are used to create an object, admin may accidentally update their own account when trying to create a new account (of course, they should be more carefully).
What are your thoughts on that?
Updated by Jens Krämer over 5 years ago
Yes, the thought that POST is not really nice there crossed my mind, but in order to keep the patch as small as possible I sticked to it since that is what the web form uses as well. If we change the API method to PUT, I would vote for changing the method used by the /my/account form to PUT, as well. What do you think?
Updated by Go MAEDA over 5 years ago
Jens Krämer wrote:
If we change the API method to PUT, I would vote for changing the method used by the /my/account form to PUT, as well. What do you think?
Sounds nice, it makes things consistent. I am in favor of it.
Updated by Jens Krämer over 5 years ago
OK, then I will come up with a patch for that :)
Updated by Jens Krämer over 5 years ago
- File 0002-changes-my-account-html-form-to-put.patch 0002-changes-my-account-html-form-to-put.patch added
here's a second patch which changes the HTML form method to PUT and removes support for POST on that endpoint.
Updated by Go MAEDA over 5 years ago
Thank you for updating the patch but some tests fail after applying the second patch. Could you look into these errors?
Failure: SudoModeTest#test_update_email_address [/Users/maeda/redmines/trunk/test/integration/sudo_mode_test.rb:153]: Expected response to be a <2XX: success>, but was a <404: Not Found> bin/rails test test/integration/sudo_mode_test.rb:147
Failure: RoutingMyTest#test_my [/Users/maeda/redmines/trunk/test/test_helper.rb:296]: No route matches "/my/account" bin/rails test test/integration/routing/my_test.rb:23
Updated by Jens Krämer over 5 years ago
- File 0003-lets-sudo-mode-handle-PUT-on-my-account-makes-tests-.patch 0003-lets-sudo-mode-handle-PUT-on-my-account-makes-tests-.patch added
Indeed there was a bug - I forgot to change the sudo mode requirement in the controller to PUT. I also changed the tests to do PUT requests now / expect PUT to be routed instead of POST.
Updated by Go MAEDA over 5 years ago
- Status changed from Needs feedback to Resolved
- Assignee changed from Jens Krämer to Go MAEDA
Committed the patch. Thank you for your contribution.
The API document should be updated later.
Updated by Jean-Philippe Lang over 5 years ago
- Status changed from Resolved to Closed
Documentation to be added here Rest_MyAccount.
Updated by Go MAEDA over 4 years ago
- Has duplicate Feature #19301: Let non admin users update their account via the REST API added