Make various improvements to the OpenIddict.Validation/System.Net.Http integration package

src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.Discovery.cs

@@ -20,11 +20,13 @@ namespace OpenIddict.Validation.SystemNetHttp
                 PrepareGetHttpRequest<PrepareConfigurationRequestContext>.Descriptor,
                 AttachQueryStringParameters<PrepareConfigurationRequestContext>.Descriptor,
                 SendHttpRequest<ApplyConfigurationRequestContext>.Descriptor,
+                DisposeHttpRequest<ApplyConfigurationRequestContext>.Descriptor,
 
                 /*
                  * Configuration response processing:
                  */
                 ExtractJsonHttpResponse<ExtractConfigurationResponseContext>.Descriptor,
+                DisposeHttpResponse<ExtractConfigurationResponseContext>.Descriptor,
 
                 /*
                  * Cryptography request processing:
@@ -32,11 +34,13 @@ namespace OpenIddict.Validation.SystemNetHttp
                 PrepareGetHttpRequest<PrepareCryptographyRequestContext>.Descriptor,
                 AttachQueryStringParameters<PrepareCryptographyRequestContext>.Descriptor,
                 SendHttpRequest<ApplyCryptographyRequestContext>.Descriptor,
+                DisposeHttpRequest<ApplyCryptographyRequestContext>.Descriptor,
 
                 /*
                  * Configuration response processing:
                  */
-                ExtractJsonHttpResponse<ExtractCryptographyResponseContext>.Descriptor);
+                ExtractJsonHttpResponse<ExtractCryptographyResponseContext>.Descriptor,
+                DisposeHttpResponse<ExtractCryptographyResponseContext>.Descriptor);
         }
     }
 }

src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.Introspection.cs

@@ -30,11 +30,13 @@ namespace OpenIddict.Validation.SystemNetHttp
                 AttachBasicAuthenticationCredentials.Descriptor,
                 AttachFormParameters<PrepareIntrospectionRequestContext>.Descriptor,
                 SendHttpRequest<ApplyIntrospectionRequestContext>.Descriptor,
+                DisposeHttpRequest<ApplyIntrospectionRequestContext>.Descriptor,
 
                 /*
                  * Introspection response processing:
                  */
-                ExtractJsonHttpResponse<ExtractIntrospectionResponseContext>.Descriptor);
+                ExtractJsonHttpResponse<ExtractIntrospectionResponseContext>.Descriptor,
+                DisposeHttpResponse<ExtractIntrospectionResponseContext>.Descriptor);
 
             /// <summary>
             /// Contains the logic responsible of attaching the client credentials to the HTTP Authorization header.
@@ -70,45 +72,44 @@ namespace OpenIddict.Validation.SystemNetHttp
                         throw new InvalidOperationException(SR.GetResourceString(SR.ID0173));
                     }
 
+                    // If no client identifier was attached to the request, skip the following logic.
+                    if (string.IsNullOrEmpty(context.Request.ClientId))
+                    {
+                        return;
+                    }
+
                     var configuration = await context.Options.ConfigurationManager.GetConfigurationAsync(default) ??
                         throw new InvalidOperationException(SR.GetResourceString(SR.ID0140));
 
                     // The OAuth 2.0 specification recommends sending the client credentials using basic authentication.
-                    // However, this authentication method is known to have compatibility issues with how the
-                    // client credentials are encoded, that MUST be formURL-encoded before being base64-encoded.
+                    // However, this authentication method is known to have compatibility issues with the way the
+                    // client credentials are encoded (they MUST be formURL-encoded before being base64-encoded).
                     // To guarantee that the OpenIddict validation handler can be used with servers implementing
                     // non-standard encoding, the client_secret_post is always preferred when it's explicitly
                     // listed as a supported client authentication method for the introspection endpoint.
                     // If client_secret_post is not listed or if the server returned an empty methods list,
