Adds AllowRedirectOnSchemeChange to configure ability to redirect to …

…different schemes
microsoft · Sep 8, 2021 · fee7d2d · fee7d2d
1 parent 7f9ba26
commit fee7d2d
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 3 deletions.
diff --git a/...otnet/httpclient/Microsoft.Kiota.Http.HttpClient.Tests/Middleware/RedirectHandlerTests.cs b/...otnet/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<RedirectHandler>(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<InvalidOperationException>(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<InvalidOperationException>(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
@@ -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 <see cref="HttpResponseMessage"/> as it's parameter and return a <see cref="bool"/>. This defaults to true.
         /// </summary>
         public Func<HttpResponseMessage, bool> ShouldRedirect { get; set; } = (response) => true;
+
+        /// <summary>
+        /// A boolean value to determine if we redirects are allowed if the scheme changes(e.g. https to http). Defaults to false. 
+        /// </summary>
+        public bool AllowRedirectOnSchemeChange { get; set; } = false;
     }
 }
diff --git a/http/dotnet/httpclient/src/Middleware/RedirectHandler.cs b/http/dotnet/httpclient/src/Middleware/RedirectHandler.cs
@@ -94,6 +94,14 @@ protected override async Task<HttpResponseMessage> 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);