Can create user without authentication if there is no admin

BTCPayServer.Client/BTCPayServerClient.cs

@@ -23,6 +23,13 @@ namespace BTCPayServer.Client
             PropertyNamingPolicy = JsonNamingPolicy.CamelCase
         };
 
+        public BTCPayServerClient(Uri btcpayHost, HttpClient httpClient = null)
+        {
+            if (btcpayHost == null)
+                throw new ArgumentNullException(nameof(btcpayHost));
+            _btcpayHost = btcpayHost;
+            _httpClient = httpClient ?? new HttpClient();
+        }
         public BTCPayServerClient(Uri btcpayHost, string APIKey, HttpClient httpClient = null)
         {
             _apiKey = APIKey;
@@ -52,7 +59,8 @@ namespace BTCPayServer.Client
             }
 
             var httpRequest = new HttpRequestMessage(method ?? HttpMethod.Get, uriBuilder.Uri);
-            httpRequest.Headers.Authorization = new AuthenticationHeaderValue("token", _apiKey);
+            if (_apiKey != null)
+                httpRequest.Headers.Authorization = new AuthenticationHeaderValue("token", _apiKey);
 
 
             return httpRequest;

BTCPayServer.Client/Models/ApplicationUserData.cs

@@ -34,9 +34,5 @@ namespace BTCPayServer.Client.Models
         /// Whether this user is an administrator. If left null and there are no admins in the system, the user will be created as an admin.
         /// </summary>
         public bool? IsAdministrator { get; set; }
-        /// <summary>
-        /// If the server requires email confirmation, this allows you to set the account as confirmed from the start
-        /// </summary>
-        public bool? EmailConfirmed { get; set; }
     }
 }

BTCPayServer.Tests/GreenfieldAPITests.cs

@@ -6,6 +6,7 @@ using BTCPayServer.Client;
 using BTCPayServer.Client.Models;
 using BTCPayServer.Controllers;
 using BTCPayServer.Controllers.RestApi.Users;
+using BTCPayServer.Services;
 using BTCPayServer.Tests.Logging;
 using Microsoft.AspNet.SignalR.Client;
 using Microsoft.AspNetCore.Mvc;
@@ -23,7 +24,7 @@ namespace BTCPayServer.Tests
 
         public GreenfieldAPITests(ITestOutputHelper helper)
         {
-            Logs.Tester = new XUnitLog(helper) {Name = "Tests"};
+            Logs.Tester = new XUnitLog(helper) { Name = "Tests" };
             Logs.LogProvider = new XUnitLogProvider(helper);
         }
 
@@ -54,6 +55,80 @@ namespace BTCPayServer.Tests
             }
         }
 
