Add support for Alipay certificate signing #1131

AigioL · 2025-11-18T08:16:42Z

improve #968

src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationConstants.cs

src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationHandler.cs

src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationOptions.cs

src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationOptionsExtensions.cs

src/AspNet.Security.OAuth.Alipay/AntCertificationUtil.cs

src/AspNet.Security.OAuth.Alipay/AlipayCertificationUtil.cs

martincostello · 2025-12-12T14:43:59Z

src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationConstants.cs

+        /// <summary>
+        /// The internal identifier for Alipay users will no longer be independently available going forward and will be replaced by OpenID.
+        /// </summary>
+        public const string UserId = "urn:alipay:user_id";


On reflection, if this is going to be deprecated, it would be better not to expose it as a public member. Otherwise it feels like we're adding a new API that's immediately obsolete.

UserId cannot be modified to private because applications created before OpenId or legacy code still require it. For newly created applications, OpenId should be used unless a support ticket is submitted to request the platform to enable legacy UserId. I have updated the code and added the ObsoleteAttribute.

Adding a new public API that is immediately obsolete serves no useful purpose to the user and adds maintenance burden for us to remove it in the future (which is then itself a breaking change).

My intention was to delete the property entirely, not just to remove the [Obsolete] attribute.

src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationHandler.cs

martincostello · 2025-12-12T14:49:09Z

src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationOptionsExtensions.cs

+namespace Microsoft.Extensions.DependencyInjection;
+
+/// <summary>
+/// Extension methods to configure Sign in with Alipay authentication capabilities for an HTTP application pipeline.


Is it actually called "Sign in with Alipay", or is this just copy-paste where "Apple" has been changed to "Alipay"?

is correct, see https://auth.alipay.com/login/global.htm

So it's called "Sign In Alipay", not "Sign In with Alipay"?

This should be “Sign in with Alipay” because, like Apple, it uses a third-party platform account for login. This will redirect you to an Alipay webpage displaying a QR code. Use the Alipay mobile app to scan the code and log in.

"Sign In with Apple" is the name of the product (docs), "Sign in with Apple" is the description of what the user does.

"Sign In Alipay" appears to be the name, as it is not grammatically correct English to be a description of what the user is doing.

src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationOptionsExtensions.cs

src/AspNet.Security.OAuth.Alipay/AlipayCertificationUtil.cs

martincostello · 2025-12-12T15:05:13Z

src/AspNet.Security.OAuth.Alipay/AlipayCertificationUtil.cs

+
+            Span<byte> hash = stackalloc byte[MD5.HashSizeInBytes];
+#pragma warning disable CA5351
+            MD5.HashData(bytes, hash);


The return value should be checked.

The MD5 algorithm has a fixed length, so there is no need to check the returned result. If the input array is too short, it will throw an exception.

martincostello · 2025-12-12T15:07:18Z

src/AspNet.Security.OAuth.Alipay/AlipayCertificationUtil.cs

+    /// </summary>
+    private const int StackallocByteThreshold = 256;
+
+    private static string CalculateMd5(ReadOnlySpan<char> chars)


I think this is still overcomplicated really. This isn't a hot path.

The whole thing could just be:

var buffer = Encoding.UTF8.GetBytes(chars); return MD5.HashData(buffer);

I have balanced high performance with code conciseness. The return value here should be a lowercase hex string. The updated code uses stackalloc instead of byte[] to avoid fragmented memory.

Technically yes, but I don't think it's of importance here against a simpler implementation.

The latest submission has been simplified.

AspNet.Security.OAuth.Providers/src/AspNet.Security.OAuth.Alipay/AlipayCertificationUtil.cs

Lines 67 to 75 in 13e690e

private static string CalculateMd5(string s)

{

var buffer = Encoding.UTF8.GetBytes(s);

Span<byte> hash = stackalloc byte[MD5.HashSizeInBytes];

#pragma warning disable CA5351

MD5.HashData(buffer, hash);

#pragma warning restore CA5351

return Convert.ToHexStringLower(hash);

}

martincostello · 2026-01-02T11:08:23Z

Please don't resolve comments where no code changes have been made - otherwise it makes things harder to review as I have to uncollapse the comments in the UI, and if I disagree I just have to unresolve them anyway.

martincostello · 2026-01-02T11:09:49Z

src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationOptions.cs

+#pragma warning disable CS0618
+        ClaimActions.MapJsonKey(Claims.UserId, "user_id");
+#pragma warning restore CS0618


