From da9a6b835a878d7a65f05b37db44dd02ca6c8326 Mon Sep 17 00:00:00 2001 From: Andrew Camilleri Date: Thu, 10 Feb 2022 06:51:10 +0100 Subject: [PATCH] Greenfield: Store Users (#3425) * Greenfield: Store Users * fixups Co-authored-by: nicolas.dorier --- .gitignore | 1 + .../BTCPayServerClient.StoreUsers.cs | 37 ++++ BTCPayServer.Client/Models/StoreData.cs | 10 + BTCPayServer.Tests/GreenfieldAPITests.cs | 59 +++++ .../GreenfieldStoreUsersController.cs | 81 +++++++ .../Controllers/UIStoresController.cs | 8 +- .../Services/Stores/StoreRepository.cs | 19 +- .../v1/swagger.template.stores-users.json | 208 ++++++++++++++++++ 8 files changed, 412 insertions(+), 11 deletions(-) create mode 100644 BTCPayServer.Client/BTCPayServerClient.StoreUsers.cs create mode 100644 BTCPayServer/Controllers/GreenField/GreenfieldStoreUsersController.cs create mode 100644 BTCPayServer/wwwroot/swagger/v1/swagger.template.stores-users.json diff --git a/.gitignore b/.gitignore index bce7c51e4..f466f2cba 100644 --- a/.gitignore +++ b/.gitignore @@ -299,3 +299,4 @@ BTCPayServer/wwwroot/bundles/* BTCPayServer/testpwd .DS_Store Packed Plugins +Plugins/packed diff --git a/BTCPayServer.Client/BTCPayServerClient.StoreUsers.cs b/BTCPayServer.Client/BTCPayServerClient.StoreUsers.cs new file mode 100644 index 000000000..d8387e5a8 --- /dev/null +++ b/BTCPayServer.Client/BTCPayServerClient.StoreUsers.cs @@ -0,0 +1,37 @@ +using System; +using System.Collections.Generic; +using System.Net.Http; +using System.Threading; +using System.Threading.Tasks; +using BTCPayServer.Client.Models; + +namespace BTCPayServer.Client +{ + public partial class BTCPayServerClient + { + public virtual async Task> GetStoreUsers(string storeId, + CancellationToken token = default) + { + using var response = await _httpClient.SendAsync(CreateHttpRequest($"api/v1/stores/{storeId}/users"), token); + return await HandleResponse>(response); + } + + public virtual async Task RemoveStoreUser(string storeId, string userId, CancellationToken token = default) + { + using var response = await _httpClient.SendAsync( + CreateHttpRequest($"api/v1/stores/{storeId}/users/{userId}", method: HttpMethod.Delete), token); + await HandleResponse(response); + } + + public virtual async Task AddStoreUser(string storeId, StoreUserData request, + CancellationToken token = default) + { + if (request == null) + throw new ArgumentNullException(nameof(request)); + using var response = await _httpClient.SendAsync( + CreateHttpRequest($"api/v1/stores/{storeId}/users", bodyPayload: request, method: HttpMethod.Post), + token); + return await HandleResponse(response); + } + } +} diff --git a/BTCPayServer.Client/Models/StoreData.cs b/BTCPayServer.Client/Models/StoreData.cs index 779fc1978..67f4a78bf 100644 --- a/BTCPayServer.Client/Models/StoreData.cs +++ b/BTCPayServer.Client/Models/StoreData.cs @@ -7,4 +7,14 @@ namespace BTCPayServer.Client.Models /// public string Id { get; set; } } + + public class StoreUserData + { + /// + /// the id of the user + /// + public string UserId { get; set; } + + public string Role { get; set; } + } } diff --git a/BTCPayServer.Tests/GreenfieldAPITests.cs b/BTCPayServer.Tests/GreenfieldAPITests.cs index 3e00be687..c671df5ec 100644 --- a/BTCPayServer.Tests/GreenfieldAPITests.cs +++ b/BTCPayServer.Tests/GreenfieldAPITests.cs @@ -2111,5 +2111,64 @@ namespace BTCPayServer.Tests } + + [Fact(Timeout = 60 * 2 * 1000)] + [Trait("Integration", "Integration")] + public async Task StoreUsersAPITest() + { + + using var tester = CreateServerTester(); + await tester.StartAsync(); + + var user = tester.NewAccount(); + await user.GrantAccessAsync(true); + + var client = await user.CreateClient(Policies.CanModifyStoreSettings, Policies.CanModifyServerSettings); + + var users = await client.GetStoreUsers(user.StoreId); + var storeuser = Assert.Single(users); + Assert.Equal(user.UserId,storeuser.UserId); + Assert.Equal(StoreRoles.Owner,storeuser.Role); + + var user2= tester.NewAccount(); + await user2.GrantAccessAsync(false); + + var user2Client =await user2.CreateClient(Policies.CanModifyStoreSettings); + + //test no access to api when unrelated to store at all + await AssertPermissionError(Policies.CanModifyStoreSettings, async () => await user2Client.GetStoreUsers(user.StoreId)); + await AssertPermissionError(Policies.CanModifyStoreSettings, async () => await user2Client.AddStoreUser(user.StoreId, new StoreUserData())); + await AssertPermissionError(Policies.CanModifyStoreSettings, async () => await user2Client.RemoveStoreUser(user.StoreId, user.UserId)); + + await client.AddStoreUser(user.StoreId, new StoreUserData() { Role = StoreRoles.Guest, UserId = user2.UserId }); + + //test no access to api when only a guest + await AssertPermissionError(Policies.CanModifyStoreSettings, async () => await user2Client.GetStoreUsers(user.StoreId)); + await AssertPermissionError(Policies.CanModifyStoreSettings, async () => await user2Client.AddStoreUser(user.StoreId, new StoreUserData())); + await AssertPermissionError(Policies.CanModifyStoreSettings, async () => await user2Client.RemoveStoreUser(user.StoreId, user.UserId)); + + await user2Client.GetStore(user.StoreId); + + await client.RemoveStoreUser(user.StoreId, user2.UserId); + await AssertHttpError(403, async () => + await user2Client.GetStore(user.StoreId)); + + + await client.AddStoreUser(user.StoreId, new StoreUserData() { Role = StoreRoles.Owner, UserId = user2.UserId }); + await AssertAPIError("duplicate-store-user-role",async ()=> + await client.AddStoreUser(user.StoreId, + new StoreUserData() { Role = StoreRoles.Owner, UserId = user2.UserId })); + await user2Client.RemoveStoreUser(user.StoreId, user.UserId); + + + //test no access to api when unrelated to store at all + await AssertPermissionError(Policies.CanModifyStoreSettings, async () => await client.GetStoreUsers(user.StoreId)); + await AssertPermissionError(Policies.CanModifyStoreSettings, async () => await client.AddStoreUser(user.StoreId, new StoreUserData())); + await AssertPermissionError(Policies.CanModifyStoreSettings, async () => await client.RemoveStoreUser(user.StoreId, user.UserId)); + + await AssertAPIError("store-user-role-orphaned", async () => await user2Client.RemoveStoreUser(user.StoreId, user2.UserId)); + + } + } } diff --git a/BTCPayServer/Controllers/GreenField/GreenfieldStoreUsersController.cs b/BTCPayServer/Controllers/GreenField/GreenfieldStoreUsersController.cs new file mode 100644 index 000000000..ae515c8ba --- /dev/null +++ b/BTCPayServer/Controllers/GreenField/GreenfieldStoreUsersController.cs @@ -0,0 +1,81 @@ +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using BTCPayServer.Abstractions.Constants; +using BTCPayServer.Client; +using BTCPayServer.Client.Models; +using BTCPayServer.Data; +using BTCPayServer.Services.Stores; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Cors; +using Microsoft.AspNetCore.Identity; +using Microsoft.AspNetCore.Mvc; + +namespace BTCPayServer.Controllers.Greenfield +{ + [ApiController] + [Authorize(AuthenticationSchemes = AuthenticationSchemes.Greenfield)] + [EnableCors(CorsPolicies.All)] + public class GreenfieldStoreUsersController : ControllerBase + { + private readonly StoreRepository _storeRepository; + + public GreenfieldStoreUsersController(StoreRepository storeRepository, UserManager userManager) + { + _storeRepository = storeRepository; + } + [Authorize(Policy = Policies.CanModifyStoreSettings, AuthenticationSchemes = AuthenticationSchemes.Greenfield)] + [HttpGet("~/api/v1/stores/{storeId}/users")] + public IActionResult GetStoreUsers() + { + + var store = HttpContext.GetStoreData(); + return store == null ? StoreNotFound() : Ok(FromModel(store)); + } + [Authorize(Policy = Policies.CanModifyStoreSettings, AuthenticationSchemes = AuthenticationSchemes.Greenfield)] + [HttpDelete("~/api/v1/stores/{storeId}/users/{userId}")] + public async Task RemoveStoreUser(string storeId, string userId) + { + var store = HttpContext.GetStoreData(); + if (store == null) + { + return StoreNotFound(); + } + + if (await _storeRepository.RemoveStoreUser(storeId, userId)) + { + + return Ok(); + } + + return 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) + { + var store = HttpContext.GetStoreData(); + if (store == null) + { + return StoreNotFound(); + } + //we do not need to validate the role string as any value other than `StoreRoles.Owner` is currently treated like a guest + if (await _storeRepository.AddStoreUser(storeId, request.UserId, request.Role)) + { + return Ok(); + } + + return this.CreateAPIError(409, "duplicate-store-user-role", "The user is already added to the store"); + } + + private IEnumerable FromModel(Data.StoreData data) + { + return data.UserStores.Select(store => new StoreUserData() { UserId = store.ApplicationUserId, Role = store.Role}); + } + private IActionResult StoreNotFound() + { + return this.CreateAPIError(404, "store-not-found", "The store was not found"); + } + } +} diff --git a/BTCPayServer/Controllers/UIStoresController.cs b/BTCPayServer/Controllers/UIStoresController.cs index 69880204e..bc7f9f650 100644 --- a/BTCPayServer/Controllers/UIStoresController.cs +++ b/BTCPayServer/Controllers/UIStoresController.cs @@ -201,8 +201,12 @@ namespace BTCPayServer.Controllers [HttpPost("{storeId}/users/{userId}/delete")] public async Task DeleteStoreUserPost(string storeId, string userId) { - await _Repo.RemoveStoreUser(storeId, userId); - TempData[WellKnownTempData.SuccessMessage] = "User removed successfully."; + if(await _Repo.RemoveStoreUser(storeId, userId)) + TempData[WellKnownTempData.SuccessMessage] = "User removed successfully."; + else + { + TempData[WellKnownTempData.ErrorMessage] = "Removing this user would result in the store having no owner."; + } return RedirectToAction(nameof(StoreUsers), new { storeId, userId }); } diff --git a/BTCPayServer/Services/Stores/StoreRepository.cs b/BTCPayServer/Services/Stores/StoreRepository.cs index 61eeaf438..714294f4f 100644 --- a/BTCPayServer/Services/Stores/StoreRepository.cs +++ b/BTCPayServer/Services/Stores/StoreRepository.cs @@ -122,17 +122,18 @@ namespace BTCPayServer.Services.Stores await ctx.SaveChangesAsync(); } - public async Task RemoveStoreUser(string storeId, string userId) + public async Task RemoveStoreUser(string storeId, string userId) { - using (var ctx = _ContextFactory.CreateContext()) - { - var userStore = new UserStore() { StoreDataId = storeId, ApplicationUserId = userId }; - ctx.UserStore.Add(userStore); - ctx.Entry(userStore).State = Microsoft.EntityFrameworkCore.EntityState.Deleted; - await ctx.SaveChangesAsync(); + await using var ctx = _ContextFactory.CreateContext(); + if (!await ctx.UserStore.AnyAsync(store => + store.StoreDataId == storeId && store.Role == StoreRoles.Owner && + userId != store.ApplicationUserId)) return false; + var userStore = new UserStore() { StoreDataId = storeId, ApplicationUserId = userId }; + ctx.UserStore.Add(userStore); + ctx.Entry(userStore).State = EntityState.Deleted; + await ctx.SaveChangesAsync(); + return true; - } - await DeleteStoreIfOrphan(storeId); } private async Task DeleteStoreIfOrphan(string storeId) diff --git a/BTCPayServer/wwwroot/swagger/v1/swagger.template.stores-users.json b/BTCPayServer/wwwroot/swagger/v1/swagger.template.stores-users.json new file mode 100644 index 000000000..7e9eb74e1 --- /dev/null +++ b/BTCPayServer/wwwroot/swagger/v1/swagger.template.stores-users.json @@ -0,0 +1,208 @@ +{ + "paths": { + "/api/v1/stores/{storeId}/users": { + "get": { + "tags": [ + "Stores (Users)" + ], + "summary": "Get store users", + "parameters": [ + { + "name": "storeId", + "in": "path", + "required": true, + "description": "The store to fetch", + "schema": { + "type": "string" + } + } + ], + "description": "View users of the specified store", + "operationId": "Stores_GetStoreUsers", + "responses": { + "200": { + "description": "specified store users", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/StoreUserDataList" + } + } + } + }, + "403": { + "description": "If you are authenticated but forbidden to view the specified store" + }, + "404": { + "description": "The key is not found for this store" + } + }, + "security": [ + { + "API_Key": [ + "btcpay.store.canmodifystoresettings" + ], + "Basic": [] + } + ] + }, + "post": { + "tags": [ + "Stores (Users)" + ], + "summary": "Add a store user", + "description": "Add a store user", + "requestBody": { + "x-name": "request", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/StoreUserData" + } + } + }, + "required": true, + "x-position": 1 + }, + "responses": { + "200": { + "description": "The user as added" + }, + "400": { + "description": "A list of errors that occurred when creating the store", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ValidationProblemDetails" + } + } + } + }, + "403": { + "description": "If you are authenticated but forbidden to add new stores" + }, + "409": { + "description": "Error code: `duplicate-store-user-role`. Removing this user would result in the store having no owner.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + } + }, + "security": [ + { + "API_Key": [ + "btcpay.store.canmodifystoresettings" + ], + "Basic": [] + } + ] + } + }, + "/api/v1/stores/{storeId}/users/{userId}": { + "delete": { + "tags": [ + "Stores (Users)" + ], + "summary": "Remove Store User", + "description": "Removes the specified store user. If there is no other owner, this endpoint will fail.", + "parameters": [ + { + "name": "storeId", + "in": "path", + "required": true, + "description": "The store", + "schema": { + "type": "string" + } + }, + { + "name": "userId", + "in": "path", + "required": true, + "description": "The user", + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "The user has been removed" + }, + "400": { + "description": "A list of errors that occurred when removing the store", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ValidationProblemDetails" + } + } + } + }, + "409": { + "description": "Error code: `store-user-role-orphaned`. Removing this user would result in the store having no owner.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, + "403": { + "description": "If you are authenticated but forbidden to remove the specified store" + }, + "404": { + "description": "The key is not found for this store" + } + }, + "security": [ + { + "API_Key": [ + "btcpay.store.canmodifystoresettings" + ], + "Basic": [] + } + ] + } + } + }, + "components": { + "schemas": { + "StoreUserDataList": { + "type": "array", + "items": { + "$ref": "#/components/schemas/StoreData" + } + }, + "StoreUserData": { + "allOf": [ + { + "type": "object", + "properties": { + "userId": { + "type": "string", + "description": "The id of the user", + "nullable": false + }, + "role": { + "type": "string", + "description": "The role of the user. Default roles are `Owner` and `Guest`", + "nullable": false + } + } + } + ] + } + } + }, + "tags": [ + { + "name": "Stores (Users)" + } + ] +}