+        [Fact(Timeout = TestTimeout)]
+        [Trait("Integration", "Integration")]
+        public async Task CanCreateUsersViaAPI()
+        {
+            using (var tester = ServerTester.Create(newDb: true))
+            {
+                await tester.StartAsync();
+                var unauthClient = new BTCPayServerClient(tester.PayTester.ServerUri);
+                await AssertHttpError(400, async () => await unauthClient.CreateUser(new CreateApplicationUserRequest()));
+                await AssertHttpError(400, async () => await unauthClient.CreateUser(new CreateApplicationUserRequest() { Email = "test@gmail.com" }));
+                // Pass too simple
+                await AssertHttpError(400, async () => await unauthClient.CreateUser(new CreateApplicationUserRequest() { Email = "test3@gmail.com", Password = "a" }));
+
+                // We have no admin, so it should work
+                var user1 = await unauthClient.CreateUser(new CreateApplicationUserRequest() { Email = "test@gmail.com", Password = "abceudhqw" });
+                // We have no admin, so it should work
+                var user2 = await unauthClient.CreateUser(new CreateApplicationUserRequest() { Email = "test2@gmail.com", Password = "abceudhqw" });
+
+                // Duplicate email
+                await AssertHttpError(400, async () => await unauthClient.CreateUser(new CreateApplicationUserRequest() { Email = "test2@gmail.com", Password = "abceudhqw" }));
+
+                // Let's make an admin
+                var admin = await unauthClient.CreateUser(new CreateApplicationUserRequest() { Email = "admin@gmail.com", Password = "abceudhqw", IsAdministrator = true });
+
+                // Creating a new user without proper creds is now impossible (unauthorized) 
+                // Because if registration are locked and that an admin exists, we don't accept unauthenticated connection
+                await AssertHttpError(401, async () => await unauthClient.CreateUser(new CreateApplicationUserRequest() { Email = "test3@gmail.com", Password = "afewfoiewiou" }));
+
+
+                // But should be ok with subscriptions unlocked
+                var settings = tester.PayTester.GetService<SettingsRepository>();
+                await settings.UpdateSetting<PoliciesSettings>(new PoliciesSettings() { LockSubscription = false });
+                await unauthClient.CreateUser(new CreateApplicationUserRequest() { Email = "test3@gmail.com", Password = "afewfoiewiou" });
+
+                // But it should be forbidden to create an admin without being authenticated
+                await AssertHttpError(403, async () => await unauthClient.CreateUser(new CreateApplicationUserRequest() { Email = "admin2@gmail.com", Password = "afewfoiewiou", IsAdministrator = true }));
+                await settings.UpdateSetting<PoliciesSettings>(new PoliciesSettings() { LockSubscription = true });
+
+                var adminAcc = tester.NewAccount();
+                adminAcc.UserId = admin.Id;
+                adminAcc.IsAdmin = true;
+                var adminClient = await adminAcc.CreateClient(Permissions.ProfileManagement);
+
+                // We should be forbidden to create a new user without proper admin permissions
+                await AssertHttpError(403, async () => await adminClient.CreateUser(new CreateApplicationUserRequest() { Email = "test4@gmail.com", Password = "afewfoiewiou" }));
+                await AssertHttpError(403, async () => await adminClient.CreateUser(new CreateApplicationUserRequest() { Email = "test4@gmail.com", Password = "afewfoiewiou", IsAdministrator = true }));
+
+                // However, should be ok with the server management permissions
+                adminClient = await adminAcc.CreateClient(Permissions.ServerManagement);
+                await adminClient.CreateUser(new CreateApplicationUserRequest() { Email = "test4@gmail.com", Password = "afewfoiewiou" });
+                // Even creating new admin should be ok
+                await adminClient.CreateUser(new CreateApplicationUserRequest() { Email = "admin4@gmail.com", Password = "afewfoiewiou", IsAdministrator = true });
+
+                var user1Acc = tester.NewAccount();
+                user1Acc.UserId = user1.Id;
+                user1Acc.IsAdmin = false;
+                var user1Client = await user1Acc.CreateClient(Permissions.ServerManagement);
+                // User1 trying to get server management would still fail to create user
+                await AssertHttpError(403, async () => await user1Client.CreateUser(new CreateApplicationUserRequest() { Email = "test8@gmail.com", Password = "afewfoiewiou" }));
+
+                // User1 should be able to create user if subscription unlocked
+                await settings.UpdateSetting<PoliciesSettings>(new PoliciesSettings() { LockSubscription = false });
+                await user1Client.CreateUser(new CreateApplicationUserRequest() { Email = "test8@gmail.com", Password = "afewfoiewiou" });
+                // But not an admin
+                await AssertHttpError(403, async () => await user1Client.CreateUser(new CreateApplicationUserRequest() { Email = "admin8@gmail.com", Password = "afewfoiewiou", IsAdministrator = true }));
+            }
+        }
+
+        private async Task AssertHttpError(int code, Func<Task> act)
+        {
+            var ex = await Assert.ThrowsAsync<HttpRequestException>(act);
+            Assert.Contains(code.ToString(), ex.Message);
+        }
+
         [Fact(Timeout = TestTimeout)]
         [Trait("Integration", "Integration")]
         public async Task UsersControllerTests()
@@ -82,11 +157,12 @@ namespace BTCPayServer.Tests
                 {
                     Email = $"{Guid.NewGuid()}@g.com",
                     Password = Guid.NewGuid().ToString()
-                }) );
+                }));
 
                 var newUser = await clientServer.CreateUser(new CreateApplicationUserRequest()
                 {
-                    Email = $"{Guid.NewGuid()}@g.com", Password = Guid.NewGuid().ToString()
+                    Email = $"{Guid.NewGuid()}@g.com",
+                    Password = Guid.NewGuid().ToString()
                 });
                 Assert.NotNull(newUser);
 
