diff --git a/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.Exchange.cs b/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.Exchange.cs index 7b89c3a0..a000101d 100644 --- a/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.Exchange.cs +++ b/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.Exchange.cs @@ -70,24 +70,30 @@ public static partial class OpenIddictClientSystemNetHttpHandlers var request = context.Transaction.GetHttpRequestMessage() ?? 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 default; - } - // The OAuth 2.0 specification recommends sending the client credentials using basic authentication. - // 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 client 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 token endpoint. + // However, this authentication method is known to have severe compatibility/interoperability issues: + // + // - While restricted to clients that have been given a secret (i.e confidential clients) by the + // specification, basic authentication is also sometimes required by server implementations for + // public clients that don't have a client secret: in this case, an empty password is used and + // the client identifier is sent alone in the Authorization header (instead of being sent using + // the standard "client_id" parameter present in the request body). + // + // - While the OAuth 2.0 specification requires that the client credentials be formURL-encoded + // before being base64-encoded, many implementations are known to implement a non-standard + // encoding scheme, where neither the client_id nor the client_secret are formURL-encoded. + // + // To guarantee that the OpenIddict implementation can be used with most servers implementions, + // basic authentication is only used when a client secret is present and client_secret_post is + // always preferred when it's explicitly listed as a supported client authentication method. // 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 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 (!context.Configuration.TokenEndpointAuthMethodsSupported.Contains(ClientAuthenticationMethods.ClientSecretPost)) + if (!string.IsNullOrEmpty(context.Request.ClientId) && + !string.IsNullOrEmpty(context.Request.ClientSecret) && + UseBasicAuthentication(context.Configuration)) { // Important: the credentials MUST be formURL-encoded before being base64-encoded. var credentials = Convert.ToBase64String(Encoding.ASCII.GetBytes(new StringBuilder() @@ -105,8 +111,19 @@ public static partial class OpenIddictClientSystemNetHttpHandlers return default; - static string? EscapeDataString(string? value) - => value is not null ? Uri.EscapeDataString(value).Replace("%20", "+") : null; + static bool UseBasicAuthentication(OpenIddictConfiguration configuration) + => configuration.TokenEndpointAuthMethodsSupported switch + { + // If at least one authentication method was explicit added, only use basic authentication + // if it's supported AND if client_secret_post is not supported or enabled by the server. + { Count: > 0 } methods => methods.Contains(ClientAuthenticationMethods.ClientSecretBasic) && + !methods.Contains(ClientAuthenticationMethods.ClientSecretPost), + + // Otherwise, if no authentication method was explicit added, assume only basic is supported. + { Count: _ } => true + }; + + static string EscapeDataString(string value) => Uri.EscapeDataString(value).Replace("%20", "+"); } } } diff --git a/src/OpenIddict.Client.WebIntegration/OpenIddictClientWebIntegrationHandlers.Exchange.cs b/src/OpenIddict.Client.WebIntegration/OpenIddictClientWebIntegrationHandlers.Exchange.cs index 0b2bd843..f90f1296 100644 --- a/src/OpenIddict.Client.WebIntegration/OpenIddictClientWebIntegrationHandlers.Exchange.cs +++ b/src/OpenIddict.Client.WebIntegration/OpenIddictClientWebIntegrationHandlers.Exchange.cs @@ -5,9 +5,13 @@ */ using System.Collections.Immutable; +using System.Diagnostics; +using System.Net.Http.Headers; +using System.Text; using OpenIddict.Extensions; using static OpenIddict.Client.SystemNetHttp.OpenIddictClientSystemNetHttpHandlerFilters; using static OpenIddict.Client.SystemNetHttp.OpenIddictClientSystemNetHttpHandlers; +using static OpenIddict.Client.SystemNetHttp.OpenIddictClientSystemNetHttpHandlers.Exchange; using static OpenIddict.Client.WebIntegration.OpenIddictClientWebIntegrationConstants; namespace OpenIddict.Client.WebIntegration; @@ -20,6 +24,7 @@ public static partial class OpenIddictClientWebIntegrationHandlers /* * Token request preparation: */ + AttachNonStandardBasicAuthenticationCredentials.Descriptor, AttachNonStandardQueryStringParameters.Descriptor, /* @@ -27,6 +32,68 @@ public static partial class OpenIddictClientWebIntegrationHandlers */ MapNonStandardResponseParameters.Descriptor); + /// + /// Contains the logic responsible for attaching the client credentials to the HTTP Authorization + /// header using a non-standard construction logic for the providers that require it. + /// + public sealed class AttachNonStandardBasicAuthenticationCredentials : IOpenIddictClientHandler + { + /// + /// Gets the default descriptor definition assigned to this handler. + /// + public static OpenIddictClientHandlerDescriptor Descriptor { get; } + = OpenIddictClientHandlerDescriptor.CreateBuilder() + .AddFilter() + .UseSingletonHandler() + .SetOrder(AttachBasicAuthenticationCredentials.Descriptor.Order + 500) + .SetType(OpenIddictClientHandlerType.BuiltIn) + .Build(); + + /// + public ValueTask HandleAsync(PrepareTokenRequestContext context) + { + if (context is null) + { + throw new ArgumentNullException(nameof(context)); + } + + // Some providers are known to incorrectly implement basic authentication support, either because + // an incorrect encoding scheme is used (e.g the credentials are not formURL-encoded as required + // by the OAuth 2.0 specification) or because basic authentication is required even for public + // clients, even though these clients don't have a secret (which requires using an empty password). + + Debug.Assert(context.Request is not null, SR.GetResourceString(SR.ID4008)); + + // 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() ?? + throw new InvalidOperationException(SR.GetResourceString(SR.ID0173)); + + // These providers require using basic authentication to flow the client_id + // for all types of client applications, even when there's no client_secret. + if (context.Registration.ProviderName is Providers.Reddit) + { + // 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)) + .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 payload to ensure they are not sent twice. + context.Request.ClientId = context.Request.ClientSecret = null; + } + + return default; + + static string? EscapeDataString(string? value) + => value is not null ? Uri.EscapeDataString(value).Replace("%20", "+") : null; + } + } + /// /// Contains the logic responsible for attaching non-standard query string /// parameters to the token request for the providers that require it. diff --git a/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.Introspection.cs b/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.Introspection.cs index a7ced04b..bd2ebd82 100644 --- a/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.Introspection.cs +++ b/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.Introspection.cs @@ -70,24 +70,30 @@ public static partial class OpenIddictValidationSystemNetHttpHandlers var request = context.Transaction.GetHttpRequestMessage() ?? 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 default; - } - // The OAuth 2.0 specification recommends sending the client credentials using basic authentication. - // 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. + // However, this authentication method is known to have severe compatibility/interoperability issues: + // + // - While restricted to clients that have been given a secret (i.e confidential clients) by the + // specification, basic authentication is also sometimes required by server implementations for + // public clients that don't have a client secret: in this case, an empty password is used and + // the client identifier is sent alone in the Authorization header (instead of being sent using + // the standard "client_id" parameter present in the request body). + // + // - While the OAuth 2.0 specification requires that the client credentials be formURL-encoded + // before being base64-encoded, many implementations are known to implement a non-standard + // encoding scheme, where neither the client_id nor the client_secret are formURL-encoded. + // + // To guarantee that the OpenIddict implementation can be used with most servers implementions, + // basic authentication is only used when a client secret is present and client_secret_post is + // always preferred when it's explicitly listed as a supported client authentication method. // 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 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 (!context.Configuration.IntrospectionEndpointAuthMethodsSupported.Contains(ClientAuthenticationMethods.ClientSecretPost)) + if (!string.IsNullOrEmpty(context.Request.ClientId) && + !string.IsNullOrEmpty(context.Request.ClientSecret) && + UseBasicAuthentication(context.Configuration)) { // Important: the credentials MUST be formURL-encoded before being base64-encoded. var credentials = Convert.ToBase64String(Encoding.ASCII.GetBytes(new StringBuilder() @@ -105,8 +111,19 @@ public static partial class OpenIddictValidationSystemNetHttpHandlers return default; - static string? EscapeDataString(string? value) - => value is not null ? Uri.EscapeDataString(value).Replace("%20", "+") : null; + static bool UseBasicAuthentication(OpenIddictConfiguration configuration) + => configuration.IntrospectionEndpointAuthMethodsSupported switch + { + // If at least one authentication method was explicit added, only use basic authentication + // if it's supported AND if client_secret_post is not supported or enabled by the server. + { Count: > 0 } methods => methods.Contains(ClientAuthenticationMethods.ClientSecretBasic) && + !methods.Contains(ClientAuthenticationMethods.ClientSecretPost), + + // Otherwise, if no authentication method was explicit added, assume only basic is supported. + { Count: _ } => true + }; + + static string EscapeDataString(string value) => Uri.EscapeDataString(value).Replace("%20", "+"); } } }