From f64e0c8d533c9db5996316bde7153036c8a44b63 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 8 May 2020 05:45:07 -0400 Subject: [PATCH] User fixes [TGSDeploy] --- .../Controllers/InstanceController.cs | 16 +++++++++++-- .../Controllers/UserController.cs | 24 +++++++------------ .../Models/InstanceUser.cs | 10 -------- .../Setup/SetupWizard.cs | 14 +++++------ .../Tgstation.Server.Tests/IntegrationTest.cs | 4 ++-- tests/Tgstation.Server.Tests/TestingServer.cs | 13 ++++++++-- tests/Tgstation.Server.Tests/UsersTest.cs | 17 ++++++++++++- 7 files changed, 57 insertions(+), 41 deletions(-) diff --git a/src/Tgstation.Server.Host/Controllers/InstanceController.cs b/src/Tgstation.Server.Host/Controllers/InstanceController.cs index 15f6eec39f..44934bb3c9 100644 --- a/src/Tgstation.Server.Host/Controllers/InstanceController.cs +++ b/src/Tgstation.Server.Host/Controllers/InstanceController.cs @@ -534,7 +534,13 @@ namespace Tgstation.Server.Host.Controllers if (!AuthenticationContext.User.InstanceManagerRights.Value.HasFlag(InstanceManagerRights.List)) query = query .Where(x => x.InstanceUsers.Any(y => y.UserId == AuthenticationContext.User.Id)) - .Where(x => x.InstanceUsers.Any(y => y.AnyRights)); + .Where(x => x.InstanceUsers.Any(instanceUser => + instanceUser.ByondRights != ByondRights.None || + instanceUser.ChatBotRights != ChatBotRights.None || + instanceUser.ConfigurationRights != ConfigurationRights.None || + instanceUser.DreamDaemonRights != DreamDaemonRights.None || + instanceUser.DreamMakerRights != DreamMakerRights.None || + instanceUser.InstanceUserRights != InstanceUserRights.None)); // Hack for EF IAsyncEnumerable BS return query.Select(x => x); @@ -589,7 +595,13 @@ namespace Tgstation.Server.Host.Controllers if (instance == null) return StatusCode((int)HttpStatusCode.Gone); - if (cantList && !instance.InstanceUsers.Any(x => x.UserId == AuthenticationContext.User.Id && x.AnyRights)) + if (cantList && !instance.InstanceUsers.Any(instanceUser => instanceUser.UserId == AuthenticationContext.User.Id && + (instanceUser.ByondRights != ByondRights.None || + instanceUser.ChatBotRights != ChatBotRights.None || + instanceUser.ConfigurationRights != ConfigurationRights.None || + instanceUser.DreamDaemonRights != DreamDaemonRights.None || + instanceUser.DreamMakerRights != DreamMakerRights.None || + instanceUser.InstanceUserRights != InstanceUserRights.None))) return Forbid(); var api = instance.ToApi(); diff --git a/src/Tgstation.Server.Host/Controllers/UserController.cs b/src/Tgstation.Server.Host/Controllers/UserController.cs index 4f33a5a388..7cd0e25b7f 100644 --- a/src/Tgstation.Server.Host/Controllers/UserController.cs +++ b/src/Tgstation.Server.Host/Controllers/UserController.cs @@ -34,11 +34,6 @@ namespace Tgstation.Server.Host.Controllers /// readonly ICryptographySuite cryptographySuite; - /// - /// The for the - /// - readonly ILogger logger; - /// /// The for the /// @@ -51,11 +46,10 @@ namespace Tgstation.Server.Host.Controllers /// The for the /// The value of /// The value of - /// The value of + /// The for the . /// The containing the value of public UserController(IDatabaseContext databaseContext, IAuthenticationContextFactory authenticationContextFactory, ISystemIdentityFactory systemIdentityFactory, ICryptographySuite cryptographySuite, ILogger logger, IOptions generalConfigurationOptions) : base(databaseContext, authenticationContextFactory, logger, false, true) { - this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); this.systemIdentityFactory = systemIdentityFactory ?? throw new ArgumentNullException(nameof(systemIdentityFactory)); this.cryptographySuite = cryptographySuite ?? throw new ArgumentNullException(nameof(cryptographySuite)); generalConfiguration = generalConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(generalConfigurationOptions)); @@ -69,7 +63,7 @@ namespace Tgstation.Server.Host.Controllers /// if is valid, a otherwise. BadRequestObjectResult CheckValidName(UserUpdate model, bool newUser) { - var userInvalidWithNullName = newUser && model.Name == null; + var userInvalidWithNullName = newUser && model.Name == null && model.SystemIdentifier == null; if (userInvalidWithNullName || (model.Name != null && String.IsNullOrWhiteSpace(model.Name))) return BadRequest(new ErrorMessage(ErrorCode.UserMissingName)); @@ -88,7 +82,7 @@ namespace Tgstation.Server.Host.Controllers /// on success, if is too short. BadRequestObjectResult TrySetPassword(Models.User dbUser, string newPassword, bool newUser) { - newPassword = newPassword ?? String.Empty; + newPassword ??= String.Empty; if (newPassword.Length < generalConfiguration.MinimumPasswordLength) return BadRequest(new ErrorMessage(ErrorCode.UserPasswordLength) { @@ -145,13 +139,11 @@ namespace Tgstation.Server.Host.Controllers if (model.SystemIdentifier != null) try { - using (var sysIdentity = await systemIdentityFactory.CreateSystemIdentity(dbUser, cancellationToken).ConfigureAwait(false)) - { - if (sysIdentity == null) - return StatusCode((int)HttpStatusCode.Gone); - dbUser.Name = sysIdentity.Username; - dbUser.SystemIdentifier = sysIdentity.Uid; - } + using var sysIdentity = await systemIdentityFactory.CreateSystemIdentity(dbUser, cancellationToken).ConfigureAwait(false); + if (sysIdentity == null) + return StatusCode((int)HttpStatusCode.Gone); + dbUser.Name = sysIdentity.Username; + dbUser.SystemIdentifier = sysIdentity.Uid; } catch (NotImplementedException) { diff --git a/src/Tgstation.Server.Host/Models/InstanceUser.cs b/src/Tgstation.Server.Host/Models/InstanceUser.cs index 35508c4417..7b9b8491d1 100644 --- a/src/Tgstation.Server.Host/Models/InstanceUser.cs +++ b/src/Tgstation.Server.Host/Models/InstanceUser.cs @@ -21,16 +21,6 @@ namespace Tgstation.Server.Host.Models [Required] public Instance Instance { get; set; } - /// - /// If the has any instance rights - /// - public bool AnyRights => ByondRights != Api.Rights.ByondRights.None || - ChatBotRights != Api.Rights.ChatBotRights.None || - ConfigurationRights != Api.Rights.ConfigurationRights.None || - DreamDaemonRights != Api.Rights.DreamDaemonRights.None || - DreamMakerRights != Api.Rights.DreamMakerRights.None || - InstanceUserRights != Api.Rights.InstanceUserRights.None; - /// /// Convert the to it's API form /// diff --git a/src/Tgstation.Server.Host/Setup/SetupWizard.cs b/src/Tgstation.Server.Host/Setup/SetupWizard.cs index 369287ff9f..35b3bdca0e 100644 --- a/src/Tgstation.Server.Host/Setup/SetupWizard.cs +++ b/src/Tgstation.Server.Host/Setup/SetupWizard.cs @@ -178,14 +178,12 @@ namespace Tgstation.Server.Host.Setup || databaseConfiguration.DatabaseType == DatabaseType.MySql) { await console.WriteAsync("Checking MySQL/MariaDB version...", true, cancellationToken).ConfigureAwait(false); - using (var command = testConnection.CreateCommand()) - { - command.CommandText = "SELECT VERSION()"; - var fullVersion = (string)await command.ExecuteScalarAsync(cancellationToken).ConfigureAwait(false); - await console.WriteAsync(String.Format(CultureInfo.InvariantCulture, "Found {0}", fullVersion), true, cancellationToken).ConfigureAwait(false); - var splits = fullVersion.Split('-'); - databaseConfiguration.MySqlServerVersion = splits.First(); - } + using var command = testConnection.CreateCommand(); + command.CommandText = "SELECT VERSION()"; + var fullVersion = (string)await command.ExecuteScalarAsync(cancellationToken).ConfigureAwait(false); + await console.WriteAsync(String.Format(CultureInfo.InvariantCulture, "Found {0}", fullVersion), true, cancellationToken).ConfigureAwait(false); + var splits = fullVersion.Split('-'); + databaseConfiguration.MySqlServerVersion = splits.First(); } if (!isSqliteDB && !dbExists) diff --git a/tests/Tgstation.Server.Tests/IntegrationTest.cs b/tests/Tgstation.Server.Tests/IntegrationTest.cs index fe31c266d5..18f390af9c 100644 --- a/tests/Tgstation.Server.Tests/IntegrationTest.cs +++ b/tests/Tgstation.Server.Tests/IntegrationTest.cs @@ -87,7 +87,7 @@ namespace Tgstation.Server.Tests try { var updatePath = Path.Combine(updatePathRoot, Guid.NewGuid().ToString()); - var server = new TestingServer(clientFactory, updatePath); + using var server = new TestingServer(clientFactory, updatePath); if (server.DatabaseType == "Sqlite") Assert.Inconclusive("Cannot run this test on SQLite yet!"); @@ -165,7 +165,7 @@ namespace Tgstation.Server.Tests public async Task TestFullStandardOperation() { RequireDiscordToken(); - var server = new TestingServer(clientFactory, null); + using var server = new TestingServer(clientFactory, null); using var serverCts = new CancellationTokenSource(); var cancellationToken = serverCts.Token; var serverTask = server.Run(cancellationToken); diff --git a/tests/Tgstation.Server.Tests/TestingServer.cs b/tests/Tgstation.Server.Tests/TestingServer.cs index efee99422b..18d38d4586 100644 --- a/tests/Tgstation.Server.Tests/TestingServer.cs +++ b/tests/Tgstation.Server.Tests/TestingServer.cs @@ -15,7 +15,7 @@ using Tgstation.Server.Host.Core; namespace Tgstation.Server.Tests { - sealed class TestingServer : IServer + sealed class TestingServer : IServer, IDisposable { public Uri Url { get; } @@ -89,7 +89,16 @@ namespace Tgstation.Server.Tests public void Dispose() { - System.IO.Directory.Delete(Directory, true); + for (int i = 0; i < 5; ++i) + try + { + System.IO.Directory.Delete(Directory, true); + } + catch + { + GC.Collect(Int32.MaxValue, GCCollectionMode.Forced, false); + Thread.Sleep(3000); + } } public async Task Run(CancellationToken cancellationToken) diff --git a/tests/Tgstation.Server.Tests/UsersTest.cs b/tests/Tgstation.Server.Tests/UsersTest.cs index 5142d6fe24..a0e49879d9 100644 --- a/tests/Tgstation.Server.Tests/UsersTest.cs +++ b/tests/Tgstation.Server.Tests/UsersTest.cs @@ -6,6 +6,7 @@ using System.Threading; using System.Threading.Tasks; using Tgstation.Server.Api.Models; using Tgstation.Server.Client; +using Tgstation.Server.Host.System; namespace Tgstation.Server.Tests { @@ -21,9 +22,10 @@ namespace Tgstation.Server.Tests public async Task Run(CancellationToken cancellationToken) { await TestRetrieveCurrentUser(cancellationToken).ConfigureAwait(false); + await TestCreateSysUser(cancellationToken); await TestSpamCreation(cancellationToken).ConfigureAwait(false); } - + async Task TestRetrieveCurrentUser(CancellationToken cancellationToken) { var user = await this.client.Read(cancellationToken).ConfigureAwait(false); @@ -33,6 +35,19 @@ namespace Tgstation.Server.Tests Assert.AreEqual(true, user.Enabled); } + async Task TestCreateSysUser(CancellationToken cancellationToken) + { + var sysId = Environment.UserName; + var update = new UserUpdate + { + SystemIdentifier = sysId + }; + if (new PlatformIdentifier().IsWindows) + await client.Create(update, cancellationToken); + else + await ApiAssert.ThrowsException(() => client.Create(update, cancellationToken), ErrorCode.RequiresPosixSystemIdentity); + } + async Task TestSpamCreation(CancellationToken cancellationToken) { ICollection> tasks = new List>();