Repository: btcpayserver/btcpayserver (csharp)

Issue:
API error when adding a non existing user to a store not clear
I was trying to add a user to a store via the API (/api/v1/stores/{storeId}/users). The following error returned:
```
{"code":"duplicate-store-user-role","message":"The user is already added to the store"}
```

But the user wasn't added to the store yet. After some investigation it appeared the user wasn't existing anymore in my test environment. So the error message should be different I guess.

Pull request:
- Adds an endpoint to update store users (before they had to be removed ad re-added)
- Checks for the existance of a user and responds with 404 in that case (fixes #6423)
- Allows retrieval of user by user id or email for add and update (consistent with the other endpoints)
- Improves the API docs for the store users endpoints

Patch:
diff --git a/BTCPayServer.Client/BTCPayServerClient.StoreUsers.cs b/BTCPayServer.Client/BTCPayServerClient.StoreUsers.cs
index 116cd6be66..2311418c73 100644
--- a/BTCPayServer.Client/BTCPayServerClient.StoreUsers.cs
+++ b/BTCPayServer.Client/BTCPayServerClient.StoreUsers.cs
@@ -29,4 +29,10 @@ public virtual async Task AddStoreUser(string storeId, StoreUserData request, Ca
         if (request == null) throw new ArgumentNullException(nameof(request));
         await SendHttpRequest<StoreUserData>($"api/v1/stores/{storeId}/users", request, HttpMethod.Post, token);
     }
+
+    public virtual async Task UpdateStoreUser(string storeId, string userId, StoreUserData request, CancellationToken token = default)
+    {
+        if (request == null) throw new ArgumentNullException(nameof(request));
+        await SendHttpRequest<StoreUserData>($"api/v1/stores/{storeId}/users/{userId}", request, HttpMethod.Put, token);
+    }
 }
diff --git a/BTCPayServer.Client/Models/StoreData.cs b/BTCPayServer.Client/Models/StoreData.cs
index ca6459e693..9f2e2b7286 100644
--- a/BTCPayServer.Client/Models/StoreData.cs
+++ b/BTCPayServer.Client/Models/StoreData.cs
@@ -17,7 +17,25 @@ public class StoreUserData
         /// </summary>
         public string UserId { get; set; }
 
+        /// <summary>
+        /// the store role of the user
+        /// </summary>
         public string Role { get; set; }
+
+        /// <summary>
+        /// the email AND username of the user
+        /// </summary>
+        public string Email { get; set; }
+        
+        /// <summary>
+        /// the name of the user
+        /// </summary>
+        public string Name { get; set; }
+
+        /// <summary>
+        /// the image url of the user
+        /// </summary>
+        public string ImageUrl { get; set; }
     }
 
     public class RoleData
diff --git a/BTCPayServer/Controllers/GreenField/GreenfieldStoreUsersController.cs b/BTCPayServer/Controllers/GreenField/GreenfieldStoreUsersController.cs
index 1839d7071d..f54da7301d 100644
--- a/BTCPayServer/Controllers/GreenField/GreenfieldStoreUsersController.cs
+++ b/BTCPayServer/Controllers/GreenField/GreenfieldStoreUsersController.cs
@@ -11,6 +11,7 @@
 using Microsoft.AspNetCore.Cors;
 using Microsoft.AspNetCore.Identity;
 using Microsoft.AspNetCore.Mvc;
