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/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/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.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. /// 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 {