Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Alibaba Cloud (Aliyun) to the list of supported providers #2256

Merged
merged 9 commits into from
Feb 26, 2025

Conversation

gehongyan
Copy link
Contributor

This pull request would like to add Alibaba Cloud (Aliyun) to the list of supported providers.

@gehongyan
Copy link
Contributor Author

gehongyan commented Feb 25, 2025

Docs

Alibaba Cloud provides services to developers in mainland China under the brand name Aliyun. Documentation is provided on both Alibaba Cloud and Aliyun. OpenID is provided in the RAM (Resource Access Management) product.

Hints

Note

Question 1: The token revocation does not work yet in the console sample when refreshing tokens. It seems that only refresh tokens can be revoked. How do we handle this issue?
image

Useful Screenshots

  • Aliyun RAM console.

image

  • Login page.

image

  • Token refresh works in the ASP.NET sample. (A pretty long token)

image

  • Extracted claims.

image

@kevinchalet
Copy link
Member

Thanks for your PR, @gehongyan!

Woooo, compared to their Alipay OAuth 2.0 implementation, it's really night and day! (because let's be honest, Alipay is the worst/most non-standard implementation I've ever seen in my entire life! 🤣)

This new Alibaba Cloud service doesn't cover Alipay, right?

Question 1: The token revocation does not work yet in the console sample when refreshing tokens. It seems that only refresh tokens can be revoked. How do we handle this issue?
image

It's rare, but it happens and it's something the specification allows. There's nothing we can do on our side, but as long as you're able to confirm it works fine by temporarily tweaking the console sample to send the refresh token instead of the access token, that's fine 👍🏻

