Improve HeadersException handling

Avoid recreating ApiHeaders in request pipeline where possible
This commit is contained in:
Jordan Dominion
2023-10-29 19:13:51 -04:00
parent 3901512693
commit eaebe733bd
9 changed files with 160 additions and 99 deletions
+22 -22
View File
@@ -190,17 +190,17 @@ namespace Tgstation.Server.Api
/// <param name="ignoreMissingAuth">If a missing <see cref="HeaderNames.Authorization"/> should be ignored.</param>
/// <exception cref="HeadersException">Thrown if the <paramref name="requestHeaders"/> constitue invalid <see cref="ApiHeaders"/>.</exception>
#pragma warning disable CA1502 // TODO: Decomplexify
public ApiHeaders(RequestHeaders requestHeaders, bool ignoreMissingAuth = false)
public ApiHeaders(RequestHeaders requestHeaders, bool ignoreMissingAuth)
{
if (requestHeaders == null)
throw new ArgumentNullException(nameof(requestHeaders));
var badHeaders = HeaderTypes.None;
var badHeaders = HeaderErrorTypes.None;
var errorBuilder = new StringBuilder();
var multipleErrors = false;
void AddError(HeaderTypes headerType, string message)
void AddError(HeaderErrorTypes headerType, string message)
{
if (badHeaders != HeaderTypes.None)
if (badHeaders != HeaderErrorTypes.None)
{
multipleErrors = true;
errorBuilder.AppendLine();
@@ -212,28 +212,28 @@ namespace Tgstation.Server.Api
var jsonAccept = new Microsoft.Net.Http.Headers.MediaTypeHeaderValue(ApplicationJsonMime);
if (!requestHeaders.Accept.Any(x => jsonAccept.IsSubsetOf(x)))
AddError(HeaderTypes.Accept, $"Client does not accept {ApplicationJsonMime}!");
AddError(HeaderErrorTypes.Accept, $"Client does not accept {ApplicationJsonMime}!");
if (!requestHeaders.Headers.TryGetValue(HeaderNames.UserAgent, out var userAgentValues) || userAgentValues.Count == 0)
AddError(HeaderTypes.UserAgent, $"Missing {HeaderNames.UserAgent} header!");
AddError(HeaderErrorTypes.UserAgent, $"Missing {HeaderNames.UserAgent} header!");
else
{
RawUserAgent = userAgentValues.First();
if (String.IsNullOrWhiteSpace(RawUserAgent))
AddError(HeaderTypes.UserAgent, $"Malformed {HeaderNames.UserAgent} header!");
AddError(HeaderErrorTypes.UserAgent, $"Malformed {HeaderNames.UserAgent} header!");
}
// make sure the api header matches ours
Version? apiVersion = null;
if (!requestHeaders.Headers.TryGetValue(ApiVersionHeader, out var apiUserAgentHeaderValues) || !ProductInfoHeaderValue.TryParse(apiUserAgentHeaderValues.FirstOrDefault(), out var apiUserAgent) || apiUserAgent.Product.Name != AssemblyName.Name)
AddError(HeaderTypes.Api, $"Missing {ApiVersionHeader} header!");
AddError(HeaderErrorTypes.Api, $"Missing {ApiVersionHeader} header!");
else if (!Version.TryParse(apiUserAgent.Product.Version, out apiVersion))
AddError(HeaderTypes.Api, $"Malformed {ApiVersionHeader} header!");
AddError(HeaderErrorTypes.Api, $"Malformed {ApiVersionHeader} header!");
if (!requestHeaders.Headers.TryGetValue(HeaderNames.Authorization, out StringValues authorization))
{
if (!ignoreMissingAuth)
AddError(HeaderTypes.Authorization, $"Missing {HeaderNames.Authorization} header!");
AddError(HeaderErrorTypes.AuthorizationMissing, $"Missing {HeaderNames.Authorization} header!");
}
else
{
@@ -241,13 +241,13 @@ namespace Tgstation.Server.Api
var splits = new List<string>(auth.Split(' '));
var scheme = splits.First();
if (String.IsNullOrWhiteSpace(scheme))
AddError(HeaderTypes.Authorization, "Missing authentication scheme!");
AddError(HeaderErrorTypes.AuthorizationInvalid, "Missing authentication scheme!");
else
{
splits.RemoveAt(0);
var parameter = String.Concat(splits);
if (String.IsNullOrEmpty(parameter))
AddError(HeaderTypes.Authorization, "Missing authentication parameter!");
AddError(HeaderErrorTypes.AuthorizationInvalid, "Missing authentication parameter!");
else
{
if (requestHeaders.Headers.TryGetValue(InstanceIdHeader, out var instanceIdValues))
@@ -266,10 +266,10 @@ namespace Tgstation.Server.Api
if (Enum.TryParse<OAuthProvider>(oauthProviderString, out var oauthProvider))
OAuthProvider = oauthProvider;
else
AddError(HeaderTypes.OAuthProvider, "Invalid OAuth provider!");
AddError(HeaderErrorTypes.OAuthProvider, "Invalid OAuth provider!");
}
else
AddError(HeaderTypes.OAuthProvider, $"Missing {OAuthProviderHeader} header!");
AddError(HeaderErrorTypes.OAuthProvider, $"Missing {OAuthProviderHeader} header!");
OAuthCode = parameter;
break;
@@ -277,7 +277,7 @@ namespace Tgstation.Server.Api
var tokenSplits = parameter.Split('.');
DateTimeOffset? expiresAt = null;
if (tokenSplits.Length != 3)
AddError(HeaderTypes.Authorization, "Invalid JWT!");
AddError(HeaderErrorTypes.AuthorizationInvalid, "Invalid JWT!");
else
try
{
@@ -290,13 +290,13 @@ namespace Tgstation.Server.Api
if (Int64.TryParse(nbf, out var unixTimeSeconds))
expiresAt = DateTimeOffset.FromUnixTimeSeconds(unixTimeSeconds);
else
AddError(HeaderTypes.Authorization, "'nbf' in JWT could not be parsed!");
AddError(HeaderErrorTypes.AuthorizationInvalid, "'nbf' in JWT could not be parsed!");
else
AddError(HeaderTypes.Authorization, "Missing 'nbf' in JWT payload!");
AddError(HeaderErrorTypes.AuthorizationInvalid, "Missing 'nbf' in JWT payload!");
}
catch
{
AddError(HeaderTypes.Authorization, "Invalid JWT payload!");
AddError(HeaderErrorTypes.AuthorizationInvalid, "Invalid JWT payload!");
}
Token = new TokenResponse
@@ -315,14 +315,14 @@ namespace Tgstation.Server.Api
}
catch
{
AddError(HeaderTypes.Authorization, badBasicAuthHeaderMessage);
AddError(HeaderErrorTypes.AuthorizationInvalid, badBasicAuthHeaderMessage);
break;
}
var basicAuthSplits = joinedString.Split(ColonSeparator, StringSplitOptions.RemoveEmptyEntries);
if (basicAuthSplits.Length < 2)
{
AddError(HeaderTypes.Authorization, badBasicAuthHeaderMessage);
AddError(HeaderErrorTypes.AuthorizationInvalid, badBasicAuthHeaderMessage);
break;
}
@@ -330,14 +330,14 @@ namespace Tgstation.Server.Api
Password = String.Concat(basicAuthSplits.Skip(1));
break;
default:
AddError(HeaderTypes.Authorization, "Invalid authentication scheme!");
AddError(HeaderErrorTypes.AuthorizationInvalid, "Invalid authentication scheme!");
break;
}
}
}
}
if (badHeaders != HeaderTypes.None)
if (badHeaders != HeaderErrorTypes.None)
{
if (multipleErrors)
errorBuilder.Insert(0, $"Multiple header validation errors occurred:{Environment.NewLine}");
@@ -0,0 +1,46 @@
using System;
namespace Tgstation.Server.Api
{
/// <summary>
/// Types of individual <see cref="ApiHeaders"/> errors.
/// </summary>
[Flags]
public enum HeaderErrorTypes
{
/// <summary>
/// No header errors.
/// </summary>
None = 0,
/// <summary>
/// The <see cref="Microsoft.Net.Http.Headers.HeaderNames.UserAgent"/> header is missing or invalid.
/// </summary>
UserAgent = 1 << 0,
/// <summary>
/// The <see cref="Microsoft.Net.Http.Headers.HeaderNames.Accept"/> header is missing or invalid.
/// </summary>
Accept = 1 << 1,
/// <summary>
/// The <see cref="ApiHeaders.ApiVersionHeader"/> header is missing or invalid.
/// </summary>
Api = 1 << 2,
/// <summary>
/// The <see cref="Microsoft.Net.Http.Headers.HeaderNames.Authorization"/> header is invalid.
/// </summary>
AuthorizationInvalid = 1 << 3,
/// <summary>
/// The <see cref="ApiHeaders.OAuthProviderHeader"/> header is missing or invalid.
/// </summary>
OAuthProvider = 1 << 4,
/// <summary>
/// The <see cref="Microsoft.Net.Http.Headers.HeaderNames.Authorization"/> header is missing.
/// </summary>
AuthorizationMissing = 1 << 5,
}
}
-41
View File
@@ -1,41 +0,0 @@
using System;
namespace Tgstation.Server.Api
{
/// <summary>
/// Types of individual <see cref="ApiHeaders"/>.
/// </summary>
[Flags]
public enum HeaderTypes
{
/// <summary>
/// No headers.
/// </summary>
None = 0,
/// <summary>
/// <see cref="Microsoft.Net.Http.Headers.HeaderNames.UserAgent"/> header.
/// </summary>
UserAgent = 1 << 0,
/// <summary>
/// <see cref="Microsoft.Net.Http.Headers.HeaderNames.Accept"/> header.
/// </summary>
Accept = 1 << 1,
/// <summary>
/// <see cref="ApiHeaders.ApiVersionHeader"/>.
/// </summary>
Api = 1 << 2,
/// <summary>
/// <see cref="Microsoft.Net.Http.Headers.HeaderNames.Authorization"/>
/// </summary>
Authorization = 1 << 3,
/// <summary>
/// <see cref="ApiHeaders.OAuthProviderHeader"/>.
/// </summary>
OAuthProvider = 1 << 4,
}
}
+5 -5
View File
@@ -8,19 +8,19 @@ namespace Tgstation.Server.Api
public sealed class HeadersException : Exception
{
/// <summary>
/// The <see cref="HeaderTypes"/>s that are missing or malformed.
/// The <see cref="HeaderErrorTypes"/>s that are missing or malformed.
/// </summary>
public HeaderTypes MissingOrMalformedHeaders { get; }
public HeaderErrorTypes ParseErrors { get; }
/// <summary>
/// Initializes a new instance of the <see cref="HeadersException"/> class.
/// </summary>
/// <param name="missingOrMalformedHeaders">The value of <see cref="MissingOrMalformedHeaders"/>.</param>
/// <param name="parseErrors">The value of <see cref="ParseErrors"/>.</param>
/// <param name="message">The error message.</param>
public HeadersException(HeaderTypes missingOrMalformedHeaders, string message)
public HeadersException(HeaderErrorTypes parseErrors, string message)
: base(message)
{
MissingOrMalformedHeaders = missingOrMalformedHeaders;
ParseErrors = parseErrors;
}
/// <summary>
@@ -48,7 +48,12 @@ namespace Tgstation.Server.Host.Controllers
/// <summary>
/// The <see cref="Api.ApiHeaders"/> for the operation.
/// </summary>
protected ApiHeaders ApiHeaders { get; }
protected ApiHeaders ApiHeaders => ApiHeadersProvider.ApiHeaders;
/// <summary>
/// The <see cref="IApiHeadersProvider"/> containing value of <see cref="ApiHeaders"/>.
/// </summary>
protected IApiHeadersProvider ApiHeadersProvider { get; }
/// <summary>
/// The <see cref="IDatabaseContext"/> for the operation.
@@ -81,7 +86,7 @@ namespace Tgstation.Server.Host.Controllers
/// <param name="databaseContext">The value of <see cref="DatabaseContext"/>.</param>
/// <param name="authenticationContext">The <see cref="IAuthenticationContext"/> for the <see cref="ApiController"/>.</param>
/// <param name="logger">The value of <see cref="Logger"/>.</param>
/// <param name="apiHeadersProvider">The <see cref="IApiHeadersProvider"/> containing value of <see cref="ApiHeaders"/>.</param>
/// <param name="apiHeadersProvider">The value of <see cref="ApiHeadersProvider"/>..</param>
/// <param name="requireHeaders">The value of <see cref="requireHeaders"/>.</param>
protected ApiController(
IDatabaseContext databaseContext,
@@ -92,11 +97,10 @@ namespace Tgstation.Server.Host.Controllers
{
DatabaseContext = databaseContext ?? throw new ArgumentNullException(nameof(databaseContext));
AuthenticationContext = authenticationContext ?? throw new ArgumentNullException(nameof(authenticationContext));
ArgumentNullException.ThrowIfNull(apiHeadersProvider);
ApiHeadersProvider = apiHeadersProvider ?? throw new ArgumentNullException(nameof(apiHeadersProvider));
Logger = logger ?? throw new ArgumentNullException(nameof(logger));
Instance = AuthenticationContext?.InstancePermissionSet?.Instance;
ApiHeaders = apiHeadersProvider.ApiHeaders;
this.requireHeaders = requireHeaders;
}
@@ -110,7 +114,7 @@ namespace Tgstation.Server.Host.Controllers
if (ApiHeaders == null)
{
if (requireHeaders)
return HeadersIssue(false);
return HeadersIssue(ApiHeadersProvider.HeadersException);
}
else if (!ApiHeaders.Compatible())
return this.StatusCode(
@@ -239,28 +243,19 @@ namespace Tgstation.Server.Host.Controllers
/// <summary>
/// Response for missing/Invalid headers.
/// </summary>
/// <param name="ignoreMissingAuth">Whether or not errors due to missing <see cref="HeaderNames.Authorization"/> should be thrown.</param>
/// <param name="headersException">The <see cref="HeadersException"/> that occurred while trying to parse the <see cref="ApiHeaders"/>.</param>
/// <returns>The appropriate <see cref="IActionResult"/>.</returns>
protected IActionResult HeadersIssue(bool ignoreMissingAuth)
protected IActionResult HeadersIssue(HeadersException headersException)
{
HeadersException headersException;
try
{
// TODO: Move this somewhere saner?
_ = new ApiHeaders(Request.GetTypedHeaders(), ignoreMissingAuth);
if (headersException == null)
throw new InvalidOperationException("Expected a header parse exception!");
}
catch (HeadersException ex)
{
headersException = ex;
}
var errorMessage = new ErrorMessageResponse(ErrorCode.BadHeaders)
{
AdditionalData = headersException.Message,
};
if (headersException.MissingOrMalformedHeaders.HasFlag(HeaderTypes.Accept))
if (headersException.ParseErrors.HasFlag(HeaderErrorTypes.Accept))
return this.StatusCode(HttpStatusCode.NotAcceptable, errorMessage);
return BadRequest(errorMessage);
@@ -180,15 +180,15 @@ namespace Tgstation.Server.Host.Controllers
try
{
// we only allow authorization header issues
var headers = new ApiHeaders(Request.GetTypedHeaders(), true);
var headers = ApiHeadersProvider.CreateAuthlessHeaders();
if (!headers.Compatible())
return this.StatusCode(
HttpStatusCode.UpgradeRequired,
new ErrorMessageResponse(ErrorCode.ApiMismatch));
}
catch (HeadersException)
catch (HeadersException ex)
{
return HeadersIssue(true);
return HeadersIssue(ex);
}
}
@@ -228,7 +228,7 @@ namespace Tgstation.Server.Host.Controllers
if (ApiHeaders == null)
{
Response.Headers.Add(HeaderNames.WWWAuthenticate, new StringValues($"basic realm=\"Create TGS {ApiHeaders.BearerAuthenticationScheme} token\""));
return HeadersIssue(false);
return HeadersIssue(ApiHeadersProvider.HeadersException);
}
if (ApiHeaders.IsTokenAuthentication)
@@ -2,6 +2,7 @@
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Logging;
using Tgstation.Server.Api;
namespace Tgstation.Server.Host.Utils
@@ -10,29 +11,76 @@ namespace Tgstation.Server.Host.Utils
sealed class ApiHeadersProvider : IApiHeadersProvider
{
/// <inheritdoc />
public ApiHeaders ApiHeaders { get; }
public ApiHeaders ApiHeaders => attemptedApiHeadersCreation
? apiHeaders
: CreateApiHeaders(true);
/// <inheritdoc />
public HeadersException HeadersException { get; private set; }
/// <summary>
/// The <see cref="IHttpContextAccessor"/> for the <see cref="ApiHeadersProvider"/>.
/// </summary>
readonly IHttpContextAccessor httpContextAccessor;
/// <summary>
/// The <see cref="ILogger"/> for the <see cref="ApiHeadersProvider"/>.
/// </summary>
readonly ILogger<ApiHeadersProvider> logger;
/// <summary>
/// Backing field for <see cref="ApiHeaders"/>.
/// </summary>
ApiHeaders apiHeaders;
/// <summary>
/// If populating <see cref="ApiHeaders"/> was previously attempted.
/// </summary>
bool attemptedApiHeadersCreation;
/// <summary>
/// Initializes a new instance of the <see cref="ApiHeadersProvider"/> class.
/// </summary>
/// <param name="httpContextAccessor">The <see cref="IHttpContextAccessor"/> for accessing the <see cref="HttpContext"/>.</param>
/// <param name="logger">The <see cref="ILogger"/> to write to.</param>
/// <param name="httpContextAccessor">The value of <see cref="httpContextAccessor"/>.</param>
/// <param name="logger">The value of <see cref="logger"/>.</param>
public ApiHeadersProvider(IHttpContextAccessor httpContextAccessor, ILogger<ApiHeadersProvider> logger)
{
ArgumentNullException.ThrowIfNull(httpContextAccessor);
this.httpContextAccessor = httpContextAccessor ?? throw new ArgumentNullException(nameof(httpContextAccessor));
this.logger = logger ?? throw new ArgumentNullException(nameof(logger));
}
/// <inheritdoc />
public ApiHeaders CreateAuthlessHeaders() => CreateApiHeaders(false);
/// <summary>
/// Attempt to parse <see cref="Api.ApiHeaders"/> from the <see cref="HttpContext"/>, optionally populating the <see langword="class"/> properties.
/// </summary>
/// <param name="includeAuthAndSetProperties">If the <see cref="HeaderErrorTypes.AuthorizationMissing"/> error should be ignored and <see cref="ApiHeaders"/>/<see cref="HeadersException"/> should be populated.</param>
/// <returns>A newly parsed <see cref="Api.ApiHeaders"/> <see langword="class"/> or <see langword="null"/> if <paramref name="includeAuthAndSetProperties"/> was set and the parse failed.</returns>
ApiHeaders CreateApiHeaders(bool includeAuthAndSetProperties)
{
if (httpContextAccessor.HttpContext == null)
throw new InvalidOperationException("httpContextAccessor has no HttpContext!");
var request = httpContextAccessor.HttpContext.Request;
var ignoreMissingAuth = !includeAuthAndSetProperties;
if (includeAuthAndSetProperties)
attemptedApiHeadersCreation = true;
try
{
ApiHeaders = new ApiHeaders(request.GetTypedHeaders());
var headers = new ApiHeaders(request.GetTypedHeaders(), ignoreMissingAuth);
if (includeAuthAndSetProperties)
apiHeaders = headers;
return headers;
}
catch (HeadersException ex)
catch (HeadersException ex) when (includeAuthAndSetProperties)
{
// we are not responsible for handling header validation issues
logger.LogTrace(ex, "Failed to validated API request headers!");
logger.LogTrace(ex, "Failed to parse API headers!");
HeadersException = ex;
return null;
}
}
}
@@ -11,5 +11,18 @@ namespace Tgstation.Server.Host.Utils
/// The created <see cref="Api.ApiHeaders"/>, if any.
/// </summary>
ApiHeaders ApiHeaders { get; }
/// <summary>
/// The <see cref="Api.HeadersException"/> thrown when attempting to parse the <see cref="ApiHeaders"/> if any.
/// </summary>
HeadersException HeadersException { get; }
/// <summary>
/// Attempt to create <see cref="Api.ApiHeaders"/> without checking for the presence of an <see cref="Microsoft.Net.Http.Headers.HeaderNames.Authorization"/> header.
/// </summary>
/// <returns>A new <see cref="Api.ApiHeaders"/> <see langword="class"/>.</returns>
/// <remarks>This does not populate the <see cref="ApiHeaders"/> property.</remarks>
/// <exception cref="Api.HeadersException">Thrown if the requested <see cref="Api.ApiHeaders"/> contain errors other than <see cref="HeaderErrorTypes.AuthorizationMissing"/>.</exception>
ApiHeaders CreateAuthlessHeaders();
}
}
@@ -43,7 +43,7 @@ namespace Tgstation.Server.Api.Tests
{ "User-Agent", userAgent }
};
return new ApiHeaders(new RequestHeaders(headers));
return new ApiHeaders(new RequestHeaders(headers), false);
};
var header = TestHeader(BrowserHeader);