From 3ff021a3e45eac783e9b99d4fb0bdb2c4ddf35b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Sun, 15 May 2022 16:04:55 +0200 Subject: [PATCH] Revamp HTTP response extraction to support WWW-Authenticate and enforce Content-Type validation --- .../OpenIddictConstants.cs | 2 + .../OpenIddictResources.resx | 38 +++- .../OpenIddictClientSystemNetHttpConstants.cs | 3 +- ...ctClientSystemNetHttpHandlers.Discovery.cs | 4 + ...ictClientSystemNetHttpHandlers.Exchange.cs | 2 + ...ictClientSystemNetHttpHandlers.Userinfo.cs | 56 +---- .../OpenIddictClientSystemNetHttpHandlers.cs | 191 +++++++++++++++--- .../OpenIddictClientService.cs | 8 +- ...nIddictValidationSystemNetHttpConstants.cs | 18 ++ ...lidationSystemNetHttpHandlers.Discovery.cs | 4 + ...tionSystemNetHttpHandlers.Introspection.cs | 2 + ...enIddictValidationSystemNetHttpHandlers.cs | 191 +++++++++++++++--- .../OpenIddictServerIntegrationTestClient.cs | 30 +-- 13 files changed, 438 insertions(+), 111 deletions(-) create mode 100644 src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpConstants.cs diff --git a/src/OpenIddict.Abstractions/OpenIddictConstants.cs b/src/OpenIddict.Abstractions/OpenIddictConstants.cs index f0fe353f..f5979ba0 100644 --- a/src/OpenIddict.Abstractions/OpenIddictConstants.cs +++ b/src/OpenIddict.Abstractions/OpenIddictConstants.cs @@ -446,8 +446,10 @@ public static class OpenIddictConstants public static class Separators { public static readonly char[] Ampersand = { '&' }; + public static readonly char[] Comma = { ',' }; public static readonly char[] Dash = { '-' }; public static readonly char[] Dot = { '.' }; + public static readonly char[] DoubleQuote = { '"' }; public static readonly char[] EqualsSign = { '=' }; public static readonly char[] Hash = { '#' }; public static readonly char[] QuestionMark = { '?' }; diff --git a/src/OpenIddict.Abstractions/OpenIddictResources.resx b/src/OpenIddict.Abstractions/OpenIddictResources.resx index 3cb589eb..a7c2e1dd 100644 --- a/src/OpenIddict.Abstractions/OpenIddictResources.resx +++ b/src/OpenIddict.Abstractions/OpenIddictResources.resx @@ -1256,6 +1256,36 @@ Alternatively, you can disable the token storage feature by calling 'services.Ad Error description: {1} Error URI: {2} + + An error occurred while preparing the userinfo request. + Error: {0} + Error description: {1} + Error URI: {2} + + + An error occurred while sending the userinfo request. + Error: {0} + Error description: {1} + Error URI: {2} + + + An error occurred while extracting the userinfo response. + Error: {0} + Error description: {1} + Error URI: {2} + + + An error occurred while handling the userinfo response. + Error: {0} + Error description: {1} + Error URI: {2} + + + A generic error was returned by the remote authorization server. + + + An unsupported response was returned by the remote authorization server. + The security token is missing. @@ -2222,7 +2252,13 @@ This may indicate that the hashed entry is corrupted or malformed. A network error occured while communicating with the remote HTTP server. - An invalid JSON response was returned by the remote HTTP server: {Payload}. + An invalid JSON payload was returned by the remote HTTP server: {Payload}. + + + A generic {StatusCode} response was returned by the remote HTTP server: {Payload}. + + + An unsupported {StatusCode} response was returned by the remote HTTP server: {ContentType} {Payload}. https://documentation.openiddict.com/errors/{0} diff --git a/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpConstants.cs b/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpConstants.cs index 5ca792c5..e8ed863c 100644 --- a/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpConstants.cs +++ b/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpConstants.cs @@ -11,8 +11,9 @@ namespace OpenIddict.Client.SystemNetHttp; /// public static class OpenIddictClientSystemNetHttpConstants { - public static class ContentTypes + public static class MediaTypes { + public const string Json = "application/json"; public const string JsonWebToken = "application/jwt"; } } diff --git a/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.Discovery.cs b/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.Discovery.cs index c928a5f5..64df5c53 100644 --- a/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.Discovery.cs +++ b/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.Discovery.cs @@ -25,6 +25,8 @@ public static partial class OpenIddictClientSystemNetHttpHandlers * Configuration response processing: */ ExtractJsonHttpResponse.Descriptor, + ExtractWwwAuthenticateHeader.Descriptor, + ValidateHttpResponse.Descriptor, DisposeHttpResponse.Descriptor, /* @@ -39,6 +41,8 @@ public static partial class OpenIddictClientSystemNetHttpHandlers * Configuration response processing: */ ExtractJsonHttpResponse.Descriptor, + ExtractWwwAuthenticateHeader.Descriptor, + ValidateHttpResponse.Descriptor, DisposeHttpResponse.Descriptor); } } diff --git a/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.Exchange.cs b/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.Exchange.cs index e88f26ca..0a8d89f5 100644 --- a/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.Exchange.cs +++ b/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.Exchange.cs @@ -29,6 +29,8 @@ public static partial class OpenIddictClientSystemNetHttpHandlers * Token response processing: */ ExtractJsonHttpResponse.Descriptor, + ExtractWwwAuthenticateHeader.Descriptor, + ValidateHttpResponse.Descriptor, DisposeHttpResponse.Descriptor); /// diff --git a/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.Userinfo.cs b/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.Userinfo.cs index 61ead133..77df0ca4 100644 --- a/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.Userinfo.cs +++ b/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.Userinfo.cs @@ -7,8 +7,6 @@ using System.Collections.Immutable; using System.Diagnostics; using System.Net.Http.Headers; -using System.Text.Json; -using Microsoft.Extensions.Logging; using static OpenIddict.Client.SystemNetHttp.OpenIddictClientSystemNetHttpConstants; namespace OpenIddict.Client.SystemNetHttp; @@ -30,7 +28,10 @@ public static partial class OpenIddictClientSystemNetHttpHandlers /* * Userinfo response processing: */ - ExtractUserinfoHttpResponse.Descriptor, + ExtractUserinfoTokenHttpResponse.Descriptor, + ExtractJsonHttpResponse.Descriptor, + ExtractWwwAuthenticateHeader.Descriptor, + ValidateHttpResponse.Descriptor, DisposeHttpResponse.Descriptor); /// @@ -77,7 +78,7 @@ public static partial class OpenIddictClientSystemNetHttpHandlers /// /// Contains the logic responsible for extracting the response from the userinfo response. /// - public class ExtractUserinfoHttpResponse : IOpenIddictClientHandler + public class ExtractUserinfoTokenHttpResponse : IOpenIddictClientHandler { /// /// Gets the default descriptor definition assigned to this handler. @@ -85,8 +86,8 @@ public static partial class OpenIddictClientSystemNetHttpHandlers public static OpenIddictClientHandlerDescriptor Descriptor { get; } = OpenIddictClientHandlerDescriptor.CreateBuilder() .AddFilter() - .UseSingletonHandler() - .SetOrder(DisposeHttpResponse.Descriptor.Order - 50_000) + .UseSingletonHandler() + .SetOrder(ExtractJsonHttpResponse.Descriptor.Order - 500) .SetType(OpenIddictClientHandlerType.BuiltIn) .Build(); @@ -116,54 +117,17 @@ public static partial class OpenIddictClientSystemNetHttpHandlers // - application/json responses containing a JSON object listing the user claims as-is. // - application/jwt responses containing a signed/encrypted JSON Web Token containing the user claims. // - // As such, this handler implements a selection routine to extract the userinfo token as-is - // if the media type is application/jwt and fall back to JSON in any other case. + // To support both types, this handler will try to extract the userinfo token as-is if the media type + // is application/jwt and will rely on other handlers in the pipeline to process regular JSON responses. if (string.Equals(response.Content.Headers.ContentType?.MediaType, - ContentTypes.JsonWebToken, StringComparison.OrdinalIgnoreCase)) + MediaTypes.JsonWebToken, StringComparison.OrdinalIgnoreCase)) { context.Response = new OpenIddictResponse(); context.UserinfoToken = await response.Content.ReadAsStringAsync(); return; } - - try - { - try - { - // Note: ReadFromJsonAsync() automatically validates the content encoding and transparently - // transcodes the response stream if a non-UTF-8 response is returned by the remote server. - context.Response = await response.Content.ReadFromJsonAsync(); - } - - // Initial versions of System.Net.Http.Json were known to eagerly validate the media type returned - // as part of the HTTP Content-Type header and throw a NotSupportedException. If such an exception - // is caught, try to extract the response using the less efficient string-based deserialization, - // that will also take care of handling non-UTF-8 encodings but won't validate the media type. - catch (NotSupportedException) - { - context.Response = JsonSerializer.Deserialize( - await response.Content.ReadAsStringAsync()); - } - } - - // If an exception is thrown at this stage, this likely means the returned response was not a valid - // JSON response or was not correctly formatted as a JSON object. This typically happens when - // a server error occurs and a default error page is returned by the remote HTTP server. - // In this case, log the error details and return a generic error to stop processing the event. - catch (Exception exception) - { - context.Logger.LogError(exception, SR.GetResourceString(SR.ID6183), - await response.Content.ReadAsStringAsync()); - - context.Reject( - error: Errors.ServerError, - description: SR.GetResourceString(SR.ID2137), - uri: SR.FormatID8000(SR.ID2137)); - - return; - } } } } diff --git a/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.cs b/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.cs index b9920aa4..69184a58 100644 --- a/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.cs +++ b/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.cs @@ -10,8 +10,9 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Net.Http.Headers; using System.Text; -using System.Text.Json; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Primitives; +using static OpenIddict.Client.SystemNetHttp.OpenIddictClientSystemNetHttpConstants; namespace OpenIddict.Client.SystemNetHttp; @@ -139,7 +140,7 @@ public static partial class OpenIddictClientSystemNetHttpHandlers var request = context.Transaction.GetHttpRequestMessage() ?? throw new InvalidOperationException(SR.GetResourceString(SR.ID0173)); - if (request.RequestUri is null || context.Transaction.Request.Count == 0) + if (request.RequestUri is null || context.Transaction.Request.Count is 0) { return default; } @@ -333,7 +334,7 @@ public static partial class OpenIddictClientSystemNetHttpHandlers = OpenIddictClientHandlerDescriptor.CreateBuilder() .AddFilter() .UseSingletonHandler>() - .SetOrder(DisposeHttpResponse.Descriptor.Order - 50_000) + .SetOrder(ExtractWwwAuthenticateHeader.Descriptor.Order - 1_000) .SetType(OpenIddictClientHandlerType.BuiltIn) .Build(); @@ -356,33 +357,24 @@ public static partial class OpenIddictClientSystemNetHttpHandlers var response = context.Transaction.GetHttpResponseMessage() ?? throw new InvalidOperationException(SR.GetResourceString(SR.ID0173)); - // The status code is deliberately not validated to ensure even errored responses - // (typically in the 4xx range) can be deserialized and handled by the event handlers. + // If the returned Content-Type doesn't indicate the response has a JSON payload, + // ignore it and allow other handlers in the pipeline to process the HTTP response. + if (!string.Equals(response.Content.Headers.ContentType?.MediaType, + MediaTypes.Json, StringComparison.OrdinalIgnoreCase)) + { + return; + } try { - try - { - // Note: ReadFromJsonAsync() automatically validates the content encoding and transparently - // transcodes the response stream if a non-UTF-8 response is returned by the remote server. - context.Transaction.Response = await response.Content.ReadFromJsonAsync(); - } - - // Initial versions of System.Net.Http.Json were known to eagerly validate the media type returned - // as part of the HTTP Content-Type header and throw a NotSupportedException. If such an exception - // is caught, try to extract the response using the less efficient string-based deserialization, - // that will also take care of handling non-UTF-8 encodings but won't validate the media type. - catch (NotSupportedException) - { - context.Transaction.Response = JsonSerializer.Deserialize( - await response.Content.ReadAsStringAsync()); - } + // Note: ReadFromJsonAsync() automatically validates the content encoding and transparently + // transcodes the response stream if a non-UTF-8 response is returned by the remote server. + context.Transaction.Response = await response.Content.ReadFromJsonAsync(); } // If an exception is thrown at this stage, this likely means the returned response was not a valid // JSON response or was not correctly formatted as a JSON object. This typically happens when - // a server error occurs and a default error page is returned by the remote HTTP server. - // In this case, log the error details and return a generic error to stop processing the event. + // a server error occurs while the JSON response is being generated and returned to the client. catch (Exception exception) { context.Logger.LogError(exception, SR.GetResourceString(SR.ID6183), @@ -398,6 +390,159 @@ public static partial class OpenIddictClientSystemNetHttpHandlers } } + /// + /// Contains the logic responsible for extracting errors from WWW-Authenticate headers. + /// + public class ExtractWwwAuthenticateHeader : IOpenIddictClientHandler where TContext : BaseExternalContext + { + /// + /// Gets the default descriptor definition assigned to this handler. + /// + public static OpenIddictClientHandlerDescriptor Descriptor { get; } + = OpenIddictClientHandlerDescriptor.CreateBuilder() + .AddFilter() + .UseSingletonHandler>() + .SetOrder(ValidateHttpResponse.Descriptor.Order - 1_000) + .SetType(OpenIddictClientHandlerType.BuiltIn) + .Build(); + + /// + public ValueTask HandleAsync(TContext context) + { + if (context is null) + { + throw new ArgumentNullException(nameof(context)); + } + + // Don't overwrite the response if one was already provided. + if (context.Transaction.Response is not null) + { + return default; + } + + // 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() ?? + throw new InvalidOperationException(SR.GetResourceString(SR.ID0173)); + + if (response.Headers.WwwAuthenticate.Count is 0) + { + return default; + } + + var parameters = new Dictionary(response.Headers.WwwAuthenticate.Count); + + foreach (var header in response.Headers.WwwAuthenticate) + { + if (string.IsNullOrEmpty(header.Parameter)) + { + continue; + } + + // Note: while initially not allowed by the core OAuth 2.0 specification, multiple + // parameters with the same name are used by derived drafts like the OAuth 2.0 + // token exchange specification. For consistency, multiple parameters with the + // same name are also supported when returned as part of WWW-Authentication headers. + + foreach (var parameter in header.Parameter.Split(Separators.Comma, StringSplitOptions.RemoveEmptyEntries)) + { + var values = parameter.Split(Separators.EqualsSign, StringSplitOptions.RemoveEmptyEntries); + if (values.Length is not 2) + { + continue; + } + + var (name, value) = ( + values[0]?.Trim(Separators.Space[0]), + values[1]?.Trim(Separators.Space[0], Separators.DoubleQuote[0])); + + if (string.IsNullOrEmpty(name)) + { + continue; + } + + parameters[name] = parameters.ContainsKey(name) ? + StringValues.Concat(parameters[name], value?.Replace("\\\"", "\"")) : + new StringValues(value?.Replace("\\\"", "\"")); + } + } + + context.Transaction.Response = new OpenIddictResponse(parameters); + + return default; + } + } + + /// + /// Contains the logic responsible for extracting errors from WWW-Authenticate headers. + /// + public class ValidateHttpResponse : IOpenIddictClientHandler where TContext : BaseExternalContext + { + /// + /// Gets the default descriptor definition assigned to this handler. + /// + public static OpenIddictClientHandlerDescriptor Descriptor { get; } + = OpenIddictClientHandlerDescriptor.CreateBuilder() + .AddFilter() + .UseSingletonHandler>() + .SetOrder(DisposeHttpResponse.Descriptor.Order - 50_000) + .SetType(OpenIddictClientHandlerType.BuiltIn) + .Build(); + + /// + public async 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() ?? + throw new InvalidOperationException(SR.GetResourceString(SR.ID0173)); + + // At this stage, return a generic error based on the HTTP status code if no + // error could be extracted from the payload or from the WWW-Authenticate header. + if (!response.IsSuccessStatusCode && string.IsNullOrEmpty(context.Transaction.Response?.Error)) + { + context.Logger.LogError(SR.GetResourceString(SR.ID6184), response.StatusCode, + await response.Content.ReadAsStringAsync()); + + context.Reject( + error: (int) response.StatusCode switch + { + 400 => Errors.InvalidRequest, + 401 => Errors.InvalidToken, + 403 => Errors.InsufficientAccess, + 429 => Errors.SlowDown, + 500 => Errors.ServerError, + 503 => Errors.TemporarilyUnavailable, + _ => Errors.ServerError + }, + description: SR.GetResourceString(SR.ID0328), + uri: SR.FormatID8000(SR.ID0328)); + + return; + } + + // If no other event handler was able to extract the response payload at this point + // (e.g because an unsupported content type was returned), return a generic error. + if (context.Transaction.Response is null) + { + context.Logger.LogError(SR.GetResourceString(SR.ID6185), response.StatusCode, + response.Content.Headers.ContentType, await response.Content.ReadAsStringAsync()); + + context.Reject( + error: Errors.ServerError, + description: SR.GetResourceString(SR.ID0329), + uri: SR.FormatID8000(SR.ID0329)); + + return; + } + } + } + /// /// Contains the logic responsible for disposing of the HTTP response message. /// diff --git a/src/OpenIddict.Client/OpenIddictClientService.cs b/src/OpenIddict.Client/OpenIddictClientService.cs index 42356489..fd3c0335 100644 --- a/src/OpenIddict.Client/OpenIddictClientService.cs +++ b/src/OpenIddict.Client/OpenIddictClientService.cs @@ -695,7 +695,7 @@ public class OpenIddictClientService if (context.IsRejected) { throw new OpenIddictExceptions.GenericException( - SR.FormatID0152(context.Error, context.ErrorDescription, context.ErrorUri), + SR.FormatID0324(context.Error, context.ErrorDescription, context.ErrorUri), context.Error, context.ErrorDescription, context.ErrorUri); } @@ -718,7 +718,7 @@ public class OpenIddictClientService if (context.IsRejected) { throw new OpenIddictExceptions.GenericException( - SR.FormatID0153(context.Error, context.ErrorDescription, context.ErrorUri), + SR.FormatID0325(context.Error, context.ErrorDescription, context.ErrorUri), context.Error, context.ErrorDescription, context.ErrorUri); } @@ -741,7 +741,7 @@ public class OpenIddictClientService if (context.IsRejected) { throw new OpenIddictExceptions.GenericException( - SR.FormatID0154(context.Error, context.ErrorDescription, context.ErrorUri), + SR.FormatID0326(context.Error, context.ErrorDescription, context.ErrorUri), context.Error, context.ErrorDescription, context.ErrorUri); } @@ -768,7 +768,7 @@ public class OpenIddictClientService if (context.IsRejected) { throw new OpenIddictExceptions.GenericException( - SR.FormatID0155(context.Error, context.ErrorDescription, context.ErrorUri), + SR.FormatID0327(context.Error, context.ErrorDescription, context.ErrorUri), context.Error, context.ErrorDescription, context.ErrorUri); } diff --git a/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpConstants.cs b/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpConstants.cs new file mode 100644 index 00000000..5ea91dec --- /dev/null +++ b/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpConstants.cs @@ -0,0 +1,18 @@ +/* + * Licensed under the Apache License, Version 2.0 (http://www.apache.org/licenses/LICENSE-2.0) + * See https://github.com/openiddict/openiddict-core for more information concerning + * the license and the contributors participating to this project. + */ + +namespace OpenIddict.Validation.SystemNetHttp; + +/// +/// Exposes common constants used by the OpenIddict System.Net.Http integration. +/// +public static class OpenIddictValidationSystemNetHttpConstants +{ + public static class MediaTypes + { + public const string Json = "application/json"; + } +} diff --git a/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.Discovery.cs b/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.Discovery.cs index f6ed6769..33e947aa 100644 --- a/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.Discovery.cs +++ b/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.Discovery.cs @@ -25,6 +25,8 @@ public static partial class OpenIddictValidationSystemNetHttpHandlers * Configuration response processing: */ ExtractJsonHttpResponse.Descriptor, + ExtractWwwAuthenticateHeader.Descriptor, + ValidateHttpResponse.Descriptor, DisposeHttpResponse.Descriptor, /* @@ -39,6 +41,8 @@ public static partial class OpenIddictValidationSystemNetHttpHandlers * Configuration response processing: */ ExtractJsonHttpResponse.Descriptor, + ExtractWwwAuthenticateHeader.Descriptor, + ValidateHttpResponse.Descriptor, DisposeHttpResponse.Descriptor); } } diff --git a/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.Introspection.cs b/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.Introspection.cs index 967bf1e9..e6fcd2f9 100644 --- a/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.Introspection.cs +++ b/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.Introspection.cs @@ -29,6 +29,8 @@ public static partial class OpenIddictValidationSystemNetHttpHandlers * Introspection response processing: */ ExtractJsonHttpResponse.Descriptor, + ExtractWwwAuthenticateHeader.Descriptor, + ValidateHttpResponse.Descriptor, DisposeHttpResponse.Descriptor); /// diff --git a/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.cs b/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.cs index 9772e18e..c3ed9070 100644 --- a/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.cs +++ b/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.cs @@ -10,8 +10,9 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Net.Http.Headers; using System.Text; -using System.Text.Json; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Primitives; +using static OpenIddict.Validation.SystemNetHttp.OpenIddictValidationSystemNetHttpConstants; namespace OpenIddict.Validation.SystemNetHttp; @@ -138,7 +139,7 @@ public static partial class OpenIddictValidationSystemNetHttpHandlers var request = context.Transaction.GetHttpRequestMessage() ?? throw new InvalidOperationException(SR.GetResourceString(SR.ID0173)); - if (request.RequestUri is null || context.Transaction.Request.Count == 0) + if (request.RequestUri is null || context.Transaction.Request.Count is 0) { return default; } @@ -332,7 +333,7 @@ public static partial class OpenIddictValidationSystemNetHttpHandlers = OpenIddictValidationHandlerDescriptor.CreateBuilder() .AddFilter() .UseSingletonHandler>() - .SetOrder(DisposeHttpResponse.Descriptor.Order - 50_000) + .SetOrder(ExtractWwwAuthenticateHeader.Descriptor.Order - 1_000) .SetType(OpenIddictValidationHandlerType.BuiltIn) .Build(); @@ -355,33 +356,24 @@ public static partial class OpenIddictValidationSystemNetHttpHandlers var response = context.Transaction.GetHttpResponseMessage() ?? throw new InvalidOperationException(SR.GetResourceString(SR.ID0173)); - // The status code is deliberately not validated to ensure even errored responses - // (typically in the 4xx range) can be deserialized and handled by the event handlers. + // If the returned Content-Type doesn't indicate the response has a JSON payload, + // ignore it and allow other handlers in the pipeline to process the HTTP response. + if (!string.Equals(response.Content.Headers.ContentType?.MediaType, + MediaTypes.Json, StringComparison.OrdinalIgnoreCase)) + { + return; + } try { - try - { - // Note: ReadFromJsonAsync() automatically validates the content encoding and transparently - // transcodes the response stream if a non-UTF-8 response is returned by the remote server. - context.Transaction.Response = await response.Content.ReadFromJsonAsync(); - } - - // Initial versions of System.Net.Http.Json were known to eagerly validate the media type returned - // as part of the HTTP Content-Type header and throw a NotSupportedException. If such an exception - // is caught, try to extract the response using the less efficient string-based deserialization, - // that will also take care of handling non-UTF-8 encodings but won't validate the media type. - catch (NotSupportedException) - { - context.Transaction.Response = JsonSerializer.Deserialize( - await response.Content.ReadAsStringAsync()); - } + // Note: ReadFromJsonAsync() automatically validates the content encoding and transparently + // transcodes the response stream if a non-UTF-8 response is returned by the remote server. + context.Transaction.Response = await response.Content.ReadFromJsonAsync(); } // If an exception is thrown at this stage, this likely means the returned response was not a valid // JSON response or was not correctly formatted as a JSON object. This typically happens when - // a server error occurs and a default error page is returned by the remote HTTP server. - // In this case, log the error details and return a generic error to stop processing the event. + // a server error occurs while the JSON response is being generated and returned to the client. catch (Exception exception) { context.Logger.LogError(exception, SR.GetResourceString(SR.ID6183), @@ -397,6 +389,159 @@ public static partial class OpenIddictValidationSystemNetHttpHandlers } } + /// + /// Contains the logic responsible for extracting errors from WWW-Authenticate headers. + /// + public class ExtractWwwAuthenticateHeader : IOpenIddictValidationHandler where TContext : BaseExternalContext + { + /// + /// Gets the default descriptor definition assigned to this handler. + /// + public static OpenIddictValidationHandlerDescriptor Descriptor { get; } + = OpenIddictValidationHandlerDescriptor.CreateBuilder() + .AddFilter() + .UseSingletonHandler>() + .SetOrder(ValidateHttpResponse.Descriptor.Order - 1_000) + .SetType(OpenIddictValidationHandlerType.BuiltIn) + .Build(); + + /// + public ValueTask HandleAsync(TContext context) + { + if (context is null) + { + throw new ArgumentNullException(nameof(context)); + } + + // Don't overwrite the response if one was already provided. + if (context.Transaction.Response is not null) + { + return default; + } + + // 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() ?? + throw new InvalidOperationException(SR.GetResourceString(SR.ID0173)); + + if (response.Headers.WwwAuthenticate.Count is 0) + { + return default; + } + + var parameters = new Dictionary(response.Headers.WwwAuthenticate.Count); + + foreach (var header in response.Headers.WwwAuthenticate) + { + if (string.IsNullOrEmpty(header.Parameter)) + { + continue; + } + + // Note: while initially not allowed by the core OAuth 2.0 specification, multiple + // parameters with the same name are used by derived drafts like the OAuth 2.0 + // token exchange specification. For consistency, multiple parameters with the + // same name are also supported when returned as part of WWW-Authentication headers. + + foreach (var parameter in header.Parameter.Split(Separators.Comma, StringSplitOptions.RemoveEmptyEntries)) + { + var values = parameter.Split(Separators.EqualsSign, StringSplitOptions.RemoveEmptyEntries); + if (values.Length is not 2) + { + continue; + } + + var (name, value) = ( + values[0]?.Trim(Separators.Space[0]), + values[1]?.Trim(Separators.Space[0], Separators.DoubleQuote[0])); + + if (string.IsNullOrEmpty(name)) + { + continue; + } + + parameters[name] = parameters.ContainsKey(name) ? + StringValues.Concat(parameters[name], value?.Replace("\\\"", "\"")) : + new StringValues(value?.Replace("\\\"", "\"")); + } + } + + context.Transaction.Response = new OpenIddictResponse(parameters); + + return default; + } + } + + /// + /// Contains the logic responsible for extracting errors from WWW-Authenticate headers. + /// + public class ValidateHttpResponse : IOpenIddictValidationHandler where TContext : BaseExternalContext + { + /// + /// Gets the default descriptor definition assigned to this handler. + /// + public static OpenIddictValidationHandlerDescriptor Descriptor { get; } + = OpenIddictValidationHandlerDescriptor.CreateBuilder() + .AddFilter() + .UseSingletonHandler>() + .SetOrder(DisposeHttpResponse.Descriptor.Order - 50_000) + .SetType(OpenIddictValidationHandlerType.BuiltIn) + .Build(); + + /// + public async 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() ?? + throw new InvalidOperationException(SR.GetResourceString(SR.ID0173)); + + // At this stage, return a generic error based on the HTTP status code if no + // error could be extracted from the payload or from the WWW-Authenticate header. + if (!response.IsSuccessStatusCode && string.IsNullOrEmpty(context.Transaction.Response?.Error)) + { + context.Logger.LogError(SR.GetResourceString(SR.ID6184), response.StatusCode, + await response.Content.ReadAsStringAsync()); + + context.Reject( + error: (int) response.StatusCode switch + { + 400 => Errors.InvalidRequest, + 401 => Errors.InvalidToken, + 403 => Errors.InsufficientAccess, + 429 => Errors.SlowDown, + 500 => Errors.ServerError, + 503 => Errors.TemporarilyUnavailable, + _ => Errors.ServerError + }, + description: SR.GetResourceString(SR.ID0328), + uri: SR.FormatID8000(SR.ID0328)); + + return; + } + + // If no other event handler was able to extract the response payload at this point + // (e.g because an unsupported content type was returned), return a generic error. + if (context.Transaction.Response is null) + { + context.Logger.LogError(SR.GetResourceString(SR.ID6185), response.StatusCode, + response.Content.Headers.ContentType, await response.Content.ReadAsStringAsync()); + + context.Reject( + error: Errors.ServerError, + description: SR.GetResourceString(SR.ID0329), + uri: SR.FormatID8000(SR.ID0329)); + + return; + } + } + } + /// /// Contains the logic responsible for disposing of the HTTP response message. /// diff --git a/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTestClient.cs b/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTestClient.cs index ea22a65e..229c4f1e 100644 --- a/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTestClient.cs +++ b/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTestClient.cs @@ -307,9 +307,9 @@ public class OpenIddictServerIntegrationTestClient : IAsyncDisposable private async Task GetResponseAsync(HttpResponseMessage message) { - if (message.Headers.WwwAuthenticate.Count != 0) + if (message.Headers.WwwAuthenticate.Count is not 0) { - var response = new OpenIddictResponse(); + var parameters = new Dictionary(message.Headers.WwwAuthenticate.Count); foreach (var header in message.Headers.WwwAuthenticate) { @@ -318,31 +318,35 @@ public class OpenIddictServerIntegrationTestClient : IAsyncDisposable continue; } - foreach (var parameter in header.Parameter.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries)) + // Note: while initially not allowed by the core OAuth 2.0 specification, multiple + // parameters with the same name are used by derived drafts like the OAuth 2.0 + // token exchange specification. For consistency, multiple parameters with the + // same name are also supported when returned as part of WWW-Authentication headers. + + foreach (var parameter in header.Parameter.Split(Separators.Comma, StringSplitOptions.RemoveEmptyEntries)) { - var values = parameter.Split(new[] { '=' }, StringSplitOptions.RemoveEmptyEntries); - if (values.Length != 2) + var values = parameter.Split(Separators.EqualsSign, StringSplitOptions.RemoveEmptyEntries); + if (values.Length is not 2) { continue; } - var name = values[0]?.Trim(' ', '"'); - if (string.IsNullOrEmpty(name)) - { - continue; - } + var (name, value) = ( + values[0]?.Trim(Separators.Space[0]), + values[1]?.Trim(Separators.Space[0], Separators.DoubleQuote[0])); - var value = values[1]?.Trim(' ', '"'); if (string.IsNullOrEmpty(name)) { continue; } - response.SetParameter(name, value); + parameters[name] = parameters.ContainsKey(name) ? + StringValues.Concat(parameters[name], value?.Replace("\\\"", "\"")) : + new StringValues(value?.Replace("\\\"", "\"")); } } - return response; + return new OpenIddictResponse(parameters); } else if (message.Headers.Location is not null)