@@ -94,17 +170,17 @@ namespace BTCPayServer.Tests
                 {
                     Email = $"{Guid.NewGuid()}",
                     Password = Guid.NewGuid().ToString()
-                }) );
+                }));
 
                 await Assert.ThrowsAsync<HttpRequestException>(async () => await clientServer.CreateUser(new CreateApplicationUserRequest()
                 {
                     Email = $"{Guid.NewGuid()}@g.com",
-                }) );
+                }));
 
                 await Assert.ThrowsAsync<HttpRequestException>(async () => await clientServer.CreateUser(new CreateApplicationUserRequest()
                 {
                     Password = Guid.NewGuid().ToString()
-                }) );
+                }));
 
             }
         }

BTCPayServer.Tests/ServerTester.cs

@@ -29,13 +29,13 @@ namespace BTCPayServer.Tests
 {
     public class ServerTester : IDisposable
     {
-        public static ServerTester Create([CallerMemberNameAttribute]string scope = null)
+        public static ServerTester Create([CallerMemberNameAttribute]string scope = null, bool newDb = false)
         {
-            return new ServerTester(scope);
+            return new ServerTester(scope, newDb);
         }
 
         string _Directory;
-        public ServerTester(string scope)
+        public ServerTester(string scope, bool newDb)
         {
             _Directory = scope;
             if (Directory.Exists(_Directory))
@@ -59,6 +59,12 @@ namespace BTCPayServer.Tests
                 Postgres = GetEnvironment("TESTS_POSTGRES", "User ID=postgres;Host=127.0.0.1;Port=39372;Database=btcpayserver"),
                 MySQL = GetEnvironment("TESTS_MYSQL", "User ID=root;Host=127.0.0.1;Port=33036;Database=btcpayserver")
             };
+            if (newDb)
+            {
+                var r = RandomUtils.GetUInt32();
+                PayTester.Postgres = PayTester.Postgres.Replace("btcpayserver", $"btcpayserver{r}");
+                PayTester.MySQL = PayTester.MySQL.Replace("btcpayserver", $"btcpayserver{r}");
+            }
             PayTester.Port = int.Parse(GetEnvironment("TESTS_PORT", Utils.FreeTcpPort().ToString(CultureInfo.InvariantCulture)), CultureInfo.InvariantCulture);
             PayTester.HostName = GetEnvironment("TESTS_HOSTNAME", "127.0.0.1");
             PayTester.InContainer = bool.Parse(GetEnvironment("TESTS_INCONTAINER", "false"));

BTCPayServer/Controllers/AccountController.cs

@@ -404,6 +404,7 @@ namespace BTCPayServer.Controllers
 
         [HttpGet]
         [AllowAnonymous]
+        [RateLimitsFilter(ZoneLimits.Register, Scope = RateLimitsScope.RemoteAddress)]
         public async Task<IActionResult> Register(string returnUrl = null, bool logon = true, bool useBasicLayout = false)
         {
             if (!CanLoginOrRegister())

BTCPayServer/Controllers/RestApi/Users/UsersController.cs

@@ -1,18 +1,21 @@
 using System;
 using System.ComponentModel.DataAnnotations;
 using System.Linq;
+using System.Threading;
 using System.Threading.Tasks;
 using BTCPayServer.Client.Models;
 using BTCPayServer.Configuration;
 using BTCPayServer.Data;
 using BTCPayServer.Events;
 using BTCPayServer.Security;
+using BTCPayServer.Security.APIKeys;
 using BTCPayServer.Services;
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Identity;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.Mvc.ModelBinding;
+using NicolasDorier.RateLimits;
 
 namespace BTCPayServer.Controllers.RestApi.Users
 {
@@ -26,11 +29,15 @@ namespace BTCPayServer.Controllers.RestApi.Users
         private readonly SettingsRepository _settingsRepository;
         private readonly EventAggregator _eventAggregator;
         private readonly IPasswordValidator<ApplicationUser> _passwordValidator;
+        private readonly RateLimitService _throttleService;
+        private readonly IAuthorizationService _authorizationService;
 
         public UsersController(UserManager<ApplicationUser> userManager, BTCPayServerOptions btcPayServerOptions,
             RoleManager<IdentityRole> roleManager, SettingsRepository settingsRepository,
             EventAggregator eventAggregator,
-            IPasswordValidator<ApplicationUser> passwordValidator)
+            IPasswordValidator<ApplicationUser> passwordValidator,
+            NicolasDorier.RateLimits.RateLimitService throttleService,
+            IAuthorizationService authorizationService)
         {
             _userManager = userManager;
             _btcPayServerOptions = btcPayServerOptions;
@@ -38,6 +45,8 @@ namespace BTCPayServer.Controllers.RestApi.Users
             _settingsRepository = settingsRepository;
             _eventAggregator = eventAggregator;
             _passwordValidator = passwordValidator;
+            _throttleService = throttleService;
+            _authorizationService = authorizationService;
         }
 
         [Authorize(Policy = Policies.CanModifyProfile.Key, AuthenticationSchemes = AuthenticationSchemes.ApiKey)]
@@ -48,9 +57,9 @@ namespace BTCPayServer.Controllers.RestApi.Users
             return FromModel(user);
         }
 
-        [Authorize(Policy = Policies.CanCreateUser.Key, AuthenticationSchemes = AuthenticationSchemes.ApiKey)]
+        [AllowAnonymous]
         [HttpPost("~/api/v1/users")]
-        public async Task<ActionResult<ApplicationUserData>> CreateUser(CreateApplicationUserRequest request)
+        public async Task<ActionResult<ApplicationUserData>> CreateUser(CreateApplicationUserRequest request, CancellationToken cancellationToken = default)
         {
             if (request?.Email is null)
                 return BadRequest(CreateValidationProblem(nameof(request.Email), "Email is missing"));
@@ -60,15 +69,39 @@ namespace BTCPayServer.Controllers.RestApi.Users
             }
             if (request?.Password is null)
                 return BadRequest(CreateValidationProblem(nameof(request.Password), "Password is missing"));
-            var policies = await _settingsRepository.GetSettingAsync<PoliciesSettings>() ?? new PoliciesSettings();
             var anyAdmin = (await _userManager.GetUsersInRoleAsync(Roles.ServerAdmin)).Any();
-            var admin = request.IsAdministrator.GetValueOrDefault(!anyAdmin);
+            var policies = await _settingsRepository.GetSettingAsync<PoliciesSettings>() ?? new PoliciesSettings();
+            var isAuth = User.Identity.AuthenticationType == APIKeyConstants.AuthenticationType;
+
+            // If registration are locked and that an admin exists, don't accept unauthenticated connection
+            if (anyAdmin && policies.LockSubscription && !isAuth)
+                return Unauthorized();
+
+            // Even if subscription are unlocked, it is forbidden to create admin unauthenticated
+            if (anyAdmin && request.IsAdministrator is true && !isAuth)
+                return Forbid(AuthenticationSchemes.ApiKey);
+            // You are de-facto admin if there is no other admin, else you need to be auth and pass policy requirements
+            bool isAdmin = anyAdmin ? (await _authorizationService.AuthorizeAsync(User, null, new PolicyRequirement(Policies.CanModifyServerSettings.Key))).Succeeded 
+                                     && isAuth
+                                    : true;
+            // You need to be admin to create an admin
+            if (request.IsAdministrator is true && !isAdmin)
+            {
+                return Forbid(AuthenticationSchemes.ApiKey);
+            }
+
+            if (!isAdmin && policies.LockSubscription)
+            {
+                // If we are not admin and subscriptions are locked, we need to check the Policies.CanCreateUser.Key permission
+                if (!isAuth || !(await _authorizationService.AuthorizeAsync(User, null, new PolicyRequirement(Policies.CanCreateUser.Key))).Succeeded)
+                    return Forbid(AuthenticationSchemes.ApiKey);
+            }
+
             var user = new ApplicationUser
             {
                 UserName = request.Email,
                 Email = request.Email,
-                RequiresEmailConfirmation = policies.RequiresConfirmedEmail,
-                EmailConfirmed = request.EmailConfirmed.GetValueOrDefault(false)
+                RequiresEmailConfirmation = policies.RequiresConfirmedEmail
             };
             var passwordValidation = await this._passwordValidator.ValidateAsync(_userManager, user, request.Password);
             if (!passwordValidation.Succeeded)
@@ -79,6 +112,11 @@ namespace BTCPayServer.Controllers.RestApi.Users
                 }
                 return BadRequest(new ValidationProblemDetails(ModelState));
             }
