From 2dd53509caebd483f946a78b22adcb02d0b730ce Mon Sep 17 00:00:00 2001 From: lordmilko Date: Fri, 20 Nov 2020 21:12:46 +1100 Subject: [PATCH] Fix API requests involving cookies not working when a user incorrectly specifies to use http:// instead of https:// against a HTTPS PRTG server (Fix #181) --- build/CI/ci.psm1 | 3 ++ .../Cmdlets/Session/ConnectPrtgServer.cs | 5 +++ .../Support/PowerShell/Init.ps1 | 2 ++ .../CSharp/Infrastructure/PrtgClientTests.cs | 31 ++++++++++++++++ .../PrtgAPI.Tests.UnitTests.csproj | 1 + .../TestResponses/HttpToHttpsResponse.cs | 14 ++++++++ src/PrtgAPI/Request/PrtgClient.Generated.cs | 2 +- src/PrtgAPI/Request/PrtgClient.Generated.tt | 2 +- src/PrtgAPI/Request/PrtgRequestMessage.cs | 8 +++-- src/PrtgAPI/Request/PrtgWebClient.cs | 3 +- src/PrtgAPI/Request/RequestEngine.cs | 35 +++++++++++++++++-- 11 files changed, 96 insertions(+), 10 deletions(-) create mode 100644 src/PrtgAPI.Tests.UnitTests/Support/TestResponses/HttpToHttpsResponse.cs diff --git a/build/CI/ci.psm1 b/build/CI/ci.psm1 index f822bd53..0178aacd 100644 --- a/build/CI/ci.psm1 +++ b/build/CI/ci.psm1 @@ -339,6 +339,9 @@ function global:Join-PathEx } } +# PowerShell Gallery requires TLS 1.2 +[Net.ServicePointManager]::SecurityProtocol = [Net.ServicePointManager]::SecurityProtocol -bor [Net.SecurityProtocolType]::Tls12 + $exports = @( "HasType" "New-PackageManager" diff --git a/src/PrtgAPI.PowerShell/PowerShell/Cmdlets/Session/ConnectPrtgServer.cs b/src/PrtgAPI.PowerShell/PowerShell/Cmdlets/Session/ConnectPrtgServer.cs index 53ce4292..6cac5f73 100644 --- a/src/PrtgAPI.PowerShell/PowerShell/Cmdlets/Session/ConnectPrtgServer.cs +++ b/src/PrtgAPI.PowerShell/PowerShell/Cmdlets/Session/ConnectPrtgServer.cs @@ -215,6 +215,11 @@ internal void Connect(PSCmdlet cmdlet) else PrtgSessionState.EnableProgress = true; + if (PrtgSessionState.Client.Server != Server && (PrtgSessionState.Client.LogLevel & PrtgAPI.LogLevel.Trace) == PrtgAPI.LogLevel.Trace) + { + WriteVerbose($"{MyInvocation.MyCommand}: Server redirected HTTP to HTTPS. Changing server URL from '{Server}' to '{PrtgSessionState.Client.Server}'"); + } + WritePassThru(); } else diff --git a/src/PrtgAPI.Tests.IntegrationTests/Support/PowerShell/Init.ps1 b/src/PrtgAPI.Tests.IntegrationTests/Support/PowerShell/Init.ps1 index 7071da7e..9c9b736a 100644 --- a/src/PrtgAPI.Tests.IntegrationTests/Support/PowerShell/Init.ps1 +++ b/src/PrtgAPI.Tests.IntegrationTests/Support/PowerShell/Init.ps1 @@ -2,6 +2,8 @@ if(!(Get-Module -ListAvailable Assert)) { + [Net.ServicePointManager]::SecurityProtocol = [Net.ServicePointManager]::SecurityProtocol -bor [Net.SecurityProtocolType]::Tls12 + Install-Package Assert -ProviderName PowerShellGet -RequiredVersion 0.8.1 -ForceBootstrap -Force | Out-Null } diff --git a/src/PrtgAPI.Tests.UnitTests/CSharp/Infrastructure/PrtgClientTests.cs b/src/PrtgAPI.Tests.UnitTests/CSharp/Infrastructure/PrtgClientTests.cs index 02b70375..645a0800 100644 --- a/src/PrtgAPI.Tests.UnitTests/CSharp/Infrastructure/PrtgClientTests.cs +++ b/src/PrtgAPI.Tests.UnitTests/CSharp/Infrastructure/PrtgClientTests.cs @@ -461,6 +461,37 @@ public void PrtgClient_HandlesTimeoutSocketException() AssertEx.Throws(() => ExecuteSocketException(SocketError.TimedOut), "Connection timed out while communicating with remote server"); } + [UnitTest] + [TestMethod] + public void PrtgClient_RedirectsHttpUrlToHttps() + { + var httpServer = "http://prtg.example.com"; + var httpsServer = "https://prtg.example.com"; + + var client = new PrtgClient(httpServer, "username", "password", AuthMode.PassHash, new MockWebClient(new HttpToHttpsResponse())); + + Assert.AreEqual(httpServer, client.Server); + + client.GetSensors(); + + Assert.AreEqual(httpsServer, client.Server); + } + + [UnitTest] + [TestMethod] + public void PrtgClient_DoesntModifyHttpsToHttps() + { + var httpsServer = "https://prtg.example.com"; + + var client = new PrtgClient(httpsServer, "username", "password", AuthMode.PassHash, new MockWebClient(new HttpToHttpsResponse())); + + Assert.AreEqual(httpsServer, client.Server); + + client.GetSensors(); + + Assert.AreEqual(httpsServer, client.Server); + } + [UnitTest] [TestMethod] public void PrtgClient_SplitsRequests_BatchingOver1500Items() diff --git a/src/PrtgAPI.Tests.UnitTests/PrtgAPI.Tests.UnitTests.csproj b/src/PrtgAPI.Tests.UnitTests/PrtgAPI.Tests.UnitTests.csproj index 78ddbf65..fa3a1978 100644 --- a/src/PrtgAPI.Tests.UnitTests/PrtgAPI.Tests.UnitTests.csproj +++ b/src/PrtgAPI.Tests.UnitTests/PrtgAPI.Tests.UnitTests.csproj @@ -131,6 +131,7 @@ + diff --git a/src/PrtgAPI.Tests.UnitTests/Support/TestResponses/HttpToHttpsResponse.cs b/src/PrtgAPI.Tests.UnitTests/Support/TestResponses/HttpToHttpsResponse.cs new file mode 100644 index 00000000..0a0ece00 --- /dev/null +++ b/src/PrtgAPI.Tests.UnitTests/Support/TestResponses/HttpToHttpsResponse.cs @@ -0,0 +1,14 @@ +namespace PrtgAPI.Tests.UnitTests.Support.TestResponses +{ + class HttpToHttpsResponse : MultiTypeResponse + { + protected override IWebResponse GetResponse(ref string address, string function) + { + var response = base.GetResponse(ref address, function); + + address = address.Replace("http://", "https://"); + + return response; + } + } +} diff --git a/src/PrtgAPI/Request/PrtgClient.Generated.cs b/src/PrtgAPI/Request/PrtgClient.Generated.cs index 1d033dc0..9c947f83 100644 --- a/src/PrtgAPI/Request/PrtgClient.Generated.cs +++ b/src/PrtgAPI/Request/PrtgClient.Generated.cs @@ -21,7 +21,7 @@ using PrtgAPI.Utilities; //Methods with complex logic surrounding sync/async function calls. -//For each method, two variants a generated. A synchronous method with the +//For each method, two variants are generated. A synchronous method with the //expected return type, and an async method that implicitly wraps the result //in a Task diff --git a/src/PrtgAPI/Request/PrtgClient.Generated.tt b/src/PrtgAPI/Request/PrtgClient.Generated.tt index 632946d4..e03fddc3 100644 --- a/src/PrtgAPI/Request/PrtgClient.Generated.tt +++ b/src/PrtgAPI/Request/PrtgClient.Generated.tt @@ -24,7 +24,7 @@ using PrtgAPI.Request; using PrtgAPI.Utilities; //Methods with complex logic surrounding sync/async function calls. -//For each method, two variants a generated. A synchronous method with the +//For each method, two variants are generated. A synchronous method with the //expected return type, and an async method that implicitly wraps the result //in a Task diff --git a/src/PrtgAPI/Request/PrtgRequestMessage.cs b/src/PrtgAPI/Request/PrtgRequestMessage.cs index 2f6eaca1..61bfcab1 100644 --- a/src/PrtgAPI/Request/PrtgRequestMessage.cs +++ b/src/PrtgAPI/Request/PrtgRequestMessage.cs @@ -18,7 +18,9 @@ internal class PrtgRequestMessage bool passFound; bool hasQueries; - public string Url { get; } + public string Url => Uri.OriginalString; + + public Uri Uri { get; } internal PrtgRequestMessage(ConnectionDetails connectionDetails, XmlFunction function, IParameters parameters) : this(connectionDetails, GetResourcePath(function), parameters) @@ -69,7 +71,7 @@ private PrtgRequestMessage(ConnectionDetails connectionDetails, string function, AddParameter(urlBuilder, Parameter.PassHash, connectionDetails.PassHash); } - Url = urlBuilder.ToString(); + Uri = new Uri(urlBuilder.ToString()); #if DEBUG Debug.WriteLine(Url); @@ -78,7 +80,7 @@ private PrtgRequestMessage(ConnectionDetails connectionDetails, string function, internal static string AddUrlPrefix(string server) { - if (server.StartsWith("http://") || server.StartsWith("https://")) + if (server.StartsWith("http://", StringComparison.OrdinalIgnoreCase) || server.StartsWith("https://", StringComparison.OrdinalIgnoreCase)) return server; return $"https://{server}"; diff --git a/src/PrtgAPI/Request/PrtgWebClient.cs b/src/PrtgAPI/Request/PrtgWebClient.cs index b3470744..0aa3bb87 100644 --- a/src/PrtgAPI/Request/PrtgWebClient.cs +++ b/src/PrtgAPI/Request/PrtgWebClient.cs @@ -16,7 +16,6 @@ internal class PrtgWebClient : IWebClient private HttpClient asyncClient = new HttpClient(); private HttpClientHandler handler = new HttpClientHandler(); - private CookieContainer cookies = new CookieContainer(); private string server; @@ -108,7 +107,7 @@ public Task SendAsync(PrtgRequestMessage request, Cancellat //needs to be refactored to have the deserialization happen in a callback to ExecuteRequest //so that there is no problem wrapping the HttpResponseMessage up in a using for both //sync/async - return asyncClient.GetAsync(request.Url, token); + return asyncClient.GetAsync(request.Uri, token); } } } diff --git a/src/PrtgAPI/Request/RequestEngine.cs b/src/PrtgAPI/Request/RequestEngine.cs index 05551bc3..8be074a1 100644 --- a/src/PrtgAPI/Request/RequestEngine.cs +++ b/src/PrtgAPI/Request/RequestEngine.cs @@ -264,7 +264,7 @@ private PrtgResponse ExecuteRequest(PrtgRequestMessage request, CancellationToke if (responseContent.Type == PrtgResponseType.String) prtgClient.Log(responseContent.StringValue, LogLevel.Response); - ValidateHttpResponse(responseMessage, responseContent); + ValidateHttpResponse(prtgClient, request, responseMessage, responseContent); return responseContent; } @@ -321,7 +321,7 @@ private async Task ExecuteRequestAsync(PrtgRequestMessage request, if (responseContent.Type == PrtgResponseType.String) prtgClient.Log(responseContent.StringValue, LogLevel.Response); - ValidateHttpResponse(responseMessage, responseContent); + ValidateHttpResponse(prtgClient, request, responseMessage, responseContent); return responseContent; } @@ -429,7 +429,36 @@ private bool HandleRequestException(Exception innerMostEx, Exception fallbackHan return true; } - internal static void ValidateHttpResponse(HttpResponseMessage responseMessage, PrtgResponse response) + private void ValidateHttpResponse(PrtgClient client, PrtgRequestMessage request, + HttpResponseMessage responseMessage, PrtgResponse response) + { + /* If the user specified http:// when their server is in fact https://, modify + * the server URL while being overley cautious not to mess with it too much; + * we want to assume that user knows what they're doing. Failing to adjust the URL + * will cause cookies tagged as Secure from the HTTPS server to be omitted + * from the insecure HTTP requests made by the HttpClient */ + if (request != null && responseMessage.RequestMessage.RequestUri.Scheme == "https" && request.Uri.Scheme == "http" && + client.ConnectionDetails.Server.StartsWith("http://", StringComparison.OrdinalIgnoreCase)) + { + var ch = "s"; + + if (client.ConnectionDetails.Server.StartsWith("HTTP")) + ch = "S"; + + var str = client.ConnectionDetails.Server.Insert(4, ch); + + if (str.StartsWith("https", StringComparison.OrdinalIgnoreCase)) + { + client.Log($"Server redirected HTTP to HTTPS. Changing server URL from '{client.ConnectionDetails.Server}' to '{str}'", LogLevel.Trace); + + client.ConnectionDetails.Server = str; + } + } + + ValidateHttpResponse(responseMessage, response); + } + + internal void ValidateHttpResponse(HttpResponseMessage responseMessage, PrtgResponse response) { if (responseMessage.StatusCode == HttpStatusCode.BadRequest) {