Create a custom injected class for claims injection

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

@@ -1,20 +1,14 @@
-using Microsoft.AspNetCore.Authentication.JwtBearer;
-using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.Mvc.Filters;
-using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using System;
-using System.Collections.Generic;
 using System.Globalization;
-using System.IdentityModel.Tokens.Jwt;
 using System.Linq;
 using System.Net;
-using System.Security.Claims;
 using System.Threading.Tasks;
 using Tgstation.Server.Api;
 using Tgstation.Server.Api.Models;
-using Tgstation.Server.Api.Rights;
 using Tgstation.Server.Host.Models;
 using Tgstation.Server.Host.Security;
 
@@ -57,64 +51,6 @@ namespace Tgstation.Server.Host.Controllers
 		/// </summary>
 		readonly bool requireInstance;
 
-		/// <summary>
-		/// Runs after a <see cref="Token"/> has been validated. Creates the <see cref="IAuthenticationContext"/> for the <see cref="ControllerBase.Request"/>
-		/// </summary>
-		/// <param name="context">The <see cref="TokenValidatedContext"/> for the operation</param>
-		/// <returns>A <see cref="Task"/> representing the running operation</returns>
-		public static async Task OnTokenValidated(TokenValidatedContext context)
-		{
-			var databaseContext = context.HttpContext.RequestServices.GetRequiredService<IDatabaseContext>();
-			var authenticationContextFactory = context.HttpContext.RequestServices.GetRequiredService<IAuthenticationContextFactory>();
-
-			var userIdClaim = context.Principal.FindFirst(JwtRegisteredClaimNames.Sub);
-
-			if (userIdClaim == default(Claim))
-				throw new InvalidOperationException("Missing required claim!");
-
-			long userId;
-			try
-			{
-				userId = Int64.Parse(userIdClaim.Value, CultureInfo.InvariantCulture);
-			}
-			catch (Exception e)
-			{
-				throw new InvalidOperationException("Failed to parse user ID!", e);
-			}
-
-			ApiHeaders apiHeaders;
-			try
-			{
-				apiHeaders = new ApiHeaders(context.HttpContext.Request.GetTypedHeaders());
-			}
-			catch
-			{
-				//let OnActionExecutionAsync handle the reponse
-				return;
-			}
-
-			await authenticationContextFactory.CreateAuthenticationContext(userId, apiHeaders.InstanceId, context.SecurityToken.ValidFrom, context.HttpContext.RequestAborted).ConfigureAwait(false);
-
-			var authenticationContext = authenticationContextFactory.CurrentAuthenticationContext;
-
-			var enumerator = Enum.GetValues(typeof(RightsType));
-			var claims = new List<Claim>();
-			foreach (RightsType I 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 rightInt = authenticationContext.User == null || (RightsHelper.IsInstanceRight(I) && authenticationContext.InstanceUser == null) ? ~0U : authenticationContext.GetRight(I);
-				var rightEnum = RightsHelper.RightToType(I);
-				var right = (Enum)Enum.ToObject(rightEnum, rightInt);
-				foreach (Enum J in Enum.GetValues(rightEnum))
-					if (right.HasFlag(J))
-						claims.Add(new Claim(ClaimTypes.Role, RightsHelper.RoleName(I, J)));
-			}
-
-			context.Principal.AddIdentity(new ClaimsIdentity(claims));
-		}
-
 		/// <summary>
 		/// Construct an <see cref="ApiController"/>
 		/// </summary>

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

@@ -101,6 +101,8 @@ namespace Tgstation.Server.Host.Core
 
 			services.AddOptions();
 
+			services.AddScoped<IClaimsInjector, ClaimsInjector>();
+
 			const string scheme = "JwtBearer";
 			services.AddAuthentication((options) =>
 			{
@@ -128,9 +130,11 @@ namespace Tgstation.Server.Host.Core
 				};
 				jwtBearerOptions.Events = new JwtBearerEvents
 				{
-					OnTokenValidated = ApiController.OnTokenValidated
+					//Application is our composition root so this monstrosity of a line is okay
+					OnTokenValidated = ctx => ctx.HttpContext.RequestServices.GetRequiredService<IClaimsInjector>().InjectClaimsIntoContext(ctx, ctx.HttpContext.RequestAborted)
 				};
 			});