+            if (!isAdmin)
+            {
+                if (!await _throttleService.Throttle(ZoneLimits.Register, this.HttpContext.Connection.RemoteIpAddress, cancellationToken))
+                    return new TooManyRequestsResult(ZoneLimits.Register);
+            }
             var identityResult = await _userManager.CreateAsync(user, request.Password);
             if (!identityResult.Succeeded)
             {
@@ -88,13 +126,23 @@ namespace BTCPayServer.Controllers.RestApi.Users
                 }
                 return BadRequest(new ValidationProblemDetails(ModelState));
             }
-            else if (admin)
+
+            if (request.IsAdministrator is true)
             {
-                await _roleManager.CreateAsync(new IdentityRole(Roles.ServerAdmin));
+                if (!anyAdmin)
+                {
+                    await _roleManager.CreateAsync(new IdentityRole(Roles.ServerAdmin));
+                }
                 await _userManager.AddToRoleAsync(user, Roles.ServerAdmin);
+                if (!anyAdmin)
+                {
+                    // automatically lock subscriptions now that we have our first admin
+                    policies.LockSubscription = true;
+                    await _settingsRepository.UpdateSetting(policies);
+                }
             }
-            _eventAggregator.Publish(new UserRegisteredEvent() {Request = Request, User = user, Admin = admin});
-            return CreatedAtAction("", user);
+            _eventAggregator.Publish(new UserRegisteredEvent() {Request = Request, User = user, Admin = request.IsAdministrator is true });
+            return CreatedAtAction(string.Empty, user);
         }
 
         private ValidationProblemDetails CreateValidationProblem(string propertyName, string errorMessage)

