Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ namespace Ocelot.Configuration.Creator
{
public interface ISecurityOptionsCreator
{
SecurityOptions Create(FileSecurityOptions securityOptions);
SecurityOptions Create(FileSecurityOptions securityOptions, FileGlobalConfiguration globalConfiguration);
}
}
2 changes: 1 addition & 1 deletion src/Ocelot/Configuration/Creator/RoutesCreator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ private DownstreamRoute SetUpDownstreamRoute(FileRoute fileRoute, FileGlobalConf

var lbOptions = _loadBalancerOptionsCreator.Create(fileRoute.LoadBalancerOptions);

var securityOptions = _securityOptionsCreator.Create(fileRoute.SecurityOptions);
var securityOptions = _securityOptionsCreator.Create(fileRoute.SecurityOptions, globalConfiguration);

var downstreamHttpVersion = _versionCreator.Create(fileRoute.DownstreamHttpVersion);

Expand Down
37 changes: 14 additions & 23 deletions src/Ocelot/Configuration/Creator/SecurityOptionsCreator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,35 +5,26 @@ namespace Ocelot.Configuration.Creator
{
public class SecurityOptionsCreator : ISecurityOptionsCreator
{
public SecurityOptions Create(FileSecurityOptions securityOptions)
{
var ipAllowedList = new List<string>();
var ipBlockedList = new List<string>();
public SecurityOptions Create(FileSecurityOptions securityOptions, FileGlobalConfiguration globalConfiguration)
=> Create(securityOptions.IsFullFilled() ? securityOptions : globalConfiguration.SecurityOptions);

foreach (var allowed in securityOptions.IPAllowedList)
{
if (IPAddressRange.TryParse(allowed, out var allowedIpAddressRange))
{
var allowedIps = allowedIpAddressRange.Select<IPAddress, string>(x => x.ToString());
ipAllowedList.AddRange(allowedIps);
}
}

foreach (var blocked in securityOptions.IPBlockedList)
{
if (IPAddressRange.TryParse(blocked, out var blockedIpAddressRange))
{
var blockedIps = blockedIpAddressRange.Select<IPAddress, string>(x => x.ToString());
ipBlockedList.AddRange(blockedIps);
}
}
private static SecurityOptions Create(FileSecurityOptions securityOptions)
{
var ipAllowedList = SetIpAddressList(securityOptions.IPAllowedList);
var ipBlockedList = SetIpAddressList(securityOptions.IPBlockedList);

if (securityOptions.ExcludeAllowedFromBlocked)
{
ipBlockedList = ipBlockedList.Except(ipAllowedList).ToList();
ipBlockedList = ipBlockedList.Except(ipAllowedList).ToArray();
}

return new SecurityOptions(ipAllowedList, ipBlockedList);
}
}

private static string[] SetIpAddressList(IList<string> ipValueList)
=> ipValueList
.Where(ipValue => IPAddressRange.TryParse(ipValue, out _))
.SelectMany(ipValue => IPAddressRange.Parse(ipValue).Select<IPAddress, string>(ip => ip.ToString()))
.ToArray();
}
}
3 changes: 3 additions & 0 deletions src/Ocelot/Configuration/File/FileGlobalConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public FileGlobalConfiguration()
HttpHandlerOptions = new FileHttpHandlerOptions();
CacheOptions = new FileCacheOptions();
MetadataOptions = new FileMetadataOptions();
SecurityOptions = new FileSecurityOptions();
}

public string RequestIdKey { get; set; }
Expand Down Expand Up @@ -48,5 +49,7 @@ public FileGlobalConfiguration()
public FileCacheOptions CacheOptions { get; set; }

public FileMetadataOptions MetadataOptions { get; set; }

public FileSecurityOptions SecurityOptions { get; set; }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
namespace Ocelot.Configuration.File
{
internal static class FileSecurityOptionsExtensions
{
internal static bool IsFullFilled(this FileSecurityOptions fileSecurityOptions)
=> fileSecurityOptions.IPAllowedList.Count > 0 || fileSecurityOptions.IPBlockedList.Count > 0;
}
}
14 changes: 7 additions & 7 deletions src/Ocelot/Configuration/SecurityOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ public class SecurityOptions
{
public SecurityOptions()
{
IPAllowedList = new();
IPBlockedList = new();
IPAllowedList = new List<string>();
IPBlockedList = new List<string>();
}

public SecurityOptions(string allowed = null, string blocked = null)
Expand All @@ -22,13 +22,13 @@ public SecurityOptions(string allowed = null, string blocked = null)
}
}

public SecurityOptions(List<string> allowedList = null, List<string> blockedList = null)
public SecurityOptions(IList<string> allowedList = null, IList<string> blockedList = null)
{
IPAllowedList = allowedList ?? new();
IPBlockedList = blockedList ?? new();
IPAllowedList = allowedList ?? new List<string>();
IPBlockedList = blockedList ?? new List<string>();
}