+
 			JwtSecurityTokenHandler.DefaultInboundClaimTypeMap.Clear(); //fucking converts 'sub' to M$ bs
 
 			services.AddMvc().AddJsonOptions(options =>

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

@@ -0,0 +1,95 @@
+using Microsoft.AspNetCore.Authentication.JwtBearer;
+using Microsoft.AspNetCore.Http;
+using System;
+using System.Collections.Generic;
+using System.Globalization;
+using System.IdentityModel.Tokens.Jwt;
+using System.Security.Claims;
+using System.Threading;
+using System.Threading.Tasks;
+using Tgstation.Server.Api;
+using Tgstation.Server.Api.Rights;
+using Tgstation.Server.Host.Models;
+
+namespace Tgstation.Server.Host.Security
+{
+	/// <inheritdoc />
+	sealed class ClaimsInjector : IClaimsInjector
+	{
+		/// <summary>
+		/// The <see cref="IDatabaseContext"/> for the <see cref="ClaimsInjector"/>
+		/// </summary>
+		readonly IDatabaseContext databaseContext;
+
+		/// <summary>
+		/// The <see cref="IAuthenticationContextFactory"/> for the <see cref="ClaimsInjector"/>
+		/// </summary>
+		readonly IAuthenticationContextFactory authenticationContextFactory;
+
+		/// <summary>
+		/// Construct a <see cref="ClaimsInjector"/>
+		/// </summary>
+		/// <param name="databaseContext">The value of <see cref="databaseContext"/></param>
+		/// <param name="authenticationContextFactory">The value of <see cref="authenticationContextFactory"/></param>
+		public ClaimsInjector(IDatabaseContext databaseContext, IAuthenticationContextFactory authenticationContextFactory)
+		{
+			this.databaseContext = databaseContext ?? throw new ArgumentNullException(nameof(databaseContext));
+			this.authenticationContextFactory = authenticationContextFactory ?? throw new ArgumentNullException(nameof(authenticationContextFactory));
+		}
+
+		/// <inheritdoc />
+		public async Task InjectClaimsIntoContext(TokenValidatedContext tokenValidatedContext, CancellationToken cancellationToken)
+		{
+			if (tokenValidatedContext == null)
+				throw new ArgumentNullException(nameof(tokenValidatedContext));
+
+			//Find the user id in the token
+			var userIdClaim = tokenValidatedContext.Principal.FindFirst(JwtRegisteredClaimNames.Sub);
+			if (userIdClaim == default)
+				throw new InvalidOperationException("Missing required claim!");
+
+			long userId;
+			try
+			{
+				userId = Int64.Parse(userIdClaim.Value, CultureInfo.InvariantCulture);
+			}
+			catch (Exception e)
+			{
+				throw new InvalidOperationException("Failed to parse user ID!", e);
+			}
+
+			ApiHeaders apiHeaders;
+			try
+			{
+				apiHeaders = new ApiHeaders(tokenValidatedContext.HttpContext.Request.GetTypedHeaders());
+			}
+			catch (InvalidOperationException)
+			{
+				//we are not responsible for handling header validation issues
+				return;
+			}
+
+			//This populates the CurrentAuthenticationContext field for use by us and subsequent controllers
+			await authenticationContextFactory.CreateAuthenticationContext(userId, apiHeaders.InstanceId, tokenValidatedContext.SecurityToken.ValidFrom, cancellationToken).ConfigureAwait(false);
+
+			var authenticationContext = authenticationContextFactory.CurrentAuthenticationContext;
+
+			var enumerator = Enum.GetValues(typeof(RightsType));
+			var claims = new List<Claim>();
+			foreach (RightsType I 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 rightInt = authenticationContext.User == null || (RightsHelper.IsInstanceRight(I) && authenticationContext.InstanceUser == null) ? ~0U : authenticationContext.GetRight(I);
+				var rightEnum = RightsHelper.RightToType(I);
+				var right = (Enum)Enum.ToObject(rightEnum, rightInt);
+				foreach (Enum J in Enum.GetValues(rightEnum))
+					if (right.HasFlag(J))
+						claims.Add(new Claim(ClaimTypes.Role, RightsHelper.RoleName(I, J)));
+			}
+
+			tokenValidatedContext.Principal.AddIdentity(new ClaimsIdentity(claims));
+		}
+	}
+}

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

@@ -0,0 +1,20 @@
+using Microsoft.AspNetCore.Authentication.JwtBearer;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace Tgstation.Server.Host.Security
+{
+	/// <summary>
+	/// For injecting <see cref="System.Security.Claims.Claim"/>s that <see cref="Controllers.TgsAuthorizeAttribute"/> can look for
+	/// </summary>
+	interface IClaimsInjector
+	{
+		/// <summary>
+		/// Setup the <see cref="System.Security.Claims.Claim"/>s for a given <paramref name="tokenValidatedContext"/>
+		/// </summary>
+		/// <param name="tokenValidatedContext">The <see cref="TokenValidatedContext"/> containing the <see cref="Microsoft.AspNetCore.Http.HttpContext"/> and <see cref="Microsoft.IdentityModel.Tokens.SecurityToken"/> of the request and the <see cref="System.Security.Claims.ClaimsPrincipal"/> to add <see cref="System.Security.Claims.Claim"/>s to</param>
+		/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation</param>
+		/// <returns>A <see cref="Task"/> representing the running operation</returns>
+		Task InjectClaimsIntoContext(TokenValidatedContext tokenValidatedContext, CancellationToken cancellationToken);
+	}
+}