BTCPayServer/Hosting/BTCPayServerServices.cs

@@ -256,10 +256,12 @@ namespace BTCPayServer.Hosting
                 if (btcPayEnv.IsDevelopping)
                 {
                     rateLimits.SetZone($"zone={ZoneLimits.Login} rate=1000r/min burst=100 nodelay");
+                    rateLimits.SetZone($"zone={ZoneLimits.Register} rate=1000r/min burst=100 nodelay");
                 }
                 else
                 {
                     rateLimits.SetZone($"zone={ZoneLimits.Login} rate=5r/min burst=3 nodelay");
+                    rateLimits.SetZone($"zone={ZoneLimits.Register} rate=2r/min burst=2 nodelay");
                 }
                 return rateLimits;
             });

BTCPayServer/ZoneLimits.cs

@@ -8,5 +8,6 @@ namespace BTCPayServer
     public class ZoneLimits
     {
         public const string Login = "btcpaylogin";
+        public const string Register = "btcpayregister";
     }
 }

BTCPayServer/wwwroot/swagger/v1/swagger.template.json

@@ -24,7 +24,7 @@
                 "parameters": [
                     {
                         "name": "permissions",
-                        "description": "The permissions to request. Current permissions available: ServerManagement, StoreManagement",
+                        "description": "The permissions to request. Current permissions available: ServerManagement, StoreManagement, ProfileManagement",
                         "in": "query",
                         "style": "form",
                         "explode": true,
@@ -111,7 +111,7 @@
                 "security": [
                     {
                         "APIKey": [
-                            "btcpay.store.canmodifyprofile"
+                            "ProfileManagement"
                         ]
                     }
                 ]
@@ -123,8 +123,7 @@
                     "Users"
                 ],
                 "summary": "Create user",
-                "description": "Create a new user",
-                "operationId": "Users_CreateUser",
+                "description": "Create a new user. This operation can be called without authentication if there is not any administrator yet on the server.",
                 "requestBody": {
                     "x-name": "request",
                     "content": {
@@ -135,19 +134,15 @@
                                 "properties": {
                                     "email": {
                                         "type": "string",
-                                        "nullable": true
+                                        "nullable": false
                                     },
                                     "password": {
-                                        "type": "string",
-                                        "nullable": true
+                                        "type": "string"
                                     },
                                     "isAdministrator": {
                                         "type": "boolean",
-                                        "nullable": true
-                                    },
-                                    "emailConfirmed": {
-                                        "type": "boolean",
-                                        "nullable": true
+                                        "nullable": true,
+                                        "default": false
                                     }
                                 }
                             }
@@ -176,13 +171,23 @@
                                 }
                             }
                         }
+                    },
+                    "401": {
+                        "description": "If you need to authenticate for this endpoint (ie. the server settings policies lock subscriptions and that an admin already exists)"
+                    },
+                    "403": {
+                        "description": "If you are authenticated but forbidden to create a new user (ie. you don't have the ServerManagement permission)"
+                    },
+                    "429": {
+                        "description": "DDoS protection if you are creating more than 2 accounts every minutes (non-admin only)"
                     }
                 },
                 "security": [
                     {
                         "APIKey": [
-                            "btcpay.store.cancreateuser"
-                        ]
+                            "ServerManagement"
+                        ],
+                        "Anonymous": [ ]
                     }
                 ]
             }