diff --git a/BTCPayServer.Client/BTCPayServerClient.Users.cs b/BTCPayServer.Client/BTCPayServerClient.Users.cs index 0879bd7ce..08063878b 100644 --- a/BTCPayServer.Client/BTCPayServerClient.Users.cs +++ b/BTCPayServer.Client/BTCPayServerClient.Users.cs @@ -1,3 +1,4 @@ +#nullable enable using System.Net.Http; using System.Threading; using System.Threading.Tasks; @@ -19,5 +20,16 @@ namespace BTCPayServer.Client var response = await _httpClient.SendAsync(CreateHttpRequest("api/v1/users", null, request, HttpMethod.Post), token); return await HandleResponse(response); } + + public virtual async Task DeleteUser(string userId, CancellationToken token = default) + { + var response = await _httpClient.SendAsync(CreateHttpRequest($"api/v1/users/{userId}", null, HttpMethod.Delete), token); + await HandleResponse(response); + } + + public virtual async Task DeleteCurrentUser(CancellationToken token = default) + { + await DeleteUser("me", token); + } } } diff --git a/BTCPayServer.Client/Permissions.cs b/BTCPayServer.Client/Permissions.cs index 08dfeda61..adc1480f5 100644 --- a/BTCPayServer.Client/Permissions.cs +++ b/BTCPayServer.Client/Permissions.cs @@ -24,6 +24,7 @@ namespace BTCPayServer.Client public const string CanManageNotificationsForUser = "btcpay.user.canmanagenotificationsforuser"; public const string CanViewNotificationsForUser = "btcpay.user.canviewnotificationsforuser"; public const string CanCreateUser = "btcpay.server.cancreateuser"; + public const string CanDeleteUser = "btcpay.user.candeleteuser"; public const string CanManagePullPayments = "btcpay.store.canmanagepullpayments"; public const string Unrestricted = "unrestricted"; public static IEnumerable AllPolicies @@ -41,6 +42,7 @@ namespace BTCPayServer.Client yield return CanModifyProfile; yield return CanViewProfile; yield return CanCreateUser; + yield return CanDeleteUser; yield return CanManageNotificationsForUser; yield return CanViewNotificationsForUser; yield return Unrestricted; diff --git a/BTCPayServer.Tests/GreenfieldAPITests.cs b/BTCPayServer.Tests/GreenfieldAPITests.cs index 558c0c6a3..acd0d84d2 100644 --- a/BTCPayServer.Tests/GreenfieldAPITests.cs +++ b/BTCPayServer.Tests/GreenfieldAPITests.cs @@ -127,6 +127,45 @@ namespace BTCPayServer.Tests } } + [Fact(Timeout = TestTimeout)] + [Trait("Integration", "Integration")] + public async Task CanDeleteUsersViaApi() + { + using var tester = ServerTester.Create(newDb: true); + await tester.StartAsync(); + var unauthClient = new BTCPayServerClient(tester.PayTester.ServerUri); + // Should not be authorized to perform this action + await AssertHttpError(401, + async () => await unauthClient.DeleteUser("lol user id")); + + var user = tester.NewAccount(); + await user.GrantAccessAsync(); + await user.MakeAdmin(); + var adminClient = await user.CreateClient(Policies.Unrestricted); + + //can't delete if the only admin + await AssertHttpError(403, + async () => await adminClient.DeleteCurrentUser()); + + // Should 404 if user doesn't exist + await AssertHttpError(404, + async () => await adminClient.DeleteUser("lol user id")); + + user = tester.NewAccount(); + await user.GrantAccessAsync(); + var badClient = await user.CreateClient(Policies.CanCreateInvoice); + + await AssertHttpError(403, + async () => await badClient.DeleteCurrentUser()); + + var goodClient = await user.CreateClient(Policies.CanDeleteUser, Policies.CanViewProfile); + await goodClient.DeleteCurrentUser(); + await AssertHttpError(404, + async () => await adminClient.DeleteUser(user.UserId)); + + tester.Stores.Remove(user.StoreId); + } + [Fact(Timeout = TestTimeout)] [Trait("Integration", "Integration")] public async Task CanCreateUsersViaAPI() diff --git a/BTCPayServer/Controllers/GreenField/UsersController.cs b/BTCPayServer/Controllers/GreenField/UsersController.cs index faab54b11..fe5c2478a 100644 --- a/BTCPayServer/Controllers/GreenField/UsersController.cs +++ b/BTCPayServer/Controllers/GreenField/UsersController.cs @@ -1,3 +1,4 @@ +#nullable enable using System; using System.Linq; using System.Threading; @@ -9,10 +10,11 @@ using BTCPayServer.Configuration; using BTCPayServer.Data; using BTCPayServer.Events; using BTCPayServer.HostedServices; -using BTCPayServer.Logging; using BTCPayServer.Security; using BTCPayServer.Security.GreenField; using BTCPayServer.Services; +using BTCPayServer.Storage.Services; +using BTCPayServer.Services.Stores; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Cors; using Microsoft.AspNetCore.Identity; @@ -35,6 +37,7 @@ namespace BTCPayServer.Controllers.GreenField private readonly BTCPayServerOptions _options; private readonly IAuthorizationService _authorizationService; private readonly CssThemeManager _themeManager; + private readonly UserService _userService; public UsersController(UserManager userManager, RoleManager roleManager, @@ -44,7 +47,8 @@ namespace BTCPayServer.Controllers.GreenField RateLimitService throttleService, BTCPayServerOptions options, IAuthorizationService authorizationService, - CssThemeManager themeManager) + CssThemeManager themeManager, + UserService userService) { _userManager = userManager; _roleManager = roleManager; @@ -55,6 +59,7 @@ namespace BTCPayServer.Controllers.GreenField _options = options; _authorizationService = authorizationService; _themeManager = themeManager; + _userService = userService; } [Authorize(Policy = Policies.CanViewProfile, AuthenticationSchemes = AuthenticationSchemes.Greenfield)] @@ -65,17 +70,24 @@ namespace BTCPayServer.Controllers.GreenField return await FromModel(user); } + [Authorize(Policy = Policies.CanDeleteUser, AuthenticationSchemes = AuthenticationSchemes.Greenfield)] + [HttpDelete("~/api/v1/users/me")] + public async Task DeleteCurrentUser() + { + return await DeleteUser(_userManager.GetUserId(User)); + } + [AllowAnonymous] [HttpPost("~/api/v1/users")] public async Task CreateUser(CreateApplicationUserRequest request, CancellationToken cancellationToken = default) { - if (request?.Email is null) + if (request.Email is null) ModelState.AddModelError(nameof(request.Email), "Email is missing"); - if (!string.IsNullOrEmpty(request?.Email) && !Validation.EmailValidator.IsEmail(request.Email)) + if (!string.IsNullOrEmpty(request.Email) && !Validation.EmailValidator.IsEmail(request.Email)) { ModelState.AddModelError(nameof(request.Email), "Invalid email"); } - if (request?.Password is null) + if (request.Password is null) ModelState.AddModelError(nameof(request.Password), "Password is missing"); if (!ModelState.IsValid) @@ -154,8 +166,10 @@ namespace BTCPayServer.Controllers.GreenField if (!anyAdmin) { var settings = await _settingsRepository.GetSettingAsync(); - settings.FirstRun = false; - await _settingsRepository.UpdateSetting(settings); + if (settings != null) { + settings.FirstRun = false; + await _settingsRepository.UpdateSetting(settings); + } await _settingsRepository.FirstAdminRegistered(policies, _options.UpdateUrl != null, _options.DisableRegistration); } @@ -165,6 +179,36 @@ namespace BTCPayServer.Controllers.GreenField return CreatedAtAction(string.Empty, model); } + [HttpDelete("~/api/v1/users/{userId}")] + [Authorize(Policy = Policies.CanModifyServerSettings, AuthenticationSchemes = AuthenticationSchemes.Greenfield)] + public async Task DeleteUser(string userId) + { + var user = await _userManager.FindByIdAsync(userId); + if (user == null) + { + return UserNotFound(); + } + + // We can safely delete the user if it's not an admin user + if (!(await _userService.IsAdminUser(user))) + { + await _userService.DeleteUserAndAssociatedData(user); + + return Ok(); + } + + // User shouldn't be deleted if it's the only admin + if (await IsUserTheOnlyOneAdmin(user)) + { + return Forbid(AuthenticationSchemes.GreenfieldBasic); + } + + // Ok, this user is an admin but there are other admins as well so safe to delete + await _userService.DeleteUserAndAssociatedData(user); + + return Ok(); + } + private async Task FromModel(ApplicationUser data) { var roles = (await _userManager.GetRolesAsync(data)).ToArray(); @@ -178,5 +222,25 @@ namespace BTCPayServer.Controllers.GreenField Created = data.Created }; } + + private async Task IsUserTheOnlyOneAdmin() + { + return await IsUserTheOnlyOneAdmin(await _userManager.GetUserAsync(User)); + } + + private async Task IsUserTheOnlyOneAdmin(ApplicationUser user) + { + var isUserAdmin = await _userService.IsAdminUser(user); + if (!isUserAdmin) { + return false; + } + + return (await _userManager.GetUsersInRoleAsync(Roles.ServerAdmin)).Count == 1; + } + + private IActionResult UserNotFound() + { + return this.CreateAPIError(404, "user-not-found", "The user was not found"); + } } } diff --git a/BTCPayServer/Controllers/ManageController.APIKeys.cs b/BTCPayServer/Controllers/ManageController.APIKeys.cs index dace755eb..945ff12b4 100644 --- a/BTCPayServer/Controllers/ManageController.APIKeys.cs +++ b/BTCPayServer/Controllers/ManageController.APIKeys.cs @@ -470,6 +470,7 @@ namespace BTCPayServer.Controllers { {BTCPayServer.Client.Policies.Unrestricted, ("Unrestricted access", "The app will have unrestricted access to your account.")}, {BTCPayServer.Client.Policies.CanCreateUser, ("Create new users", "The app will be able to create new users on this server.")}, + {BTCPayServer.Client.Policies.CanDeleteUser, ("Delete user", "The app will be able to delete the user to whom it is assigned. Admin users can delete any user without this permission.")}, {BTCPayServer.Client.Policies.CanModifyStoreSettings, ("Modify your stores", "The app will be able to view, modify, delete and create new invoices on all your stores.")}, {$"{BTCPayServer.Client.Policies.CanModifyStoreSettings}:", ("Manage selected stores", "The app will be able to view, modify, delete and create new invoices on the selected stores.")}, {BTCPayServer.Client.Policies.CanModifyStoreWebhooks, ("Modify stores webhooks", "The app will modify the webhooks of all your stores.")}, diff --git a/BTCPayServer/Controllers/ServerController.Users.cs b/BTCPayServer/Controllers/ServerController.Users.cs index a3bc89f71..7a42734b7 100644 --- a/BTCPayServer/Controllers/ServerController.Users.cs +++ b/BTCPayServer/Controllers/ServerController.Users.cs @@ -79,16 +79,11 @@ namespace BTCPayServer.Controllers Id = user.Id, Email = user.Email, Verified = user.EmailConfirmed || !user.RequiresEmailConfirmation, - IsAdmin = IsAdmin(roles) + IsAdmin = _userService.IsRoleAdmin(roles) }; return View(userVM); } - private static bool IsAdmin(IList roles) - { - return roles.Contains(Roles.ServerAdmin, StringComparer.Ordinal); - } - [Route("server/users/{userId}")] [HttpPost] public new async Task User(string userId, UsersViewModel.UserViewModel viewModel) @@ -99,7 +94,7 @@ namespace BTCPayServer.Controllers var admins = await _UserManager.GetUsersInRoleAsync(Roles.ServerAdmin); var roles = await _UserManager.GetRolesAsync(user); - var wasAdmin = IsAdmin(roles); + var wasAdmin = _userService.IsRoleAdmin(roles); if (!viewModel.IsAdmin && admins.Count == 1 && wasAdmin) { TempData[WellKnownTempData.ErrorMessage] = "This is the only Admin, so their role can't be removed until another Admin is added."; @@ -206,7 +201,7 @@ namespace BTCPayServer.Controllers return NotFound(); var roles = await _UserManager.GetRolesAsync(user); - if (IsAdmin(roles)) + if (_userService.IsRoleAdmin(roles)) { var admins = await _UserManager.GetUsersInRoleAsync(Roles.ServerAdmin); if (admins.Count == 1) @@ -236,15 +231,8 @@ namespace BTCPayServer.Controllers if (user == null) return NotFound(); - var files = await _StoredFileRepository.GetFiles(new StoredFileRepository.FilesQuery() - { - UserIds = new[] { userId }, - }); + await _userService.DeleteUserAndAssociatedData(user); - await Task.WhenAll(files.Select(file => _FileService.RemoveFile(file.Id, userId))); - - await _UserManager.DeleteAsync(user); - await _StoreRepository.CleanUnreachableStores(); TempData[WellKnownTempData.SuccessMessage] = "User deleted"; return RedirectToAction(nameof(ListUsers)); } diff --git a/BTCPayServer/Controllers/ServerController.cs b/BTCPayServer/Controllers/ServerController.cs index 221d0e465..247184325 100644 --- a/BTCPayServer/Controllers/ServerController.cs +++ b/BTCPayServer/Controllers/ServerController.cs @@ -46,6 +46,7 @@ namespace BTCPayServer.Controllers public partial class ServerController : Controller { private readonly UserManager _UserManager; + private readonly UserService _userService; readonly SettingsRepository _SettingsRepository; private readonly NBXplorerDashboard _dashBoard; private readonly StoreRepository _StoreRepository; @@ -61,7 +62,9 @@ namespace BTCPayServer.Controllers private readonly FileService _FileService; private readonly IEnumerable _StorageProviderServices; - public ServerController(UserManager userManager, + public ServerController( + UserManager userManager, + UserService userService, StoredFileRepository storedFileRepository, FileService fileService, IEnumerable storageProviderServices, @@ -83,6 +86,7 @@ namespace BTCPayServer.Controllers _FileService = fileService; _StorageProviderServices = storageProviderServices; _UserManager = userManager; + _userService = userService; _SettingsRepository = settingsRepository; _dashBoard = dashBoard; HttpClientFactory = httpClientFactory; diff --git a/BTCPayServer/Hosting/BTCPayServerServices.cs b/BTCPayServer/Hosting/BTCPayServerServices.cs index 084b39f52..b9db7bdd9 100644 --- a/BTCPayServer/Hosting/BTCPayServerServices.cs +++ b/BTCPayServer/Hosting/BTCPayServerServices.cs @@ -112,6 +112,7 @@ namespace BTCPayServer.Hosting services.TryAddSingleton(); services.TryAddSingleton(); services.TryAddSingleton(); + services.TryAddSingleton(); services.AddSingleton(); services.AddOptions().Configure( (options) => diff --git a/BTCPayServer/Security/GreenField/GreenFieldAuthorizationHandler.cs b/BTCPayServer/Security/GreenField/GreenFieldAuthorizationHandler.cs index ffb65688f..c545f72dc 100644 --- a/BTCPayServer/Security/GreenField/GreenFieldAuthorizationHandler.cs +++ b/BTCPayServer/Security/GreenField/GreenFieldAuthorizationHandler.cs @@ -93,6 +93,7 @@ namespace BTCPayServer.Security.GreenField case Policies.CanViewNotificationsForUser: case Policies.CanModifyProfile: case Policies.CanViewProfile: + case Policies.CanDeleteUser: case Policies.Unrestricted: success = context.HasPermission(Permission.Create(policy), requiredUnscoped); break; diff --git a/BTCPayServer/Services/UserService.cs b/BTCPayServer/Services/UserService.cs new file mode 100644 index 000000000..bfd8ddf16 --- /dev/null +++ b/BTCPayServer/Services/UserService.cs @@ -0,0 +1,61 @@ +#nullable enable +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Identity; +using BTCPayServer.Data; +using BTCPayServer.Storage.Services; +using BTCPayServer.Services.Stores; + +namespace BTCPayServer.Services +{ + public class UserService + { + private readonly UserManager _userManager; + private readonly IAuthorizationService _authorizationService; + private readonly StoredFileRepository _storedFileRepository; + private readonly FileService _fileService; + private readonly StoreRepository _storeRepository; + + public UserService( + UserManager userManager, + IAuthorizationService authorizationService, + StoredFileRepository storedFileRepository, + FileService fileService, + StoreRepository storeRepository + ) + { + _userManager = userManager; + _authorizationService = authorizationService; + _storedFileRepository = storedFileRepository; + _fileService = fileService; + _storeRepository = storeRepository; + } + + public async Task IsAdminUser(ApplicationUser user) + { + return IsRoleAdmin(await _userManager.GetRolesAsync(user)); + } + + public async Task DeleteUserAndAssociatedData(ApplicationUser user) + { + var userId = user.Id; + var files = await _storedFileRepository.GetFiles(new StoredFileRepository.FilesQuery() + { + UserIds = new[] { userId }, + }); + + await Task.WhenAll(files.Select(file => _fileService.RemoveFile(file.Id, userId))); + + await _userManager.DeleteAsync(user); + await _storeRepository.CleanUnreachableStores(); + } + + public bool IsRoleAdmin(IList roles) + { + return roles.Contains(Roles.ServerAdmin, StringComparer.Ordinal); + } + } +} diff --git a/BTCPayServer/wwwroot/swagger/v1/swagger.template.json b/BTCPayServer/wwwroot/swagger/v1/swagger.template.json index 5753d73e7..5000aafe7 100644 --- a/BTCPayServer/wwwroot/swagger/v1/swagger.template.json +++ b/BTCPayServer/wwwroot/swagger/v1/swagger.template.json @@ -76,7 +76,7 @@ "securitySchemes": { "API Key": { "type": "apiKey", - "description": "BTCPay Server supports authenticating and authorizing users through an API Key that is generated by them. Send the API Key as a header value to Authorization with the format: `token {token}`. For a smoother experience, you can generate a url that redirects users to an API key creation screen.\n\n The following permissions are available to the context of the user creating the API Key:\n\n* `unrestricted`: Unrestricted access\n* `btcpay.user.canviewprofile`: View your profile\n* `btcpay.user.canmodifyprofile`: Manage your profile\n* `btcpay.user.canmanagenotificationsforuser`: Manage your notifications\n* `btcpay.user.canviewnotificationsforuser`: View your notifications\n\nThe following permissions are available if the user is an administrator:\n\n* `btcpay.server.cancreateuser`: Create new users\n* `btcpay.server.canmodifyserversettings`: Manage your server\n* `btcpay.server.canuseinternallightningnode`: Use the internal lightning node\n* `btcpay.server.cancreatelightninginvoiceinternalnode`: Create invoices with internal lightning node\n\nThe following permissions applies to all stores of the user, you can limit to a specific store with the following format: `btcpay.store.cancreateinvoice:6HSHAEU4iYWtjxtyRs9KyPjM9GAQp8kw2T9VWbGG1FnZ`:\n\n* `btcpay.store.canmodifystoresettings`: Modify your stores\n* `btcpay.store.webhooks.canmodifywebhooks`: Modify stores webhooks\n* `btcpay.store.canviewstoresettings`: View your stores\n* `btcpay.store.cancreateinvoice`: Create an invoice\n* `btcpay.store.canviewinvoices`: View invoices\n* `btcpay.store.canmodifypaymentrequests`: Modify your payment requests\n* `btcpay.store.canviewpaymentrequests`: View your payment requests\n* `btcpay.store.canuselightningnode`: Use the lightning nodes associated with your stores\n* `btcpay.store.cancreatelightninginvoice`: Create invoices the lightning nodes associated with your stores\n\nNote that API Keys only limits permission of a user and can never expand it. If an API Key has the permission `btcpay.server.canmodifyserversettings` but that the user account creating this API Key is not administrator, the API Key will not be able to modify the server settings.\n", + "description": "BTCPay Server supports authenticating and authorizing users through an API Key that is generated by them. Send the API Key as a header value to Authorization with the format: `token {token}`. For a smoother experience, you can generate a url that redirects users to an API key creation screen.\n\n The following permissions are available to the context of the user creating the API Key:\n\n* `unrestricted`: Unrestricted access\n* `btcpay.user.candeleteuser`: Delete user\n* `btcpay.user.canviewprofile`: View your profile\n* `btcpay.user.canmodifyprofile`: Manage your profile\n* `btcpay.user.canmanagenotificationsforuser`: Manage your notifications\n* `btcpay.user.canviewnotificationsforuser`: View your notifications\n\nThe following permissions are available if the user is an administrator:\n\n* `btcpay.server.cancreateuser`: Create new users\n* `btcpay.server.canmodifyserversettings`: Manage your server\n* `btcpay.server.canuseinternallightningnode`: Use the internal lightning node\n* `btcpay.server.cancreatelightninginvoiceinternalnode`: Create invoices with internal lightning node\n\nThe following permissions applies to all stores of the user, you can limit to a specific store with the following format: `btcpay.store.cancreateinvoice:6HSHAEU4iYWtjxtyRs9KyPjM9GAQp8kw2T9VWbGG1FnZ`:\n\n* `btcpay.store.canmodifystoresettings`: Modify your stores\n* `btcpay.store.webhooks.canmodifywebhooks`: Modify stores webhooks\n* `btcpay.store.canviewstoresettings`: View your stores\n* `btcpay.store.cancreateinvoice`: Create an invoice\n* `btcpay.store.canviewinvoices`: View invoices\n* `btcpay.store.canmodifypaymentrequests`: Modify your payment requests\n* `btcpay.store.canviewpaymentrequests`: View your payment requests\n* `btcpay.store.canuselightningnode`: Use the lightning nodes associated with your stores\n* `btcpay.store.cancreatelightninginvoice`: Create invoices the lightning nodes associated with your stores\n\nNote that API Keys only limits permission of a user and can never expand it. If an API Key has the permission `btcpay.server.canmodifyserversettings` but that the user account creating this API Key is not administrator, the API Key will not be able to modify the server settings.\n", "name": "Authorization", "in": "header", "scheme": "token" diff --git a/BTCPayServer/wwwroot/swagger/v1/swagger.template.users.json b/BTCPayServer/wwwroot/swagger/v1/swagger.template.users.json index 57764207d..1ecd2586e 100644 --- a/BTCPayServer/wwwroot/swagger/v1/swagger.template.users.json +++ b/BTCPayServer/wwwroot/swagger/v1/swagger.template.users.json @@ -28,6 +28,27 @@ "Basic": [] } ] + }, + "delete": { + "tags": [ + "Users" + ], + "summary": "Deletes user profile", + "description": "Deletes user profile and associated user data for user making the request", + "operationId": "Users_DeleteCurrentUser", + "responses": { + "200": { + "description": "User and associated data deleted successfully" + } + }, + "security": [ + { + "API Key": [ + "btcpay.user.candeleteuser" + ], + "Basic": [] + } + ] } }, "/api/v1/users": { @@ -107,6 +128,41 @@ } ] } + }, + "/api/v1/users/{userId}": { + "delete": { + "tags": [ + "Users" + ], + "summary": "Delete user", + "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": "userId", + "in": "path", + "required": true, + "description": "The ID of the user to be deleted", + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "User has been successfully deleted" + }, + "401": { + "description": "Missing authorization for deleting the user" + }, + "403": { + "description": "Authorized but forbidden to delete the user. Can happen if you attempt to delete the only admin user." + }, + "404": { + "description": "User with provided ID was not found" + } + }, + "security": [] + } } }, "components": {