+using StoreData = BTCPayServer.Data.StoreData;
 
 namespace BTCPayServer.Controllers.Greenfield
 {
@@ -30,10 +31,10 @@ public GreenfieldStoreUsersController(StoreRepository storeRepository, UserManag
 
         [Authorize(Policy = Policies.CanViewStoreSettings, AuthenticationSchemes = AuthenticationSchemes.Greenfield)]
         [HttpGet("~/api/v1/stores/{storeId}/users")]
-        public IActionResult GetStoreUsers()
+        public async Task<IActionResult> GetStoreUsers()
         {
             var store = HttpContext.GetStoreData();
-            return store == null ? StoreNotFound() : Ok(FromModel(store));
+            return store == null ? StoreNotFound() : Ok(await FromModel(store));
         }
 
         [Authorize(Policy = Policies.CanModifyStoreSettings, AuthenticationSchemes = AuthenticationSchemes.Greenfield)]
@@ -41,31 +42,28 @@ public IActionResult GetStoreUsers()
         public async Task<IActionResult> RemoveStoreUser(string storeId, string idOrEmail)
         {
             var store = HttpContext.GetStoreData();
-            if (store == null)
-            {
-                return StoreNotFound();
-            }
-
-            var userId = await _userManager.FindByIdOrEmail(idOrEmail);
-            if (userId != null && await _storeRepository.RemoveStoreUser(storeId, idOrEmail))
-            {
-                return Ok();
-            }
+            if (store == null) return StoreNotFound();
 
-            return this.CreateAPIError(409, "store-user-role-orphaned", "Removing this user would result in the store having no owner.");
+            var user = await _userManager.FindByIdOrEmail(idOrEmail);
+            if (user == null) return UserNotFound();
+            
+            return await _storeRepository.RemoveStoreUser(storeId, user.Id)
+                ? Ok()
+                : this.CreateAPIError(409, "store-user-role-orphaned", "Removing this user would result in the store having no owner.");
         }
 
         [Authorize(Policy = Policies.CanModifyStoreSettings, AuthenticationSchemes = AuthenticationSchemes.Greenfield)]
         [HttpPost("~/api/v1/stores/{storeId}/users")]
-        public async Task<IActionResult> AddStoreUser(string storeId, StoreUserData request)
+        [HttpPut("~/api/v1/stores/{storeId}/users/{idOrEmail?}")]
+        public async Task<IActionResult> AddOrUpdateStoreUser(string storeId, StoreUserData request, string idOrEmail = null)
         {
             var store = HttpContext.GetStoreData();
-            if (store == null)
-            {
-                return StoreNotFound();
-            }
-            StoreRoleId roleId = null;
+            if (store == null) return StoreNotFound();
 
+            var user = await _userManager.FindByIdOrEmail(idOrEmail ?? request.UserId);
+            if (user == null) return UserNotFound();
+            
+            StoreRoleId roleId = null;
             if (request.Role is not null)
             {
                 roleId = await _storeRepository.ResolveStoreRoleId(storeId, request.Role);
@@ -76,21 +74,42 @@ public async Task<IActionResult> AddStoreUser(string storeId, StoreUserData requ
             if (!ModelState.IsValid)
                 return this.CreateValidationError(ModelState);
 
-            if (await _storeRepository.AddStoreUser(storeId, request.UserId, roleId))
-            {
-                return Ok();
-            }
-
-            return this.CreateAPIError(409, "duplicate-store-user-role", "The user is already added to the store");
+            var result = string.IsNullOrEmpty(idOrEmail)
+                ? await _storeRepository.AddStoreUser(storeId, user.Id, roleId)
+                : await _storeRepository.AddOrUpdateStoreUser(storeId, user.Id, roleId);
+            return result
+                ? Ok()
+                : this.CreateAPIError(409, "duplicate-store-user-role", "The user is already added to the store");
         }
 
-        private IEnumerable<StoreUserData> FromModel(Data.StoreData data)
+        private async Task<IEnumerable<StoreUserData>> FromModel(StoreData data)
         {
-            return data.UserStores.Select(store => new StoreUserData() { UserId = store.ApplicationUserId, Role = store.StoreRoleId });
+            var storeUsers = new List<StoreUserData>();
+            foreach (var storeUser in data.UserStores)
+            {
+                var user = await _userManager.FindByIdOrEmail(storeUser.ApplicationUserId);
+                var blob = user?.GetBlob();
+                storeUsers.Add(new StoreUserData
+                {
+                    UserId = storeUser.ApplicationUserId,
+                    Role = storeUser.StoreRoleId,
+                    Email = user?.Email,
+                    Name = blob?.Name,
+                    ImageUrl = blob?.ImageUrl,
+
+                });
+            }
+            return storeUsers;
         }
+
         private IActionResult StoreNotFound()
         {
             return this.CreateAPIError(404, "store-not-found", "The store was not found");
         }
+
+        private IActionResult UserNotFound()
+        {
+            return this.CreateAPIError(404, "user-not-found", "The user was not found");
+        }
     }
 }
diff --git a/BTCPayServer/Controllers/GreenField/LocalBTCPayServerClient.cs b/BTCPayServer/Controllers/GreenField/LocalBTCPayServerClient.cs
index 2a4b898f3c..95e2c30b2c 100644
--- a/BTCPayServer/Controllers/GreenField/LocalBTCPayServerClient.cs
+++ b/BTCPayServer/Controllers/GreenField/LocalBTCPayServerClient.cs
@@ -985,17 +985,22 @@ public override async Task<ApplicationUserData[]> GetUsers(CancellationToken tok
             return GetFromActionResult(await GetController<GreenfieldUsersController>().GetUsers());
         }
 
-        public override Task<IEnumerable<StoreUserData>> GetStoreUsers(string storeId,
+        public override async Task<IEnumerable<StoreUserData>> GetStoreUsers(string storeId,
             CancellationToken token = default)
         {
-            return Task.FromResult(
-                GetFromActionResult<IEnumerable<StoreUserData>>(GetController<GreenfieldStoreUsersController>().GetStoreUsers()));
+            return GetFromActionResult<IEnumerable<StoreUserData>>(await GetController<GreenfieldStoreUsersController>().GetStoreUsers());
         }
 
         public override async Task AddStoreUser(string storeId, StoreUserData request,
             CancellationToken token = default)
         {
-            HandleActionResult(await GetController<GreenfieldStoreUsersController>().AddStoreUser(storeId, request));
+            HandleActionResult(await GetController<GreenfieldStoreUsersController>().AddOrUpdateStoreUser(storeId, request));
+        }
+
+        public override async Task UpdateStoreUser(string storeId, string userId, StoreUserData request,
+            CancellationToken token = default)
+        {
+            HandleActionResult(await GetController<GreenfieldStoreUsersController>().AddOrUpdateStoreUser(storeId, request, userId));
         }
 
         public override async Task RemoveStoreUser(string storeId, string userId, CancellationToken token = default)
diff --git a/BTCPayServer/wwwroot/swagger/v1/swagger.template.api-keys.json b/BTCPayServer/wwwroot/swagger/v1/swagger.template.api-keys.json
index 81a4651289..f42c791d7f 100644
--- a/BTCPayServer/wwwroot/swagger/v1/swagger.template.api-keys.json
+++ b/BTCPayServer/wwwroot/swagger/v1/swagger.template.api-keys.json
@@ -47,13 +47,7 @@
                 "description": "Revoke the API key of a target user so that it cannot be used anymore",
                 "parameters": [
                     {
-                        "name": "idOrEmail",
-                        "in": "path",
-                        "required": true,
-                        "description": "The target user's id or email",
-                        "schema": {
-                            "type": "string"
-                        }
+                        "$ref": "#/components/parameters/UserIdOrEmail"
                     },
                     {
                         "name": "apikey",
@@ -244,13 +238,7 @@
                 "description": "Create a new API Key for a user",
                 "parameters": [
                     {
-                        "name": "idOrEmail",
-                        "in": "path",
-                        "required": true,
-                        "description": "The target user's id or email",
-                        "schema": {
-                            "type": "string"
-                        }
+                        "$ref": "#/components/parameters/UserIdOrEmail"
                     }
                 ],
                 "responses": {
diff --git a/BTCPayServer/wwwroot/swagger/v1/swagger.template.json b/BTCPayServer/wwwroot/swagger/v1/swagger.template.json
index 90b4a55e75..46d63ef805 100644
--- a/BTCPayServer/wwwroot/swagger/v1/swagger.template.json
+++ b/BTCPayServer/wwwroot/swagger/v1/swagger.template.json
@@ -42,6 +42,15 @@
                 "schema": {
                     "type": "string"
                 }
+            },
+            "UserIdOrEmail": {
+                "name": "idOrEmail",
+                "in": "path",
+                "required": true,
+                "description": "The user's id or email",
+                "schema": {
+                    "type": "string"
+                }
             }
         },
         "schemas": {
diff --git a/BTCPayServer/wwwroot/swagger/v1/swagger.template.stores-users.json b/BTCPayServer/wwwroot/swagger/v1/swagger.template.stores-users.json
index 37c61d82ff..4a631da808 100644
--- a/BTCPayServer/wwwroot/swagger/v1/swagger.template.stores-users.json
+++ b/BTCPayServer/wwwroot/swagger/v1/swagger.template.stores-users.json
@@ -25,10 +25,10 @@
                         }
                     },
                     "403": {
-                        "description": "If you are authenticated but forbidden to view the specified store"
+                        "description": "If you are authenticated but forbidden to view the specified store's users"
                     },
                     "404": {
-                        "description": "The key is not found for this store"
+                        "description": "The store could not be found"
                     }
                 },
                 "security": [
@@ -69,7 +69,7 @@
                         "description": "The user was added"
                     },
                     "400": {
-                        "description": "A list of errors that occurred when creating the store",
+                        "description": "A list of errors that occurred when adding the store user",
                         "content": {
                             "application/json": {
                                 "schema": {
@@ -79,7 +79,10 @@
                         }
                     },
                     "403": {
-                        "description": "If you are authenticated but forbidden to add new stores"
+                        "description": "If you are authenticated but forbidden to add new store users"
+                    },
+                    "404": {
+                        "description": "The store or user could not be found"
                     },
                     "409": {
                         "description": "Error code: `duplicate-store-user-role`. Removing this user would result in the store having no owner.",
@@ -103,6 +106,63 @@
             }
         },
         "/api/v1/stores/{storeId}/users/{idOrEmail}": {
+            "put": {
+                "tags": [
+                    "Stores (Users)"
+                ],
+                "summary": "Updates a store user",
+                "description": "Updates a store user",
+                "operationId": "Stores_UpdateStoreUser",
+                "parameters": [
+                    {
+                        "$ref": "#/components/parameters/StoreId"
+                    },
+                    {
+                        "$ref": "#/components/parameters/UserIdOrEmail"
+                    }
+                ],
+                "requestBody": {
+                    "x-name": "request",
+                    "content": {
+                        "application/json": {
+                            "schema": {
+                                "$ref": "#/components/schemas/StoreUserData"
+                            }
+                        }
+                    },
+                    "required": true,
+                    "x-position": 1
+                },
+                "responses": {
+                    "200": {
+                        "description": "The user was updated"
+                    },
+                    "400": {
+                        "description": "A list of errors that occurred when updating the store user",
+                        "content": {
+                            "application/json": {
+                                "schema": {
+                                    "$ref": "#/components/schemas/ValidationProblemDetails"
+                                }
+                            }
+                        }
+                    },
+                    "403": {
+                        "description": "If you are authenticated but forbidden to update store users"
+                    },
+                    "404": {
+                        "description": "The store or user could not be found"
+                    }
+                },
+                "security": [
+                    {
+                        "API_Key": [
+                            "btcpay.store.canmodifystoresettings"
+                        ],
+                        "Basic": []
+                    }
+                ]
+            },
             "delete": {
                 "tags": [
                     "Stores (Users)"
@@ -115,13 +175,7 @@
                         "$ref": "#/components/parameters/StoreId"
                     },
                     {
-                        "name": "idOrEmail",
-                        "in": "path",
-                        "required": true,
-                        "description": "The user's id or email",
-                        "schema": {
-                            "type": "string"
-                        }
+                        "$ref": "#/components/parameters/UserIdOrEmail"
                     }
                 ],
                 "responses": {
@@ -129,7 +183,7 @@
                         "description": "The user has been removed"
                     },
                     "400": {
-                        "description": "A list of errors that occurred when removing the store",
+                        "description": "A list of errors that occurred when removing the store user",
                         "content": {
                             "application/json": {
                                 "schema": {
@@ -149,10 +203,10 @@
                         }
                     },
                     "403": {
-                        "description": "If you are authenticated but forbidden to remove the specified store"
+                        "description": "If you are authenticated but forbidden to remove the specified store user"
                     },
                     "404": {
-                        "description": "The key is not found for this store"
+                        "description": "The store or user could not be found"
                     }
                 },
                 "security": [
@@ -186,8 +240,23 @@
                             },
                             "role": {
                                 "type": "string",
-                                "description": "The role of the user. Default roles are `Owner` and `Guest`",
+                                "description": "The role of the user. Default roles are `Owner`, `Manager`, `Employee` and `Guest`",
                                 "nullable": false
+                            },
+                            "email": {
+                                "type": "string",
+                                "description": "The email of the user",
+                                "nullable": true
+                            },
+                            "name": {
+                                "type": "string",
+                                "description": "The name of the user",
+                                "nullable": true
+                            },
+                            "imageUrl": {
+                                "type": "string",
+                                "description": "The profile picture URL of the user",
+                                "nullable": true
                             }
                         }
                     }
diff --git a/BTCPayServer/wwwroot/swagger/v1/swagger.template.users.json b/BTCPayServer/wwwroot/swagger/v1/swagger.template.users.json
index a0756bc794..9e005d6766 100644
--- a/BTCPayServer/wwwroot/swagger/v1/swagger.template.users.json
+++ b/BTCPayServer/wwwroot/swagger/v1/swagger.template.users.json
@@ -330,13 +330,7 @@
                 "description": "Get 1 user by ID or Email.",
                 "parameters": [
                     {
-                        "name": "idOrEmail",
-                        "in": "path",
-                        "required": true,
-                        "description": "The ID or email of the user to load",
-                        "schema": {
-                            "type": "string"
-                        }
+                        "$ref": "#/components/parameters/UserIdOrEmail"
                     }
                 ],
                 "responses": {
@@ -378,13 +372,7 @@
                 "description": "Delete a user.\n\nMust be an admin to perform this operation.\n\nAttempting to delete the only admin user will not succeed.\n\nAll data associated with the user will be deleted as well if the operation succeeds.",
                 "parameters": [
                     {
-                        "name": "idOrEmail",
-                        "in": "path",
-                        "required": true,
-                        "description": "The ID or email of the user to be deleted",
-                        "schema": {
-                            "type": "string"
-                        }
+                        "$ref": "#/components/parameters/UserIdOrEmail"
                     }
                 ],
                 "responses": {
@@ -421,13 +409,7 @@
                 "description": "Lock or unlock a user.\n\nMust be an admin to perform this operation.\n\nAttempting to lock the only admin user will not succeed.",
                 "parameters": [
                     {
-                        "name": "idOrEmail",
-                        "in": "path",
-                        "required": true,
-                        "description": "The ID of the user to be un/locked",
-                        "schema": {
-                            "type": "string"
-                        }
+                        "$ref": "#/components/parameters/UserIdOrEmail"
                     }
                 ],
                 "requestBody": {
@@ -474,13 +456,7 @@
                 "description": "Approve or unapprove a user.\n\nMust be an admin to perform this operation.\n\nAttempting to (un)approve a user for which this requirement does not exist will not succeed.",
                 "parameters": [
                     {
-                        "name": "idOrEmail",
-                        "in": "path",
-                        "required": true,
-                        "description": "The ID of the user to be un/approved",
-                        "schema": {
-                            "type": "string"
-                        }
+                        "$ref": "#/components/parameters/UserIdOrEmail"
                     }
                 ],
                 "requestBody": {


Test patch:
diff --git a/BTCPayServer.Tests/GreenfieldAPITests.cs b/BTCPayServer.Tests/GreenfieldAPITests.cs
index bfebb99b71..2906043a27 100644
--- a/BTCPayServer.Tests/GreenfieldAPITests.cs
+++ b/BTCPayServer.Tests/GreenfieldAPITests.cs
@@ -3985,7 +3985,12 @@ public async Task StoreUsersAPITest()
             var user = tester.NewAccount();
             await user.GrantAccessAsync(true);
 
-            var client = await user.CreateClient(Policies.CanModifyStoreSettings, Policies.CanModifyServerSettings);
+            var client = await user.CreateClient(Policies.CanModifyStoreSettings, Policies.CanModifyServerSettings, Policies.CanModifyProfile);
+            await client.UpdateCurrentUser(new UpdateApplicationUserRequest
+            {
+                Name = "The Admin",
+                ImageUrl = "avatar.jpg"
+            });
 
             var roles = await client.GetServerRoles();
             Assert.Equal(4, roles.Count);
@@ -3999,6 +4004,9 @@ public async Task StoreUsersAPITest()
             var storeUser = Assert.Single(users);
             Assert.Equal(user.UserId, storeUser.UserId);
             Assert.Equal(ownerRole.Id, storeUser.Role);
+            Assert.Equal(user.Email, storeUser.Email);
+            Assert.Equal("The Admin", storeUser.Name);
+            Assert.Equal("avatar.jpg", storeUser.ImageUrl);
             var manager = tester.NewAccount();
             await manager.GrantAccessAsync();
             var employee = tester.NewAccount();
@@ -4029,7 +4037,14 @@ public async Task StoreUsersAPITest()
             // add users to store
             await client.AddStoreUser(user.StoreId, new StoreUserData { Role = managerRole.Id, UserId = manager.UserId });
             await client.AddStoreUser(user.StoreId, new StoreUserData { Role = employeeRole.Id, UserId = employee.UserId });
-            await client.AddStoreUser(user.StoreId, new StoreUserData { Role = guestRole.Id, UserId = guest.UserId });
+
+            // add with email
+            await client.AddStoreUser(user.StoreId, new StoreUserData { Role = guestRole.Id, UserId = guest.Email });
+            
+            // test unknown user
+            await AssertAPIError("user-not-found", async () => await client.AddStoreUser(user.StoreId, new StoreUserData { Role = managerRole.Id, UserId = "unknown" }));
+            await AssertAPIError("user-not-found", async () => await client.UpdateStoreUser(user.StoreId, "unknown", new StoreUserData { Role = ownerRole.Id }));
+            await AssertAPIError("user-not-found", async () => await client.RemoveStoreUser(user.StoreId, "unknown"));
 
             //test no access to api for employee
             await AssertPermissionError(Policies.CanViewStoreSettings, async () => await employeeClient.GetStore(user.StoreId));
@@ -4050,9 +4065,14 @@ public async Task StoreUsersAPITest()
             await AssertPermissionError(Policies.CanModifyStoreSettings, async () => await managerClient.RemoveStoreUser(user.StoreId, user.UserId));
 
             // updates
+            await client.UpdateStoreUser(user.StoreId, employee.UserId, new StoreUserData { Role = ownerRole.Id });
+            await employeeClient.GetStore(user.StoreId);
+            
+            // remove
             await client.RemoveStoreUser(user.StoreId, employee.UserId);
             await AssertHttpError(403, async () => await employeeClient.GetStore(user.StoreId));
 
+            // test duplicate add
             await client.AddStoreUser(user.StoreId, new StoreUserData { Role = ownerRole.Id, UserId = employee.UserId });
             await AssertAPIError("duplicate-store-user-role", async () =>
                  await client.AddStoreUser(user.StoreId, new StoreUserData { Role = ownerRole.Id, UserId = employee.UserId }));