public List<string> IPAllowedList { get; }
public List<string> IPBlockedList { get; }
public IList<string> IPAllowedList { get; }
public IList<string> IPBlockedList { get; }
}
}
8 changes: 4 additions & 4 deletions src/Ocelot/Security/IPSecurity/IPSecurityPolicy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,18 @@ public async Task<Response> Security(DownstreamRoute downstreamRoute, HttpContex
return new OkResponse();
}

if (securityOptions.IPBlockedList != null)
if (securityOptions.IPBlockedList != null && securityOptions.IPBlockedList.Count > 0 && clientIp != null)
{
if (securityOptions.IPBlockedList.Exists(f => f == clientIp.ToString()))
if (securityOptions.IPBlockedList.Contains(clientIp.ToString()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ClientIp could be null, it's not handled

Copy link
Contributor Author

@Fabman08 Fabman08 Nov 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed checking clientIp

Copy link
Member

@raman-m raman-m Nov 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not to develop operators aka equality operator methods for IpAddress and string objects, if their classes have no already implemented ones? 😉
After operators implementation it will be possible to simply write: IPBlockedList.Contains(clientIp) because of IEquatable<T> interface.

See examples of operator implementations, current ones. The awesome possibility of operators as long as other methods is having 2 arguments of different types. But classic operators override the operation for 2 operands of the same type.

Different types of 2 operands:

public static bool operator ==(ServiceHostAndPort h, Lease l) => h == l.HostAndPort;
public static bool operator !=(ServiceHostAndPort h, Lease l) => !(h == l);
public static bool operator ==(Lease l, ServiceHostAndPort h) => l.HostAndPort == h;
public static bool operator !=(Lease l, ServiceHostAndPort h) => !(l == h);

The same types of 2 operands:

public static bool operator ==(Lease x, Lease y) => x.HostAndPort == y.HostAndPort; // ignore -> x.Connections == y.Connections;
public static bool operator !=(Lease x, Lease y) => !(x == y);

and
public static bool operator ==(MessageInvokerCacheKey left, MessageInvokerCacheKey right) => left.Equals(right);
public static bool operator !=(MessageInvokerCacheKey left, MessageInvokerCacheKey right) => !(left == right);

Cool, right?

Copy link
Member

@raman-m raman-m Nov 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The funny fact is that YARP project developers sometimes implement operators: only 4 types in whole project ! 🤣
But they are Microsoft senior engineers with 150K+ salaries, right @ggnaegi ? 😄
And those operators are pretty elementary, simple equality.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe in a next Feature PR ? 🤣

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, next time... 😋

{
var error = new UnauthenticatedError($" This request rejects access to {clientIp} IP");
return new ErrorResponse(error);
}
}

if (securityOptions.IPAllowedList?.Count > 0)
if (securityOptions.IPAllowedList != null && securityOptions.IPAllowedList.Count > 0 && clientIp != null)
{
if (!securityOptions.IPAllowedList.Exists(f => f == clientIp.ToString()))
if (!securityOptions.IPAllowedList.Contains(clientIp.ToString()))
{
var error = new UnauthenticatedError($"{clientIp} does not allow access, the request is invalid");
return new ErrorResponse(error);
Expand Down
163 changes: 163 additions & 0 deletions test/Ocelot.AcceptanceTests/Security/SecurityOptionsTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
using Microsoft.AspNetCore.Http;
using Ocelot.Configuration.File;

namespace Ocelot.AcceptanceTests.Security
{
public sealed class SecurityOptionsTests: Steps
{
private readonly ServiceHandler _serviceHandler;

public SecurityOptionsTests()
{
_serviceHandler = new ServiceHandler();
}

public override void Dispose()
{
_serviceHandler.Dispose();
base.Dispose();
}

[Fact]
public void should_call_with_allowed_ip_in_global_config()
{
var port = PortFinder.GetRandomPort();
var ip = Dns.GetHostAddresses("192.168.1.35")[0];
var route = GivenRoute(port, "/myPath", "/worldPath");
var configuration = GivenConfigurationWithSecurityOptions(route);

this.Given(x => x.GivenThereIsAServiceRunningOn(DownstreamUrl(port), ip))
.And(x => GivenThereIsAConfiguration(configuration))
.And(x => GivenOcelotIsRunning())
.When(x => WhenIGetUrlOnTheApiGateway("/worldPath"))
.Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK));
}

[Fact]
public void should_block_call_with_blocked_ip_in_global_config()
{
var port = PortFinder.GetRandomPort();
var ip = Dns.GetHostAddresses("192.168.1.55")[0];
var route = GivenRoute(port, "/myPath", "/worldPath");
var configuration = GivenConfigurationWithSecurityOptions(route);

this.Given(x => x.GivenThereIsAServiceRunningOn(DownstreamUrl(port), ip))
.And(x => GivenThereIsAConfiguration(configuration))
.And(x => GivenOcelotIsRunning())
.When(x => WhenIGetUrlOnTheApiGateway("/worldPath"))
.Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.Unauthorized));
}

[Fact]
public void should_call_with_allowed_ip_in_route_config()
{
var port = PortFinder.GetRandomPort();
var ip = Dns.GetHostAddresses("192.168.1.1")[0];
var securityConfig = new FileSecurityOptions
{
IPAllowedList = new List<string> { "192.168.1.1" },
};

var route = GivenRoute(port, "/myPath", "/worldPath", securityConfig);
var configuration = GivenConfiguration(route);

this.Given(x => x.GivenThereIsAServiceRunningOn(DownstreamUrl(port), ip))
.And(x => GivenThereIsAConfiguration(configuration))
.And(x => GivenOcelotIsRunning())
.When(x => WhenIGetUrlOnTheApiGateway("/worldPath"))
.Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK));
}

