From 7ef26f5f4b114f5fe29450bdc19da1e14de6cc77 Mon Sep 17 00:00:00 2001 From: Tom Gardham-Pallister Date: Tue, 29 Aug 2017 20:47:52 +0100 Subject: [PATCH 1/4] fixes issue #117 --- .../Finder/DownstreamRouteFinder.cs | 3 ++ .../DownstreamRouteFinderMiddleware.cs | 2 +- test/Ocelot.AcceptanceTests/RoutingTests.cs | 37 +++++++++++++++- .../DownstreamRouteFinderTests.cs | 43 ++++++++++++++++++- 4 files changed, 81 insertions(+), 4 deletions(-) diff --git a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs index bf0d5a527..cd2c098f6 100644 --- a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs +++ b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs @@ -6,6 +6,7 @@ using Ocelot.DownstreamRouteFinder.UrlMatcher; using Ocelot.Errors; using Ocelot.Responses; +using Ocelot.Utilities; namespace Ocelot.DownstreamRouteFinder.Finder { @@ -24,6 +25,8 @@ public DownstreamRouteFinder(IOcelotConfigurationProvider configProvider, IUrlPa public async Task> FindDownstreamRoute(string upstreamUrlPath, string upstreamHttpMethod) { + upstreamUrlPath = upstreamUrlPath.SetLastCharacterAs('/'); + var configuration = await _configProvider.Get(); var applicableReRoutes = configuration.Data.ReRoutes.Where(r => r.UpstreamHttpMethod.Count == 0 || r.UpstreamHttpMethod.Select(x => x.Method.ToLower()).Contains(upstreamHttpMethod.ToLower())); diff --git a/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs b/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs index 51cb3a0b0..f421d6d8e 100644 --- a/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs +++ b/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs @@ -30,7 +30,7 @@ public DownstreamRouteFinderMiddleware(RequestDelegate next, public async Task Invoke(HttpContext context) { - var upstreamUrlPath = context.Request.Path.ToString().SetLastCharacterAs('/'); + var upstreamUrlPath = context.Request.Path.ToString(); _logger.LogDebug("upstream url path is {upstreamUrlPath}", upstreamUrlPath); diff --git a/test/Ocelot.AcceptanceTests/RoutingTests.cs b/test/Ocelot.AcceptanceTests/RoutingTests.cs index b32c46902..fbdf57e11 100644 --- a/test/Ocelot.AcceptanceTests/RoutingTests.cs +++ b/test/Ocelot.AcceptanceTests/RoutingTests.cs @@ -6,6 +6,7 @@ using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Ocelot.Configuration.File; +using Shouldly; using TestStack.BDDfy; using Xunit; @@ -15,6 +16,7 @@ public class RoutingTests : IDisposable { private IWebHost _builder; private readonly Steps _steps; + private string _downstreamPath; public RoutingTests() { @@ -232,6 +234,34 @@ public void should_return_response_200_with_complex_url() .BDDfy(); } + + [Fact] + public void should_not_add_trailing_slash_to_downstream_url() + { + var configuration = new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/api/products/{productId}", + DownstreamScheme = "http", + DownstreamHost = "localhost", + DownstreamPort = 51879, + UpstreamPathTemplate = "/products/{productId}", + UpstreamHttpMethod = new List { "Get" }, + } + } + }; + + this.Given(x => GivenThereIsAServiceRunningOn("http://localhost:51879/api/products/1", 200, "Some Product")) + .And(x => _steps.GivenThereIsAConfiguration(configuration)) + .And(x => _steps.GivenOcelotIsRunning()) + .When(x => _steps.WhenIGetUrlOnTheApiGateway("/products/1")) + .Then(x => ThenTheDownstreamUrlPathShouldBe("/api/products/1")) + .BDDfy(); + } + [Fact] public void should_return_response_201_with_simple_url() { @@ -379,11 +409,11 @@ private void GivenThereIsAServiceRunningOn(string url, int statusCode, string re .UseKestrel() .UseContentRoot(Directory.GetCurrentDirectory()) .UseIISIntegration() - .UseUrls(url) .Configure(app => { app.Run(async context => { + _downstreamPath = context.Request.PathBase.Value; context.Response.StatusCode = statusCode; await context.Response.WriteAsync(responseBody); }); @@ -393,6 +423,11 @@ private void GivenThereIsAServiceRunningOn(string url, int statusCode, string re _builder.Start(); } + internal void ThenTheDownstreamUrlPathShouldBe(string expectedDownstreamPath) + { + _downstreamPath.ShouldBe(expectedDownstreamPath); + } + public void Dispose() { _builder?.Dispose(); diff --git a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs index 8514786dd..8dbe6ef49 100644 --- a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs +++ b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs @@ -36,7 +36,7 @@ public DownstreamRouteFinderTests() [Fact] public void should_return_route() { - this.Given(x => x.GivenThereIsAnUpstreamUrlPath("matchInUrlMatcher")) + this.Given(x => x.GivenThereIsAnUpstreamUrlPath("matchInUrlMatcher/")) .And(x =>x.GivenTheTemplateVariableAndNameFinderReturns( new OkResponse>( new List()))) @@ -65,6 +65,39 @@ public void should_return_route() .BDDfy(); } + + [Fact] + public void should_append_slash_to_upstream_url_path() + { + this.Given(x => x.GivenThereIsAnUpstreamUrlPath("matchInUrlMatcher")) + .And(x =>x.GivenTheTemplateVariableAndNameFinderReturns( + new OkResponse>( + new List()))) + .And(x => x.GivenTheConfigurationIs(new List + { + new ReRouteBuilder() + .WithDownstreamPathTemplate("someDownstreamPath") + .WithUpstreamPathTemplate("someUpstreamPath") + .WithUpstreamHttpMethod(new List { "Get" }) + .WithUpstreamTemplatePattern("someUpstreamPath") + .Build() + }, string.Empty + )) + .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) + .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) + .When(x => x.WhenICallTheFinder()) + .Then( + x => x.ThenTheFollowingIsReturned(new DownstreamRoute( + new List(), + new ReRouteBuilder() + .WithDownstreamPathTemplate("someDownstreamPath") + .WithUpstreamHttpMethod(new List { "Get" }) + .Build() + ))) + .And(x => x.ThenTheUrlMatcherIsCalledCorrectly("matchInUrlMatcher/")) + .BDDfy(); + } + [Fact] public void should_return_route_if_upstream_path_and_upstream_template_are_the_same() { @@ -137,7 +170,7 @@ public void should_return_correct_route_for_http_verb() [Fact] public void should_not_return_route() { - this.Given(x => x.GivenThereIsAnUpstreamUrlPath("dontMatchPath")) + this.Given(x => x.GivenThereIsAnUpstreamUrlPath("dontMatchPath/")) .And(x => x.GivenTheConfigurationIs(new List { new ReRouteBuilder() @@ -269,6 +302,12 @@ private void ThenTheUrlMatcherIsCalledCorrectly() .Verify(x => x.Match(_upstreamUrlPath, _reRoutesConfig[0].UpstreamPathTemplate.Value), Times.Once); } + private void ThenTheUrlMatcherIsCalledCorrectly(string expectedUpstreamUrlPath) + { + _mockMatcher + .Verify(x => x.Match(expectedUpstreamUrlPath, _reRoutesConfig[0].UpstreamPathTemplate.Value), Times.Once); + } + private void ThenTheUrlMatcherIsNotCalled() { _mockMatcher From 3cfa7fe6edfae047453a3fdace863e720d95915a Mon Sep 17 00:00:00 2001 From: Tom Gardham-Pallister Date: Wed, 30 Aug 2017 18:25:48 +0100 Subject: [PATCH 2/4] added more tests to get build working --- .../Infrastructure/ScopesAuthoriserTests.cs | 124 ++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 test/Ocelot.UnitTests/Infrastructure/ScopesAuthoriserTests.cs diff --git a/test/Ocelot.UnitTests/Infrastructure/ScopesAuthoriserTests.cs b/test/Ocelot.UnitTests/Infrastructure/ScopesAuthoriserTests.cs new file mode 100644 index 000000000..eef569826 --- /dev/null +++ b/test/Ocelot.UnitTests/Infrastructure/ScopesAuthoriserTests.cs @@ -0,0 +1,124 @@ +using Xunit; +using Shouldly; +using Ocelot.Authorisation; +using Ocelot.Infrastructure.Claims.Parser; +using Moq; +using System.Collections.Generic; +using System.Security.Claims; +using Ocelot.Responses; +using TestStack.BDDfy; +using Ocelot.Errors; + +namespace Ocelot.UnitTests.Infrastructure +{ + public class ScopesAuthoriserTests + { + private ScopesAuthoriser _authoriser; + public Mock _parser; + private ClaimsPrincipal _principal; + private List _allowedScopes; + private Response _result; + + public ScopesAuthoriserTests() + { + _parser = new Mock(); + _authoriser = new ScopesAuthoriser(_parser.Object); + } + + [Fact] + public void should_return_ok_if_no_allowed_scopes() + { + this.Given(_ => GivenTheFollowing(new ClaimsPrincipal())) + .And(_ => GivenTheFollowing(new List())) + .When(_ => WhenIAuthorise()) + .Then(_ => ThenTheFollowingIsReturned(new OkResponse(true))) + .BDDfy(); + } + + + [Fact] + public void should_return_ok_if_null_allowed_scopes() + { + this.Given(_ => GivenTheFollowing(new ClaimsPrincipal())) + .And(_ => GivenTheFollowing((List)null)) + .When(_ => WhenIAuthorise()) + .Then(_ => ThenTheFollowingIsReturned(new OkResponse(true))) + .BDDfy(); + } + + [Fact] + public void should_return_error_if_claims_parser_returns_error() + { + var fakeError = new FakeError(); + this.Given(_ => GivenTheFollowing(new ClaimsPrincipal())) + .And(_ => GivenTheParserReturns(new ErrorResponse>(fakeError))) + .And(_ => GivenTheFollowing(new List(){"doesntmatter"})) + .When(_ => WhenIAuthorise()) + .Then(_ => ThenTheFollowingIsReturned(new ErrorResponse(fakeError))) + .BDDfy(); + } + + [Fact] + public void should_match_scopes_and_return_ok_result() + { + var claimsPrincipal = new ClaimsPrincipal(); + var allowedScopes = new List(){"someScope"}; + + this.Given(_ => GivenTheFollowing(claimsPrincipal)) + .And(_ => GivenTheParserReturns(new OkResponse>(allowedScopes))) + .And(_ => GivenTheFollowing(allowedScopes)) + .When(_ => WhenIAuthorise()) + .Then(_ => ThenTheFollowingIsReturned(new OkResponse(true))) + .BDDfy(); + } + + [Fact] + public void should_not_match_scopes_and_return_error_result() + { + var fakeError = new FakeError(); + var claimsPrincipal = new ClaimsPrincipal(); + var allowedScopes = new List(){"someScope"}; + var userScopes = new List(){"anotherScope"}; + + this.Given(_ => GivenTheFollowing(claimsPrincipal)) + .And(_ => GivenTheParserReturns(new OkResponse>(userScopes))) + .And(_ => GivenTheFollowing(allowedScopes)) + .When(_ => WhenIAuthorise()) + .Then(_ => ThenTheFollowingIsReturned(new ErrorResponse(fakeError))) + .BDDfy(); + } + + private void GivenTheParserReturns(Response> response) + { + _parser.Setup(x => x.GetValuesByClaimType(It.IsAny>(), It.IsAny())).Returns(response); + } + + private void GivenTheFollowing(ClaimsPrincipal principal) + { + _principal = principal; + } + + private void GivenTheFollowing(List allowedScopes) + { + _allowedScopes = allowedScopes; + } + + private void WhenIAuthorise() + { + _result = _authoriser.Authorise(_principal, _allowedScopes); + } + + private void ThenTheFollowingIsReturned(Response expected) + { + _result.Data.ShouldBe(expected.Data); + _result.IsError.ShouldBe(expected.IsError); + } + } + + public class FakeError : Error + { + public FakeError() : base("fake error", OcelotErrorCode.CannotAddDataError) + { + } + } +} \ No newline at end of file From 207a3882dcad94083620ad6c452053e4d92da56a Mon Sep 17 00:00:00 2001 From: TomPallister Date: Mon, 11 Sep 2017 21:48:49 +0100 Subject: [PATCH 3/4] attempt to fix consul problem --- .../Configuration/Creator/FileOcelotConfigurationCreator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs b/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs index c6f5f4fe9..9b5676d4b 100644 --- a/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs +++ b/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs @@ -178,7 +178,7 @@ private async Task SetUpReRoute(FileReRoute fileReRoute, FileGlobalConf private string CreateReRouteKey(FileReRoute fileReRoute) { //note - not sure if this is the correct key, but this is probably the only unique key i can think of given my poor brain - var loadBalancerKey = $"{fileReRoute.UpstreamPathTemplate}{fileReRoute.UpstreamHttpMethod}"; + var loadBalancerKey = $"{fileReRoute.UpstreamPathTemplate}|{string.Join(",", fileReRoute.UpstreamHttpMethod)}"; return loadBalancerKey; } From 7ac6df71fbd9c9b31da01c7e14eba6a53d274278 Mon Sep 17 00:00:00 2001 From: Tom Pallister Date: Mon, 9 Oct 2017 11:21:19 +0100 Subject: [PATCH 4/4] fixed build --- build.cake | 3 +- build.ps1 | 121 +++++++++++++++++++++++++++++------------- tools/packages.config | 2 +- 3 files changed, 86 insertions(+), 40 deletions(-) diff --git a/build.cake b/build.cake index 9960c5880..f64519bd6 100644 --- a/build.cake +++ b/build.cake @@ -1,6 +1,7 @@ #tool "nuget:?package=GitVersion.CommandLine" #tool "nuget:?package=GitReleaseNotes" -#addin "nuget:?package=Cake.Json" +#addin nuget:?package=Cake.Json +#addin nuget:?package=Newtonsoft.Json&version=9.0.1 #tool "nuget:?package=OpenCover" #tool "nuget:?package=ReportGenerator" #tool coveralls.net diff --git a/build.ps1 b/build.ps1 index 44de57932..265bd1238 100644 --- a/build.ps1 +++ b/build.ps1 @@ -21,34 +21,35 @@ The build script target to run. The build configuration to use. .PARAMETER Verbosity Specifies the amount of information to be displayed. +.PARAMETER ShowDescription +Shows description about tasks. +.PARAMETER DryRun +Performs a dry run. .PARAMETER Experimental -Tells Cake to use the latest Roslyn release. -.PARAMETER WhatIf -Performs a dry run of the build script. -No tasks will be executed. +Uses the nightly builds of the Roslyn script engine. .PARAMETER Mono -Tells Cake to use the Mono scripting engine. +Uses the Mono Compiler rather than the Roslyn script engine. .PARAMETER SkipToolPackageRestore Skips restoring of packages. .PARAMETER ScriptArgs Remaining arguments are added here. .LINK -http://cakebuild.net +https://cakebuild.net #> [CmdletBinding()] Param( [string]$Script = "build.cake", - [string]$Target = "Default", - [ValidateSet("Release", "Debug")] - [string]$Configuration = "Release", + [string]$Target, + [string]$Configuration, [ValidateSet("Quiet", "Minimal", "Normal", "Verbose", "Diagnostic")] - [string]$Verbosity = "Verbose", + [string]$Verbosity, + [switch]$ShowDescription, + [Alias("WhatIf", "Noop")] + [switch]$DryRun, [switch]$Experimental, - [Alias("DryRun","Noop")] - [switch]$WhatIf, [switch]$Mono, [switch]$SkipToolPackageRestore, [Parameter(Position=0,Mandatory=$false,ValueFromRemainingArguments=$true)] @@ -80,6 +81,15 @@ function MD5HashFile([string] $filePath) } } +function GetProxyEnabledWebClient +{ + $wc = New-Object System.Net.WebClient + $proxy = [System.Net.WebRequest]::GetSystemWebProxy() + $proxy.Credentials = [System.Net.CredentialCache]::DefaultCredentials + $wc.Proxy = $proxy + return $wc +} + Write-Host "Preparing to run build script..." if(!$PSScriptRoot){ @@ -87,31 +97,15 @@ if(!$PSScriptRoot){ } $TOOLS_DIR = Join-Path $PSScriptRoot "tools" +$ADDINS_DIR = Join-Path $TOOLS_DIR "Addins" +$MODULES_DIR = Join-Path $TOOLS_DIR "Modules" $NUGET_EXE = Join-Path $TOOLS_DIR "nuget.exe" $CAKE_EXE = Join-Path $TOOLS_DIR "Cake/Cake.exe" $NUGET_URL = "https://dist.nuget.org/win-x86-commandline/latest/nuget.exe" $PACKAGES_CONFIG = Join-Path $TOOLS_DIR "packages.config" $PACKAGES_CONFIG_MD5 = Join-Path $TOOLS_DIR "packages.config.md5sum" - -# Should we use mono? -$UseMono = ""; -if($Mono.IsPresent) { - Write-Verbose -Message "Using the Mono based scripting engine." - $UseMono = "-mono" -} - -# Should we use the new Roslyn? -$UseExperimental = ""; -if($Experimental.IsPresent -and !($Mono.IsPresent)) { - Write-Verbose -Message "Using experimental version of Roslyn." - $UseExperimental = "-experimental" -} - -# Is this a dry run? -$UseDryRun = ""; -if($WhatIf.IsPresent) { - $UseDryRun = "-dryrun" -} +$ADDINS_PACKAGES_CONFIG = Join-Path $ADDINS_DIR "packages.config" +$MODULES_PACKAGES_CONFIG = Join-Path $MODULES_DIR "packages.config" # Make sure tools folder exists if ((Test-Path $PSScriptRoot) -and !(Test-Path $TOOLS_DIR)) { @@ -121,8 +115,10 @@ if ((Test-Path $PSScriptRoot) -and !(Test-Path $TOOLS_DIR)) { # Make sure that packages.config exist. if (!(Test-Path $PACKAGES_CONFIG)) { - Write-Verbose -Message "Downloading packages.config..." - try { (New-Object System.Net.WebClient).DownloadFile("http://cakebuild.net/download/bootstrapper/packages", $PACKAGES_CONFIG) } catch { + Write-Verbose -Message "Downloading packages.config..." + try { + $wc = GetProxyEnabledWebClient + $wc.DownloadFile("https://cakebuild.net/download/bootstrapper/packages", $PACKAGES_CONFIG) } catch { Throw "Could not download packages.config." } } @@ -130,7 +126,7 @@ if (!(Test-Path $PACKAGES_CONFIG)) { # Try find NuGet.exe in path if not exists if (!(Test-Path $NUGET_EXE)) { Write-Verbose -Message "Trying to find nuget.exe in PATH..." - $existingPaths = $Env:Path -Split ';' | Where-Object { (![string]::IsNullOrEmpty($_)) -and (Test-Path $_) } + $existingPaths = $Env:Path -Split ';' | Where-Object { (![string]::IsNullOrEmpty($_)) -and (Test-Path $_ -PathType Container) } $NUGET_EXE_IN_PATH = Get-ChildItem -Path $existingPaths -Filter "nuget.exe" | Select -First 1 if ($NUGET_EXE_IN_PATH -ne $null -and (Test-Path $NUGET_EXE_IN_PATH.FullName)) { Write-Verbose -Message "Found in PATH at $($NUGET_EXE_IN_PATH.FullName)." @@ -142,7 +138,8 @@ if (!(Test-Path $NUGET_EXE)) { if (!(Test-Path $NUGET_EXE)) { Write-Verbose -Message "Downloading NuGet.exe..." try { - (New-Object System.Net.WebClient).DownloadFile($NUGET_URL, $NUGET_EXE) + $wc = GetProxyEnabledWebClient + $wc.DownloadFile($NUGET_URL, $NUGET_EXE) } catch { Throw "Could not download NuGet.exe." } @@ -175,6 +172,41 @@ if(-Not $SkipToolPackageRestore.IsPresent) { $md5Hash | Out-File $PACKAGES_CONFIG_MD5 -Encoding "ASCII" } Write-Verbose -Message ($NuGetOutput | out-string) + + Pop-Location +} + +# Restore addins from NuGet +if (Test-Path $ADDINS_PACKAGES_CONFIG) { + Push-Location + Set-Location $ADDINS_DIR + + Write-Verbose -Message "Restoring addins from NuGet..." + $NuGetOutput = Invoke-Expression "&`"$NUGET_EXE`" install -ExcludeVersion -OutputDirectory `"$ADDINS_DIR`"" + + if ($LASTEXITCODE -ne 0) { + Throw "An error occured while restoring NuGet addins." + } + + Write-Verbose -Message ($NuGetOutput | out-string) + + Pop-Location +} + +# Restore modules from NuGet +if (Test-Path $MODULES_PACKAGES_CONFIG) { + Push-Location + Set-Location $MODULES_DIR + + Write-Verbose -Message "Restoring modules from NuGet..." + $NuGetOutput = Invoke-Expression "&`"$NUGET_EXE`" install -ExcludeVersion -OutputDirectory `"$MODULES_DIR`"" + + if ($LASTEXITCODE -ne 0) { + Throw "An error occured while restoring NuGet modules." + } + + Write-Verbose -Message ($NuGetOutput | out-string) + Pop-Location } @@ -183,7 +215,20 @@ if (!(Test-Path $CAKE_EXE)) { Throw "Could not find Cake.exe at $CAKE_EXE" } + + +# Build Cake arguments +$cakeArguments = @("$Script"); +if ($Target) { $cakeArguments += "-target=$Target" } +if ($Configuration) { $cakeArguments += "-configuration=$Configuration" } +if ($Verbosity) { $cakeArguments += "-verbosity=$Verbosity" } +if ($ShowDescription) { $cakeArguments += "-showdescription" } +if ($DryRun) { $cakeArguments += "-dryrun" } +if ($Experimental) { $cakeArguments += "-experimental" } +if ($Mono) { $cakeArguments += "-mono" } +$cakeArguments += $ScriptArgs + # Start Cake Write-Host "Running build script..." -Invoke-Expression "& `"$CAKE_EXE`" `"$Script`" -target=`"$Target`" -configuration=`"$Configuration`" -verbosity=`"$Verbosity`" $UseMono $UseDryRun $UseExperimental $ScriptArgs" -exit $LASTEXITCODE \ No newline at end of file +&$CAKE_EXE $cakeArguments +exit $LASTEXITCODE diff --git a/tools/packages.config b/tools/packages.config index 70a1e5995..e0dd39bd2 100644 --- a/tools/packages.config +++ b/tools/packages.config @@ -1,4 +1,4 @@ - +