From fee7d2d33b3c5eab95113a1cfa1f68cc70430540 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Wed, 8 Sep 2021 09:53:43 +0300 Subject: [PATCH] Adds AllowRedirectOnSchemeChange to configure ability to redirect to different schemes --- .../Middleware/RedirectHandlerTests.cs | 29 +++++++++++++++++-- .../Options/RedirectHandlerOption.cs | 5 ++++ .../src/Middleware/RedirectHandler.cs | 8 +++++ 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/http/dotnet/httpclient/Microsoft.Kiota.Http.HttpClient.Tests/Middleware/RedirectHandlerTests.cs b/http/dotnet/httpclient/Microsoft.Kiota.Http.HttpClient.Tests/Middleware/RedirectHandlerTests.cs index 0fafc8d64e..11d9faa9ce 100644 --- a/http/dotnet/httpclient/Microsoft.Kiota.Http.HttpClient.Tests/Middleware/RedirectHandlerTests.cs +++ b/http/dotnet/httpclient/Microsoft.Kiota.Http.HttpClient.Tests/Middleware/RedirectHandlerTests.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Net; using System.Net.Http; using System.Net.Http.Headers; @@ -59,10 +59,11 @@ public void RedirectHandler_HttpMessageHandlerConstructor() public void RedirectHandler_RedirectOptionConstructor() { // Assert - using RedirectHandler redirect = new RedirectHandler(new RedirectHandlerOption { MaxRedirect = 2 }); + using RedirectHandler redirect = new RedirectHandler(new RedirectHandlerOption { MaxRedirect = 2, AllowRedirectOnSchemeChange = true}); Assert.Null(redirect.InnerHandler); Assert.NotNull(redirect.RedirectOption); Assert.Equal(2, redirect.RedirectOption.MaxRedirect); + Assert.True(redirect.RedirectOption.AllowRedirectOnSchemeChange); Assert.IsType(redirect); } @@ -151,7 +152,7 @@ public async Task RedirectWithDifferentHostShouldRemoveAuthHeader(HttpStatusCode [InlineData(HttpStatusCode.Found)] // 302 [InlineData(HttpStatusCode.TemporaryRedirect)] // 307 [InlineData(HttpStatusCode.PermanentRedirect)] // 308 - public async Task RedirectWithDifferentSchemeShouldRemoveAuthHeader(HttpStatusCode statusCode) + public async Task RedirectWithDifferentSchemeThrowsInvalidOperationExceptionIfAllowRedirectOnSchemeChangeIsDisabled(HttpStatusCode statusCode) { // Arrange var httpRequestMessage = new HttpRequestMessage(HttpMethod.Get, "https://example.org/foo"); @@ -160,6 +161,28 @@ public async Task RedirectWithDifferentSchemeShouldRemoveAuthHeader(HttpStatusCo redirectResponse.Headers.Location = new Uri("http://example.org/bar"); this._testHttpMessageHandler.SetHttpResponse(redirectResponse, new HttpResponseMessage(HttpStatusCode.OK));// sets the mock response // Act + var exception = await Assert.ThrowsAsync(async () => await this._invoker.SendAsync(httpRequestMessage, CancellationToken.None)); + // Assert + Assert.Contains("Redirects with changing schemes not allowed by default", exception.Message); + Assert.Equal("Scheme changed from https to http.", exception.InnerException?.Message); + Assert.IsType(exception); + } + + [Theory] + [InlineData(HttpStatusCode.MovedPermanently)] // 301 + [InlineData(HttpStatusCode.Found)] // 302 + [InlineData(HttpStatusCode.TemporaryRedirect)] // 307 + [InlineData(HttpStatusCode.PermanentRedirect)] // 308 + public async Task RedirectWithDifferentSchemeShouldRemoveAuthHeaderIfAllowRedirectOnSchemeChangeIsEnabled(HttpStatusCode statusCode) + { + // Arrange + var httpRequestMessage = new HttpRequestMessage(HttpMethod.Get, "https://example.org/foo"); + httpRequestMessage.Headers.Authorization = new AuthenticationHeaderValue("fooAuth", "aparam"); + var redirectResponse = new HttpResponseMessage(statusCode); + redirectResponse.Headers.Location = new Uri("http://example.org/bar"); + this._redirectHandler.RedirectOption.AllowRedirectOnSchemeChange = true;// Enable redirects on scheme change + this._testHttpMessageHandler.SetHttpResponse(redirectResponse, new HttpResponseMessage(HttpStatusCode.OK));// sets the mock response + // Act var response = await _invoker.SendAsync(httpRequestMessage, new CancellationToken()); // Assert Assert.NotSame(response.RequestMessage, httpRequestMessage); diff --git a/http/dotnet/httpclient/src/Middleware/Options/RedirectHandlerOption.cs b/http/dotnet/httpclient/src/Middleware/Options/RedirectHandlerOption.cs index daac10ae19..8a7a194e0f 100644 --- a/http/dotnet/httpclient/src/Middleware/Options/RedirectHandlerOption.cs +++ b/http/dotnet/httpclient/src/Middleware/Options/RedirectHandlerOption.cs @@ -39,5 +39,10 @@ public int MaxRedirect /// A delegate that's called to determine whether a response should be redirected or not. The delegate method should accept as it's parameter and return a . This defaults to true. /// public Func ShouldRedirect { get; set; } = (response) => true; + + /// + /// A boolean value to determine if we redirects are allowed if the scheme changes(e.g. https to http). Defaults to false. + /// + public bool AllowRedirectOnSchemeChange { get; set; } = false; } } diff --git a/http/dotnet/httpclient/src/Middleware/RedirectHandler.cs b/http/dotnet/httpclient/src/Middleware/RedirectHandler.cs index 952535d555..c1d5d065d0 100644 --- a/http/dotnet/httpclient/src/Middleware/RedirectHandler.cs +++ b/http/dotnet/httpclient/src/Middleware/RedirectHandler.cs @@ -94,6 +94,14 @@ protected override async Task SendAsync(HttpRequestMessage newRequest.Headers.Authorization = null; } + // If scheme has changed. Ensure that this has been opted in for security reasons + if(!newRequest.RequestUri.Scheme.Equals(httpRequestMessage.RequestUri?.Scheme) && !RedirectOption.AllowRedirectOnSchemeChange) + { + throw new InvalidOperationException( + $"Redirects with changing schemes not allowed by default. You can change this by modifying the {nameof(RedirectOption.AllowRedirectOnSchemeChange)} option", + new Exception($"Scheme changed from {httpRequestMessage.RequestUri?.Scheme} to {newRequest.RequestUri.Scheme}.")); + } + // Send redirect request to get response response = await base.SendAsync(newRequest, cancellationToken);