[Fact]
public void should_block_call_with_blocked_ip_in_route_config()
{
var port = PortFinder.GetRandomPort();
var ip = Dns.GetHostAddresses("192.168.1.1")[0];
var securityConfig = new FileSecurityOptions
{
IPBlockedList = new List<string> { "192.168.1.1" },
};

var route = GivenRoute(port, "/myPath", "/worldPath", securityConfig);
var configuration = GivenConfiguration(route);

this.Given(x => x.GivenThereIsAServiceRunningOn(DownstreamUrl(port), ip))
.And(x => GivenThereIsAConfiguration(configuration))
.And(x => GivenOcelotIsRunning())
.When(x => WhenIGetUrlOnTheApiGateway("/worldPath"))
.Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.Unauthorized));
}

[Fact]
public void should_call_with_allowed_ip_in_route_config_and_blocked_ip_in_global_config()
{
var port = PortFinder.GetRandomPort();
var ip = Dns.GetHostAddresses("192.168.1.55")[0];
var securityConfig = new FileSecurityOptions
{
IPAllowedList = new List<string> { "192.168.1.55" },
};

var route = GivenRoute(port, "/myPath", "/worldPath", securityConfig);
var configuration = GivenConfigurationWithSecurityOptions(route);

this.Given(x => x.GivenThereIsAServiceRunningOn(DownstreamUrl(port), ip))
.And(x => GivenThereIsAConfiguration(configuration))
.And(x => GivenOcelotIsRunning())
.When(x => WhenIGetUrlOnTheApiGateway("/worldPath"))
.Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK));
}

[Fact]
public void should_block_call_with_blocked_ip_in_route_config_and_allowed_ip_in_global_config()
{
var port = PortFinder.GetRandomPort();
var ip = Dns.GetHostAddresses("192.168.1.35")[0];
var securityConfig = new FileSecurityOptions
{
IPBlockedList = new List<string> { "192.168.1.35" },
};

var route = GivenRoute(port, "/myPath", "/worldPath", securityConfig);
var configuration = GivenConfigurationWithSecurityOptions(route);

this.Given(x => x.GivenThereIsAServiceRunningOn(DownstreamUrl(port), ip))
.And(x => GivenThereIsAConfiguration(configuration))
.And(x => GivenOcelotIsRunning())
.When(x => WhenIGetUrlOnTheApiGateway("/worldPath"))
.Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.Unauthorized));
}

private void GivenThereIsAServiceRunningOn(string url, IPAddress ipAddess)
{
_serviceHandler.GivenThereIsAServiceRunningOn(url, async context =>
{
context.Connection.RemoteIpAddress = ipAddess;
context.Response.StatusCode = (int)HttpStatusCode.OK;
await context.Response.WriteAsync("a valida response body");
});
}

private static FileConfiguration GivenConfigurationWithSecurityOptions(params FileRoute[] routes)
{
var config = GivenConfiguration(routes);
config.GlobalConfiguration.SecurityOptions = new FileSecurityOptions
{
IPAllowedList = new List<string> { "192.168.1.30-50" },
IPBlockedList = new List<string> { "192.168.1.1-100" },
ExcludeAllowedFromBlocked = true
};

return config;
}

private FileRoute GivenRoute(int port, string downstream, string upstream, FileSecurityOptions fileSecurityOptions = null)
=> new()
{
DownstreamPathTemplate = downstream,
UpstreamPathTemplate = upstream,
UpstreamHttpMethod = new List<string> { "Get" },
SecurityOptions = fileSecurityOptions ?? new FileSecurityOptions(),
};
}
}
2 changes: 1 addition & 1 deletion test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ private void ThenTheDepsAreCalledFor(FileRoute fileRoute, FileGlobalConfiguratio
_hfarCreator.Verify(x => x.Create(fileRoute), Times.Once);
_daCreator.Verify(x => x.Create(fileRoute), Times.Once);
_lboCreator.Verify(x => x.Create(fileRoute.LoadBalancerOptions), Times.Once);
_soCreator.Verify(x => x.Create(fileRoute.SecurityOptions), Times.Once);
_soCreator.Verify(x => x.Create(fileRoute.SecurityOptions, globalConfig), Times.Once);
_metadataCreator.Verify(x => x.Create(fileRoute.Metadata, globalConfig), Times.Once);
}
}
Expand Down
Loading