-                    // client_secret_basic is always used, as it MUST be supported by all OAuth 2.0 servers.
+                    // client_secret_basic is always used, as it MUST be implemented by all OAuth 2.0 servers.
                     //
                     // See https://tools.ietf.org/html/rfc8414#section-2
                     // and https://tools.ietf.org/html/rfc6749#section-2.3.1 for more information.
                     if (!configuration.IntrospectionEndpointAuthMethodsSupported.Contains(ClientAuthenticationMethods.ClientSecretPost))
                     {
-                        var builder = new StringBuilder()
+                        // Important: the credentials MUST be formURL-encoded before being base64-encoded.
+                        var credentials = Convert.ToBase64String(Encoding.ASCII.GetBytes(new StringBuilder()
                             .Append(EscapeDataString(context.Request.ClientId))
                             .Append(':')
-                            .Append(EscapeDataString(context.Request.ClientSecret));
-
-                        var credentials = Convert.ToBase64String(Encoding.ASCII.GetBytes(builder.ToString()));
+                            .Append(EscapeDataString(context.Request.ClientSecret))
+                            .ToString()));
 
                         // Attach the authorization header containing the client credentials to the HTTP request.
                         request.Headers.Authorization = new AuthenticationHeaderValue(Schemes.Basic, credentials);
 
-                        // Remove the client credentials from the request.
+                        // Remove the client credentials from the request payload to ensure they are not sent twice.
                         context.Request.ClientId = context.Request.ClientSecret = null;
                     }
 
                     static string? EscapeDataString(string? value)
-                    {
-                        if (string.IsNullOrEmpty(value))
-                        {
-                            return null;
-                        }
-
-                        return Uri.EscapeDataString(value).Replace("%20", "+");
-                    }
+                        => value is not null ? Uri.EscapeDataString(value).Replace("%20", "+") : null;
                 }
             }
         }

src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.cs

@@ -9,10 +9,12 @@ using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.ComponentModel;
 using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Net.Http;
 using System.Net.Http.Headers;
 using System.Net.Http.Json;
+using System.Text;
 using System.Threading.Tasks;
 using OpenIddict.Abstractions;
 using static OpenIddict.Validation.OpenIddictValidationEvents;
@@ -46,6 +48,8 @@ namespace OpenIddict.Validation.SystemNetHttp
                     .Build();
 
             /// <inheritdoc/>
+            [SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope",
+                Justification = "The HTTP request message is disposed later by a dedicated handler.")]
             public ValueTask HandleAsync(TContext context)
             {
                 if (context is null)
@@ -53,12 +57,17 @@ namespace OpenIddict.Validation.SystemNetHttp
                     throw new ArgumentNullException(nameof(context));
                 }
 
-                var request = new HttpRequestMessage(HttpMethod.Get, context.Address);
-                request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
-                request.Headers.AcceptCharset.Add(new StringWithQualityHeaderValue("utf-8"));
+                var request = new HttpRequestMessage(HttpMethod.Get, context.Address)
+                {
+                    Headers =
+                    {
+                        Accept = { new MediaTypeWithQualityHeaderValue("application/json") },
+                        AcceptCharset = { new StringWithQualityHeaderValue("utf-8") }
+                    }
+                };
 
                 // Store the HttpRequestMessage in the transaction properties.
-                context.Transaction.Properties[typeof(HttpRequestMessage).FullName!] = request;
+                context.Transaction.SetProperty(typeof(HttpRequestMessage).FullName!, request);
 
                 return default;
             }
@@ -81,6 +90,8 @@ namespace OpenIddict.Validation.SystemNetHttp
                     .Build();
 
             /// <inheritdoc/>
+            [SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope",
+                Justification = "The HTTP request message is disposed later by a dedicated handler.")]
             public ValueTask HandleAsync(TContext context)
             {
                 if (context is null)
@@ -88,12 +99,17 @@ namespace OpenIddict.Validation.SystemNetHttp
                     throw new ArgumentNullException(nameof(context));
                 }
 
-                var request = new HttpRequestMessage(HttpMethod.Post, context.Address);
-                request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
-                request.Headers.AcceptCharset.Add(new StringWithQualityHeaderValue("utf-8"));
+                var request = new HttpRequestMessage(HttpMethod.Post, context.Address)
+                {
+                    Headers =
+                    {
+                        Accept = { new MediaTypeWithQualityHeaderValue("application/json") },
+                        AcceptCharset = { new StringWithQualityHeaderValue("utf-8") }
+                    }
+                };
 
                 // Store the HttpRequestMessage in the transaction properties.
