enhance: serialize user update methods

[eternal-flame-AD]

Serializes user update actions to prevent race conditions leading to unexpected results.

I removed the 'Test_UpdateUserByID_EmptyPassword_Expect400' test as it seemed to be a mistake - it should return 200, it returned 400 in the test because there wasn't a second admin.

[codecov]

Codecov Report

❌ Patch coverage is 83.78378% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.79%. Comparing base (f4de619) to head (2bf405a).

Files with missing lines	Patch %	Lines
api/user.go	74.41%	6 Missing and 5 partials ⚠️
plugin/manager.go	89.61%	3 Missing and 5 partials ⚠️
database/database.go	75.00%	1 Missing and 1 partial ⚠️
database/user.go	60.00%	1 Missing and 1 partial ⚠️
api/oidc.go	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1042      +/-   ##
==========================================
+ Coverage   76.66%   76.79%   +0.13%     
==========================================
  Files          68       68              
  Lines        3724     3771      +47     
==========================================
+ Hits         2855     2896      +41     
- Misses        657      660       +3     
- Partials      212      215       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jmattheis]

(optional) Can we move this into the transaction too? Otherwise they can still be some race here. Similar for the UpdateUser api.

[eternal-flame-AD]

I think this is fine as is, it's just an early filter making sure the ID exists. The risk would be someone deleted the user right before the check and your transaction runs, in such case returning a success instead of "user does not exist" is probably acceptable.

[jmattheis]

For the update endpoint it caches oidc_id before the transaction and then set's it inside the transaction. There probably could be a race condition where a user is linked to an oidc identity & maybe updates the admin permissions, and at the same time the update method is called, this could unset the oidc_id and the change of permission.

For the delete endpoint it could be that at the same time another admin changes the admin flag of the user, so it's the last admin. The admin flag is false on the read outside of the transaction, but before the transaction is called, it was actually updated, and then the admin check isn't done correctly.

But yeah, it's optional, but it seems like a similar problem we are trying to fix here.