Merge pull request #1749 from tgstation/FixDiscordNoAccess [NugetDeploy][TGSDeploy]

v6.0.1 Patches

README.md

@@ -191,7 +191,7 @@ On Linux, as long as OpenDream and TGS do not use the same .NET major version, y
 
 1. Install `tgstation-server` using any of the above methods.
 1. [Download the Linux SDK binaries](https://dotnet.microsoft.com/en-us/download/dotnet/7.0) for your selected architecture.
-1. Extract everything EXCEPT the `dotnet` executable, `LICENSE.txt``, and `ThirdPartyNotices.txt` in the `.tar.gz` on top of the existing installation directory `/usr/share/dotnet/`
+1. Extract everything EXCEPT the `dotnet` executable, `LICENSE.txt`, and `ThirdPartyNotices.txt` in the `.tar.gz` on top of the existing installation directory `/usr/share/dotnet/`
 1. Run `sudo chown -R root /usr/share/dotnet`
 
 You should now be able to run the `dotnet --list-sdks` command and see an entry for `7.0.XXX [/usr/share/dotnet/sdk]`.

build/Version.props

@@ -3,12 +3,12 @@
   <!-- Integration tests will ensure they match across the board -->
   <Import Project="WebpanelVersion.props" />
   <PropertyGroup>
-    <TgsCoreVersion>6.0.0</TgsCoreVersion>
+    <TgsCoreVersion>6.0.1</TgsCoreVersion>
     <TgsConfigVersion>5.0.0</TgsConfigVersion>
     <TgsApiVersion>10.0.0</TgsApiVersion>
     <TgsCommonLibraryVersion>7.0.0</TgsCommonLibraryVersion>
-    <TgsApiLibraryVersion>13.0.0</TgsApiLibraryVersion>
-    <TgsClientVersion>15.0.0</TgsClientVersion>
+    <TgsApiLibraryVersion>13.0.1</TgsApiLibraryVersion>
+    <TgsClientVersion>15.0.1</TgsClientVersion>
     <TgsDmapiVersion>7.0.0</TgsDmapiVersion>
     <TgsInteropVersion>5.7.0</TgsInteropVersion>
     <TgsHostWatchdogVersion>1.4.1</TgsHostWatchdogVersion>

src/Tgstation.Server.Api/Models/Response/ErrorMessageResponse.cs

@@ -1,6 +1,8 @@
 using System;
 using System.ComponentModel.DataAnnotations;
 
+using Newtonsoft.Json;
+
 namespace Tgstation.Server.Api.Models.Response
 {
 	/// <summary>
@@ -28,6 +30,7 @@ namespace Tgstation.Server.Api.Models.Response
 		/// The <see cref="ErrorCode"/> of the <see cref="ErrorMessageResponse"/>.
 		/// </summary>
 		[EnumDataType(typeof(ErrorCode))]
+		[JsonProperty(Required = Required.Always)]
 		public ErrorCode ErrorCode { get; set; }
 
 		/// <summary>

src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs

@@ -687,9 +687,21 @@ namespace Tgstation.Server.Host.Components.Chat.Providers
 						channelId,
 						discordChannelResponse.LogFormat());
 
-					remapRequired |= !(discordChannelResponse.Error is RestResultError<RestError> restResultError
+					var remapConditional = !(discordChannelResponse.Error is RestResultError<RestError> restResultError
 						&& (restResultError.Error?.Code == DiscordError.MissingAccess
 						|| restResultError.Error?.Code == DiscordError.UnknownChannel));
+
+					if (remapConditional)
+					{
+						Logger.Log(
+							remapRequired
+								? LogLevel.Trace
+								: LogLevel.Debug,
+							"Error on channel {channelId} is not an access/thread issue. Will retry remap...",
+							channelId);
+						remapRequired = true;
+					}
+
 					return null;
 				}
 
@@ -721,7 +733,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers
 							"Error retrieving discord guild {guildID}: {result}",
 							guildId,
 							guildsResponse.LogFormat());
-						remapRequired |= true;
+						remapRequired = true;
 					}
 
 					return null;

src/Tgstation.Server.Host/Controllers/ControlPanelController.cs

@@ -123,7 +123,7 @@ namespace Tgstation.Server.Host.Controllers
 		/// </summary>
 		/// <param name="appRoute">The value of the route.</param>
 		/// <returns>The <see cref="VirtualFileResult"/> to use.</returns>
-		[Route("/{**appRoute}")]
+		[Route("{**appRoute}")]
 		[HttpGet]
 		public IActionResult Get([FromRoute] string appRoute)
 		{

src/Tgstation.Server.Host/Controllers/RootController.cs

@@ -94,21 +94,21 @@ namespace Tgstation.Server.Host.Controllers
 		[AllowAnonymous]
 		public IActionResult Index()
 		{
-			const string ApiDocumentationRoute = "/" + SwaggerConfiguration.DocumentationSiteRouteExtension;
 			var panelEnabled = controlPanelConfiguration.Enable;
 			var apiDocsEnabled = generalConfiguration.HostApiDocumentation;
 
+			var controlPanelRoute = ControlPanelController.ControlPanelRoute.TrimStart('/');
 			if (panelEnabled ^ apiDocsEnabled)
 				if (panelEnabled)
-					return Redirect(ControlPanelController.ControlPanelRoute);
+					return Redirect(controlPanelRoute);
 				else
-					return Redirect(ApiDocumentationRoute);
+					return Redirect(SwaggerConfiguration.DocumentationSiteRouteExtension);
 
 			Dictionary<string, string>? links;
 			if (panelEnabled)
 				links = new Dictionary<string, string>()
 				{
-					{ "Web Control Panel", ControlPanelController.ControlPanelRoute.TrimStart('/') },
+					{ "Web Control Panel", controlPanelRoute },
 					{ "API Documentation", SwaggerConfiguration.DocumentationSiteRouteExtension },
 				};
 			else

src/Tgstation.Server.Host/Core/Application.cs

@@ -16,7 +16,6 @@ using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Http.Connections;
 using Microsoft.AspNetCore.Identity;
-using Microsoft.AspNetCore.Mvc.Filters;
 using Microsoft.AspNetCore.Mvc.Infrastructure;
 using Microsoft.AspNetCore.SignalR;
 using Microsoft.Extensions.Configuration;
@@ -629,7 +628,6 @@ namespace Tgstation.Server.Host.Core
 				.GetRequiredService<AuthenticationContextFactory>()
 				.CurrentAuthenticationContext);
 			services.AddScoped<IClaimsTransformation, AuthenticationContextClaimsTransformation>();
-			services.AddScoped<IAuthorizationFilter, AuthenticationContextAuthorizationFilter>();
 
 			services
 				.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)

src/Tgstation.Server.Host/Security/AuthenticationContextAuthorizationFilter.cs

@@ -1,55 +0,0 @@
-using System;
-using System.Security.Claims;
-
-using Microsoft.AspNetCore.Mvc;
-using Microsoft.AspNetCore.Mvc.Filters;
-using Microsoft.Extensions.Logging;
-
-using Tgstation.Server.Host.Models;
-
-namespace Tgstation.Server.Host.Security
-{
-	/// <summary>
-	/// An <see cref="IAuthorizationFilter"/> that maps <see cref="Claim"/>s using an <see cref="IAuthenticationContext"/>.
-	/// </summary>
-	sealed class AuthenticationContextAuthorizationFilter : IAuthorizationFilter
-	{
-		/// <summary>
-		/// The <see cref="IAuthenticationContext"/> for the <see cref="AuthenticationContextAuthorizationFilter"/>.
-		/// </summary>
-		readonly IAuthenticationContext authenticationContext;
-
-		/// <summary>
-		/// The <see cref="ILogger"/> for the <see cref="AuthenticationContextAuthorizationFilter"/>.
-		/// </summary>
-		readonly ILogger<AuthenticationContextAuthorizationFilter> logger;
-
-		/// <summary>
-		/// Initializes a new instance of the <see cref="AuthenticationContextAuthorizationFilter"/> class.
-		/// </summary>
-		/// <param name="authenticationContext">The value of <see cref="authenticationContext"/>.</param>
-		/// <param name="logger">The value of <see cref="logger"/>.</param>
-		public AuthenticationContextAuthorizationFilter(IAuthenticationContext authenticationContext, ILogger<AuthenticationContextAuthorizationFilter> logger)
-		{
-			this.authenticationContext = authenticationContext ?? throw new ArgumentNullException(nameof(authenticationContext));
-			this.logger = logger ?? throw new ArgumentNullException(nameof(logger));
-		}
-
-		/// <inheritdoc />
-		public void OnAuthorization(AuthorizationFilterContext context)
-		{
-			if (!authenticationContext.Valid)
-			{
-				logger.LogTrace("authenticationContext is invalid!");
-				context.Result = new UnauthorizedResult();
-				return;
-			}
-
-			if (authenticationContext.User.Require(x => x.Enabled))
-				return;
-
-			logger.LogTrace("authenticationContext is for a disabled user!");
-			context.Result = new ForbidResult();
-		}
-	}
-}

src/Tgstation.Server.Host/Security/AuthenticationContextClaimsTransformation.cs

@@ -83,32 +83,28 @@ namespace Tgstation.Server.Host.Security
 				nbf,
 				CancellationToken.None); // DCT: None available
 
-			if (authenticationContext.Valid)
+			var enumerator = Enum.GetValues(typeof(RightsType));
+			var claims = new List<Claim>();
+			foreach (RightsType rightType in enumerator)
 			{
-				var enumerator = Enum.GetValues(typeof(RightsType));
-				var claims = new List<Claim>();
-				foreach (RightsType rightType in enumerator)
-				{
-					// if there's no instance user, do a weird thing and add all the instance roles
-					// we need it so we can get to OnActionExecutionAsync where we can properly decide between BadRequest and Forbid
-					// if user is null that means they got the token with an expired password
-					var rightAsULong = authenticationContext.User == null
-						|| (RightsHelper.IsInstanceRight(rightType) && authenticationContext.InstancePermissionSet == null)
-						? ~0UL
-						: authenticationContext.GetRight(rightType);
-					var rightEnum = RightsHelper.RightToType(rightType);
-					var right = (Enum)Enum.ToObject(rightEnum, rightAsULong);
-					foreach (Enum enumeratedRight in Enum.GetValues(rightEnum))
-						if (right.HasFlag(enumeratedRight))
-							claims.Add(
-								new Claim(
-									ClaimTypes.Role,
-									RightsHelper.RoleName(rightType, enumeratedRight)));
-				}
-
-				principal.AddIdentity(new ClaimsIdentity(claims));
+				// if there's a bad condition, do a weird thing and add all the roles
+				// we need it so we can get to TgsAuthorizeAttribute where we can properly decide between BadRequest and Forbid
+				var rightAsULong = !authenticationContext.Valid
+					|| (RightsHelper.IsInstanceRight(rightType) && authenticationContext.InstancePermissionSet == null)
+					? ~0UL
+					: authenticationContext.GetRight(rightType);
+				var rightEnum = RightsHelper.RightToType(rightType);
+				var right = (Enum)Enum.ToObject(rightEnum, rightAsULong);
+				foreach (Enum enumeratedRight in Enum.GetValues(rightEnum))
+					if (right.HasFlag(enumeratedRight))
+						claims.Add(
+							new Claim(
+								ClaimTypes.Role,
+								RightsHelper.RoleName(rightType, enumeratedRight)));
 			}
 
+			principal.AddIdentity(new ClaimsIdentity(claims));
+
 			return principal;
 		}
 	}

src/Tgstation.Server.Host/Security/AuthenticationContextFactory.cs

@@ -102,7 +102,7 @@ namespace Tgstation.Server.Host.Security
 				systemIdentity = identityCache.LoadCachedIdentity(user);
 			else
 			{
-				if (user.LastPasswordUpdate.HasValue && user.LastPasswordUpdate > notBefore)
+				if (user.LastPasswordUpdate.HasValue && user.LastPasswordUpdate >= notBefore)
 				{
 					logger.LogDebug("Rejecting token for user {userId} created before last password update: {lastPasswordUpdate}", userId, user.LastPasswordUpdate.Value);
 					return currentAuthenticationContext;

src/Tgstation.Server.Host/Security/README.md

@@ -64,7 +64,7 @@
 	1. If a valid authentication context is returned from the [IAuthenticationContextFactory](./IAuthenticationContextFactory.cs), the [AuthenticationContextClaimsTransformation](./AuthenticationContextClaimsTransformation.cs) uses the context to add claims for each permission bit to the user's identity principal.
 		- Internally, ASP.NET Core uses this to determine whether or not a request to an endpoint will 403 or not based on the parameters of its [TgsAuthorizeAttribute](./TgsAuthorizeAttribute.cs).
 1. The authorization filter is invoked
-	- For non-SignalR hub requests, this is the [AuthenticationContextAuthorizationFilter](./AuthenticationContextAuthorizationFilter.cs). It does two simple things:
+	- For non-SignalR hub requests, this is the `IAuthorizationFilter` part of the [TgsAuthorizeAttribute](./TgsAuthorizeAttribute.cs). It does two simple things:
 		1. It checks the validity of the scope's [IAuthenticationContext](./IAuthenticationContext.cs). If it is invalid (indicating the user is not authorized either due to not existing (Only possible with a forged and signed JWT) or if their token was outdated compared to the last time their password or `Enabled` status was updated), HTTP 401 will be returned.
 		1. It checks the user's `Enabled` status. If the user is disabled, HTTP 403 will be returned.
 	- For SignalR hub requests, this is the [AuthorizationContextHubFilter](./AuthorizationContextHubFilter.cs).

src/Tgstation.Server.Host/Security/TgsAuthorizeAttribute.cs

@@ -1,8 +1,13 @@
 using System;
 
 using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Mvc;
+using Microsoft.AspNetCore.Mvc.Filters;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
 
 using Tgstation.Server.Api.Rights;
+using Tgstation.Server.Host.Models;
 
 namespace Tgstation.Server.Host.Security
 {
@@ -11,7 +16,7 @@ namespace Tgstation.Server.Host.Security
 	/// </summary>
 #pragma warning disable CA1019
 	[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = true, Inherited = true)]
-	sealed class TgsAuthorizeAttribute : AuthorizeAttribute
+	sealed class TgsAuthorizeAttribute : AuthorizeAttribute, IAuthorizationFilter
 	{
 		/// <summary>
 		/// Gets the <see cref="Api.Rights.RightsType"/> associated with the <see cref="TgsAuthorizeAttribute"/> if any.
@@ -114,5 +119,26 @@ namespace Tgstation.Server.Host.Security
 			Roles = RightsHelper.RoleNames(requiredRights);
 			RightsType = Api.Rights.RightsType.InstancePermissionSet;
 		}
+
+		/// <inheritdoc />
+		public void OnAuthorization(AuthorizationFilterContext context)
+		{
+			var services = context.HttpContext.RequestServices;
+			var authenticationContext = services.GetRequiredService<IAuthenticationContext>();
+			var logger = services.GetRequiredService<ILogger<TgsAuthorizeAttribute>>();
+
+			if (!authenticationContext.Valid)
+			{
+				logger.LogTrace("authenticationContext is invalid!");
+				context.Result = new UnauthorizedResult();
+				return;
+			}
+
+			if (authenticationContext.User.Require(x => x.Enabled))
+				return;
+
+			logger.LogTrace("authenticationContext is for a disabled user!");
+			context.Result = new ForbidResult();
+		}
 	}
 }

tests/Tgstation.Server.Tests/Live/Instance/DeploymentTest.cs

@@ -123,7 +123,7 @@ namespace Tgstation.Server.Tests.Live.Instance
 				var updatedDM = await dreamMakerClient.Update(new DreamMakerRequest
 				{
 					ProjectName = "tests/DMAPI/ApiFree/api_free",
-					ApiValidationPort = dmPort
+					ApiValidationPort = dmPort,
 				}, cancellationToken);
 				Assert.AreEqual(dmPort, updatedDM.ApiValidationPort);
 				Assert.AreEqual("tests/DMAPI/ApiFree/api_free", updatedDM.ProjectName);
@@ -139,10 +139,10 @@ namespace Tgstation.Server.Tests.Live.Instance
 
 			var updatedDD = await dreamDaemonClient.Update(new DreamDaemonRequest
 			{
-				StartupTimeout = 30,
+				StartupTimeout = 60,
 				Port = ddPort
 			}, cancellationToken);
-			Assert.AreEqual(30U, updatedDD.StartupTimeout);
+			Assert.AreEqual(60U, updatedDD.StartupTimeout);
 			Assert.AreEqual(ddPort, updatedDD.Port);
 
 			async Task<JobResponse> CompileAfterByondInstall()
@@ -153,7 +153,7 @@ namespace Tgstation.Server.Tests.Live.Instance
 
 			var deployJobTask = CompileAfterByondInstall();
 			var deployJob = await deployJobTask;
-			var deploymentJobWaitTask = WaitForJob(deployJob, 40, true, ErrorCode.DeploymentNeverValidated, cancellationToken);
+			var deploymentJobWaitTask = WaitForJob(deployJob, 120, true, ErrorCode.DeploymentNeverValidated, cancellationToken);
 
 			await CheckDreamDaemonPriority(deploymentJobWaitTask, cancellationToken);
 

tests/Tgstation.Server.Tests/Live/Instance/InstanceTest.cs

@@ -159,10 +159,27 @@ namespace Tgstation.Server.Tests.Live.Instance
 				Origin = new Uri(Origin),
 			}, cancellationToken).AsTask();
 
-			var dmUpdateRequest = instanceClient.DreamMaker.Update(new DreamMakerRequest
+			async Task UpdateDMSettings()
 			{
-				ApiValidationPort = dmPort,
-			}, cancellationToken);
+				for (var i = 0; i < 5; ++i)
+					try
+					{
+						await instanceClient.DreamMaker.Update(new DreamMakerRequest
+						{
+							ApiValidationPort = dmPort,
+						}, cancellationToken);
+					}
+					catch (ConflictException ex) when (ex.ErrorCode == ErrorCode.PortNotAvailable)
+					{
+						if (i == 4)
+							throw;
+
+						// I have no idea why this happens sometimes
+						await Task.Delay(TimeSpan.FromSeconds(3), cancellationToken);
+					}
+			}
+
+			var dmUpdateRequest = UpdateDMSettings();
 
 			// need at least one chat bot to satisfy DMAPI test,
 			// use discord as it allows multi-botting on on token unlike IRC
@@ -230,7 +247,7 @@ namespace Tgstation.Server.Tests.Live.Instance
 			await Task.WhenAll(
 				jrt.WaitForJob(installJob2.InstallJob, EngineTest.EngineInstallationTimeout(compatVersion) + 30, false, null, cancellationToken),
 				jrt.WaitForJob(cloneRequest.Result.ActiveJob, 60, false, null, cancellationToken),
-				dmUpdateRequest.AsTask(),
+				dmUpdateRequest,
 				cloneRequest);
 
 			if (compatVersion.Engine.Value == EngineType.OpenDream)

tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs

@@ -129,17 +129,34 @@ namespace Tgstation.Server.Tests.Live.Instance
 				}
 			}
 
+			async Task UpdateDDSettings()
+			{
+				for (var i = 0; i < 5; ++i)
+					try
+					{
+						// Increase startup timeout, disable heartbeats, enable map threads because we've tested without for years
+						await instanceClient.DreamDaemon.Update(new DreamDaemonRequest
+						{
+							StartupTimeout = 60,
+							HealthCheckSeconds = 0,
+							Port = ddPort,
+							MapThreads = 2,
+							LogOutput = false,
+							AdditionalParameters = BaseAdditionalParameters
+						}, cancellationToken);
+					}
+					catch (ConflictException ex) when (ex.ErrorCode == ErrorCode.PortNotAvailable)
+					{
+						if (i == 4)
+							throw;
+
+						// I have no idea why this happens sometimes
+						await Task.Delay(TimeSpan.FromSeconds(3), cancellationToken);
+					}
+			}
+
 			await Task.WhenAll(
-				// Increase startup timeout, disable heartbeats, enable map threads because we've tested without for years
-				instanceClient.DreamDaemon.Update(new DreamDaemonRequest
-				{
-					StartupTimeout = 30,
-					HealthCheckSeconds = 0,
-					Port = ddPort,
-					MapThreads = 2,
-					LogOutput = false,
-					AdditionalParameters = BaseAdditionalParameters
-				}, cancellationToken).AsTask(),
+				UpdateDDSettings(),
 				CheckByondVersions(),
 				ApiAssert.ThrowsException<ApiConflictException, DreamDaemonResponse>(() => instanceClient.DreamDaemon.Update(new DreamDaemonRequest
 				{

tests/Tgstation.Server.Tests/Live/TestLiveServer.cs

@@ -1342,7 +1342,20 @@ namespace Tgstation.Server.Tests.Live
 			{
 				Api.Models.Instance instance;
 				long initialStaged, initialActive, initialSessionId;
+
 				await using var firstAdminClient = await CreateAdminClient(server.ApiUrl, cancellationToken);
+				await using (var tokenOnlyClient = clientFactory.CreateFromToken(server.RootUrl, firstAdminClient.Token))
+				{
+					// regression test for password change issue
+					var currentUser = await tokenOnlyClient.Users.Read(cancellationToken);
+					var updatedUser = await tokenOnlyClient.Users.Update(new UserUpdateRequest
+					{
+						Id = currentUser.Id,
+						Password = DefaultCredentials.DefaultAdminUserPassword,
+					}, cancellationToken);
+
+					await ApiAssert.ThrowsException<UnauthorizedException, UserResponse>(() => tokenOnlyClient.Users.Read(cancellationToken), null);
+				}
 
 				async ValueTask<IServerClient> CreateUserWithNoInstancePerms()
 				{