From d8a4451603ce2b5b48422c6ee658ef930ad1d41d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Sat, 10 Dec 2022 10:45:29 +0100 Subject: [PATCH] Support OpenIddictClientOptions.ClientUri/OpenIddictServerOptions.Issuer/OpenIddictValidationOptions.Issuer URIs created with DangerousDisablePathAndQueryCanonicalization=true --- .../OpenIddictResources.resx | 3 +++ .../OpenIddictClientHandlers.Discovery.cs | 4 ++-- .../OpenIddictClientHandlers.Protection.cs | 24 +++++++++++++++++-- .../OpenIddictServerHandlers.Protection.cs | 12 +++++++++- ...OpenIddictValidationHandlers.Protection.cs | 20 ++++++++++++---- 5 files changed, 53 insertions(+), 10 deletions(-) diff --git a/src/OpenIddict.Abstractions/OpenIddictResources.resx b/src/OpenIddict.Abstractions/OpenIddictResources.resx index 13050f5d..83772ba4 100644 --- a/src/OpenIddict.Abstractions/OpenIddictResources.resx +++ b/src/OpenIddict.Abstractions/OpenIddictResources.resx @@ -1885,6 +1885,9 @@ Consider registering a certificate using 'services.AddOpenIddict().AddClient().A The request forgery protection is missing or doesn't contain the expected value. + + The issuer returned in the server configuration doesn't match the value set in the client registration options. + The '{0}' parameter shouldn't be null or empty at this point. diff --git a/src/OpenIddict.Client/OpenIddictClientHandlers.Discovery.cs b/src/OpenIddict.Client/OpenIddictClientHandlers.Discovery.cs index 60298ce2..f9bd7995 100644 --- a/src/OpenIddict.Client/OpenIddictClientHandlers.Discovery.cs +++ b/src/OpenIddict.Client/OpenIddictClientHandlers.Discovery.cs @@ -230,8 +230,8 @@ public static partial class OpenIddictClientHandlers { context.Reject( error: Errors.ServerError, - description: SR.GetResourceString(SR.ID2098), - uri: SR.FormatID8000(SR.ID2098)); + description: SR.GetResourceString(SR.ID2165), + uri: SR.FormatID8000(SR.ID2165)); return default; } diff --git a/src/OpenIddict.Client/OpenIddictClientHandlers.Protection.cs b/src/OpenIddict.Client/OpenIddictClientHandlers.Protection.cs index d75924a9..4b822767 100644 --- a/src/OpenIddict.Client/OpenIddictClientHandlers.Protection.cs +++ b/src/OpenIddict.Client/OpenIddictClientHandlers.Protection.cs @@ -102,7 +102,7 @@ public static partial class OpenIddictClientHandlers { null => null, - // If the client URI doesn't contain any path/query/fragment, allow both http://www.fabrikam.com + // If the client URI doesn't contain any query/fragment, allow both http://www.fabrikam.com // and http://www.fabrikam.com/ (the recommended URI representation) to be considered valid. // See https://datatracker.ietf.org/doc/html/rfc3986#section-6.2.3 for more information. { AbsolutePath: "/", Query.Length: 0, Fragment.Length: 0 } uri => new[] @@ -111,6 +111,16 @@ public static partial class OpenIddictClientHandlers uri.AbsoluteUri[..^1] }, + // When properly normalized, Uri.AbsolutePath should never be empty and should at least + // contain a leading slash. While dangerous, System.Uri now offers a way to create a URI + // instance without applying the default canonicalization logic. To support such URIs, + // a special case is added here to add back the missing trailing slash when necessary. + { AbsolutePath.Length: 0, Query.Length: 0, Fragment.Length: 0 } uri => new[] + { + uri.AbsoluteUri, + uri.AbsoluteUri + "/" + }, + Uri uri => new[] { uri.AbsoluteUri } }; @@ -130,7 +140,7 @@ public static partial class OpenIddictClientHandlers { null => null, - // If the issuer URI doesn't contain any path/query/fragment, allow both http://www.fabrikam.com + // If the issuer URI doesn't contain any query/fragment, allow both http://www.fabrikam.com // and http://www.fabrikam.com/ (the recommended URI representation) to be considered valid. // See https://datatracker.ietf.org/doc/html/rfc3986#section-6.2.3 for more information. { AbsolutePath: "/", Query.Length: 0, Fragment.Length: 0 } uri => new[] @@ -139,6 +149,16 @@ public static partial class OpenIddictClientHandlers uri.AbsoluteUri[..^1] }, + // When properly normalized, Uri.AbsolutePath should never be empty and should at least + // contain a leading slash. While dangerous, System.Uri now offers a way to create a URI + // instance without applying the default canonicalization logic. To support such URIs, + // a special case is added here to add back the missing trailing slash when necessary. + { AbsolutePath.Length: 0, Query.Length: 0, Fragment.Length: 0 } uri => new[] + { + uri.AbsoluteUri, + uri.AbsoluteUri + "/" + }, + Uri uri => new[] { uri.AbsoluteUri } }; diff --git a/src/OpenIddict.Server/OpenIddictServerHandlers.Protection.cs b/src/OpenIddict.Server/OpenIddictServerHandlers.Protection.cs index 3b32fe62..bf31d50e 100644 --- a/src/OpenIddict.Server/OpenIddictServerHandlers.Protection.cs +++ b/src/OpenIddict.Server/OpenIddictServerHandlers.Protection.cs @@ -73,7 +73,7 @@ public static partial class OpenIddictServerHandlers { null => null, - // If the issuer URI doesn't contain any path/query/fragment, allow both http://www.fabrikam.com + // If the issuer URI doesn't contain any query/fragment, allow both http://www.fabrikam.com // and http://www.fabrikam.com/ (the recommended URI representation) to be considered valid. // See https://datatracker.ietf.org/doc/html/rfc3986#section-6.2.3 for more information. { AbsolutePath: "/", Query.Length: 0, Fragment.Length: 0 } uri => new[] @@ -82,6 +82,16 @@ public static partial class OpenIddictServerHandlers uri.AbsoluteUri[..^1] }, + // When properly normalized, Uri.AbsolutePath should never be empty and should at least + // contain a leading slash. While dangerous, System.Uri now offers a way to create a URI + // instance without applying the default canonicalization logic. To support such URIs, + // a special case is added here to add back the missing trailing slash when necessary. + { AbsolutePath.Length: 0, Query.Length: 0, Fragment.Length: 0 } uri => new[] + { + uri.AbsoluteUri, + uri.AbsoluteUri + "/" + }, + Uri uri => new[] { uri.AbsoluteUri } }; diff --git a/src/OpenIddict.Validation/OpenIddictValidationHandlers.Protection.cs b/src/OpenIddict.Validation/OpenIddictValidationHandlers.Protection.cs index dbc08b76..ddd0d47f 100644 --- a/src/OpenIddict.Validation/OpenIddictValidationHandlers.Protection.cs +++ b/src/OpenIddict.Validation/OpenIddictValidationHandlers.Protection.cs @@ -71,16 +71,26 @@ public static partial class OpenIddictValidationHandlers // issued by a local authorization server outside a request context). null => null, - // If the issuer URI doesn't contain any path/query/fragment, allow both http://www.fabrikam.com + // If the issuer URI doesn't contain any query/fragment, allow both http://www.fabrikam.com // and http://www.fabrikam.com/ (the recommended URI representation) to be considered valid. // See https://datatracker.ietf.org/doc/html/rfc3986#section-6.2.3 for more information. - { AbsolutePath: "/", Query.Length: 0, Fragment.Length: 0 } issuer => new[] + { AbsolutePath: "/", Query.Length: 0, Fragment.Length: 0 } uri => new[] { - issuer.AbsoluteUri, // Uri.AbsoluteUri is normalized and always contains a trailing slash. - issuer.AbsoluteUri[..^1] + uri.AbsoluteUri, // Uri.AbsoluteUri is normalized and always contains a trailing slash. + uri.AbsoluteUri[..^1] }, - Uri issuer => new[] { issuer.AbsoluteUri } + // When properly normalized, Uri.AbsolutePath should never be empty and should at least + // contain a leading slash. While dangerous, System.Uri now offers a way to create a URI + // instance without applying the default canonicalization logic. To support such URIs, + // a special case is added here to add back the missing trailing slash when necessary. + { AbsolutePath.Length: 0, Query.Length: 0, Fragment.Length: 0 } uri => new[] + { + uri.AbsoluteUri, + uri.AbsoluteUri + "/" + }, + + Uri uri => new[] { uri.AbsoluteUri } }; parameters.ValidateIssuer = parameters.ValidIssuers is not null;