fix(security): remove proxy and cookie auth headers on insecure redirect

.vscode/settings.json

@@ -3,5 +3,6 @@
 		"Kiota"
 	],
 	"editor.formatOnSave": true,
-	"dotnet-test-explorer.testProjectPath": "**/*.Tests.csproj"
+	"dotnet-test-explorer.testProjectPath": "**/*.Tests.csproj",
+	"sarif-viewer.connectToGithubCodeScanning": "on"
 }

src/http/httpClient/Middleware/RedirectHandler.cs

@@ -41,7 +41,7 @@
         /// <param name="request">The <see cref="HttpRequestMessage"/> to send.</param>
         /// <param name="cancellationToken">The <see cref="CancellationToken"/>for the request.</param>
         /// <returns>The <see cref="HttpResponseMessage"/>.</returns>
         protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
         {
             if(request == null) throw new ArgumentNullException(nameof(request));
 
@@ -124,6 +124,8 @@
                         !newRequest.RequestUri.Scheme.Equals(request.RequestUri?.Scheme))
                         {
                             newRequest.Headers.Authorization = null;
+                            newRequest.Headers.ProxyAuthorization = null;
+                            newRequest.Headers.Remove("Cookie");
                         }
 
                         // If scheme has changed. Ensure that this has been opted in for security reasons
@@ -160,7 +162,7 @@
             }
         }
 
         private bool ShouldRedirect(HttpResponseMessage responseMessage, RedirectHandlerOption redirectOption)
         {
             return IsRedirect(responseMessage.StatusCode) && redirectOption.ShouldRedirect(responseMessage) && redirectOption.MaxRedirect > 0;
         }

tests/http/httpClient/Middleware/RedirectHandlerTests.cs

@@ -240,5 +240,127 @@ public async Task ExceedMaxRedirectsShouldThrowsException()
             Assert.Equal("Max redirects exceeded. Redirect count : 5", exception.InnerException?.Message);
             Assert.IsType<InvalidOperationException>(exception);
         }
+
+        [Theory]
+        [InlineData(HttpStatusCode.MovedPermanently)]  // 301
+        [InlineData(HttpStatusCode.Found)]  // 302
+        [InlineData(HttpStatusCode.TemporaryRedirect)]  // 307
+        [InlineData((HttpStatusCode)308)] // 308
+        public async Task RedirectWithDifferentHostShouldRemoveProxyAuthHeader(HttpStatusCode statusCode)
+        {
+            // Arrange
+            var httpRequestMessage = new HttpRequestMessage(HttpMethod.Get, "http://example.org/foo");
+            httpRequestMessage.Headers.ProxyAuthorization = new AuthenticationHeaderValue("fooAuth", "aparam");
+            var redirectResponse = new HttpResponseMessage(statusCode);
+            redirectResponse.Headers.Location = new Uri("http://example.net/bar");
+            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);
+            Assert.NotSame(response.RequestMessage?.RequestUri?.Host, httpRequestMessage.RequestUri?.Host);
+            Assert.Null(response.RequestMessage?.Headers.ProxyAuthorization);
+        }
+
+        [Theory]
+        [InlineData(HttpStatusCode.MovedPermanently)]  // 301
+        [InlineData(HttpStatusCode.Found)]  // 302
+        [InlineData(HttpStatusCode.TemporaryRedirect)]  // 307
+        [InlineData((HttpStatusCode)308)] // 308
+        public async Task RedirectWithDifferentHostShouldRemoveCookieHeader(HttpStatusCode statusCode)
+        {
+            // Arrange
+            using (var httpRequestMessage = new HttpRequestMessage(HttpMethod.Get, "http://example.org/foo"))
+            {
+                httpRequestMessage.Headers.Add("Cookie", "session=abc123");
+                var redirectResponse = new HttpResponseMessage(statusCode);
+                redirectResponse.Headers.Location = new Uri("http://example.net/bar");
+                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);
+                Assert.NotSame(response.RequestMessage?.RequestUri?.Host, httpRequestMessage.RequestUri?.Host);
+                Assert.False(response.RequestMessage?.Headers.Contains("Cookie"));
+            }
+        }
+
+        [Theory]
+        [InlineData(HttpStatusCode.MovedPermanently)]  // 301
+        [InlineData(HttpStatusCode.Found)]  // 302
+        [InlineData(HttpStatusCode.TemporaryRedirect)]  // 307
+        [InlineData((HttpStatusCode)308)] // 308
+        public async Task RedirectWithDifferentSchemeShouldRemoveProxyAuthHeaderIfAllowRedirectOnSchemeChangeIsEnabled(HttpStatusCode statusCode)
+        {
+            // Arrange
+            var httpRequestMessage = new HttpRequestMessage(HttpMethod.Get, "https://example.org/foo");
+            httpRequestMessage.Headers.ProxyAuthorization = 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);
+            Assert.NotSame(response.RequestMessage?.RequestUri?.Scheme, httpRequestMessage.RequestUri?.Scheme);
+            Assert.Null(response.RequestMessage?.Headers.ProxyAuthorization);
+        }
+
+        [Theory]
+        [InlineData(HttpStatusCode.MovedPermanently)]  // 301
+        [InlineData(HttpStatusCode.Found)]  // 302
+        [InlineData(HttpStatusCode.TemporaryRedirect)]  // 307
+        [InlineData((HttpStatusCode)308)] // 308
+        public async Task RedirectWithDifferentSchemeShouldRemoveCookieHeaderIfAllowRedirectOnSchemeChangeIsEnabled(HttpStatusCode statusCode)
+        {
+            // Arrange
+            var httpRequestMessage = new HttpRequestMessage(HttpMethod.Get, "https://example.org/foo");
+            httpRequestMessage.Headers.Add("Cookie", "session=abc123");
+            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);
+            Assert.NotSame(response.RequestMessage?.RequestUri?.Scheme, httpRequestMessage.RequestUri?.Scheme);
+            Assert.False(response.RequestMessage?.Headers.Contains("Cookie"));
+        }
+
+        [Fact]
+        public async Task RedirectWithSameHostShouldKeepProxyAuthHeader()
+        {
+            // Arrange
+            var httpRequestMessage = new HttpRequestMessage(HttpMethod.Post, "http://example.org/foo");
+            httpRequestMessage.Headers.ProxyAuthorization = new AuthenticationHeaderValue("fooAuth", "aparam");
+            var redirectResponse = new HttpResponseMessage(HttpStatusCode.Redirect);
+            redirectResponse.Headers.Location = new Uri("http://example.org/bar");
+            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);
+            Assert.Equal(response.RequestMessage?.RequestUri?.Host, httpRequestMessage.RequestUri?.Host);
+            Assert.NotNull(response.RequestMessage?.Headers.ProxyAuthorization);
+        }
+
+        [Fact]
+        public async Task RedirectWithSameHostShouldKeepCookieHeader()
+        {
+            // Arrange
+            var httpRequestMessage = new HttpRequestMessage(HttpMethod.Post, "http://example.org/foo");
+            httpRequestMessage.Headers.Add("Cookie", "session=abc123");
+            var redirectResponse = new HttpResponseMessage(HttpStatusCode.Redirect);
+            redirectResponse.Headers.Location = new Uri("http://example.org/bar");
+            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);
+            Assert.Equal(response.RequestMessage?.RequestUri?.Host, httpRequestMessage.RequestUri?.Host);
+            Assert.True(response.RequestMessage?.Headers.Contains("Cookie"));
+        }
     }
 }