Skip to content

Commit

Permalink
[ODS-5481] Improve use of X-Forwarded-XYZ headers (5.3) (#524)
Browse files Browse the repository at this point in the history 
* Allow X-Forward-Proto from Elasticbeanstalk

* Determine rootUrl only once

* Introduce appsettings for default X-Forwarded-Host
and X-Forwarded-Port in case they are not included

* Rename file

* Tests for X-Forwarded-Host

* Change to manual test; add instructions

* Typo

* Address review comments

* Refactor to pass around an object with settings
And to throw an exception for settings misconfiguration.

* Refactor from "default" to "overrideFor"

* Fix unit test

Co-authored-by: Axel Marquez <[email protected]>
  • Loading branch information
@stephenfuqua @AxelMarquez
stephenfuqua and AxelMarquez authored Jul 7, 2022
1 parent 92d760a commit 6ae63dc
Show file tree
Hide file tree
Showing 12 changed files with 862 additions and 56 deletions.
11 changes: 5 additions & 6 deletions Application/EdFi.Ods.Api/Controllers/DataManagementControllerBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ public abstract class DataManagementControllerBase<TResourceReadModel, TResource

private readonly IRESTErrorProvider _restErrorProvider;
private readonly int _defaultPageLimitSize;
private readonly bool _useProxyHeaders;

private readonly ReverseProxySettings _reverseProxySettings;
private ILog _logger;
protected Lazy<DeletePipeline> DeletePipeline;
protected Lazy<GetPipeline<TResourceReadModel, TAggregateRoot>> GetByIdPipeline;
Expand Down Expand Up @@ -107,7 +106,7 @@ protected DataManagementControllerBase(
SchoolYearContextProvider = schoolYearContextProvider;
_restErrorProvider = restErrorProvider;
_defaultPageLimitSize = defaultPageSizeLimitProvider.GetDefaultPageSizeLimit();
_useProxyHeaders = apiSettings.UseReverseProxyHeaders.HasValue && apiSettings.UseReverseProxyHeaders.Value;
_reverseProxySettings = apiSettings.GetReverseProxySettings();

GetByIdPipeline = new Lazy<GetPipeline<TResourceReadModel, TAggregateRoot>>
(pipelineFactory.CreateGetPipeline<TResourceReadModel, TAggregateRoot>);
Expand Down Expand Up @@ -369,9 +368,9 @@ protected string GetResourceUrl()
try
{
var uriBuilder = new UriBuilder(
Request.Scheme(_useProxyHeaders),
Request.Host(_useProxyHeaders),
Request.Port(_useProxyHeaders),
Request.Scheme(this._reverseProxySettings),
Request.Host(this._reverseProxySettings),
Request.Port(this._reverseProxySettings),
Request.Path);

return uriBuilder.Uri.ToString().TrimEnd('/');
Expand Down
14 changes: 7 additions & 7 deletions Application/EdFi.Ods.Api/Controllers/VersionController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,13 @@ Dictionary<string, string> GetUrlsByName()
bool isYearSpecific = _apiSettings.GetApiMode().Equals(ApiMode.YearSpecific)
|| isInstanceYearSpecific;

bool useReverseProxyHeaders = _apiSettings.UseReverseProxyHeaders ?? false;

var urlsByName = new Dictionary<string, string>(StringComparer.InvariantCultureIgnoreCase);

var rootUrl = Request.RootUrl(_apiSettings.GetReverseProxySettings());

if (_apiSettings.IsFeatureEnabled(ApiFeature.AggregateDependencies.GetConfigKeyName()))
{
urlsByName["dependencies"] = Request.RootUrl(useReverseProxyHeaders) +
urlsByName["dependencies"] = rootUrl +
(isInstanceYearSpecific
? $"/metadata/data/v{ApiVersionConstants.Ods}/" + $"{instance}/" +
currentYear + "/dependencies"
Expand All @@ -100,7 +100,7 @@ Dictionary<string, string> GetUrlsByName()

if (_apiSettings.IsFeatureEnabled(ApiFeature.OpenApiMetadata.GetConfigKeyName()))
{
urlsByName["openApiMetadata"] = Request.RootUrl(useReverseProxyHeaders) + "/metadata/" +
urlsByName["openApiMetadata"] = rootUrl + "/metadata/" +
(isInstanceYearSpecific
? $"{instance}/"
: string.Empty) +
Expand All @@ -109,13 +109,13 @@ Dictionary<string, string> GetUrlsByName()
: string.Empty);
}

urlsByName["oauth"] = Request.RootUrl(useReverseProxyHeaders) +
urlsByName["oauth"] = rootUrl +
(isInstanceYearSpecific
? $"/{instance}"
: string.Empty) +
"/oauth/token";

urlsByName["dataManagementApi"] = Request.RootUrl(useReverseProxyHeaders) +
urlsByName["dataManagementApi"] = rootUrl +
$"/data/v{ApiVersionConstants.Ods}/" +
(isInstanceYearSpecific
? $"{instance}/"
Expand All @@ -126,7 +126,7 @@ Dictionary<string, string> GetUrlsByName()

if (_apiSettings.IsFeatureEnabled(ApiFeature.XsdMetadata.GetConfigKeyName()))
{
urlsByName["xsdMetadata"] = Request.RootUrl(useReverseProxyHeaders) + "/metadata/" +
urlsByName["xsdMetadata"] = rootUrl + "/metadata/" +
(isInstanceYearSpecific
? $"{instance}/"
: string.Empty) +
Expand Down
86 changes: 57 additions & 29 deletions Application/EdFi.Ods.Api/Extensions/HttpRequestExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,30 @@
using System.Linq;
using EdFi.Common.Extensions;
using EdFi.Ods.Api.Constants;
using EdFi.Ods.Common.Extensions;
using EdFi.Ods.Common.Configuration;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Primitives;

namespace EdFi.Ods.Api.Extensions
{
public static class HttpRequestExtensions
{
public static string RootUrl(this HttpRequest request, bool useProxyHeaders = false)
public static string RootUrl(this HttpRequest request, ReverseProxySettings reverseProxySettings)
{
var uriBuilder = new UriBuilder(
request.Scheme(useProxyHeaders),
request.Host(useProxyHeaders),
request.Port(useProxyHeaders),
request.Scheme(reverseProxySettings),
request.Host(reverseProxySettings),
request.Port(reverseProxySettings),
request.PathBase);

return uriBuilder.Uri.AbsoluteUri.TrimEnd('/');
}

public static string Scheme(this HttpRequest request, bool useProxyHeaders = false)
public static string Scheme(this HttpRequest request, ReverseProxySettings reverseProxySettings)
{
string scheme = request.Scheme;

if (!useProxyHeaders)
if (!reverseProxySettings.UseReverseProxyHeaders)
{
return scheme;
}
Expand All @@ -43,45 +43,73 @@ public static string Scheme(this HttpRequest request, bool useProxyHeaders = fal
scheme = proxyHeaderValue;
}

return scheme;
// The x-forwarded-proto header can contain a single originating protocol or, in some
// cases, multiple protocols. See ODS-5481 for more information. We care about the
// _first_ protocol listed.
if (scheme == null) { return "http"; }

return scheme.Split(',')[0];
}

public static string Host(this HttpRequest request, bool useProxyHeaders = false)
public static string Host(this HttpRequest request, ReverseProxySettings reverseProxySettings)
{
string host = request.Host.Host;

if (!useProxyHeaders)
// Use actual request host when not configured for use behind a reverse proxy
if (!reverseProxySettings.UseReverseProxyHeaders)
{
return host;
return request.Host.Host;
}

request.TryGetRequestHeader(HeaderConstants.XForwardedHost, out string proxyHeaderValue);
// Use override value if available
if (!string.IsNullOrWhiteSpace(reverseProxySettings.OverrideForForwardingHostServer))
{
return reverseProxySettings.OverrideForForwardingHostServer;
}

// Pass through for any value provided, null means header wasn't found so default to request
if (proxyHeaderValue != null)
// Try to extract a X-Forwarded-Host value
if (request.TryGetRequestHeader(HeaderConstants.XForwardedHost, out string proxyHeaderValue) &&
!string.IsNullOrWhiteSpace(proxyHeaderValue))
{
host = proxyHeaderValue;
// Return the forwarding host
return proxyHeaderValue;
}

return host;
return request.Host.Host;
}

public static int Port(this HttpRequest request, bool useProxyHeaders = false)
public static int Port(this HttpRequest request, ReverseProxySettings reverseProxySettings)
{
var defaultPortForScheme = Scheme(request, useProxyHeaders) == "https"
? 443
: 80;
// User actual request host when not configured for use behind a reverse proxy
if (!reverseProxySettings.UseReverseProxyHeaders)
{
return request.Host.Port ?? getDefaultPort();
}

if (!useProxyHeaders)
// Use override value if available
if (reverseProxySettings.OverrideForForwardingHostPort.HasValue)
{
return request.Host.Port ?? defaultPortForScheme;
return reverseProxySettings.OverrideForForwardingHostPort.Value;
}

request.TryGetRequestHeader(HeaderConstants.XForwardedPort, out string proxyHeaderValue);
// Try to extract a X-Forwarded-Port value
if (request.TryGetRequestHeader(HeaderConstants.XForwardedPort, out string proxyHeaderValue) &&
int.TryParse(proxyHeaderValue, out int port))
{
// Return the forwarding port
return port;
}

// Try to send the requested port
if (request.Host.Port.HasValue)
{
return request.Host.Port.Value;
}

return getDefaultPort();

return !int.TryParse(proxyHeaderValue, out int port)
? request.Host.Port ?? defaultPortForScheme
: port;
int getDefaultPort()
{
return Scheme(request, reverseProxySettings) == "https" ? 443 : 80;
}
}

public static string VirtualPath(this HttpRequest request)
Expand All @@ -103,4 +131,4 @@ public static bool TryGetRequestHeader(this HttpRequest request, string headerNa
return !string.IsNullOrEmpty(value);
}
}
}
}
9 changes: 9 additions & 0 deletions Application/EdFi.Ods.Common/Configuration/ApiSettings.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,15 @@ public ApiSettings()

public bool? UseReverseProxyHeaders { get; set; }

public string OverrideForForwardingHostServer { get; set; }

public int? OverrideForForwardingHostPort { get; set; }

public ReverseProxySettings GetReverseProxySettings()
{
return new ReverseProxySettings(this.UseReverseProxyHeaders, this.OverrideForForwardingHostServer, this.OverrideForForwardingHostPort);
}

public string PathBase { get; set; }

public DatabaseEngine GetDatabaseEngine() => _databaseEngine.Value;
Expand Down
19 changes: 19 additions & 0 deletions Application/EdFi.Ods.Common/Configuration/ReverseProxySettings.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@

namespace EdFi.Ods.Common.Configuration
{
public class ReverseProxySettings
{
public bool UseReverseProxyHeaders { get; private set; }

public string OverrideForForwardingHostServer { get; private set; }

public int? OverrideForForwardingHostPort { get; private set; }

public ReverseProxySettings(bool? useReverseProxyHeaders, string overrideForForwardingHostServer, int? overrideForForwardingHostPort)
{
this.UseReverseProxyHeaders = useReverseProxyHeaders ?? false;
this.OverrideForForwardingHostPort = overrideForForwardingHostPort;
this.OverrideForForwardingHostServer = overrideForForwardingHostServer;
}
}
}
8 changes: 5 additions & 3 deletions Application/EdFi.Ods.Features/Controllers/OpenApiMetadataController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ public class OpenApiMetadataController : ControllerBase
private readonly ILog _logger = LogManager.GetLogger(typeof(OpenApiMetadataController));

private readonly IOpenApiMetadataCacheProvider _openApiMetadataCacheProvider;
private readonly bool _useProxyHeaders;
private readonly ReverseProxySettings _reverseProxySettings;

public OpenApiMetadataController(
IOpenApiMetadataCacheProvider openApiMetadataCacheProvider,
ApiSettings apiSettings)
{
_openApiMetadataCacheProvider = openApiMetadataCacheProvider;
_useProxyHeaders = apiSettings.UseReverseProxyHeaders.HasValue && apiSettings.UseReverseProxyHeaders.Value;
_reverseProxySettings = apiSettings.GetReverseProxySettings();
_isEnabled = apiSettings.IsFeatureEnabled(ApiFeature.OpenApiMetadata.GetConfigKeyName());
}

Expand Down Expand Up @@ -86,11 +86,13 @@ public IActionResult Get([FromRoute] OpenApiMetadataSectionRequest request)

OpenApiMetadataSectionDetails GetSwaggerSectionDetailsForCacheItem(OpenApiContent apiContent)
{
var rootUrl = Request.RootUrl(this._reverseProxySettings);

// Construct fully qualified metadata url
var url =
new Uri(
new Uri(
new Uri(Request.RootUrl(_useProxyHeaders).EnsureSuffixApplied("/")),
new Uri(rootUrl.EnsureSuffixApplied("/")),
"metadata/"),
GetMetadataUrlSegmentForCacheItem(apiContent, request.SchoolYearFromRoute, request.InstanceIdFromRoute));

Expand Down
14 changes: 9 additions & 5 deletions ...ion/EdFi.Ods.Features/OpenApiMetadata/Providers/EnabledOpenApiMetadataDocumentProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public class EnabledOpenApiMetadataDocumentProvider : IOpenApiMetadataDocumentPr

private readonly IOpenApiMetadataCacheProvider _openApiMetadataCacheProvider;
private readonly IList<IOpenApiMetadataRouteInformation> _routeInformations;
private readonly bool _useReverseProxyHeaders;
private readonly ReverseProxySettings _reverseProxySettings;
private readonly Lazy<IReadOnlyList<SchemaNameMap>> _schemaNameMaps;

public EnabledOpenApiMetadataDocumentProvider(
Expand All @@ -38,9 +38,10 @@ public EnabledOpenApiMetadataDocumentProvider(
{
_openApiMetadataCacheProvider = openApiMetadataCacheProvider;
_routeInformations = routeInformations;
_useReverseProxyHeaders = apiSettings.UseReverseProxyHeaders.HasValue && apiSettings.UseReverseProxyHeaders.Value;
this._reverseProxySettings = apiSettings.GetReverseProxySettings();

_schemaNameMaps = new Lazy<IReadOnlyList<SchemaNameMap>>(schemaNameMapProvider.GetSchemaNameMaps);

}

public bool TryGetSwaggerDocument(HttpRequest request, out string document)
Expand Down Expand Up @@ -76,11 +77,14 @@ private string GetMetadataForContent(OpenApiContent content, HttpRequest request
.Replace("%HOST%", Host())
.Replace("%TOKEN_URL%", TokenUrl())
.Replace("%BASE_PATH%", basePath)
.Replace("%SCHEME%", request.Scheme(_useReverseProxyHeaders));
.Replace("%SCHEME%", request.Scheme(this._reverseProxySettings));

string TokenUrl() => $"{request.RootUrl(_useReverseProxyHeaders)}/{instanceId}oauth/token";
string TokenUrl() {
var rootUrl = request.RootUrl(this._reverseProxySettings);
return $"{rootUrl}/{instanceId}oauth/token";
}

string Host() => $"{request.Host(_useReverseProxyHeaders)}:{request.Port(_useReverseProxyHeaders)}";
string Host() => $"{request.Host(this._reverseProxySettings)}:{request.Port(this._reverseProxySettings)}";
}

private OpenApiMetadataRequest CreateOpenApiMetadataRequest(string path)
Expand Down
5 changes: 2 additions & 3 deletions Application/EdFi.Ods.Features/XsdMetadata/XsdMetadataController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,8 @@ private string GetMetadataAbsoluteUrl(string schemaFile, string uriSegment)
bool isYearSpecific = _apiSettings.GetApiMode().Equals(ApiMode.YearSpecific)
|| isInstanceYearSpecific;

bool useReverseProxyHeaders = _apiSettings.UseReverseProxyHeaders ?? false;

string basicPath = Request.RootUrl(useReverseProxyHeaders) + "/metadata/" +
string basicPath = Request.RootUrl(_apiSettings.GetReverseProxySettings())
+ "/metadata/" +
(isInstanceYearSpecific
? $"{_instanceIdContextProvider.GetInstanceId()}/"
: string.Empty) +
Expand Down
Loading

0 comments on commit 6ae63dc

Please sign in to comment.