Skip to content

Commit

Permalink
Fix API requests involving cookies not working when a user incorrectl…
Browse files Browse the repository at this point in the history
…y specifies to use http:// instead of https:// against a HTTPS PRTG server (Fix #181)
  • Loading branch information
lordmilko committed Nov 20, 2020
1 parent 6ea6bad commit 2dd5350
Show file tree
Hide file tree
Showing 11 changed files with 96 additions and 10 deletions.
3 changes: 3 additions & 0 deletions build/CI/ci.psm1
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,37 @@ public void PrtgClient_HandlesTimeoutSocketException()
AssertEx.Throws<TimeoutException>(() => 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()
Expand Down
1 change: 1 addition & 0 deletions src/PrtgAPI.Tests.UnitTests/PrtgAPI.Tests.UnitTests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@
<Compile Include="Support\Infrastructure\Progress\ProgressHierarchy.cs" />
<Compile Include="Support\TestItems\SystemInfoItem.cs" />
<Compile Include="Support\TestResponses\GeoLocatorResponse.cs" />
<Compile Include="Support\TestResponses\HttpToHttpsResponse.cs" />
<Compile Include="Support\TestResponses\InfiniteLogPostProcessValidatorResponse.cs" />
<Compile Include="Support\TestResponses\InfiniteLogResponse.cs" />
<Compile Include="Support\TestResponses\InfiniteLogValidatorResponse.cs" />
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
2 changes: 1 addition & 1 deletion src/PrtgAPI/Request/PrtgClient.Generated.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/PrtgAPI/Request/PrtgClient.Generated.tt
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 5 additions & 3 deletions src/PrtgAPI/Request/PrtgRequestMessage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand All @@ -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}";
Expand Down
3 changes: 1 addition & 2 deletions src/PrtgAPI/Request/PrtgWebClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -108,7 +107,7 @@ public Task<HttpResponseMessage> 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);
}
}
}
35 changes: 32 additions & 3 deletions src/PrtgAPI/Request/RequestEngine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -321,7 +321,7 @@ private async Task<PrtgResponse> ExecuteRequestAsync(PrtgRequestMessage request,
if (responseContent.Type == PrtgResponseType.String)
prtgClient.Log(responseContent.StringValue, LogLevel.Response);

ValidateHttpResponse(responseMessage, responseContent);
ValidateHttpResponse(prtgClient, request, responseMessage, responseContent);

return responseContent;
}
Expand Down Expand Up @@ -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)
{
Expand Down

0 comments on commit 2dd5350

Please sign in to comment.