From db4fb6fc370e57e46d9eb1dac8b19dc0d4e9b0fe Mon Sep 17 00:00:00 2001 From: Eric Quist Date: Sat, 1 Aug 2026 10:27:36 +0200 Subject: [PATCH 1/4] Nullable improvments for StringExtensions. --- .../src/Extensions/StringsExtensions.cs | 29 ++++++++++--------- .../Utility/InternalStringExtensions.cs | 5 ++-- .../src/Extensions/StringsExtensions.cs | 8 +++-- 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/src/Open.IdentityServer/src/Extensions/StringsExtensions.cs b/src/Open.IdentityServer/src/Extensions/StringsExtensions.cs index 31f86870b..d12d60a53 100644 --- a/src/Open.IdentityServer/src/Extensions/StringsExtensions.cs +++ b/src/Open.IdentityServer/src/Extensions/StringsExtensions.cs @@ -7,10 +7,13 @@ using System.Collections.Generic; using System.Collections.Specialized; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Text; using System.Text.Encodings.Web; +#nullable enable + namespace Open.IdentityServer.Extensions; internal static class StringExtensions @@ -47,7 +50,7 @@ public static IEnumerable FromSpaceSeparatedString(this string input) return input.Split(new[] { ' ' }, StringSplitOptions.RemoveEmptyEntries).ToList(); } - public static List ParseScopesString(this string scopes) + public static List? ParseScopesString(this string? scopes) { if (scopes.IsMissing()) { @@ -67,13 +70,13 @@ public static List ParseScopesString(this string scopes) } [DebuggerStepThrough] - public static bool IsMissing(this string value) + public static bool IsMissing([NotNullWhen(false)] this string? value) { return string.IsNullOrWhiteSpace(value); } [DebuggerStepThrough] - public static bool IsMissingOrTooLong(this string value, int maxLength) + public static bool IsMissingOrTooLong(this string? value, int maxLength) { if (string.IsNullOrWhiteSpace(value)) { @@ -89,13 +92,13 @@ public static bool IsMissingOrTooLong(this string value, int maxLength) } [DebuggerStepThrough] - public static bool IsPresent(this string value) + public static bool IsPresent([NotNullWhen(true)] this string? value) { return !string.IsNullOrWhiteSpace(value); } [DebuggerStepThrough] - public static string EnsureLeadingSlash(this string url) + public static string? EnsureLeadingSlash(this string? url) { if (url != null && !url.StartsWith("/")) { @@ -106,7 +109,7 @@ public static string EnsureLeadingSlash(this string url) } [DebuggerStepThrough] - public static string EnsureTrailingSlash(this string url) + public static string? EnsureTrailingSlash(this string? url) { if (url != null && !url.EndsWith("/")) { @@ -117,7 +120,7 @@ public static string EnsureTrailingSlash(this string url) } [DebuggerStepThrough] - public static string RemoveLeadingSlash(this string url) + public static string? RemoveLeadingSlash(this string? url) { if (url != null && url.StartsWith("/")) { @@ -128,7 +131,7 @@ public static string RemoveLeadingSlash(this string url) } [DebuggerStepThrough] - public static string RemoveTrailingSlash(this string url) + public static string? RemoveTrailingSlash(this string? url) { if (url != null && url.EndsWith("/")) { @@ -139,9 +142,9 @@ public static string RemoveTrailingSlash(this string url) } [DebuggerStepThrough] - public static string CleanUrlPath(this string url) + public static string CleanUrlPath(this string? url) { - if (String.IsNullOrWhiteSpace(url)) url = "/"; + if (string.IsNullOrWhiteSpace(url)) url = "/"; if (url != "/" && url.EndsWith("/")) { @@ -153,7 +156,7 @@ public static string CleanUrlPath(this string url) [DebuggerStepThrough] // Clone of UrlHelperBase.CheckIsLocalUrl from https://github.com/dotnet/aspnetcore/blob/3f1acb59718cadf111a0a796681e3d3509bb3381/src/Mvc/Mvc.Core/src/Routing/UrlHelperBase.cs - public static bool IsLocalUrl(this string url) + public static bool IsLocalUrl(this string? url) { if (string.IsNullOrEmpty(url)) { @@ -246,7 +249,7 @@ public static string AddHashFragment(this string url, string query) } [DebuggerStepThrough] - public static NameValueCollection ReadQueryStringAsNameValueCollection(this string url) + public static NameValueCollection ReadQueryStringAsNameValueCollection(this string? url) { if (url != null) { @@ -266,7 +269,7 @@ public static NameValueCollection ReadQueryStringAsNameValueCollection(this stri return new NameValueCollection(); } - public static string GetOrigin(this string url) + public static string? GetOrigin(this string? url) { if (url != null) { diff --git a/src/Open.IdentityServer/test/Open.IdentityServer.IntegrationTests/Utility/InternalStringExtensions.cs b/src/Open.IdentityServer/test/Open.IdentityServer.IntegrationTests/Utility/InternalStringExtensions.cs index 06fc17f4d..c3fce0580 100644 --- a/src/Open.IdentityServer/test/Open.IdentityServer.IntegrationTests/Utility/InternalStringExtensions.cs +++ b/src/Open.IdentityServer/test/Open.IdentityServer.IntegrationTests/Utility/InternalStringExtensions.cs @@ -1,4 +1,5 @@ // Copyright (c) Brock Allen & Dominick Baier. All rights reserved. +// Modified by Rock Solid Knowledge Ltd. Copyright in modifications 2026, Rock Solid Knowledge Ltd. // Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. #nullable enable @@ -11,13 +12,13 @@ namespace IdentityServer.IntegrationTests.Utility; internal static class InternalStringExtensions { [DebuggerStepThrough] - public static bool IsMissing(this string value) + public static bool IsMissing([NotNullWhen(false)] this string? value) { return string.IsNullOrWhiteSpace(value); } [DebuggerStepThrough] - public static bool IsPresent(this string value) + public static bool IsPresent([NotNullWhen(true)] this string? value) { return !(value.IsMissing()); } diff --git a/src/Storage/src/Extensions/StringsExtensions.cs b/src/Storage/src/Extensions/StringsExtensions.cs index 4aec2bf9b..8c46339be 100644 --- a/src/Storage/src/Extensions/StringsExtensions.cs +++ b/src/Storage/src/Extensions/StringsExtensions.cs @@ -1,21 +1,25 @@ // Copyright (c) Brock Allen & Dominick Baier. All rights reserved. +// Modified by Rock Solid Knowledge Ltd. Copyright in modifications 2026, Rock Solid Knowledge Ltd. // Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; + +#nullable enable namespace Open.IdentityServer.Extensions; internal static class StringExtensions { [DebuggerStepThrough] - public static bool IsMissing(this string value) + public static bool IsMissing([NotNullWhen(false)] this string? value) { return string.IsNullOrWhiteSpace(value); } [DebuggerStepThrough] - public static bool IsPresent(this string value) + public static bool IsPresent([NotNullWhen(true)] this string? value) { return !string.IsNullOrWhiteSpace(value); } From 2b783499e9d2973ccee89a0344a9d94ec8bce0b5 Mon Sep 17 00:00:00 2001 From: Eric Quist Date: Thu, 6 Aug 2026 10:41:04 +0200 Subject: [PATCH 2/4] Small refactoring to DataProtectedIdentityServerKeyMaterialConverter more readable and parts of it more re-usable. --- .../src/Configuration/CryptoHelper.cs | 37 +++++++++ ...ectedIdentityServerKeyMaterialConverter.cs | 15 +--- .../Configuration/CryptoHelperTests.cs | 79 +++++++++++++++++++ .../Open.IdentityServer.UnitTests.csproj | 16 ++-- 4 files changed, 128 insertions(+), 19 deletions(-) create mode 100644 src/Open.IdentityServer/test/Open.IdentityServer.UnitTests/Configuration/CryptoHelperTests.cs diff --git a/src/Open.IdentityServer/src/Configuration/CryptoHelper.cs b/src/Open.IdentityServer/src/Configuration/CryptoHelper.cs index dee7b712d..936cc7b3d 100644 --- a/src/Open.IdentityServer/src/Configuration/CryptoHelper.cs +++ b/src/Open.IdentityServer/src/Configuration/CryptoHelper.cs @@ -97,6 +97,43 @@ public static HashAlgorithm GetHashAlgorithmForSigningAlgorithm(string signingAl }; } + /// + /// Returns if the algorithm is an RSA algorithm (RSxxx or PSxxx) + /// + /// The algorithm to check. + /// if the algorithm is an RSA algorithm; otherwise, . + public static bool IsRsaAlgorithm(this string algorithm) + { + return algorithm.StartsWith('R') || algorithm.StartsWith('P'); + } + + /// + /// Returns if the algorithm is an EC algorithm (Exxx) + /// + /// The algorithm to check. + /// if the algorithm is an EC algorithm; otherwise, . + public static bool IsEcAlgorithm(this string algorithm) + { + return algorithm.StartsWith('E'); + } + + /// + /// Returns the matching named curve for a given algorithm + /// + /// The algorithm to get the curve name for. + /// The name of the curve corresponding to the algorithm. + /// + public static string? GetCurveNameForAlgorithm(this string algorithm) + { + return algorithm switch + { + "ES256" => "P-256", + "ES384" => "P-384", + "ES512" => "P-521", + _ => throw new ArgumentOutOfRangeException(nameof(algorithm), "Unexpected algorithm value for EC Curve") + }; + } + /// /// Returns the matching named curve for RFC 7518 crv value /// diff --git a/src/Open.IdentityServer/src/DataProtection/DataProtectedIdentityServerKeyMaterialConverter.cs b/src/Open.IdentityServer/src/DataProtection/DataProtectedIdentityServerKeyMaterialConverter.cs index 44e2950c1..d586a93cd 100644 --- a/src/Open.IdentityServer/src/DataProtection/DataProtectedIdentityServerKeyMaterialConverter.cs +++ b/src/Open.IdentityServer/src/DataProtection/DataProtectedIdentityServerKeyMaterialConverter.cs @@ -45,8 +45,7 @@ public SigningKey Convert(IdentityServerKeyMaterial keyMaterial) dataProtector.Unprotect(keyMaterial.Data) : keyMaterial.Data; - if (!keyMaterial.IsX509Certificate && - (keyMaterial.Algorithm.StartsWith('R') || keyMaterial.Algorithm.StartsWith('P'))) + if (!keyMaterial.IsX509Certificate && keyMaterial.Algorithm.IsRsaAlgorithm()) { var keyData = JsonSerializer.Deserialize(unprotectedData, Settings); @@ -54,18 +53,12 @@ public SigningKey Convert(IdentityServerKeyMaterial keyMaterial) signingKey.Credentials = new SigningCredentials(new RsaSecurityKey(keyData.Parameters) { KeyId = keyData.Id }, keyData.Algorithm); } - if (!keyMaterial.IsX509Certificate && keyMaterial.Algorithm.StartsWith('E')) + if (!keyMaterial.IsX509Certificate && keyMaterial.Algorithm.IsEcAlgorithm()) { var keyData = JsonSerializer.Deserialize(unprotectedData, Settings); - ECCurve curve = keyMaterial.Algorithm switch - { - "ES256" => CryptoHelper.GetCurveFromCrvValue("P-256"), - "ES384" => CryptoHelper.GetCurveFromCrvValue("P-384"), - "ES521" => CryptoHelper.GetCurveFromCrvValue("P-521"), - _ => throw new ArgumentOutOfRangeException(nameof(keyMaterial.Algorithm), "Unexpected algorithm value for EC Curve") - }; - + ECCurve curve = CryptoHelper.GetCurveFromCrvValue( + keyMaterial.Algorithm.GetCurveNameForAlgorithm()); var ecdsa = ECDsa.Create(new ECParameters { Curve = curve, D = keyData.D, Q = keyData.Q }); signingKey.Created = keyData.Created; diff --git a/src/Open.IdentityServer/test/Open.IdentityServer.UnitTests/Configuration/CryptoHelperTests.cs b/src/Open.IdentityServer/test/Open.IdentityServer.UnitTests/Configuration/CryptoHelperTests.cs new file mode 100644 index 000000000..4f2deb377 --- /dev/null +++ b/src/Open.IdentityServer/test/Open.IdentityServer.UnitTests/Configuration/CryptoHelperTests.cs @@ -0,0 +1,79 @@ +using AwesomeAssertions; +using Open.IdentityServer.Configuration; +using System; +using Xunit; + +namespace Open.IdentityServer.UnitTests.Configuration; + +public class CryptoHelperTests +{ + [Theory] + [InlineData("RS256")] + [InlineData("RS384")] + [InlineData("RS512")] + [InlineData("PS256")] + [InlineData("PS384")] + [InlineData("PS512")] + public void IsRsaAlgorithm_ShouldReturnTrueForRsaAlgorithms(string algorithm) + { + algorithm.IsRsaAlgorithm().Should().BeTrue(); + } + + [Theory] + [InlineData("ES256")] + [InlineData("ES384")] + [InlineData("ES512")] + [InlineData("HS256")] + [InlineData("AES256")] + public void IsRsaAlgorithm_ShouldReturnFalseForNonRsaAlgorithms(string algorithm) + { + algorithm.IsRsaAlgorithm().Should().BeFalse(); + } + + [Theory] + [InlineData("ES256")] + [InlineData("ES384")] + [InlineData("ES512")] + public void IsEcAlgorithm_ShouldReturnTrueForEcAlgorithms(string algorithm) + { + algorithm.IsEcAlgorithm().Should().BeTrue(); + } + + [Theory] + [InlineData("RS256")] + [InlineData("RS384")] + [InlineData("RS512")] + [InlineData("PS256")] + [InlineData("PS384")] + [InlineData("PS512")] + [InlineData("HS256")] + [InlineData("AES256")] + public void IsEcAlgorithm_ShouldReturnFalseForNonEcAlgorithms(string algorithm) + { + algorithm.IsEcAlgorithm().Should().BeFalse(); + } + + [Theory] + [InlineData("ES256", "P-256")] + [InlineData("ES384", "P-384")] + [InlineData("ES512", "P-521")] + public void GetCurveNameForAlgorithm_ShouldReturnCorrectCurveNameForEcAlgorithms(string algorithm, string expectedCurveName) + { + algorithm.GetCurveNameForAlgorithm().Should().Be(expectedCurveName); + } + + [Theory] + [InlineData("RS256")] + [InlineData("RS384")] + [InlineData("RS512")] + [InlineData("PS256")] + [InlineData("PS384")] + [InlineData("PS512")] + [InlineData("HS256")] + [InlineData("AES256")] + public void GetCurveNameForAlgorithm_ShouldThrowArgumentOutOfRangeExceptionForNonEcAlgorithms(string algorithm) + { + Action act = () => algorithm.GetCurveNameForAlgorithm(); + act.Should().Throw(); + } +} diff --git a/src/Open.IdentityServer/test/Open.IdentityServer.UnitTests/Open.IdentityServer.UnitTests.csproj b/src/Open.IdentityServer/test/Open.IdentityServer.UnitTests/Open.IdentityServer.UnitTests.csproj index 46644c63d..50baae4c4 100644 --- a/src/Open.IdentityServer/test/Open.IdentityServer.UnitTests/Open.IdentityServer.UnitTests.csproj +++ b/src/Open.IdentityServer/test/Open.IdentityServer.UnitTests/Open.IdentityServer.UnitTests.csproj @@ -12,18 +12,18 @@ - + - + - - - - - + + + + + @@ -36,6 +36,6 @@ - + From 0ef90718007bad3fc9d3a5d02c75eb5460043e6e Mon Sep 17 00:00:00 2001 From: Eric Quist Date: Thu, 6 Aug 2026 10:47:42 +0200 Subject: [PATCH 3/4] Fixed typo. --- src/Open.IdentityServer/src/Configuration/CryptoHelper.cs | 2 +- .../Configuration/CryptoHelperTests.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Open.IdentityServer/src/Configuration/CryptoHelper.cs b/src/Open.IdentityServer/src/Configuration/CryptoHelper.cs index 936cc7b3d..bd52067ca 100644 --- a/src/Open.IdentityServer/src/Configuration/CryptoHelper.cs +++ b/src/Open.IdentityServer/src/Configuration/CryptoHelper.cs @@ -129,7 +129,7 @@ public static bool IsEcAlgorithm(this string algorithm) { "ES256" => "P-256", "ES384" => "P-384", - "ES512" => "P-521", + "ES521" => "P-521", _ => throw new ArgumentOutOfRangeException(nameof(algorithm), "Unexpected algorithm value for EC Curve") }; } diff --git a/src/Open.IdentityServer/test/Open.IdentityServer.UnitTests/Configuration/CryptoHelperTests.cs b/src/Open.IdentityServer/test/Open.IdentityServer.UnitTests/Configuration/CryptoHelperTests.cs index 4f2deb377..282643c71 100644 --- a/src/Open.IdentityServer/test/Open.IdentityServer.UnitTests/Configuration/CryptoHelperTests.cs +++ b/src/Open.IdentityServer/test/Open.IdentityServer.UnitTests/Configuration/CryptoHelperTests.cs @@ -33,7 +33,7 @@ public void IsRsaAlgorithm_ShouldReturnFalseForNonRsaAlgorithms(string algorithm [Theory] [InlineData("ES256")] [InlineData("ES384")] - [InlineData("ES512")] + [InlineData("ES521")] public void IsEcAlgorithm_ShouldReturnTrueForEcAlgorithms(string algorithm) { algorithm.IsEcAlgorithm().Should().BeTrue(); @@ -56,7 +56,7 @@ public void IsEcAlgorithm_ShouldReturnFalseForNonEcAlgorithms(string algorithm) [Theory] [InlineData("ES256", "P-256")] [InlineData("ES384", "P-384")] - [InlineData("ES512", "P-521")] + [InlineData("ES521", "P-521")] public void GetCurveNameForAlgorithm_ShouldReturnCorrectCurveNameForEcAlgorithms(string algorithm, string expectedCurveName) { algorithm.GetCurveNameForAlgorithm().Should().Be(expectedCurveName); From 45ac31d62f9d0859a1d54b7d0a1b2884c1ac3f47 Mon Sep 17 00:00:00 2001 From: Eric Quist Date: Thu, 6 Aug 2026 10:55:41 +0200 Subject: [PATCH 4/4] Wrong nullable annotation. --- src/Open.IdentityServer/src/Configuration/CryptoHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Open.IdentityServer/src/Configuration/CryptoHelper.cs b/src/Open.IdentityServer/src/Configuration/CryptoHelper.cs index bd52067ca..f3e4c45be 100644 --- a/src/Open.IdentityServer/src/Configuration/CryptoHelper.cs +++ b/src/Open.IdentityServer/src/Configuration/CryptoHelper.cs @@ -123,7 +123,7 @@ public static bool IsEcAlgorithm(this string algorithm) /// The algorithm to get the curve name for. /// The name of the curve corresponding to the algorithm. /// - public static string? GetCurveNameForAlgorithm(this string algorithm) + public static string GetCurveNameForAlgorithm(this string algorithm) { return algorithm switch {