Add Weibo to the list of supported providers

[gehongyan]

This pull request would like to add Weibo to the list of supported providers, which is also supported by AspNet.Security.OAuth.Providers - Weibo.

[gehongyan]

Docs

The Chinese and English versions of some documents are not completely identical, but the differences are minor. Therefore, I have included both versions here.

• Authorization mechanism: Chinese (Chinese only)
• API that requests the code: Chinese, English
• API that gets the access token: Chinese, English
• API that gets the token info: Chinese (Chinese only)
• API that revokes the token: Chinese (Chinese only)

Hint

The single English version of OAuth docs seem to be OAuth 1.0.

The refresh_token grant type is only supported in their SDK. Web applications should be reauthorized instead.

---

Useful Screenshots

Authorization page

Authorization result

Weibo returns lots of fields from the user info request.

Token revocation

Authorization denials

---

What I Did

The scopes should be joined with the comma , instead of the space, so I adjusted the OpenIddictClientWebIntegrationHandlers.FormatNonStandardScopeParameter.

The redirect URI supports additional settings, which are appended in OpenIddictClientWebIntegrationHandlers.AttachAdditionalChallengeParameters.

The user info request requires the access_token as a urencoded parameter instead of the header, so I adjusted it in OpenIddictClientWebIntegrationHandlers.Userinfo.AttachAccessTokenParameter.

The user info request still requires the uid parameter, so I appended it in OpenIddictClientWebIntegrationHandlers.AttachAdditionalUserinfoRequestParameters.

The user identifier is mapped from id in OpenIddictClientWebIntegrationHandlers.MapCustomWebServicesFederationClaims.

---

Questions

Weibo provided an API for getting token info, which returns:

The negative number value of expire_in represents that the token has expired. Is what this API does similar to token introspection? If so, is there a way we can benefit from this API?

[kevinchalet]

Thanks for your PR! Looks great!

Weibo provided an API for getting token info, which returns:

The negative number value of expire_in represents that the token has expired. Is what this API does similar to token introspection? If so, is there a way we can benefit from this API?

In general, OAuth 2.0 introspection implementations return a JSON response containing only active: false when the access token expired (instead of returning a real response with a negative or zero expires_in node).

We could probably use that endpoint for introspection by adding some mapping, but the claims returned by that endpoint seem quite limited, so not sure it's worth the extra work.

[gehongyan]

We could probably use that endpoint for introspection by adding some mapping, but the claims returned by that endpoint seem quite limited, so not sure it's worth the extra work.

I am not sure where to map the introspection. I did not find any existing mappers in the WebIntegration project. I also didn't find any providers configured with the introspection endpoint in the XML statically. If you prefer to ignore it in case of lack of benefits, then let's just ignore it. If there will be an introspection mapper, then let's implement the Weibo introspection. Which do you prefer at present? 😃

[kevinchalet]

I did not find any existing mappers in the WebIntegration project. I also didn't find any providers configured with the introspection endpoint in the XML statically.

Yeah, introspection is not widely supported 😭

If you prefer to ignore it in case of lack of benefits, then let's just ignore it.

Let's ignore it for now and see if there's a demand for that later 👍🏻

[gehongyan]

Let's ignore it for now and see if there's a demand for that later 👍🏻

Sounds good! 👍

[gehongyan]

It seems that adding a Boolean needs me to check the entire source generator logic. For example, the latest commit added support for setting the default value set to a bool setting. I am not sure whether there should be more fixtures needed. If more are found, let's create new pull requests to fix them.

[kevinchalet]

It seems that adding a Boolean needs me to check the entire source generator logic. For example, the latest commit added support for setting the default value set to a bool setting. I am not sure whether there should be more fixtures needed. If more are found, let's create new pull requests to fix them.

I took a look and it seems you covered all the cases, except one in this part (responsible for throw an exception if a setting wasn't set):

https://github.com/openiddict/openiddict-core/blob/17fa2cac528f7b1eb78f41e94ee311371c6dd301/gen/OpenIddict.Client.WebIntegration.Generators/OpenIddictClientWebIntegrationGenerator.cs#L950-L968

[gehongyan]

Copy that. Thanks for your hint. I will implememt it. 🍻

[gehongyan]

I took a look and it seems you covered all the cases, except one in this part (responsible for throw an exception if a setting wasn't set):

It seems that the current generator will also generate proper assertions for bool properties.

// Generated

if (settings.ForceLogin is null)
{
    throw new InvalidOperationException(SR.FormatID0332(nameof(settings.ForceLogin), Providers.Weibo));
}

Do we need some additional checks for Boolean just like what was done in the Uri condition? Should we still split the bool condition with a different if block such as:

Index: gen/OpenIddict.Client.WebIntegration.Generators/OpenIddictClientWebIntegrationGenerator.cs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/gen/OpenIddict.Client.WebIntegration.Generators/OpenIddictClientWebIntegrationGenerator.cs b/gen/OpenIddict.Client.WebIntegration.Generators/OpenIddictClientWebIntegrationGenerator.cs
--- a/gen/OpenIddict.Client.WebIntegration.Generators/OpenIddictClientWebIntegrationGenerator.cs    (revision ec411581de26f474c631a8e986183e91b3d87dda)
+++ b/gen/OpenIddict.Client.WebIntegration.Generators/OpenIddictClientWebIntegrationGenerator.cs    (date 1718761954304)
@@ -970,6 +970,8 @@
             {{~ if setting.required ~}}
             {{~ if setting.type == 'String' ~}}
             if (string.IsNullOrEmpty(settings.{{ setting.property_name }}))
+            {{~ else if setting.type == 'Boolean' ~}}
+            if (!settings.{{ setting.property_name }}.HasValue)
             {{~ else ~}}
             if (settings.{{ setting.property_name }} is null)
             {{~ end ~}}
             {{~ end ~}}
             {
                 throw new InvalidOperationException(SR.FormatID0332(nameof(settings.{{ setting.property_name }}), Providers.{{ provider.name }}));
             }
             {{~ end ~}}

[kevinchalet]

Do we need some additional checks for Boolean just like what was done in the Uri condition? Should we still split the bool condition with a different if block such as:

Ah, my bad! Yeah, the existing condition works fine for booleans so no need for a specialized check 👍🏻

[kevinchalet]

Looks great! If you don't have anything else to add, let's merge it 😃

[gehongyan]

Thanks for your help! Nothing more is planned at present. ❤️

[kevinchalet]

Merged. Thanks a lot for your contribution! ❤️

[kevinchalet]

Hey 👋🏻

FYI, OpenIddict 5.7.0 just shipped and includes the 5 providers you contributed recently: https://github.com/openiddict/openiddict-core/releases/tag/5.7.0 🎉

Cheers!

[gehongyan]

Cheers! 🍻