From efa68e99492238d0c02e54219e352dc087be85b2 Mon Sep 17 00:00:00 2001 From: Juan Carlos Santana Herrera Date: Wed, 31 May 2017 18:27:28 +0100 Subject: [PATCH] Allowed scopes checking added to AuthorisationMiddleware. Acceptance tests added. --- src/Ocelot/Authorisation/ClaimsAuthoriser.cs | 2 +- .../{IAuthoriser.cs => IClaimsAuthoriser.cs} | 5 +- src/Ocelot/Authorisation/IScopesAuthoriser.cs | 12 +++ .../Middleware/AuthorisationMiddleware.cs | 54 +++++++++++-- .../Authorisation/ScopeNotAuthorisedError.cs | 12 +++ src/Ocelot/Authorisation/ScopesAuthoriser.cs | 51 ++++++++++++ .../ServiceCollectionExtensions.cs | 3 +- src/Ocelot/Errors/OcelotErrorCode.cs | 1 + .../Claims/Parser/ClaimsParser.cs | 15 +++- .../Claims/Parser/IClaimsParser.cs | 1 + .../Responder/ErrorsToHttpStatusCodeMapper.cs | 1 + .../AuthenticationTests.cs | 39 --------- .../AuthorisationTests.cs | 81 ++++++++++++++++++- .../ClaimsToHeadersForwardingTests.cs | 2 +- .../ClaimsToQueryStringForwardingTests.cs | 2 +- .../AuthorisationMiddlewareTests.cs | 7 +- 16 files changed, 232 insertions(+), 56 deletions(-) rename src/Ocelot/Authorisation/{IAuthoriser.cs => IClaimsAuthoriser.cs} (67%) create mode 100644 src/Ocelot/Authorisation/IScopesAuthoriser.cs create mode 100644 src/Ocelot/Authorisation/ScopeNotAuthorisedError.cs create mode 100644 src/Ocelot/Authorisation/ScopesAuthoriser.cs diff --git a/src/Ocelot/Authorisation/ClaimsAuthoriser.cs b/src/Ocelot/Authorisation/ClaimsAuthoriser.cs index 96f7acd6..13c18bb3 100644 --- a/src/Ocelot/Authorisation/ClaimsAuthoriser.cs +++ b/src/Ocelot/Authorisation/ClaimsAuthoriser.cs @@ -7,7 +7,7 @@ namespace Ocelot.Authorisation { using Infrastructure.Claims.Parser; - public class ClaimsAuthoriser : IAuthoriser + public class ClaimsAuthoriser : IClaimsAuthoriser { private readonly IClaimsParser _claimsParser; diff --git a/src/Ocelot/Authorisation/IAuthoriser.cs b/src/Ocelot/Authorisation/IClaimsAuthoriser.cs similarity index 67% rename from src/Ocelot/Authorisation/IAuthoriser.cs rename to src/Ocelot/Authorisation/IClaimsAuthoriser.cs index 08a7307f..0288403a 100644 --- a/src/Ocelot/Authorisation/IAuthoriser.cs +++ b/src/Ocelot/Authorisation/IClaimsAuthoriser.cs @@ -5,9 +5,8 @@ namespace Ocelot.Authorisation { using System.Collections.Generic; - public interface IAuthoriser + public interface IClaimsAuthoriser { - Response Authorise(ClaimsPrincipal claimsPrincipal, - Dictionary routeClaimsRequirement); + Response Authorise(ClaimsPrincipal claimsPrincipal, Dictionary routeClaimsRequirement); } } diff --git a/src/Ocelot/Authorisation/IScopesAuthoriser.cs b/src/Ocelot/Authorisation/IScopesAuthoriser.cs new file mode 100644 index 00000000..62b7bf93 --- /dev/null +++ b/src/Ocelot/Authorisation/IScopesAuthoriser.cs @@ -0,0 +1,12 @@ +using System.Security.Claims; +using Ocelot.Responses; + +namespace Ocelot.Authorisation +{ + using System.Collections.Generic; + + public interface IScopesAuthoriser + { + Response Authorise(ClaimsPrincipal claimsPrincipal, List routeAllowedScopes); + } +} diff --git a/src/Ocelot/Authorisation/Middleware/AuthorisationMiddleware.cs b/src/Ocelot/Authorisation/Middleware/AuthorisationMiddleware.cs index a86643a4..bfcd5f8d 100644 --- a/src/Ocelot/Authorisation/Middleware/AuthorisationMiddleware.cs +++ b/src/Ocelot/Authorisation/Middleware/AuthorisationMiddleware.cs @@ -1,6 +1,7 @@ using Ocelot.Infrastructure.RequestData; using Ocelot.Logging; using Ocelot.Responses; +using Ocelot.Configuration; namespace Ocelot.Authorisation.Middleware { @@ -13,17 +14,20 @@ namespace Ocelot.Authorisation.Middleware public class AuthorisationMiddleware : OcelotMiddleware { private readonly RequestDelegate _next; - private readonly IAuthoriser _authoriser; + private readonly IClaimsAuthoriser _claimsAuthoriser; + private readonly IScopesAuthoriser _scopesAuthoriser; private readonly IOcelotLogger _logger; public AuthorisationMiddleware(RequestDelegate next, IRequestScopedDataRepository requestScopedDataRepository, - IAuthoriser authoriser, + IClaimsAuthoriser claimsAuthoriser, + IScopesAuthoriser scopesAuthoriser, IOcelotLoggerFactory loggerFactory) : base(requestScopedDataRepository) { _next = next; - _authoriser = authoriser; + _claimsAuthoriser = claimsAuthoriser; + _scopesAuthoriser = scopesAuthoriser; _logger = loggerFactory.CreateLogger(); } @@ -31,11 +35,41 @@ namespace Ocelot.Authorisation.Middleware { _logger.LogDebug("started authorisation"); - if (DownstreamRoute.ReRoute.IsAuthorised) + if (IsAuthenticatedRoute(DownstreamRoute.ReRoute)) + { + _logger.LogDebug("route is authenticated scopes must be checked"); + + var authorised = _scopesAuthoriser.Authorise(context.User, DownstreamRoute.ReRoute.AuthenticationOptions.AllowedScopes); + + if (authorised.IsError) + { + _logger.LogDebug("error authorising user scopes"); + + SetPipelineError(authorised.Errors); + return; + } + + if (IsAuthorised(authorised)) + { + _logger.LogDebug("user scopes is authorised calling next authorisation checks"); + } + else + { + _logger.LogDebug("user scopes is not authorised setting pipeline error"); + + SetPipelineError(new List + { + new UnauthorisedError( + $"{context.User.Identity.Name} unable to access {DownstreamRoute.ReRoute.UpstreamPathTemplate.Value}") + }); + } + } + + if (IsAuthorisedRoute(DownstreamRoute.ReRoute)) { _logger.LogDebug("route is authorised"); - var authorised = _authoriser.Authorise(context.User, DownstreamRoute.ReRoute.RouteClaimsRequirement); + var authorised = _claimsAuthoriser.Authorise(context.User, DownstreamRoute.ReRoute.RouteClaimsRequirement); if (authorised.IsError) { @@ -78,5 +112,15 @@ namespace Ocelot.Authorisation.Middleware { return authorised.Data; } + + private static bool IsAuthenticatedRoute(ReRoute reRoute) + { + return reRoute.IsAuthenticated; + } + + private static bool IsAuthorisedRoute(ReRoute reRoute) + { + return reRoute.IsAuthorised; + } } } diff --git a/src/Ocelot/Authorisation/ScopeNotAuthorisedError.cs b/src/Ocelot/Authorisation/ScopeNotAuthorisedError.cs new file mode 100644 index 00000000..5460cb90 --- /dev/null +++ b/src/Ocelot/Authorisation/ScopeNotAuthorisedError.cs @@ -0,0 +1,12 @@ +using Ocelot.Errors; + +namespace Ocelot.Authorisation +{ + public class ScopeNotAuthorisedError : Error + { + public ScopeNotAuthorisedError(string message) + : base(message, OcelotErrorCode.ScopeNotAuthorisedError) + { + } + } +} diff --git a/src/Ocelot/Authorisation/ScopesAuthoriser.cs b/src/Ocelot/Authorisation/ScopesAuthoriser.cs new file mode 100644 index 00000000..6f32d6c3 --- /dev/null +++ b/src/Ocelot/Authorisation/ScopesAuthoriser.cs @@ -0,0 +1,51 @@ +using IdentityModel; +using Ocelot.Errors; +using Ocelot.Responses; +using System.Collections.Generic; +using System.Security.Claims; +using System.Linq; + +namespace Ocelot.Authorisation +{ + using Infrastructure.Claims.Parser; + + public class ScopesAuthoriser : IScopesAuthoriser + { + private readonly IClaimsParser _claimsParser; + + public ScopesAuthoriser(IClaimsParser claimsParser) + { + _claimsParser = claimsParser; + } + + public Response Authorise(ClaimsPrincipal claimsPrincipal, List routeAllowedScopes) + { + if (routeAllowedScopes == null || routeAllowedScopes.Count == 0) + { + return new OkResponse(true); + } + + var values = _claimsParser.GetValuesByClaimType(claimsPrincipal.Claims, JwtClaimTypes.Scope); + + if (values.IsError) + { + return new ErrorResponse(values.Errors); + } + + var userScopes = values.Data; + + List matchesScopes = routeAllowedScopes.Intersect(userScopes).ToList(); + + if (matchesScopes == null || matchesScopes.Count == 0) + { + return new ErrorResponse(new List + { + new ScopeNotAuthorisedError( + $"no one user scope: '{string.Join(",", userScopes)}' match with some allowed scope: '{string.Join(",", routeAllowedScopes)}'") + }); + } + + return new OkResponse(true); + } + } +} \ No newline at end of file diff --git a/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs b/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs index ef022923..d52341fe 100644 --- a/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs +++ b/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs @@ -144,7 +144,8 @@ namespace Ocelot.DependencyInjection services.TryAddSingleton(); services.TryAddSingleton(); services.TryAddSingleton(); - services.TryAddSingleton(); + services.TryAddSingleton(); + services.TryAddSingleton(); services.TryAddSingleton(); services.TryAddSingleton(); services.TryAddSingleton(); diff --git a/src/Ocelot/Errors/OcelotErrorCode.cs b/src/Ocelot/Errors/OcelotErrorCode.cs index de7960c4..cf191e63 100644 --- a/src/Ocelot/Errors/OcelotErrorCode.cs +++ b/src/Ocelot/Errors/OcelotErrorCode.cs @@ -17,6 +17,7 @@ InstructionNotForClaimsError, UnauthorizedError, ClaimValueNotAuthorisedError, + ScopeNotAuthorisedError, UserDoesNotHaveClaimError, DownstreamPathTemplateContainsSchemeError, DownstreamPathNullOrEmptyError, diff --git a/src/Ocelot/Infrastructure/Claims/Parser/ClaimsParser.cs b/src/Ocelot/Infrastructure/Claims/Parser/ClaimsParser.cs index e37f48fa..f5f3e3b4 100644 --- a/src/Ocelot/Infrastructure/Claims/Parser/ClaimsParser.cs +++ b/src/Ocelot/Infrastructure/Claims/Parser/ClaimsParser.cs @@ -1,10 +1,10 @@ namespace Ocelot.Infrastructure.Claims.Parser { + using Errors; + using Responses; using System.Collections.Generic; using System.Linq; using System.Security.Claims; - using Errors; - using Responses; public class ClaimsParser : IClaimsParser { @@ -37,6 +37,17 @@ return new OkResponse(value); } + + public Response> GetValuesByClaimType(IEnumerable claims, string claimType) + { + List values = new List(); + + values.AddRange(claims.Where(x => x.Type == claimType).Select(x => x.Value).ToList()); + + return new OkResponse>(values); + } + + private Response GetValue(IEnumerable claims, string key) { var claim = claims.FirstOrDefault(c => c.Type == key); diff --git a/src/Ocelot/Infrastructure/Claims/Parser/IClaimsParser.cs b/src/Ocelot/Infrastructure/Claims/Parser/IClaimsParser.cs index fa94cd22..1f6e65d3 100644 --- a/src/Ocelot/Infrastructure/Claims/Parser/IClaimsParser.cs +++ b/src/Ocelot/Infrastructure/Claims/Parser/IClaimsParser.cs @@ -7,5 +7,6 @@ public interface IClaimsParser { Response GetValue(IEnumerable claims, string key, string delimiter, int index); + Response> GetValuesByClaimType(IEnumerable claims, string claimType); } } \ No newline at end of file diff --git a/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs b/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs index 5c1e9fd0..be7c59b9 100644 --- a/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs +++ b/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs @@ -15,6 +15,7 @@ namespace Ocelot.Responder if (errors.Any(e => e.Code == OcelotErrorCode.UnauthorizedError || e.Code == OcelotErrorCode.ClaimValueNotAuthorisedError + || e.Code == OcelotErrorCode.ScopeNotAuthorisedError || e.Code == OcelotErrorCode.UserDoesNotHaveClaimError || e.Code == OcelotErrorCode.CannotFindClaimError)) { diff --git a/test/Ocelot.AcceptanceTests/AuthenticationTests.cs b/test/Ocelot.AcceptanceTests/AuthenticationTests.cs index 40ff7047..c0d143b9 100644 --- a/test/Ocelot.AcceptanceTests/AuthenticationTests.cs +++ b/test/Ocelot.AcceptanceTests/AuthenticationTests.cs @@ -189,45 +189,6 @@ namespace Ocelot.AcceptanceTests .BDDfy(); } - [Fact] - public void should_return_response_403_using_identity_server_with_scope_not_allowed() - { - var configuration = new FileConfiguration - { - ReRoutes = new List - { - new FileReRoute - { - DownstreamPathTemplate = _downstreamServicePath, - DownstreamPort = _downstreamServicePort, - DownstreamHost = _downstreamServiceHost, - DownstreamScheme = _downstreamServiceScheme, - UpstreamPathTemplate = "/", - UpstreamHttpMethod = new List { "Get" }, - AuthenticationOptions = new FileAuthenticationOptions - { - AllowedScopes = new List{ "api", "openid", "offline_access" }, - Provider = "IdentityServer", - ProviderRootUrl = _identityServerRootUrl, - RequireHttps = false, - ApiName = "api", - ApiSecret = "secret" - } - } - } - }; - - this.Given(x => x.GivenThereIsAnIdentityServerOn(_identityServerRootUrl, "api", "api2", AccessTokenType.Jwt)) - .And(x => x.GivenThereIsAServiceRunningOn(_downstreamServiceUrl, 200, "Hello from Laura")) - .And(x => _steps.GivenIHaveATokenForApiReadOnlyScope(_identityServerRootUrl)) - .And(x => _steps.GivenThereIsAConfiguration(configuration)) - .And(x => _steps.GivenOcelotIsRunning()) - .And(x => _steps.GivenIHaveAddedATokenToMyRequest()) - .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) - .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.Forbidden)) - .BDDfy(); - } - [Fact] public void should_return_201_using_identity_server_access_token() { diff --git a/test/Ocelot.AcceptanceTests/AuthorisationTests.cs b/test/Ocelot.AcceptanceTests/AuthorisationTests.cs index 2e4a446e..011bb679 100644 --- a/test/Ocelot.AcceptanceTests/AuthorisationTests.cs +++ b/test/Ocelot.AcceptanceTests/AuthorisationTests.cs @@ -140,6 +140,84 @@ namespace Ocelot.AcceptanceTests .BDDfy(); } + [Fact] + public void should_return_response_200_using_identity_server_with_allowed_scope() + { + var configuration = new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/", + DownstreamPort = 51876, + DownstreamHost = "localhost", + DownstreamScheme = "http", + UpstreamPathTemplate = "/", + UpstreamHttpMethod = new List { "Get" }, + AuthenticationOptions = new FileAuthenticationOptions + { + AllowedScopes = new List{ "api", "api.readOnly", "openid", "offline_access" }, + Provider = "IdentityServer", + ProviderRootUrl = "http://localhost:51888", + RequireHttps = false, + ApiName = "api", + ApiSecret = "secret" + } + } + } + }; + + this.Given(x => x.GivenThereIsAnIdentityServerOn("http://localhost:51888", "api", AccessTokenType.Jwt)) + .And(x => x.GivenThereIsAServiceRunningOn("http://localhost:51876", 200, "Hello from Laura")) + .And(x => _steps.GivenIHaveATokenForApiReadOnlyScope("http://localhost:51888")) + .And(x => _steps.GivenThereIsAConfiguration(configuration)) + .And(x => _steps.GivenOcelotIsRunning()) + .And(x => _steps.GivenIHaveAddedATokenToMyRequest()) + .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) + .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) + .BDDfy(); + } + + [Fact] + public void should_return_response_403_using_identity_server_with_scope_not_allowed() + { + var configuration = new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/", + DownstreamPort = 51876, + DownstreamHost = "localhost", + DownstreamScheme = "http", + UpstreamPathTemplate = "/", + UpstreamHttpMethod = new List { "Get" }, + AuthenticationOptions = new FileAuthenticationOptions + { + AllowedScopes = new List{ "api", "openid", "offline_access" }, + Provider = "IdentityServer", + ProviderRootUrl = "http://localhost:51888", + RequireHttps = false, + ApiName = "api", + ApiSecret = "secret" + } + } + } + }; + + this.Given(x => x.GivenThereIsAnIdentityServerOn("http://localhost:51888", "api", AccessTokenType.Jwt)) + .And(x => x.GivenThereIsAServiceRunningOn("http://localhost:51876", 200, "Hello from Laura")) + .And(x => _steps.GivenIHaveATokenForApiReadOnlyScope("http://localhost:51888")) + .And(x => _steps.GivenThereIsAConfiguration(configuration)) + .And(x => _steps.GivenOcelotIsRunning()) + .And(x => _steps.GivenIHaveAddedATokenToMyRequest()) + .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) + .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.Forbidden)) + .BDDfy(); + } + private void GivenThereIsAServiceRunningOn(string url, int statusCode, string responseBody) { _servicebuilder = new WebHostBuilder() @@ -185,6 +263,7 @@ namespace Ocelot.AcceptanceTests Scopes = new List() { new Scope("api"), + new Scope("api.readOnly"), new Scope("openid"), new Scope("offline_access") }, @@ -209,7 +288,7 @@ namespace Ocelot.AcceptanceTests ClientId = "client", AllowedGrantTypes = GrantTypes.ResourceOwnerPassword, ClientSecrets = new List {new Secret("secret".Sha256())}, - AllowedScopes = new List { apiName, "openid", "offline_access" }, + AllowedScopes = new List { apiName, "api.readOnly", "openid", "offline_access" }, AccessTokenType = tokenType, Enabled = true, RequireClientSecret = false diff --git a/test/Ocelot.AcceptanceTests/ClaimsToHeadersForwardingTests.cs b/test/Ocelot.AcceptanceTests/ClaimsToHeadersForwardingTests.cs index c2caa19e..88a294a0 100644 --- a/test/Ocelot.AcceptanceTests/ClaimsToHeadersForwardingTests.cs +++ b/test/Ocelot.AcceptanceTests/ClaimsToHeadersForwardingTests.cs @@ -61,7 +61,7 @@ namespace Ocelot.AcceptanceTests { AllowedScopes = new List { - "openid", "offline_access" + "openid", "offline_access", "api" }, Provider = "IdentityServer", ProviderRootUrl = "http://localhost:52888", diff --git a/test/Ocelot.AcceptanceTests/ClaimsToQueryStringForwardingTests.cs b/test/Ocelot.AcceptanceTests/ClaimsToQueryStringForwardingTests.cs index f2f1ca66..a6162c5f 100644 --- a/test/Ocelot.AcceptanceTests/ClaimsToQueryStringForwardingTests.cs +++ b/test/Ocelot.AcceptanceTests/ClaimsToQueryStringForwardingTests.cs @@ -61,7 +61,7 @@ namespace Ocelot.AcceptanceTests { AllowedScopes = new List { - "openid", "offline_access" + "openid", "offline_access", "api" }, Provider = "IdentityServer", ProviderRootUrl = "http://localhost:57888", diff --git a/test/Ocelot.UnitTests/Authorization/AuthorisationMiddlewareTests.cs b/test/Ocelot.UnitTests/Authorization/AuthorisationMiddlewareTests.cs index 9a890089..9bf15b42 100644 --- a/test/Ocelot.UnitTests/Authorization/AuthorisationMiddlewareTests.cs +++ b/test/Ocelot.UnitTests/Authorization/AuthorisationMiddlewareTests.cs @@ -25,7 +25,8 @@ namespace Ocelot.UnitTests.Authorization public class AuthorisationMiddlewareTests : IDisposable { private readonly Mock _scopedRepository; - private readonly Mock _authService; + private readonly Mock _authService; + private readonly Mock _authScopesService; private readonly string _url; private readonly TestServer _server; private readonly HttpClient _client; @@ -36,7 +37,8 @@ namespace Ocelot.UnitTests.Authorization { _url = "http://localhost:51879"; _scopedRepository = new Mock(); - _authService = new Mock(); + _authService = new Mock(); + _authScopesService = new Mock(); var builder = new WebHostBuilder() .ConfigureServices(x => @@ -44,6 +46,7 @@ namespace Ocelot.UnitTests.Authorization x.AddSingleton(); x.AddLogging(); x.AddSingleton(_authService.Object); + x.AddSingleton(_authScopesService.Object); x.AddSingleton(_scopedRepository.Object); }) .UseUrls(_url)