Merge pull request #847 from tgstation/826-UserAgentValidationRemoval

Removes strict validation of client user agents

docs/API.dox

@@ -32,7 +32,7 @@ This document will reference the canonical C# models in the @ref Tgstation.Serve
 
 TGS4 expects this set of headers. Failure to provide them may result in 400 error responses
 
-- User-Agent: The user agent product header value of the calling program. Should be in the form Agent/Version (i.e. SomeTgsClient/1.2.4)
+- User-Agent: The user agent product header value of the calling program
 - Accept: application/json
 - Api: Another product header value representing the version of the API to use. Currently this must be: Tgstation.Server.Api/4.0.0.0
 

src/Tgstation.Server.Api/ApiHeaders.cs

@@ -61,9 +61,14 @@ namespace Tgstation.Server.Api
 		public long? InstanceId { get; set; }
 
 		/// <summary>
-		/// The client's user agent
+		/// The client's user agent as a <see cref="ProductHeaderValue"/> if valid
 		/// </summary>
-		public ProductHeaderValue UserAgent { get; }
+		public ProductHeaderValue UserAgent => ProductInfoHeaderValue.TryParse(RawUserAgent, out var userAgent) ? userAgent.Product : null;
+
+		/// <summary>
+		/// The client's raw user agent
+		/// </summary>
+		public string RawUserAgent { get; }
 
 		/// <summary>
 		/// The client's API version
@@ -136,12 +141,12 @@ namespace Tgstation.Server.Api
 			if (!requestHeaders.Accept.Any(x => x.MediaType == jsonAccept.MediaType))
 				throw new InvalidOperationException(String.Format(CultureInfo.InvariantCulture, "Client does not accept {0}!", ApplicationJson));
 
-			if (!requestHeaders.Headers.TryGetValue(HeaderNames.UserAgent, out var userAgentValues) || !ProductInfoHeaderValue.TryParse(userAgentValues.FirstOrDefault(), out var clientUserAgent))
+			if (!requestHeaders.Headers.TryGetValue(HeaderNames.UserAgent, out var userAgentValues) || userAgentValues.Count == 0)
 				throw new InvalidOperationException(String.Format(CultureInfo.InvariantCulture, "Missing {0} headers!", HeaderNames.UserAgent));
 
-			// assure the client user agent has a name and version
-			if (String.IsNullOrWhiteSpace(clientUserAgent.Product.Name) || !Version.TryParse(clientUserAgent.Product.Version, out var clientVersion))
-				throw new InvalidOperationException("Malformed client user agent!");
+			RawUserAgent = userAgentValues.First();
+			if (String.IsNullOrWhiteSpace(RawUserAgent))
+				throw new InvalidOperationException("Malformed client User-Agent!");
 
 			// make sure the api header matches ours
 			if (!requestHeaders.Headers.TryGetValue(ApiVersionHeader, out var apiUserAgentHeaderValues) || !ProductInfoHeaderValue.TryParse(apiUserAgentHeaderValues.FirstOrDefault(), out var apiUserAgent) || apiUserAgent.Product.Name != AssemblyName.Name)
@@ -151,7 +156,6 @@ namespace Tgstation.Server.Api
 				throw new InvalidOperationException("Malformed API version!");
 
 			ApiVersion = apiVersion;
-			UserAgent = clientUserAgent.Product;
 
 			if (!requestHeaders.Headers.TryGetValue(HeaderNames.Authorization, out StringValues authorization))
 				throw new InvalidOperationException(String.Format(CultureInfo.InvariantCulture, "Missing {0} header!", HeaderNames.Authorization));