If it's going to be obsolete, just hard-code the value here without the need for the constant. Then we can just delete it in the future without having to also change the public API surface.

martincostello · 2026-01-02T11:10:41Z

src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationOptions.cs

+    public Func<string, CancellationToken, Task<ReadOnlyMemory<char>>>? PublicKey { get; set; }
+
+    /// <inheritdoc />
+    public override void Validate()


Shouldn't this also validate that PublicKey is not null?

- Removed the [Obsolete] marker from AlipayAuthenticationConstants.UserId while retaining explanatory comments. - ClaimActions now directly maps “urn:alipay:user_id” to “user_id” and removes the obsolete mapping for Claims.UserId. - Added non-null validation for PublicKey in certificate mode.

… certificate resolution This submission simplifies Alipay certificate signing configurations by directly passing the certificate serial number (SN) string. It removes the ApplicationCertificateSnKeyId, RootCertificateSnKeyId, and PublicKey delegate properties, with corresponding methods adjusted to directly read the SN. The certificate SN calculation utility class and extension methods for reading public keys via files have been eliminated. This removes the complexity of dynamic certificate resolution, enhancing configuration intuitiveness and usability.

AigioL · 2026-01-21T13:33:38Z

Based on aicodeguard's comment, there is indeed no need to parse the PEM certificate file with every request. I removed the certificate-related content and simplified all changes by using fixed configuration values.

fe4159d

martincostello · 2026-02-08T11:26:17Z

src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationConstants.cs

+        /// <summary>
+        /// The internal identifier for Alipay users will no longer be independently available going forward and will be replaced by OpenID.
+        /// </summary>
+        public const string UserId = "urn:alipay:user_id";


My intention was to delete the property entirely, not just to remove the [Obsolete] attribute.

martincostello · 2026-02-08T11:29:40Z

src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationOptionsExtensions.cs

+namespace Microsoft.Extensions.DependencyInjection;
+
+/// <summary>
+/// Extension methods to configure Sign in with Alipay authentication capabilities for an HTTP application pipeline.


"Sign In with Apple" is the name of the product (docs), "Sign in with Apple" is the description of what the user does.

"Sign In Alipay" appears to be the name, as it is not grammatically correct English to be a description of what the user is doing.

martincostello · 2026-02-08T11:31:31Z

src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationHandler.cs

+        ArgumentNullException.ThrowIfNull(Options.ApplicationCertificateSn);
+        ArgumentNullException.ThrowIfNull(Options.RootCertificateSn);


These aren't arguments to the method. They were already validated in the options. If they are subsequently messed with by the integrator that's user error.

Suggested change

ArgumentNullException.ThrowIfNull(Options.ApplicationCertificateSn);

ArgumentNullException.ThrowIfNull(Options.RootCertificateSn);

Add support for Alipay certificate signing

acc2fe0

martincostello reviewed Nov 27, 2025

View reviewed changes

AigioL and others added 4 commits December 10, 2025 18:23

Merge branch 'aspnet-contrib:dev' into dev

eabb122

Improve naming conventions and documentation comments

e54d133

Streamline code

db212e0

Corrected Annotations

872164d

martincostello reviewed Dec 12, 2025

View reviewed changes

AigioL added 2 commits December 14, 2025 16:22

Revise based on review

ab6e026

Fix the naming certificate key to public key

59de161

martincostello reviewed Jan 2, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

AigioL and others added 3 commits January 17, 2026 10:39

Merge branch 'aspnet-contrib:dev' into dev

c9aae07

martincostello reviewed Feb 8, 2026

View reviewed changes

	private static string CalculateMd5(string s)
	{
	var buffer = Encoding.UTF8.GetBytes(s);
	Span<byte> hash = stackalloc byte[MD5.HashSizeInBytes];
	#pragma warning disable CA5351
	MD5.HashData(buffer, hash);
	#pragma warning restore CA5351
	return Convert.ToHexStringLower(hash);
	}

		ArgumentNullException.ThrowIfNull(Options.ApplicationCertificateSn);
		ArgumentNullException.ThrowIfNull(Options.RootCertificateSn);

Add support for Alipay certificate signing #1131

Are you sure you want to change the base?

Add support for Alipay certificate signing #1131

Conversation

AigioL commented Nov 18, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AigioL Jan 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

martincostello commented Jan 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

AigioL commented Jan 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

AigioL Jan 17, 2026 •

edited

Loading