-                context.Transaction.Properties[typeof(HttpRequestMessage).FullName!] = request;
+                context.Transaction.SetProperty(typeof(HttpRequestMessage).FullName!, request);
 
                 return default;
             }
@@ -116,7 +132,7 @@ namespace OpenIddict.Validation.SystemNetHttp
                     .Build();
 
             /// <inheritdoc/>
-            public async ValueTask HandleAsync(TContext context)
+            public ValueTask HandleAsync(TContext context)
             {
                 if (context is null)
                 {
@@ -134,25 +150,35 @@ namespace OpenIddict.Validation.SystemNetHttp
                     throw new InvalidOperationException(SR.GetResourceString(SR.ID0173));
                 }
 
-                if (request.RequestUri is not null)
+                if (request.RequestUri is null || context.Transaction.Request.Count == 0)
+                {
+                    return default;
+                }
+
+                var builder = new StringBuilder();
+
+                foreach (var (key, value) in
+                    from parameter in context.Transaction.Request.GetParameters()
+                    let values = (string?[]?) parameter.Value
+                    where values is not null
+                    from value in values
+                    where !string.IsNullOrEmpty(value)
+                    select (parameter.Key, Value: value))
                 {
-                    // Note: System.Net.Http doesn't expose convenient methods allowing to create
-                    // query strings from existing key/value pairs. To work around this limitation,
-                    // a FormUrlEncodedContent is instantiated and used to manually create the URL.
-                    using var content = new FormUrlEncodedContent(
-                        from parameter in context.Transaction.Request.GetParameters()
-                        let values = (string[]?) parameter.Value
-                        where values is not null
-                        from value in values
-                        select new KeyValuePair<string, string>(parameter.Key, value));
-
-                    var builder = new UriBuilder(request.RequestUri)
+                    if (builder.Length > 0)
                     {
-                        Query = await content.ReadAsStringAsync()
-                    };
+                        builder.Append('&');
+                    }
 
-                    request.RequestUri = builder.Uri;
+                    builder.Append(Uri.EscapeDataString(key));
+                    builder.Append('=');
+                    builder.Append(Uri.EscapeDataString(value));
                 }
+
+                // Compute the final request URI using the base address and the query string.
+                request.RequestUri = new UriBuilder(request.RequestUri) { Query = builder.ToString() }.Uri;
+
+                return default;
             }
         }
 
@@ -218,7 +244,7 @@ namespace OpenIddict.Validation.SystemNetHttp
                 = OpenIddictValidationHandlerDescriptor.CreateBuilder<TContext>()
                     .AddFilter<RequireHttpMetadataAddress>()
                     .UseSingletonHandler<SendHttpRequest<TContext>>()
-                    .SetOrder(int.MaxValue - 100_000)
+                    .SetOrder(DisposeHttpRequest<TContext>.Descriptor.Order - 50_000)
                     .SetType(OpenIddictValidationHandlerType.BuiltIn)
                     .Build();
 
@@ -252,7 +278,48 @@ namespace OpenIddict.Validation.SystemNetHttp
                 }
 
                 // Store the HttpResponseMessage in the transaction properties.
