From f3ed1d1f0a72ea8b487adb2cfa1696a18ebecfc4 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 11 Jan 2019 14:52:28 -0500 Subject: [PATCH 1/6] Allow non-conformant client user agents --- src/Tgstation.Server.Api/ApiHeaders.cs | 19 ++++++++++--------- .../Controllers/ApiController.cs | 2 +- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/Tgstation.Server.Api/ApiHeaders.cs b/src/Tgstation.Server.Api/ApiHeaders.cs index 2f2fabbb89..f62173962a 100644 --- a/src/Tgstation.Server.Api/ApiHeaders.cs +++ b/src/Tgstation.Server.Api/ApiHeaders.cs @@ -61,9 +61,14 @@ namespace Tgstation.Server.Api public long? InstanceId { get; set; } /// - /// The client's user agent + /// The client's user agent as a if valid /// - public ProductHeaderValue UserAgent { get; } + public ProductHeaderValue UserAgent => ProductInfoHeaderValue.TryParse(RawUserAgent, out var userAgent) ? userAgent.Product : null; + + /// + /// The client's raw user agent + /// + public string RawUserAgent { get; } /// /// The client's API version @@ -136,13 +141,9 @@ 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!"); - // 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) throw new InvalidOperationException("Missing API version!"); @@ -151,7 +152,7 @@ namespace Tgstation.Server.Api throw new InvalidOperationException("Malformed API version!"); ApiVersion = apiVersion; - UserAgent = clientUserAgent.Product; + RawUserAgent = userAgentValues.First(); if (!requestHeaders.Headers.TryGetValue(HeaderNames.Authorization, out StringValues authorization)) throw new InvalidOperationException(String.Format(CultureInfo.InvariantCulture, "Missing {0} header!", HeaderNames.Authorization)); @@ -204,7 +205,7 @@ namespace Tgstation.Server.Api /// The value of ApiHeaders(ProductHeaderValue userAgent, string token, string username, string password) { - UserAgent = userAgent; + RawUserAgent = userAgent.ToString(); Token = token; Username = username; Password = password; diff --git a/src/Tgstation.Server.Host/Controllers/ApiController.cs b/src/Tgstation.Server.Host/Controllers/ApiController.cs index 194901e380..f937018e41 100644 --- a/src/Tgstation.Server.Host/Controllers/ApiController.cs +++ b/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 { From d6bdc400b3e2951d7e95a2955f31f9de2598b057 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 11 Jan 2019 14:54:16 -0500 Subject: [PATCH 2/6] API version bump to 4.0.2.0 --- src/Tgstation.Server.Api/Tgstation.Server.Api.csproj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Tgstation.Server.Api/Tgstation.Server.Api.csproj b/src/Tgstation.Server.Api/Tgstation.Server.Api.csproj index 5d5623792e..cc6aea401c 100644 --- a/src/Tgstation.Server.Api/Tgstation.Server.Api.csproj +++ b/src/Tgstation.Server.Api/Tgstation.Server.Api.csproj @@ -14,8 +14,8 @@ https://github.com/tgstation/tgstation-server 2018 json web api tgstation-server tgstation ss13 byond - Initial release - 4.0.1.1 + Added ApiHeaders.RawUserAgent + 4.0.2.0 ../../build/analyzers.ruleset latest From d1a45281d75edae20f73bc905f21abb7b4db0dfd Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 11 Jan 2019 15:04:20 -0500 Subject: [PATCH 3/6] Add test for issue #826 --- .../TestApiHeaders.cs | 30 ++++++++++++++++++- .../Tgstation.Server.Api.Tests.csproj | 1 + 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/tests/Tgstation.Server.Api.Tests/TestApiHeaders.cs b/tests/Tgstation.Server.Api.Tests/TestApiHeaders.cs index f6347eff63..fd91e30a27 100644 --- a/tests/Tgstation.Server.Api.Tests/TestApiHeaders.cs +++ b/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,31 @@ namespace Tgstation.Server.Api.Tests Assert.ThrowsException(() => 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(); + headers.Add("Accept", ApiHeaders.ApplicationJson); + headers.Add("Api", "Tgstation.Server.Api/4.0.0.0"); + headers.Add("Authorization", "Bearer asdfasdf"); + headers.Add("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); + } } } diff --git a/tests/Tgstation.Server.Api.Tests/Tgstation.Server.Api.Tests.csproj b/tests/Tgstation.Server.Api.Tests/Tgstation.Server.Api.Tests.csproj index 72f5fb422a..8d6c705910 100644 --- a/tests/Tgstation.Server.Api.Tests/Tgstation.Server.Api.Tests.csproj +++ b/tests/Tgstation.Server.Api.Tests/Tgstation.Server.Api.Tests.csproj @@ -9,6 +9,7 @@ + From 0ec3f2ed4d12c56a4e1375fa3f7b0ad641d1170b Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 11 Jan 2019 15:11:25 -0500 Subject: [PATCH 4/6] Fix a NullReferenceException that should be an ArgumentNullException --- src/Tgstation.Server.Api/ApiHeaders.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Api/ApiHeaders.cs b/src/Tgstation.Server.Api/ApiHeaders.cs index f62173962a..abaccef4f4 100644 --- a/src/Tgstation.Server.Api/ApiHeaders.cs +++ b/src/Tgstation.Server.Api/ApiHeaders.cs @@ -205,7 +205,7 @@ namespace Tgstation.Server.Api /// The value of ApiHeaders(ProductHeaderValue userAgent, string token, string username, string password) { - RawUserAgent = userAgent.ToString(); + RawUserAgent = userAgent?.ToString(); Token = token; Username = username; Password = password; From c5fb97b913f6381f4aa51f2d6fc20128b2fcb558 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 11 Jan 2019 15:11:50 -0500 Subject: [PATCH 5/6] Update `User-Agent` header requirements in the docs --- docs/API.dox | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/API.dox b/docs/API.dox index 5ba541489d..15416efd79 100644 --- a/docs/API.dox +++ b/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 From d7173e3c5fa49c8698c8ad325ce8b5c5cbe4ccfb Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 11 Jan 2019 15:14:25 -0500 Subject: [PATCH 6/6] Empty `User-Agent`s are still invalid --- src/Tgstation.Server.Api/ApiHeaders.cs | 5 ++++- tests/Tgstation.Server.Api.Tests/TestApiHeaders.cs | 14 +++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/Tgstation.Server.Api/ApiHeaders.cs b/src/Tgstation.Server.Api/ApiHeaders.cs index abaccef4f4..6128de76be 100644 --- a/src/Tgstation.Server.Api/ApiHeaders.cs +++ b/src/Tgstation.Server.Api/ApiHeaders.cs @@ -144,6 +144,10 @@ namespace Tgstation.Server.Api if (!requestHeaders.Headers.TryGetValue(HeaderNames.UserAgent, out var userAgentValues) || userAgentValues.Count == 0) throw new InvalidOperationException(String.Format(CultureInfo.InvariantCulture, "Missing {0} headers!", HeaderNames.UserAgent)); + 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) throw new InvalidOperationException("Missing API version!"); @@ -152,7 +156,6 @@ namespace Tgstation.Server.Api throw new InvalidOperationException("Malformed API version!"); ApiVersion = apiVersion; - RawUserAgent = userAgentValues.First(); if (!requestHeaders.Headers.TryGetValue(HeaderNames.Authorization, out StringValues authorization)) throw new InvalidOperationException(String.Format(CultureInfo.InvariantCulture, "Missing {0} header!", HeaderNames.Authorization)); diff --git a/tests/Tgstation.Server.Api.Tests/TestApiHeaders.cs b/tests/Tgstation.Server.Api.Tests/TestApiHeaders.cs index fd91e30a27..5420a0fb29 100644 --- a/tests/Tgstation.Server.Api.Tests/TestApiHeaders.cs +++ b/tests/Tgstation.Server.Api.Tests/TestApiHeaders.cs @@ -30,11 +30,13 @@ namespace Tgstation.Server.Api.Tests ApiHeaders TestHeader(string userAgent) { - var headers = new HeaderDictionary(); - headers.Add("Accept", ApiHeaders.ApplicationJson); - headers.Add("Api", "Tgstation.Server.Api/4.0.0.0"); - headers.Add("Authorization", "Bearer asdfasdf"); - headers.Add("User-Agent", 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)); }; @@ -46,6 +48,8 @@ namespace Tgstation.Server.Api.Tests header = TestHeader(ConformantHeader); Assert.AreEqual(ConformantHeader, header.RawUserAgent); Assert.IsNotNull(header.UserAgent); + + Assert.ThrowsException(() => TestHeader(String.Empty)); } } }