From 1e48a97294abb066cfd2a8f20ddc8e23907ffa05 Mon Sep 17 00:00:00 2001 From: Tom Pallister Date: Wed, 21 Feb 2018 18:54:11 +0000 Subject: [PATCH 1/2] tests showing how this could work (#241) * tests showing how this could work * test passing * test needs work * skip test as doesnt really do anything --- .../Middleware/ExceptionHandlerMiddleware.cs | 1 - .../CustomMiddlewareTests.cs | 94 +++++++++++++++++-- test/Ocelot.AcceptanceTests/Steps.cs | 29 ++++++ 3 files changed, 115 insertions(+), 9 deletions(-) diff --git a/src/Ocelot/Errors/Middleware/ExceptionHandlerMiddleware.cs b/src/Ocelot/Errors/Middleware/ExceptionHandlerMiddleware.cs index a08031ea..ee1f0de5 100644 --- a/src/Ocelot/Errors/Middleware/ExceptionHandlerMiddleware.cs +++ b/src/Ocelot/Errors/Middleware/ExceptionHandlerMiddleware.cs @@ -42,7 +42,6 @@ namespace Ocelot.Errors.Middleware _logger.LogDebug("ocelot pipeline started"); await _next.Invoke(context); - } catch (Exception e) { diff --git a/test/Ocelot.AcceptanceTests/CustomMiddlewareTests.cs b/test/Ocelot.AcceptanceTests/CustomMiddlewareTests.cs index 1e389881..3a2cc730 100644 --- a/test/Ocelot.AcceptanceTests/CustomMiddlewareTests.cs +++ b/test/Ocelot.AcceptanceTests/CustomMiddlewareTests.cs @@ -1,10 +1,12 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Net; using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; using Ocelot.Configuration.File; using Ocelot.Middleware; using Shouldly; @@ -61,7 +63,7 @@ namespace Ocelot.AcceptanceTests } }; - this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:41879", 200)) + this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:41879", 200, "")) .And(x => _steps.GivenThereIsAConfiguration(fileConfiguration, _configurationPath)) .And(x => _steps.GivenOcelotIsRunning(configuration)) .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) @@ -104,7 +106,7 @@ namespace Ocelot.AcceptanceTests } }; - this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:41879", 200)) + this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:41879", 200, "")) .And(x => _steps.GivenThereIsAConfiguration(fileConfiguration, _configurationPath)) .And(x => _steps.GivenOcelotIsRunning(configuration)) .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) @@ -147,7 +149,7 @@ namespace Ocelot.AcceptanceTests } }; - this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:41879", 200)) + this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:41879", 200, "")) .And(x => _steps.GivenThereIsAConfiguration(fileConfiguration, _configurationPath)) .And(x => _steps.GivenOcelotIsRunning(configuration)) .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) @@ -190,7 +192,7 @@ namespace Ocelot.AcceptanceTests } }; - this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:41879", 200)) + this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:41879", 200, "")) .And(x => _steps.GivenThereIsAConfiguration(fileConfiguration, _configurationPath)) .And(x => _steps.GivenOcelotIsRunning(configuration)) .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) @@ -233,7 +235,7 @@ namespace Ocelot.AcceptanceTests } }; - this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:41879", 200)) + this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:41879", 200, "")) .And(x => _steps.GivenThereIsAConfiguration(fileConfiguration, _configurationPath)) .And(x => _steps.GivenOcelotIsRunning(configuration)) .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) @@ -276,7 +278,7 @@ namespace Ocelot.AcceptanceTests } }; - this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:41879", 200)) + this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:41879", 200, "")) .And(x => _steps.GivenThereIsAConfiguration(fileConfiguration, _configurationPath)) .And(x => _steps.GivenOcelotIsRunning(configuration)) .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) @@ -285,12 +287,59 @@ namespace Ocelot.AcceptanceTests .BDDfy(); } + + [Fact(Skip = "This is just an example to show how you could hook into Ocelot pipeline with your own middleware. At the moment you must use Response.OnCompleted callback and cannot change the response :( I will see if this can be changed one day!")] + public void should_fix_issue_237() + { + Func callback = state => + { + var httpContext = (HttpContext)state; + + if (httpContext.Response.StatusCode > 400) + { + Debug.WriteLine("COUNT CALLED"); + Console.WriteLine("COUNT CALLED"); + } + + return Task.CompletedTask; + }; + + var fileConfiguration = new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/west", + DownstreamHostAndPorts = new List + { + new FileHostAndPort + { + Host = "localhost", + Port = 41880, + } + }, + DownstreamScheme = "http", + UpstreamPathTemplate = "/", + UpstreamHttpMethod = new List { "Get" }, + } + } + }; + + this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:41880", 200, "/test")) + .And(x => _steps.GivenThereIsAConfiguration(fileConfiguration, _configurationPath)) + .And(x => _steps.GivenOcelotIsRunningWithMiddleareBeforePipeline(callback)) + .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) + .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.NotFound)) + .BDDfy(); + } + private void ThenTheCounterIs(int expected) { _counter.ShouldBe(expected); } - private void GivenThereIsAServiceRunningOn(string url, int statusCode) + private void GivenThereIsAServiceRunningOn(string url, int statusCode, string basePath) { _builder = new WebHostBuilder() .UseUrls(url) @@ -300,9 +349,19 @@ namespace Ocelot.AcceptanceTests .UseUrls(url) .Configure(app => { + app.UsePathBase(basePath); app.Run(context => { - context.Response.StatusCode = statusCode; + + if(string.IsNullOrEmpty(basePath)) + { + context.Response.StatusCode = statusCode; + } + else if(context.Request.Path.Value != basePath) + { + context.Response.StatusCode = 404;; + } + return Task.CompletedTask; }); }) @@ -316,5 +375,24 @@ namespace Ocelot.AcceptanceTests _builder?.Dispose(); _steps.Dispose(); } + + public class FakeMiddleware + { + private readonly RequestDelegate _next; + private readonly Func _callback; + + public FakeMiddleware(RequestDelegate next, Func callback) + { + _next = next; + _callback = callback; + } + + public async Task Invoke(HttpContext context) + { + await _next(context); + + context.Response.OnCompleted(_callback, context); + } + } } } diff --git a/test/Ocelot.AcceptanceTests/Steps.cs b/test/Ocelot.AcceptanceTests/Steps.cs index 5aabc750..0f976473 100644 --- a/test/Ocelot.AcceptanceTests/Steps.cs +++ b/test/Ocelot.AcceptanceTests/Steps.cs @@ -103,6 +103,35 @@ namespace Ocelot.AcceptanceTests _ocelotClient = _ocelotServer.CreateClient(); } + public void GivenOcelotIsRunningWithMiddleareBeforePipeline(Func callback) + { + _webHostBuilder = new WebHostBuilder(); + + _webHostBuilder + .ConfigureAppConfiguration((hostingContext, config) => + { + config.SetBasePath(hostingContext.HostingEnvironment.ContentRootPath); + var env = hostingContext.HostingEnvironment; + config.AddJsonFile("appsettings.json", optional: true, reloadOnChange: true) + .AddJsonFile($"appsettings.{env.EnvironmentName}.json", optional: true, reloadOnChange: true); + config.AddJsonFile("configuration.json"); + config.AddEnvironmentVariables(); + }) + .ConfigureServices(s => + { + s.AddOcelot(); + }) + .Configure(app => + { + app.UseMiddleware(callback); + app.UseOcelot().Wait(); + }); + + _ocelotServer = new TestServer(_webHostBuilder); + + _ocelotClient = _ocelotServer.CreateClient(); + } + /// /// This is annoying cos it should be in the constructor but we need to set up the file before calling startup so its a step. /// From 9f7478c91f14cf834c72a019716576668c4efbb1 Mon Sep 17 00:00:00 2001 From: Tom Pallister Date: Wed, 21 Feb 2018 20:53:46 +0000 Subject: [PATCH 2/2] Feature/fix #240 (#243) * testing issue on train * check multiple claims of the same type for authorisation --- src/Ocelot/Authorisation/ClaimsAuthoriser.cs | 12 +- .../AuthorisationTests.cs | 128 ++++++++++++++++++ .../Authorization/ClaimsAuthoriserTests.cs | 20 ++- 3 files changed, 153 insertions(+), 7 deletions(-) diff --git a/src/Ocelot/Authorisation/ClaimsAuthoriser.cs b/src/Ocelot/Authorisation/ClaimsAuthoriser.cs index 29acb685..4db1045c 100644 --- a/src/Ocelot/Authorisation/ClaimsAuthoriser.cs +++ b/src/Ocelot/Authorisation/ClaimsAuthoriser.cs @@ -20,22 +20,22 @@ namespace Ocelot.Authorisation { foreach (var required in routeClaimsRequirement) { - var value = _claimsParser.GetValue(claimsPrincipal.Claims, required.Key, string.Empty, 0); + var values = _claimsParser.GetValuesByClaimType(claimsPrincipal.Claims, required.Key); - if (value.IsError) + if (values.IsError) { - return new ErrorResponse(value.Errors); + return new ErrorResponse(values.Errors); } - if (value.Data != null) + if (values.Data != null) { - var authorised = value.Data == required.Value; + var authorised = values.Data.Contains(required.Value); if (!authorised) { return new ErrorResponse(new List { new ClaimValueNotAuthorisedError( - $"claim value: {value.Data} is not the same as required value: {required.Value} for type: {required.Key}") + $"claim value: {values.Data} is not the same as required value: {required.Value} for type: {required.Key}") }); } } diff --git a/test/Ocelot.AcceptanceTests/AuthorisationTests.cs b/test/Ocelot.AcceptanceTests/AuthorisationTests.cs index b30f1db0..aaa4faff 100644 --- a/test/Ocelot.AcceptanceTests/AuthorisationTests.cs +++ b/test/Ocelot.AcceptanceTests/AuthorisationTests.cs @@ -235,6 +235,66 @@ namespace Ocelot.AcceptanceTests .BDDfy(); } + [Fact] + public void should_fix_issue_240() + { + var configuration = new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/", + DownstreamHostAndPorts = new List + { + new FileHostAndPort + { + Host = "localhost", + Port = 51876, + } + }, + DownstreamScheme = "http", + UpstreamPathTemplate = "/", + UpstreamHttpMethod = new List { "Get" }, + AuthenticationOptions = new FileAuthenticationOptions + { + AuthenticationProviderKey = "Test" + }, + RouteClaimsRequirement = + { + {"Role", "User"} + } + } + } + }; + + var users = new List + { + new TestUser + { + Username = "test", + Password = "test", + SubjectId = "registered|1231231", + Claims = new List + { + new Claim("Role", "AdminUser"), + new Claim("Role", "User") + }, + } + }; + + this.Given(x => x.GivenThereIsAnIdentityServerOn("http://localhost:51888", "api", AccessTokenType.Jwt, users)) + .And(x => x.GivenThereIsAServiceRunningOn("http://localhost:51876", 200, "Hello from Laura")) + .And(x => _steps.GivenIHaveAToken("http://localhost:51888")) + .And(x => _steps.GivenThereIsAConfiguration(configuration)) + .And(x => _steps.GivenOcelotIsRunning(_options, "Test")) + .And(x => _steps.GivenIHaveAddedATokenToMyRequest()) + .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) + .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) + .And(x => _steps.ThenTheResponseBodyShouldBe("Hello from Laura")) + .BDDfy(); + } + private void GivenThereIsAServiceRunningOn(string url, int statusCode, string responseBody) { _servicebuilder = new WebHostBuilder() @@ -335,7 +395,75 @@ namespace Ocelot.AcceptanceTests _identityServerBuilder.Start(); _steps.VerifyIdentiryServerStarted(url); + } + private void GivenThereIsAnIdentityServerOn(string url, string apiName, AccessTokenType tokenType, List users) + { + _identityServerBuilder = new WebHostBuilder() + .UseUrls(url) + .UseKestrel() + .UseContentRoot(Directory.GetCurrentDirectory()) + .UseIISIntegration() + .UseUrls(url) + .ConfigureServices(services => + { + services.AddLogging(); + services.AddIdentityServer() + .AddDeveloperSigningCredential() + .AddInMemoryApiResources(new List + { + new ApiResource + { + Name = apiName, + Description = "My API", + Enabled = true, + DisplayName = "test", + Scopes = new List() + { + new Scope("api"), + new Scope("api.readOnly"), + new Scope("openid"), + new Scope("offline_access"), + }, + ApiSecrets = new List() + { + new Secret + { + Value = "secret".Sha256() + } + }, + UserClaims = new List() + { + "CustomerId", "LocationId", "UserType", "UserId", "Role" + } + }, + + }) + .AddInMemoryClients(new List + { + new Client + { + ClientId = "client", + AllowedGrantTypes = GrantTypes.ResourceOwnerPassword, + ClientSecrets = new List {new Secret("secret".Sha256())}, + AllowedScopes = new List { apiName, "api.readOnly", "openid", "offline_access" }, + AccessTokenType = tokenType, + Enabled = true, + RequireClientSecret = false, + + } + }) + .AddTestUsers(users); + }) + .Configure(app => + { + app.UseIdentityServer(); + }) + .Build(); + + _identityServerBuilder.Start(); + + _steps.VerifyIdentiryServerStarted(url); } public void Dispose() diff --git a/test/Ocelot.UnitTests/Authorization/ClaimsAuthoriserTests.cs b/test/Ocelot.UnitTests/Authorization/ClaimsAuthoriserTests.cs index b435f7e7..0333268b 100644 --- a/test/Ocelot.UnitTests/Authorization/ClaimsAuthoriserTests.cs +++ b/test/Ocelot.UnitTests/Authorization/ClaimsAuthoriserTests.cs @@ -27,7 +27,25 @@ namespace Ocelot.UnitTests.Authorization { this.Given(x => x.GivenAClaimsPrincipal(new ClaimsPrincipal(new ClaimsIdentity(new List { - new Claim("UserType", "registered") + new Claim("UserType", "registered"), + + })))) + .And(x => x.GivenARouteClaimsRequirement(new Dictionary + { + {"UserType", "registered"} + })) + .When(x => x.WhenICallTheAuthoriser()) + .Then(x => x.ThenTheUserIsAuthorised()) + .BDDfy(); + } + + [Fact] + public void should_authorise_user_multiple_claims_of_same_type() + { + this.Given(x => x.GivenAClaimsPrincipal(new ClaimsPrincipal(new ClaimsIdentity(new List + { + new Claim("UserType", "guest"), + new Claim("UserType", "registered"), })))) .And(x => x.GivenARouteClaimsRequirement(new Dictionary {