From c48f277279e666b968f6de2fcb81e948fc1061ef Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Tue, 29 Dec 2020 20:11:07 -0500 Subject: [PATCH 1/5] Document 410 on POST /User --- src/Tgstation.Server.Host/Controllers/UserController.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Tgstation.Server.Host/Controllers/UserController.cs b/src/Tgstation.Server.Host/Controllers/UserController.cs index f0f3c80888..6acadfae0a 100644 --- a/src/Tgstation.Server.Host/Controllers/UserController.cs +++ b/src/Tgstation.Server.Host/Controllers/UserController.cs @@ -182,10 +182,12 @@ namespace Tgstation.Server.Host.Controllers /// A resulting in the of the operation. /// updated successfully. /// Requested does not exist. + /// Requested does not exist. [HttpPost] [TgsAuthorize(AdministrationRights.WriteUsers | AdministrationRights.EditOwnPassword | AdministrationRights.EditOwnOAuthConnections)] [ProducesResponseType(typeof(Api.Models.User), 200)] [ProducesResponseType(typeof(ErrorMessage), 404)] + [ProducesResponseType(typeof(ErrorMessage), 410)] #pragma warning disable CA1502 // TODO: Decomplexify #pragma warning disable CA1506 public async Task Update([FromBody] UserUpdate model, CancellationToken cancellationToken) From 989b39d7232e5756dfbc135ee902260c4007eb4a Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Tue, 29 Dec 2020 20:14:18 -0500 Subject: [PATCH 2/5] Add user group limit to config --- .../Models/Internal/ServerInformation.cs | 7 ++++++- .../Configuration/GeneralConfiguration.cs | 6 ++++++ src/Tgstation.Server.Host/Controllers/HomeController.cs | 1 + src/Tgstation.Server.Host/appsettings.json | 1 + tests/Tgstation.Server.Tests/RootTest.cs | 1 + tests/Tgstation.Server.Tests/TestingServer.cs | 1 + 6 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Api/Models/Internal/ServerInformation.cs b/src/Tgstation.Server.Api/Models/Internal/ServerInformation.cs index 0c6768e53a..21f4ed26eb 100644 --- a/src/Tgstation.Server.Api/Models/Internal/ServerInformation.cs +++ b/src/Tgstation.Server.Api/Models/Internal/ServerInformation.cs @@ -1,4 +1,4 @@ -using System.Collections.Generic; +using System.Collections.Generic; namespace Tgstation.Server.Api.Models.Internal { @@ -22,6 +22,11 @@ namespace Tgstation.Server.Api.Models.Internal /// public uint UserLimit { get; set; } + /// + /// The maximum number of s allowed. + /// + public uint UserGroupLimit { get; set; } + /// /// Limits the locations instances may be created or attached from. /// diff --git a/src/Tgstation.Server.Host/Configuration/GeneralConfiguration.cs b/src/Tgstation.Server.Host/Configuration/GeneralConfiguration.cs index e58c8d036e..766e4d4cd4 100644 --- a/src/Tgstation.Server.Host/Configuration/GeneralConfiguration.cs +++ b/src/Tgstation.Server.Host/Configuration/GeneralConfiguration.cs @@ -43,6 +43,11 @@ namespace Tgstation.Server.Host.Configuration /// const uint DefaultUserLimit = 100; + /// + /// The default value for . + /// + const uint DefaultUserGroupLimit = 25; + /// /// The default value for /// @@ -102,6 +107,7 @@ namespace Tgstation.Server.Host.Configuration MinimumPasswordLength = DefaultMinimumPasswordLength; InstanceLimit = DefaultInstanceLimit; UserLimit = DefaultUserLimit; + UserGroupLimit = DefaultUserGroupLimit; } /// diff --git a/src/Tgstation.Server.Host/Controllers/HomeController.cs b/src/Tgstation.Server.Host/Controllers/HomeController.cs index 23a539405e..e08e0cde15 100644 --- a/src/Tgstation.Server.Host/Controllers/HomeController.cs +++ b/src/Tgstation.Server.Host/Controllers/HomeController.cs @@ -191,6 +191,7 @@ namespace Tgstation.Server.Host.Controllers MinimumPasswordLength = generalConfiguration.MinimumPasswordLength, InstanceLimit = generalConfiguration.InstanceLimit, UserLimit = generalConfiguration.UserLimit, + UserGroupLimit = generalConfiguration.UserGroupLimit, ValidInstancePaths = generalConfiguration.ValidInstancePaths, WindowsHost = platformIdentifier.IsWindows, SwarmServers = swarmService.GetSwarmServers(), diff --git a/src/Tgstation.Server.Host/appsettings.json b/src/Tgstation.Server.Host/appsettings.json index b0df570838..efc04c7461 100644 --- a/src/Tgstation.Server.Host/appsettings.json +++ b/src/Tgstation.Server.Host/appsettings.json @@ -8,6 +8,7 @@ "ApiPort": 5000, "UseBasicWatchdog": false, "UserLimit": 100, + "UserGroupLimit": 25, "InstanceLimit": 10, "ValidInstancePaths": null, "HostApiDocumentation": false diff --git a/tests/Tgstation.Server.Tests/RootTest.cs b/tests/Tgstation.Server.Tests/RootTest.cs index f3fd6feb68..705269c9cf 100644 --- a/tests/Tgstation.Server.Tests/RootTest.cs +++ b/tests/Tgstation.Server.Tests/RootTest.cs @@ -162,6 +162,7 @@ namespace Tgstation.Server.Tests Assert.AreEqual(10U, serverInfo.MinimumPasswordLength); Assert.AreEqual(11U, serverInfo.InstanceLimit); Assert.AreEqual(150U, serverInfo.UserLimit); + Assert.AreEqual(47U, serverInfo.UserGroupLimit); Assert.AreEqual(RuntimeInformation.IsOSPlatform(OSPlatform.Windows), serverInfo.WindowsHost); //check that modifying the token even slightly fucks up the auth diff --git a/tests/Tgstation.Server.Tests/TestingServer.cs b/tests/Tgstation.Server.Tests/TestingServer.cs index f9c888e341..12f04a0b1b 100644 --- a/tests/Tgstation.Server.Tests/TestingServer.cs +++ b/tests/Tgstation.Server.Tests/TestingServer.cs @@ -80,6 +80,7 @@ namespace Tgstation.Server.Tests String.Format(CultureInfo.InvariantCulture, "General:MinimumPasswordLength={0}", 10), String.Format(CultureInfo.InvariantCulture, "General:InstanceLimit={0}", 11), String.Format(CultureInfo.InvariantCulture, "General:UserLimit={0}", 150), + String.Format(CultureInfo.InvariantCulture, "General:UserGroupLimit={0}", 47), String.Format(CultureInfo.InvariantCulture, "General:HostApiDocumentation={0}", DumpOpenApiSpecpath), String.Format(CultureInfo.InvariantCulture, "FileLogging:Directory={0}", Path.Combine(Directory, "Logs")), String.Format(CultureInfo.InvariantCulture, "FileLogging:LogLevel={0}", "Trace"), From 38a8fde66cc5eb653d547889d1b77e1e3635ed54 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Tue, 29 Dec 2020 20:14:40 -0500 Subject: [PATCH 3/5] API/Client version bumps --- build/Version.props | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build/Version.props b/build/Version.props index 5bdc17ce8b..563a1e6bb5 100644 --- a/build/Version.props +++ b/build/Version.props @@ -5,8 +5,8 @@ 4.7.0 2.2.0 - 8.0.0 - 9.0.0 + 8.1.0 + 9.1.0 5.2.10 1.1.0 1.2.0 From 29485dec97756e7e838c2b31f5528afb4f87499b Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Tue, 29 Dec 2020 20:39:23 -0500 Subject: [PATCH 4/5] Actually enforce user and group limits --- src/Tgstation.Server.Api/Models/ErrorCode.cs | 12 ++++++++++++ .../Controllers/UserController.cs | 8 ++++++++ .../Controllers/UserGroupController.cs | 18 ++++++++++++++++++ 3 files changed, 38 insertions(+) diff --git a/src/Tgstation.Server.Api/Models/ErrorCode.cs b/src/Tgstation.Server.Api/Models/ErrorCode.cs index 2f6821868d..36f14a5ee0 100644 --- a/src/Tgstation.Server.Api/Models/ErrorCode.cs +++ b/src/Tgstation.Server.Api/Models/ErrorCode.cs @@ -608,5 +608,17 @@ namespace Tgstation.Server.Api.Models /// [Description("Cannot delete the user group as it is not empty!")] UserGroupNotEmpty, + + /// + /// Attempted to create an but the configured limit has been reached. + /// + [Description("The user cannot be created because the configured limit has been reached!")] + UserLimitReached, + + /// + /// Attempted to create an but the configured limit has been reached. + /// + [Description("The user group cannot be created because the configured limit has been reached!")] + UserGroupLimitReached, } } diff --git a/src/Tgstation.Server.Host/Controllers/UserController.cs b/src/Tgstation.Server.Host/Controllers/UserController.cs index 6acadfae0a..ffd098b961 100644 --- a/src/Tgstation.Server.Host/Controllers/UserController.cs +++ b/src/Tgstation.Server.Host/Controllers/UserController.cs @@ -139,6 +139,14 @@ namespace Tgstation.Server.Host.Controllers if (fail != null) return fail; + var totalUsers = await DatabaseContext + .Users + .AsQueryable() + .CountAsync(cancellationToken) + .ConfigureAwait(false); + if (totalUsers >= generalConfiguration.UserLimit) + return Conflict(new ErrorMessage(ErrorCode.UserLimitReached)); + var dbUser = await CreateNewUserFromModel(model, cancellationToken).ConfigureAwait(false); if (dbUser == null) return Gone(); diff --git a/src/Tgstation.Server.Host/Controllers/UserGroupController.cs b/src/Tgstation.Server.Host/Controllers/UserGroupController.cs index c1be2818ad..d666e0b748 100644 --- a/src/Tgstation.Server.Host/Controllers/UserGroupController.cs +++ b/src/Tgstation.Server.Host/Controllers/UserGroupController.cs @@ -1,6 +1,7 @@ using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using System; using System.Linq; using System.Threading; @@ -8,6 +9,7 @@ using System.Threading.Tasks; using Tgstation.Server.Api; using Tgstation.Server.Api.Models; using Tgstation.Server.Api.Rights; +using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.Database; using Tgstation.Server.Host.Security; using Z.EntityFramework.Plus; @@ -20,15 +22,22 @@ namespace Tgstation.Server.Host.Controllers [Route(Routes.UserGroup)] public class UserGroupController : ApiController { + /// + /// The for the . + /// + readonly GeneralConfiguration generalConfiguration; + /// /// Initializes a new instance of the . /// /// The for the /// The for the + /// The containing the value of . /// The for the . public UserGroupController( IDatabaseContext databaseContext, IAuthenticationContextFactory authenticationContextFactory, + IOptions generalConfigurationOptions, ILogger logger) : base( databaseContext, @@ -36,6 +45,7 @@ namespace Tgstation.Server.Host.Controllers logger, true) { + generalConfiguration = generalConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(generalConfigurationOptions)); } /// @@ -56,6 +66,14 @@ namespace Tgstation.Server.Host.Controllers if (model.Name == null) return BadRequest(new ErrorMessage(ErrorCode.ModelValidationFailure)); + var totalGroups = await DatabaseContext + .Groups + .AsQueryable() + .CountAsync(cancellationToken) + .ConfigureAwait(false); + if (totalGroups >= generalConfiguration.UserGroupLimit) + return Conflict(new ErrorMessage(ErrorCode.UserGroupLimitReached)); + var permissionSet = new Models.PermissionSet { AdministrationRights = model.PermissionSet?.AdministrationRights ?? AdministrationRights.None, From 256e8f242fb7ba6c28d8c340561eacc2a5c958b3 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Tue, 29 Dec 2020 21:03:16 -0500 Subject: [PATCH 5/5] Fix the build --- src/Tgstation.Server.Host/Controllers/UserController.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Tgstation.Server.Host/Controllers/UserController.cs b/src/Tgstation.Server.Host/Controllers/UserController.cs index ffd098b961..08f83e35cf 100644 --- a/src/Tgstation.Server.Host/Controllers/UserController.cs +++ b/src/Tgstation.Server.Host/Controllers/UserController.cs @@ -113,6 +113,7 @@ namespace Tgstation.Server.Host.Controllers [HttpPut] [TgsAuthorize(AdministrationRights.WriteUsers)] [ProducesResponseType(typeof(Api.Models.User), 201)] +#pragma warning disable CA1502, CA1506 public async Task Create([FromBody] UserUpdate model, CancellationToken cancellationToken) { if (model == null) @@ -181,6 +182,7 @@ namespace Tgstation.Server.Host.Controllers return Created(dbUser.ToApi(true)); } +#pragma warning restore CA1502, CA1506 /// /// Update a .