From a103d588fea1e09781c8a480a375cbf85ad34f1e Mon Sep 17 00:00:00 2001 From: Anthony Simmon Date: Thu, 27 Jun 2024 11:16:44 -0400 Subject: [PATCH 1/3] Do not enforce HTTPS when clients communicate with servers by default --- .../ClientCredentialsOptions.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Workleap.Extensions.Http.Authentication.ClientCredentialsGrant/ClientCredentialsOptions.cs b/src/Workleap.Extensions.Http.Authentication.ClientCredentialsGrant/ClientCredentialsOptions.cs index 8aa1613..c6e60d7 100644 --- a/src/Workleap.Extensions.Http.Authentication.ClientCredentialsGrant/ClientCredentialsOptions.cs +++ b/src/Workleap.Extensions.Http.Authentication.ClientCredentialsGrant/ClientCredentialsOptions.cs @@ -53,9 +53,9 @@ public string Scope internal string CacheKey { get; set; } = string.Empty; /// - /// Enforce https for all authenticated requests + /// Enforce https for all authenticated requests. Default value is false. /// - public bool EnforceHttps { get; set; } = true; + public bool EnforceHttps { get; set; } /// /// When set to true, the library will attempt to acquire a token on app startup, From 61135539af633ec6b7f44b3e7858b6213bdb5f6c Mon Sep 17 00:00:00 2001 From: Anthony Simmon Date: Thu, 27 Jun 2024 11:19:23 -0400 Subject: [PATCH 2/3] Update docs and usage in tests --- README.md | 4 ++-- .../ClientCredentialsTokenHttpMessageHandlerTests.cs | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 42b606a..ae4107b 100644 --- a/README.md +++ b/README.md @@ -83,8 +83,8 @@ services.AddOptions("MyClient").Bind(configuration.Get services.AddHttpClient().AddClientCredentialsHandler( /* [...] */); ``` -Note on `EnforceHttps`. -It is possible to allow http authenticated requests, however, this should be limited to exceptional scenarios. +Note on `EnforceHttps`, which is disabled by default. +It is possible to allow http authenticated requests, however, this should be limited to specific scenarios, such as intra-cluster communication. It is strongly advised that you always use https for authenticated requests transmitted as the token sent will be in clear. Then, instantiate the `HttpClient` later on using `IHttpClientFactory` or directly inject it in the constructor if you used the generic registration: diff --git a/src/Workleap.Authentication.ClientCredentialsGrant.Tests/ClientCredentialsTokenHttpMessageHandlerTests.cs b/src/Workleap.Authentication.ClientCredentialsGrant.Tests/ClientCredentialsTokenHttpMessageHandlerTests.cs index f9755b3..096396b 100644 --- a/src/Workleap.Authentication.ClientCredentialsGrant.Tests/ClientCredentialsTokenHttpMessageHandlerTests.cs +++ b/src/Workleap.Authentication.ClientCredentialsGrant.Tests/ClientCredentialsTokenHttpMessageHandlerTests.cs @@ -54,7 +54,6 @@ public async Task Throws_ClientCredentialsException_When_Http_By_Default() [Fact] public async Task SendAsync_When_EnforceHttps_False_For_Http_Requests() { - this._options.EnforceHttps = false; this._mockPrimaryHttpMessageHandler.ExpectedHttpResponseMessages = new[] { new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent("Access granted on first try") }, From d5c21dee81535a970dd625bdc7aa13e29d8403bc Mon Sep 17 00:00:00 2001 From: Anthony Simmon Date: Thu, 27 Jun 2024 11:25:51 -0400 Subject: [PATCH 3/3] Fix tests --- .../ClientCredentialsTokenHttpMessageHandlerTests.cs | 3 ++- .../IntegrationTests.cs | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Workleap.Authentication.ClientCredentialsGrant.Tests/ClientCredentialsTokenHttpMessageHandlerTests.cs b/src/Workleap.Authentication.ClientCredentialsGrant.Tests/ClientCredentialsTokenHttpMessageHandlerTests.cs index 096396b..7084a38 100644 --- a/src/Workleap.Authentication.ClientCredentialsGrant.Tests/ClientCredentialsTokenHttpMessageHandlerTests.cs +++ b/src/Workleap.Authentication.ClientCredentialsGrant.Tests/ClientCredentialsTokenHttpMessageHandlerTests.cs @@ -46,8 +46,9 @@ public async Task SendAsync_When_First_Response_Is_Ok_Returns_Ok_And_Skips_Secon } [Fact] - public async Task Throws_ClientCredentialsException_When_Http_By_Default() + public async Task Throws_ClientCredentialsException_When_Https_Enforced_But_Request_Is_Http() { + this._options.EnforceHttps = true; await Assert.ThrowsAsync(() => this._clientCredentialsHttpClient.GetStringAsync("http://whatever", CancellationToken.None)); } diff --git a/src/Workleap.Authentication.ClientCredentialsGrant.Tests/IntegrationTests.cs b/src/Workleap.Authentication.ClientCredentialsGrant.Tests/IntegrationTests.cs index 1603111..eb99262 100644 --- a/src/Workleap.Authentication.ClientCredentialsGrant.Tests/IntegrationTests.cs +++ b/src/Workleap.Authentication.ClientCredentialsGrant.Tests/IntegrationTests.cs @@ -104,6 +104,7 @@ public async Task Real_Client_Server_Communication() options.ClientSecret = "invoices_read_client_secret"; options.Scope = $"{Audience}:read"; options.CacheLifetimeBuffer = tokenCacheLifetimeBuffer; + options.EnforceHttps = true; }); // Here begins ASP.NET Core middleware pipelines registration