-                context.Transaction.Properties[typeof(HttpResponseMessage).FullName!] = response;
+                context.Transaction.SetProperty(typeof(HttpResponseMessage).FullName!, response);
+            }
+        }
+
+        /// <summary>
+        /// Contains the logic responsible of disposing of the HTTP request message.
+        /// </summary>
+        public class DisposeHttpRequest<TContext> : IOpenIddictValidationHandler<TContext> where TContext : BaseExternalContext
+        {
+            /// <summary>
+            /// Gets the default descriptor definition assigned to this handler.
+            /// </summary>
+            public static OpenIddictValidationHandlerDescriptor Descriptor { get; }
+                = OpenIddictValidationHandlerDescriptor.CreateBuilder<TContext>()
+                    .AddFilter<RequireHttpMetadataAddress>()
+                    .UseSingletonHandler<DisposeHttpRequest<TContext>>()
+                    .SetOrder(int.MaxValue - 100_000)
+                    .SetType(OpenIddictValidationHandlerType.BuiltIn)
+                    .Build();
+
+            /// <inheritdoc/>
+            public ValueTask HandleAsync(TContext context)
+            {
+                if (context is null)
+                {
+                    throw new ArgumentNullException(nameof(context));
+                }
+
+                // This handler only applies to System.Net.Http requests. If the HTTP request cannot be resolved,
+                // this may indicate that the request was incorrectly processed by another client stack.
+                var request = context.Transaction.GetHttpRequestMessage();
+                if (request is null)
+                {
+                    throw new InvalidOperationException(SR.GetResourceString(SR.ID0173));
+                }
+
+                request.Dispose();
+
+                // Remove the request from the transaction properties.
+                context.Transaction.SetProperty<HttpRequestMessage>(typeof(HttpRequestMessage).FullName!, null);
+
+                return default;
             }
         }
 
@@ -268,7 +335,7 @@ namespace OpenIddict.Validation.SystemNetHttp
                 = OpenIddictValidationHandlerDescriptor.CreateBuilder<TContext>()
                     .AddFilter<RequireHttpMetadataAddress>()
                     .UseSingletonHandler<ExtractJsonHttpResponse<TContext>>()
-                    .SetOrder(int.MaxValue - 100_000)
+                    .SetOrder(DisposeHttpResponse<TContext>.Descriptor.Order - 50_000)
                     .SetType(OpenIddictValidationHandlerType.BuiltIn)
                     .Build();
 
@@ -296,5 +363,46 @@ namespace OpenIddict.Validation.SystemNetHttp
                 context.Transaction.Response = await response.Content.ReadFromJsonAsync<OpenIddictResponse>();
             }
         }
+
+        /// <summary>
+        /// Contains the logic responsible of disposing of the HTTP response message.
+        /// </summary>
+        public class DisposeHttpResponse<TContext> : IOpenIddictValidationHandler<TContext> where TContext : BaseExternalContext
+        {
+            /// <summary>
+            /// Gets the default descriptor definition assigned to this handler.
+            /// </summary>
+            public static OpenIddictValidationHandlerDescriptor Descriptor { get; }
+                = OpenIddictValidationHandlerDescriptor.CreateBuilder<TContext>()
+                    .AddFilter<RequireHttpMetadataAddress>()
+                    .UseSingletonHandler<DisposeHttpResponse<TContext>>()
+                    .SetOrder(int.MaxValue - 100_000)
+                    .SetType(OpenIddictValidationHandlerType.BuiltIn)
+                    .Build();
+
+            /// <inheritdoc/>
+            public ValueTask HandleAsync(TContext context)
+            {
+                if (context is null)
+                {
+                    throw new ArgumentNullException(nameof(context));
+                }
+
+                // This handler only applies to System.Net.Http requests. If the HTTP response cannot be resolved,
+                // this may indicate that the request was incorrectly processed by another client stack.
+                var response = context.Transaction.GetHttpResponseMessage();
+                if (response is null)
+                {
+                    throw new InvalidOperationException(SR.GetResourceString(SR.ID0173));
+                }
+
+                response.Dispose();
+
+                // Remove the response from the transaction properties.
+                context.Transaction.SetProperty<HttpResponseMessage>(typeof(HttpResponseMessage).FullName!, null);
+
+                return default;
+            }
+        }
     }
 }

src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpOptions.cs

@@ -23,6 +23,6 @@ namespace OpenIddict.Validation.SystemNetHttp
         public IAsyncPolicy<HttpResponseMessage>? HttpErrorPolicy { get; set; }
             = HttpPolicyExtensions.HandleTransientHttpError()
                 .OrResult(response => response.StatusCode == HttpStatusCode.NotFound)
-                .WaitAndRetryAsync(4, attempt => TimeSpan.FromSeconds(Math.Pow(2, attempt)));
+                .WaitAndRetryAsync(3, attempt => TimeSpan.FromSeconds(Math.Pow(2, attempt)));
     }
 }