From a682d811f384bd31047d9e6b75488d3f99655289 Mon Sep 17 00:00:00 2001 From: maliming Date: Thu, 12 Mar 2026 13:49:21 +0800 Subject: [PATCH] fix: address Copilot review issues in OpenIddict JWKS demo app and CLI command --- .../Abp/Cli/Commands/GenerateJwksCommand.cs | 56 +++++++++++-------- .../OpenIddict.Demo.Client.Console.csproj | 2 +- .../OpenIddict.Demo.Client.Console/Program.cs | 16 +++++- .../ServerDataSeedContributor.cs | 40 ++++++++----- 4 files changed, 73 insertions(+), 41 deletions(-) diff --git a/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/GenerateJwksCommand.cs b/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/GenerateJwksCommand.cs index 0629bd3214..f8474fea46 100644 --- a/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/GenerateJwksCommand.cs +++ b/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/GenerateJwksCommand.cs @@ -2,6 +2,7 @@ using System; using System.IO; using System.Security.Cryptography; using System.Text; +using System.Text.Json; using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; @@ -21,7 +22,7 @@ public class GenerateJwksCommand : IConsoleCommand, ITransientDependency Logger = NullLogger.Instance; } - public async Task ExecuteAsync(CommandLineArgs commandLineArgs) + public Task ExecuteAsync(CommandLineArgs commandLineArgs) { var outputDir = commandLineArgs.Options.GetOrNull("output", "o") ?? Directory.GetCurrentDirectory(); @@ -33,13 +34,13 @@ public class GenerateJwksCommand : IConsoleCommand, ITransientDependency if (!int.TryParse(keySizeStr, out var keySize) || (keySize != 2048 && keySize != 4096)) { Logger.LogError("Invalid key size '{0}'. Supported values: 2048, 4096.", keySizeStr); - return; + return Task.CompletedTask; } if (!IsValidAlgorithm(alg)) { Logger.LogError("Invalid algorithm '{0}'. Supported values: RS256, RS384, RS512, PS256, PS384, PS512.", alg); - return; + return Task.CompletedTask; } if (!Directory.Exists(outputDir)) @@ -49,7 +50,8 @@ public class GenerateJwksCommand : IConsoleCommand, ITransientDependency Logger.LogInformation("Generating RSA {0}-bit key pair (algorithm: {1})...", keySize, alg); - using var rsa = RSA.Create(keySize); + using var rsa = RSA.Create(); + rsa.KeySize = keySize; var jwksJson = BuildJwksJson(rsa, alg, kid); var privateKeyPem = ExportPrivateKeyPem(rsa); @@ -57,8 +59,8 @@ public class GenerateJwksCommand : IConsoleCommand, ITransientDependency var jwksFilePath = Path.Combine(outputDir, $"{filePrefix}.json"); var privateKeyFilePath = Path.Combine(outputDir, $"{filePrefix}-private.pem"); - await File.WriteAllTextAsync(jwksFilePath, jwksJson, Encoding.UTF8); - await File.WriteAllTextAsync(privateKeyFilePath, privateKeyPem, Encoding.UTF8); + File.WriteAllText(jwksFilePath, jwksJson, Encoding.UTF8); + File.WriteAllText(privateKeyFilePath, privateKeyPem, Encoding.UTF8); Logger.LogInformation(""); Logger.LogInformation("Generated files:"); @@ -72,7 +74,7 @@ public class GenerateJwksCommand : IConsoleCommand, ITransientDependency Logger.LogInformation("IMPORTANT: Keep the private key file safe. Never share it or commit it to source control."); Logger.LogInformation(" The JWKS file contains only the public key and is safe to share."); - await Task.CompletedTask; + return Task.CompletedTask; } private static string BuildJwksJson(RSA rsa, string alg, string kid) @@ -82,27 +84,34 @@ public class GenerateJwksCommand : IConsoleCommand, ITransientDependency var n = Base64UrlEncode(parameters.Modulus); var e = Base64UrlEncode(parameters.Exponent); - var sb = new StringBuilder(); - sb.AppendLine("{"); - sb.AppendLine(" \"keys\": ["); - sb.AppendLine(" {"); - sb.AppendLine(" \"kty\": \"RSA\","); - sb.AppendLine(" \"use\": \"sig\","); - sb.AppendLine($" \"kid\": \"{kid}\","); - sb.AppendLine($" \"alg\": \"{alg}\","); - sb.AppendLine($" \"n\": \"{n}\","); - sb.AppendLine($" \"e\": \"{e}\""); - sb.AppendLine(" }"); - sb.AppendLine(" ]"); - sb.Append("}"); - - return sb.ToString(); + using var stream = new System.IO.MemoryStream(); + using var writer = new Utf8JsonWriter(stream, new JsonWriterOptions { Indented = true }); + + writer.WriteStartObject(); + writer.WriteStartArray("keys"); + writer.WriteStartObject(); + writer.WriteString("kty", "RSA"); + writer.WriteString("use", "sig"); + writer.WriteString("kid", kid); + writer.WriteString("alg", alg); + writer.WriteString("n", n); + writer.WriteString("e", e); + writer.WriteEndObject(); + writer.WriteEndArray(); + writer.WriteEndObject(); + writer.Flush(); + + return Encoding.UTF8.GetString(stream.ToArray()); } private static string ExportPrivateKeyPem(RSA rsa) { #if NET5_0_OR_GREATER return rsa.ExportPkcs8PrivateKeyPem(); +#elif NETSTANDARD2_0 + // RSA.ExportPkcs8PrivateKey() was introduced in .NET Standard 2.1. + // The ABP CLI always runs on .NET 5+, so this path is never reached at runtime. + throw new PlatformNotSupportedException("Private key export requires .NET Standard 2.1 or later."); #else var privateKeyBytes = rsa.ExportPkcs8PrivateKey(); var base64 = Convert.ToBase64String(privateKeyBytes, Base64FormattingOptions.InsertLineBreaks); @@ -120,7 +129,8 @@ public class GenerateJwksCommand : IConsoleCommand, ITransientDependency private static bool IsValidAlgorithm(string alg) { - return alg is "RS256" or "RS384" or "RS512" or "PS256" or "PS384" or "PS512"; + return alg == "RS256" || alg == "RS384" || alg == "RS512" || + alg == "PS256" || alg == "PS384" || alg == "PS512"; } public string GetUsageInfo() diff --git a/modules/openiddict/app/OpenIddict.Demo.Client.Console/OpenIddict.Demo.Client.Console.csproj b/modules/openiddict/app/OpenIddict.Demo.Client.Console/OpenIddict.Demo.Client.Console.csproj index c9ad3fc760..d2938a216e 100644 --- a/modules/openiddict/app/OpenIddict.Demo.Client.Console/OpenIddict.Demo.Client.Console.csproj +++ b/modules/openiddict/app/OpenIddict.Demo.Client.Console/OpenIddict.Demo.Client.Console.csproj @@ -17,7 +17,7 @@ - + PreserveNewest diff --git a/modules/openiddict/app/OpenIddict.Demo.Client.Console/Program.cs b/modules/openiddict/app/OpenIddict.Demo.Client.Console/Program.cs index 9d2b269d60..bb1142b76c 100644 --- a/modules/openiddict/app/OpenIddict.Demo.Client.Console/Program.cs +++ b/modules/openiddict/app/OpenIddict.Demo.Client.Console/Program.cs @@ -215,8 +215,20 @@ if (!File.Exists(privateKeyPath)) using var rsaKey = RSA.Create(); rsaKey.ImportFromPem(await File.ReadAllTextAsync(privateKeyPath)); -// The kid must match the "kid" field in the JWKS registered on the server. -var signingKey = new RsaSecurityKey(rsaKey) { KeyId = "6444499c0f3e43c98db72bb85db5edee" }; +// Read the kid dynamically from the JWKS file so it stays in sync with the server-registered JWKS. +string? signingKid = null; +var jwksForKidPath = Path.Combine(AppContext.BaseDirectory, "jwks.json"); +if (File.Exists(jwksForKidPath)) +{ + using var jwksDoc = JsonDocument.Parse(await File.ReadAllTextAsync(jwksForKidPath)); + if (jwksDoc.RootElement.TryGetProperty("keys", out var keysElem) && + keysElem.GetArrayLength() > 0 && + keysElem[0].TryGetProperty("kid", out var kidElem)) + { + signingKid = kidElem.GetString(); + } +} +var signingKey = new RsaSecurityKey(rsaKey) { KeyId = signingKid }; var signingCredentials = new SigningCredentials(signingKey, SecurityAlgorithms.RsaSha256); var now = DateTime.UtcNow; diff --git a/modules/openiddict/app/OpenIddict.Demo.Server/EntityFrameworkCore/ServerDataSeedContributor.cs b/modules/openiddict/app/OpenIddict.Demo.Server/EntityFrameworkCore/ServerDataSeedContributor.cs index 8aeb899631..ae6687a883 100644 --- a/modules/openiddict/app/OpenIddict.Demo.Server/EntityFrameworkCore/ServerDataSeedContributor.cs +++ b/modules/openiddict/app/OpenIddict.Demo.Server/EntityFrameworkCore/ServerDataSeedContributor.cs @@ -167,23 +167,33 @@ public class ServerDataSeedContributor : IDataSeedContributor, ITransientDepende // and used by OpenIddict.Demo.Client.Console to sign JWT client assertions. // Both files are generated with: abp generate-jwks var jwksPath = Path.Combine(AppContext.BaseDirectory, "jwks.json"); - var jwks = new JsonWebKeySet(await File.ReadAllTextAsync(jwksPath)); - - await _applicationManager.CreateAsync(new OpenIddictApplicationDescriptor + if (!File.Exists(jwksPath)) { - ApplicationType = OpenIddictConstants.ApplicationTypes.Web, - ClientId = "AbpConsoleAppWithJwks", - ClientType = OpenIddictConstants.ClientTypes.Confidential, - DisplayName = "Abp Console App (private_key_jwt)", - JsonWebKeySet = jwks, - Permissions = + Console.WriteLine( + $"[OpenIddict] WARNING: JWKS file not found at '{jwksPath}'. " + + "Skipping creation of the 'AbpConsoleAppWithJwks' client. " + + "Run 'abp generate-jwks' in the app/ directory to generate the key pair."); + } + else + { + var jwks = new JsonWebKeySet(await File.ReadAllTextAsync(jwksPath)); + + await _applicationManager.CreateAsync(new OpenIddictApplicationDescriptor { - OpenIddictConstants.Permissions.Endpoints.Token, - OpenIddictConstants.Permissions.Endpoints.Introspection, - OpenIddictConstants.Permissions.GrantTypes.ClientCredentials, - OpenIddictConstants.Permissions.Prefixes.Scope + "AbpAPI" - } - }); + ApplicationType = OpenIddictConstants.ApplicationTypes.Web, + ClientId = "AbpConsoleAppWithJwks", + ClientType = OpenIddictConstants.ClientTypes.Confidential, + DisplayName = "Abp Console App (private_key_jwt)", + JsonWebKeySet = jwks, + Permissions = + { + OpenIddictConstants.Permissions.Endpoints.Token, + OpenIddictConstants.Permissions.Endpoints.Introspection, + OpenIddictConstants.Permissions.GrantTypes.ClientCredentials, + OpenIddictConstants.Permissions.Prefixes.Scope + "AbpAPI" + } + }); + } } if (await _applicationManager.FindByClientIdAsync("Swagger") == null)