@@ -204,7 +208,7 @@ namespace Tgstation.Server.Api
 		/// <param name="password">The value of <see cref="Password"/></param>
 		ApiHeaders(ProductHeaderValue userAgent, string token, string username, string password)
 		{
-			UserAgent = userAgent;
+			RawUserAgent = userAgent?.ToString();
 			Token = token;
 			Username = username;
 			Password = password;

src/Tgstation.Server.Api/Tgstation.Server.Api.csproj

@@ -14,8 +14,8 @@
     <RepositoryUrl>https://github.com/tgstation/tgstation-server</RepositoryUrl>
     <Copyright>2018</Copyright>
     <PackageTags>json web api tgstation-server tgstation ss13 byond</PackageTags>
-    <PackageReleaseNotes>Initial release</PackageReleaseNotes>
-    <Version>4.0.1.1</Version>
+    <PackageReleaseNotes>Added ApiHeaders.RawUserAgent</PackageReleaseNotes>
+    <Version>4.0.2.0</Version>
     <CodeAnalysisRuleSet>../../build/analyzers.ruleset</CodeAnalysisRuleSet>
     <LangVersion>latest</LangVersion>
   </PropertyGroup>

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

@@ -152,7 +152,7 @@ namespace Tgstation.Server.Host.Controllers
 			}
 
 			if (ApiHeaders != null)
-				Logger.LogDebug("Request made by User ID {0}. Api version: {1}. User-Agent: {2}. Type: {3}. Route {4}{5} to Instance {6}", AuthenticationContext?.User.Id.ToString(CultureInfo.InvariantCulture), ApiHeaders.ApiVersion, ApiHeaders.UserAgent, Request.Method, Request.Path, Request.QueryString, ApiHeaders.InstanceId);
+				Logger.LogDebug("Request made by User ID {0}. Api version: {1}. User-Agent: {2}. Type: {3}. Route {4}{5} to Instance {6}", AuthenticationContext?.User.Id.ToString(CultureInfo.InvariantCulture), ApiHeaders.ApiVersion, ApiHeaders.RawUserAgent, Request.Method, Request.Path, Request.QueryString, ApiHeaders.InstanceId);
 
 			try
 			{

tests/Tgstation.Server.Api.Tests/TestApiHeaders.cs

@@ -1,4 +1,6 @@
-using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Http.Headers;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
 using System;
 using System.Net.Http.Headers;
 
@@ -19,5 +21,35 @@ namespace Tgstation.Server.Api.Tests
 			Assert.ThrowsException<ArgumentNullException>(() => new ApiHeaders(productHeaderValue, null));
 			var headers = new ApiHeaders(productHeaderValue, String.Empty);
 		}
+
+		[TestMethod]
+		public void TestUserAgentsAreValid()
+		{
+			const string BrowserHeader = "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36.";
+			const string ConformantHeader = "TGSClient/3.2.1.4";
+
+			ApiHeaders TestHeader(string userAgent)
+			{
+				var headers = new HeaderDictionary
+				{
+					{ "Accept", ApiHeaders.ApplicationJson },
+					{ "Api", "Tgstation.Server.Api/4.0.0.0" },
+					{ "Authorization", "Bearer asdfasdf" },
+					{ "User-Agent", userAgent }
+				};
+
+				return new ApiHeaders(new RequestHeaders(headers));
+			};
+
+			var header = TestHeader(BrowserHeader);
+			Assert.AreEqual(BrowserHeader, header.RawUserAgent);
+			Assert.IsNull(header.UserAgent);
+
+			header = TestHeader(ConformantHeader);
+			Assert.AreEqual(ConformantHeader, header.RawUserAgent);
+			Assert.IsNotNull(header.UserAgent);
+
+			Assert.ThrowsException<InvalidOperationException>(() => TestHeader(String.Empty));
+		}
 	}
 }

tests/Tgstation.Server.Api.Tests/Tgstation.Server.Api.Tests.csproj

@@ -9,6 +9,7 @@
   </PropertyGroup>
 
   <ItemGroup>
+    <PackageReference Include="Microsoft.AspNetCore.Http" Version="2.2.0" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.9.0" />
     <PackageReference Include="MSTest.TestAdapter" Version="1.3.2" />
     <PackageReference Include="MSTest.TestFramework" Version="1.3.2" />