From 898f0f448101cd7093ae7628cdedd75a428a62e8 Mon Sep 17 00:00:00 2001 From: d11n Date: Mon, 2 Dec 2024 15:35:33 +0100 Subject: [PATCH] Greenfield: Improve store users API (#6427) * Greenfield: Improve store users API - 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 * Swagger: Reuse UserIdOrEmail parameter component * Add details to store user data --- .../BTCPayServerClient.StoreUsers.cs | 6 ++ BTCPayServer.Client/Models/StoreData.cs | 18 ++++ BTCPayServer.Tests/GreenfieldAPITests.cs | 24 ++++- .../GreenfieldStoreUsersController.cs | 73 +++++++++----- .../GreenField/LocalBTCPayServerClient.cs | 13 ++- .../swagger/v1/swagger.template.api-keys.json | 16 +-- .../wwwroot/swagger/v1/swagger.template.json | 9 ++ .../v1/swagger.template.stores-users.json | 99 ++++++++++++++++--- .../swagger/v1/swagger.template.users.json | 32 +----- 9 files changed, 200 insertions(+), 90 deletions(-) diff --git a/BTCPayServer.Client/BTCPayServerClient.StoreUsers.cs b/BTCPayServer.Client/BTCPayServerClient.StoreUsers.cs index 116cd6be6..2311418c7 100644 --- a/BTCPayServer.Client/BTCPayServerClient.StoreUsers.cs +++ b/BTCPayServer.Client/BTCPayServerClient.StoreUsers.cs @@ -29,4 +29,10 @@ public partial class BTCPayServerClient if (request == null) throw new ArgumentNullException(nameof(request)); await SendHttpRequest($"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($"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 ca6459e69..9f2e2b728 100644 --- a/BTCPayServer.Client/Models/StoreData.cs +++ b/BTCPayServer.Client/Models/StoreData.cs @@ -17,7 +17,25 @@ namespace BTCPayServer.Client.Models /// public string UserId { get; set; } + /// + /// the store role of the user + /// public string Role { get; set; } + + /// + /// the email AND username of the user + /// + public string Email { get; set; } + + /// + /// the name of the user + /// + public string Name { get; set; } + + /// + /// the image url of the user + /// + public string ImageUrl { get; set; } } public class RoleData diff --git a/BTCPayServer.Tests/GreenfieldAPITests.cs b/BTCPayServer.Tests/GreenfieldAPITests.cs index 1c5cebdfd..ce12b8e57 100644 --- a/BTCPayServer.Tests/GreenfieldAPITests.cs +++ b/BTCPayServer.Tests/GreenfieldAPITests.cs @@ -3985,7 +3985,12 @@ namespace BTCPayServer.Tests 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 @@ namespace BTCPayServer.Tests 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 @@ namespace BTCPayServer.Tests // 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 @@ namespace BTCPayServer.Tests 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 })); diff --git a/BTCPayServer/Controllers/GreenField/GreenfieldStoreUsersController.cs b/BTCPayServer/Controllers/GreenField/GreenfieldStoreUsersController.cs index 1839d7071..f54da7301 100644 --- a/BTCPayServer/Controllers/GreenField/GreenfieldStoreUsersController.cs +++ b/BTCPayServer/Controllers/GreenField/GreenfieldStoreUsersController.cs @@ -11,6 +11,7 @@ using Microsoft.AspNetCore.Authorization; 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 @@ namespace BTCPayServer.Controllers.Greenfield [Authorize(Policy = Policies.CanViewStoreSettings, AuthenticationSchemes = AuthenticationSchemes.Greenfield)] [HttpGet("~/api/v1/stores/{storeId}/users")] - public IActionResult GetStoreUsers() + public async Task 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 @@ namespace BTCPayServer.Controllers.Greenfield public async Task RemoveStoreUser(string storeId, string idOrEmail) { var store = HttpContext.GetStoreData(); - if (store == null) - { - return StoreNotFound(); - } + if (store == null) return StoreNotFound(); - var userId = await _userManager.FindByIdOrEmail(idOrEmail); - if (userId != null && await _storeRepository.RemoveStoreUser(storeId, idOrEmail)) - { - return Ok(); - } - - 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 AddStoreUser(string storeId, StoreUserData request) + [HttpPut("~/api/v1/stores/{storeId}/users/{idOrEmail?}")] + public async Task 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 @@ namespace BTCPayServer.Controllers.Greenfield 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 FromModel(Data.StoreData data) + private async Task> FromModel(StoreData data) { - return data.UserStores.Select(store => new StoreUserData() { UserId = store.ApplicationUserId, Role = store.StoreRoleId }); + var storeUsers = new List(); + 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 2a4b898f3..95e2c30b2 100644 --- a/BTCPayServer/Controllers/GreenField/LocalBTCPayServerClient.cs +++ b/BTCPayServer/Controllers/GreenField/LocalBTCPayServerClient.cs @@ -985,17 +985,22 @@ namespace BTCPayServer.Controllers.Greenfield return GetFromActionResult(await GetController().GetUsers()); } - public override Task> GetStoreUsers(string storeId, + public override async Task> GetStoreUsers(string storeId, CancellationToken token = default) { - return Task.FromResult( - GetFromActionResult>(GetController().GetStoreUsers())); + return GetFromActionResult>(await GetController().GetStoreUsers()); } public override async Task AddStoreUser(string storeId, StoreUserData request, CancellationToken token = default) { - HandleActionResult(await GetController().AddStoreUser(storeId, request)); + HandleActionResult(await GetController().AddOrUpdateStoreUser(storeId, request)); + } + + public override async Task UpdateStoreUser(string storeId, string userId, StoreUserData request, + CancellationToken token = default) + { + HandleActionResult(await GetController().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 81a465128..f42c791d7 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 90b4a55e7..46d63ef80 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 37c61d82f..4a631da80 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 a0756bc79..9e005d676 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": {