// Alibaba Cloud requires attaching the "client_id" parameter to revocation requests.
if (context.Registration.ProviderType is ProviderTypes.AlibabaCloud)
{
context.Request.ClientId = context.Registration.ClientId;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: OpenIddict is expected to take care of sending the client credentials automatically (client_id/client_secret/client_assertion). Were you seeing an error without that custom handler?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my recollection, the first time I invoked the token revocation, it returned an error saying the request was missing the client_id parameter. So, I immediately tried adding this handler. After that, the error message changed to indicating that only refresh_token could be revoked.

Strangely, after removing the handler, I haven't been able to reproduce the original error. Furthermore, after adjusting the console sample, the token revocation request succeeded. Therefore, given the current situation, I'll remove it for now.

image

Copy link
Member

@kevinchalet kevinchalet Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the first time you tried, your client was confidential and was assigned a client secret?

When OpenIddict doesn't find an explicit revocation_endpoint_auth_methods_supported node in the server configuration, it uses client_secret_basic - i.e the default method - if both a client identifier and client secret are expected to be sent. In that case, the client credentials are not sent as regular OAuth 2.0 parameters but are sent using the HTTP WWW-Authenticate header. If no client secret is configured, the client_id is sent alone as a regular OAuth 2.0 parameter.

Can you please check the type of client you configured with Alibaba's portal?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please check the type of client you configured with Alibaba's portal?

I tested both WebApp and NativeApp, and both can be revoked successfully.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested both WebApp and NativeApp, and both can be revoked successfully.

After changing to web app, you updated your OpenIddict registration to call SetClientSecret(...), right?

Anyway, I took a look at the docs and they don't mention basic authentication support.
Just in case, we should probably tweak the returned configuration response to add client_secret_post to both token_endpoint_auth_methods_supported and revocation_endpoint_auth_methods_supported so that client credentials are always sent as part of the request form.

Can you please give it a try?

/// <summary>
/// Contains the logic responsible for amending the supported client
/// authentication methods for the providers that require it.
/// </summary>
public sealed class AmendClientAuthenticationMethods : IOpenIddictClientHandler<HandleConfigurationResponseContext>
{
/// <summary>
/// Gets the default descriptor definition assigned to this handler.
/// </summary>
public static OpenIddictClientHandlerDescriptor Descriptor { get; }
= OpenIddictClientHandlerDescriptor.CreateBuilder<HandleConfigurationResponseContext>()
.UseSingletonHandler<AmendClientAuthenticationMethods>()
.SetOrder(ExtractTokenEndpointClientAuthenticationMethods.Descriptor.Order + 500)
.SetType(OpenIddictClientHandlerType.BuiltIn)
.Build();
/// <inheritdoc/>
public ValueTask HandleAsync(HandleConfigurationResponseContext context)
{
if (context is null)
{
throw new ArgumentNullException(nameof(context));
}
// Apple implements a non-standard client authentication method for its endpoints that
// is inspired by the standard private_key_jwt method but doesn't use the standard
// client_assertion/client_assertion_type parameters. Instead, the client assertion
// must be sent as a "dynamic" client secret using client_secret_post. Since the logic
// is the same as private_key_jwt, the configuration is amended to assume Apple supports
// private_key_jwt and an event handler is responsible for populating the client_secret
// parameter using the client assertion once it has been generated by OpenIddict.
if (context.Registration.ProviderType is ProviderTypes.Apple)
{
context.Configuration.RevocationEndpointAuthMethodsSupported.Add(
ClientAuthenticationMethods.PrivateKeyJwt);
context.Configuration.TokenEndpointAuthMethodsSupported.Add(
ClientAuthenticationMethods.PrivateKeyJwt);
}
// Atlassian doesn't return a "revocation_endpoint_auth_methods_supported" node in its
// server configuration but only supports the "client_secret_post" authentication method.
else if (context.Registration.ProviderType is ProviderTypes.Atlassian)
{
context.Configuration.RevocationEndpointAuthMethodsSupported.Add(
ClientAuthenticationMethods.ClientSecretPost);
}
// Google doesn't properly implement the device authorization grant, doesn't support
// basic client authentication for the device authorization endpoint and returns
// a generic "invalid_request" error when using "client_secret_basic" instead of
// sending the client identifier in the request form. To work around this limitation,
// "client_secret_post" is listed as the only supported client authentication method.
else if (context.Registration.ProviderType is ProviderTypes.Google)
{
context.Configuration.DeviceAuthorizationEndpointAuthMethodsSupported.Clear();
context.Configuration.DeviceAuthorizationEndpointAuthMethodsSupported.Add(
ClientAuthenticationMethods.ClientSecretPost);
}
// Huawei doesn't support sending the client credentials using basic authentication when
// using the device authorization grant, making basic authentication the default authentication
// method. To work around this compliance issue, "client_secret_post" is manually added here.
else if (context.Registration.ProviderType is ProviderTypes.Huawei)
{
context.Configuration.DeviceAuthorizationEndpointAuthMethodsSupported.Add(
ClientAuthenticationMethods.ClientSecretPost);
}
// LinkedIn doesn't support sending the client credentials using basic authentication but
// doesn't return a "token_endpoint_auth_methods_supported" node containing alternative
// authentication methods, making basic authentication the default authentication method.
// To work around this compliance issue, "client_secret_post" is manually added here.
else if (context.Registration.ProviderType is ProviderTypes.LinkedIn)
{
context.Configuration.TokenEndpointAuthMethodsSupported.Add(
ClientAuthenticationMethods.ClientSecretPost);
}
// Pro Santé Connect lists private_key_jwt as a supported client authentication method but
// only supports client_secret_basic/client_secret_post and tls_client_auth and plans to
// remove secret-based authentication support in late 2024 to force clients to use mTLS.
else if (context.Registration.ProviderType is ProviderTypes.ProSantéConnect)
{
context.Configuration.DeviceAuthorizationEndpointAuthMethodsSupported.Remove(
ClientAuthenticationMethods.PrivateKeyJwt);
context.Configuration.IntrospectionEndpointAuthMethodsSupported.Remove(
ClientAuthenticationMethods.PrivateKeyJwt);
context.Configuration.RevocationEndpointAuthMethodsSupported.Remove(
ClientAuthenticationMethods.PrivateKeyJwt);
context.Configuration.TokenEndpointAuthMethodsSupported.Remove(
ClientAuthenticationMethods.PrivateKeyJwt);
}
return default;
}
}

Copy link
Contributor Author

@gehongyan gehongyan Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After changing to web app, you updated your OpenIddict registration to call SetClientSecret(...), right?

Actually, I added registrations for both application types in my test code to conduct these tests.

Test code
.AddAlibabaCloud(options =>
    options
        .SetRegion("CN")
        .SetProviderName("AliyunWeb")
        .SetProviderDisplayName("Aliyun Web")
        .SetClientId("******************")
        .SetClientSecret("***********************************************")
        .SetAccessType("offline")
        // .SetPrompt("admin_consent")
        .SetRedirectUri("callback/login/aliyun-web")
        .AddScopes("aliuid", "profile"))
// Aliyun NativeApp
.AddAlibabaCloud(options =>
    options
        .SetRegion("CN")
        .SetProviderName("AliyunNative")
        .SetProviderDisplayName("Aliyun Native")
        .SetClientId("******************")
        .SetClientSecret("***********************************************")
        // .SetPrompt("admin_consent")
        .SetRedirectUri("callback/login/aliyun-native")
        .AddScopes("aliuid", "profile"))
// Alibaba Cloud WebApp
.AddAlibabaCloud(options =>
    options
        .SetProviderName("AlibabaWeb")
        .SetProviderDisplayName("Alibaba Web")
        .SetClientId("******************")
        .SetClientSecret("***********************************************")
        .SetRedirectUri("callback/login/alibabacloud-web")
        .SetAccessType("offline")
        // .SetPrompt("admin_consent")
        .AddScopes("aliuid", "profile"))
// Alibaba Cloud NativeApp
.AddAlibabaCloud(options =>
    options
        .SetProviderName("AlibabaNative")
        .SetProviderDisplayName("Alibaba Native")
        .SetClientId("******************")
        .SetClientSecret("***********************************************")
        // .SetPrompt("admin_consent")
        .SetRedirectUri("callback/login/alibabacloud-native")
        .AddScopes("aliuid", "profile"))

Just in case, we should probably tweak the returned configuration response.

Sounds good. Completed. All of the registrations above still work well.

@gehongyan
Copy link
Contributor Author

This new Alibaba Cloud service doesn't cover Alipay, right?

Yes, Alibaba Cloud and Alipay are different products.

@gehongyan
Copy link
Contributor Author

Alibaba Cloud categorizes OAuth applications into WebApp and NativeApp. There appear to be subtle differences in their refresh_token return policies. WebApp should specify access_type as offline in the token request parameters to obtain a refresh_token, but NativeApp always returns refresh_token.

image
image
image

This seems similar to Huawei's implementation. Using the Native application type directly can bypass this, but if a developer specifies WebApp, we should also support this scenario. I'll implement this later.

@kevinchalet
Copy link
Member

Using the Native application type directly can bypass this, but if a developer specifies WebApp, we should also support this scenario. I'll implement this later.

It's indeed fairly common: Google does that too... and if you don't specify access_type=offline the very first time the user approves the authorization demand, you'll never get a refresh token, even if future authorization requests include access_type=offline (which is a TERRIBLE design!).

@gehongyan
Copy link
Contributor Author

I've implemented the access_type and prompt parameters in the token request.

The access_type parameter successfully allows requesting a refresh_token in a WebApp, and I was able to revoke it successfully.

Regarding the prompt parameter, the documentation mentions that the admin_consent value should force the server to display an authorization page before returning to the client. The authorization page does appear, but clicking the authorization button on the page results in an error. I encountered the issue in both Alibaba Cloud and Aliyun. I have submitted a support ticket to Aliyun regarding this issue and am currently awaiting their response.

image

var settings = context.Registration.GetAlibabaCloudSettings();

context.Request["access_type"] = settings.AccessType;
context.Request["prompt"] = settings.Prompt;
Copy link
Member

@kevinchalet kevinchalet Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
context.Request["prompt"] = settings.Prompt;
context.Request.Prompt = settings.Prompt;

You can directly use the Prompt property since it's a standard OIDC parameter exposed by the OpenIddictRequest primitive 😃

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While you're changing that, let's also change line 1862 to use the same pattern:

context.Request.Resources = [settings.Resource];

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completed.

I also changed "display" to the Display property in Huawei and Weibo providers. I hope that I haven't made things worse.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, thanks!

I also changed "display" to the Display property in Huawei and Weibo providers. I hope that I haven't made things worse.

Great initiative 👍🏻

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type of "resource" is changed from a string? to a string?[]? likely resulting in a breaking change.

How that property is represented internally by the OpenIddictRequest primitive doesn't affect how it's eventually represented as an OAuth 2.0 parameter: for authorization request parameters, multi-valued parameters are always represented as multiple parameters (which was explicitly forbidden in the initial OAuth 2.0 specification, but was allowed in subsequent specifications, including the specification that defines the standard version of the resource parameter). Here, since there's just one value, the representation in the authorization request doesn't actually change at all.

That said, I had not realized that setting was optional in the AD provider, so it's possible that an empty parameter value may be set by using the property. Eventually, there shouldn't be any difference in the actual authorization request as OpenIddict excludes parameters with empty values when generating the URI (e.g multiple resource parameters with string.Empty or null as the value), but you're right it's technically a behavior change that is observable from custom event handlers, so I reverted it (I just used the appropriate constant since it's a standard parameter) 👍🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I made the change, I did have doubts about whether it was a breaking change, which is why I wrote that deleted reply. Later, however, I realized it was about the internal handling of parameters, so I deleted it.

I truly couldn't think of or understand if it would have any other impacts. I appreciate that you considered and pointed it out, which is truly admirable. It seems that raising reasonable doubts is never a bad idea after all. 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate that you considered and pointed it out, which is truly admirable. It seems that raising reasonable doubts is never a bad idea after all. 👍

🙏🏻

(using the dedicated property actually looked nicer so I pushed a commit to bring it back but guard the call with a string.IsNullOrEmpty() check 😄)

@gehongyan
Copy link
Contributor Author

Response from Aliyun support:

admin_consent is only used for cross-account authorization. Therefore, when the client ID and the logged-in account belong to the same account, even though passing prompt=admin_consent will display the authorization page, it won't be possible to grant consent to an application within the same account. An error message, indicating failure to install, will be displayed. This behavior is by design.

@gehongyan gehongyan marked this pull request as ready for review February 26, 2025 03:44
@kevinchalet
Copy link
Member

Response from Aliyun support:

admin_consent is only used for cross-account authorization. Therefore, when the client ID and the logged-in account belong to the same account, even though passing prompt=admin_consent will display the authorization page, it won't be possible to grant consent to an application within the same account. An error message, indicating failure to install, will be displayed. This behavior is by design.

Interesting 👍🏻 (their response time is also top notch!)

If you don't plan on making additional changes, I'll merge your PR later today 😃

@gehongyan
Copy link
Contributor Author

If you don't plan on making additional changes, I'll merge your PR later today 😃

Sure! Big thanks for your support! ❤️

@kevinchalet kevinchalet self-assigned this Feb 26, 2025
@kevinchalet kevinchalet added this to the 6.2.0 milestone Feb 26, 2025
@kevinchalet kevinchalet merged commit 9eae2a4 into openiddict:dev Feb 26, 2025
6 checks passed
@kevinchalet
Copy link
Member

Merged. Thanks a lot for this great new contribution! 🎉 🙏🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants