Add Feishu (Lark) to the list of supported providers

[gehongyan]

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

[gehongyan]

Hints

• The docs referenced by AspNet.Security.OAuth.Feishu implementation deprecated and is not recommended. See Login Process - Developer Guides - Documentation - Feishu Open Platform (English avaliable).

---

Docs (English avaliable)

Feishu

• Login Overview - Developer Guides - Documentation - Feishu Open Platform
• Obtain OAuth code - Server API - Documentation - Feishu Open Platform
• Get user_access_token - Server API - Documentation - Feishu Open Platform
• Obtain login user information - Server API - Documentation - Feishu Open Platform
• Refresh user_access_token - Server API - Documentation - Feishu Open Platform

Lark

Lark Suite is the global version of Feishu. It seems that all the docs are almost the same as above. The API host is changed to open.larksuite.com from open.feishu.cn.

• Login Overview - Developer Guides - Documentation - Lark Developer
• Obtain OAuth code - Server API - Documentation - Feishu Open Platform
• Get user_access_token - Server API - Documentation - Lark Developer
• Obtain login user information - Server API - Documentation - Lark Developer
• Refresh user_access_token - Server API - Documentation - Lark Developer

---

Current Implementation

• Feishu

  • Authorization

    

  • Refreshing

    

• Lark

  • Authorization

    

  • Refreshing

    

---

What I Did

• Multi environment

I assume that Feishu and Lark can be two different environments, so I added two environments to the XML configuration.

I tried naming them Feishu and Lark, but the compiler complains that the environment Production is not found. So at present, I named them Production and Lark.

Tip

Question 1: Is it property to name them as Production and Lark? Or is it better to consider them as the same environment (because both of them are production environments), add another Setting to allow users to modify the API host, and assemble all API request URI in codes when the user code specifies the non-default Lark platform? Or even should we split them into two providers?

• Token request requires additional header

Their token request requires a Authorization header with the value of Bearer 'access_token', where the access_token should be a app_access_token. Here are the docs how to obtain them:

Obtain access tokens - Server API - Documentation - Lark Developer
Get custom app app_access_token - Server API - Documentation - Lark Developer

Hence, I provided a setting to allow the user code to set the app_access_token.

The header value is set in OpenIddictClientWebIntegrationHandlers.Exchange.AttachNonStandardBasicAuthenticationCredentials.

• Extract data from the token response

They respond the token request with a non-standard JSON, so I extracted the error code and successful data in OpenIddictClientWebIntegrationHandlers.Exchange.MapNonStandardResponseParameters.

• Extract data from the user info response

They also return a nested data object, which will be handled in OpenIddictClientWebIntegrationHandlers.Userinfo.UnwrapUserinfoResponse.

• Assemble different token endpoint for the refresh token grant

They use a different token endpoint when refreshing tokens, which will be handled in OpenIddictClientWebIntegrationHandlers.OverrideTokenEndpoint.

---

Issues not Fixed Well

When users deny an authorization request, they do redirect back with a state parameter:

http://localhost:49152/callback/login/feishu?error=access_denied

If I append the state as a query parameter to the redirect URI just like what you taught me yesterday in Huawei OAuth implementation, the denying will work,

http://localhost:49152/callback/login/feishu?state=G_8BHA5dv*******************hobtjP5Nw&error=access_denied

but they will redirect back with an empty state parameter when users authorize:

http://localhost:49152/callback/login/feishu?code=ca0i847***************31fqhi25u&state=

This breaks the normal authorization flow.

Tip

Question 2: How do we deal with it better?

[kevinchalet]

Hey,

Thanks for your PR!

I took a look at the docs and Good Lord, that OAuth 2.0 implementation is just terrible. Some companies are really good at inventing their own non-standard horrors 🤣

Question 1: Is it property to name them as Production and Lark? Or is it better to consider them as the same environment (because both of them are production environments), add another Setting to allow users to modify the API host, and assemble all API request URI in codes when the user code specifies the non-default Lark platform? Or even should we split them into two providers?

Using environments is likely not the best option here (as you figured out, we require a Production environment, so it would be weird to call one Production and the other Lark when both are actually production environments).

We have two options:

• Declare two separate providers, which would be the way to go if we think the two services may diverge at some point (are they operated by the same organization?)

• Use a Region setting with China and Global as the possible values and use it the compute the domain part of the issuer/endpoints. It's a pattern we use for Battle.Net:

  openiddict-core/src/OpenIddict.Client.WebIntegration/OpenIddictClientWebIntegrationProviders.xml

  Lines 236 to 250 in 7194d58

  	<Provider Name="BattleNet" DisplayName="Battle.net" Id="9baed518-5e8d-4544-b562-686218326b14"
  	Documentation="https://develop.battle.net/documentation/guides/using-oauth">
  	<!--
  	Note: most Battle.net regions use the same issuer URI but a different domain is required for China.
  	-->
  	<Environment Issuer="https://oauth.{settings.Region switch {
  	string region when string.Equals(region, 'CN', StringComparison.OrdinalIgnoreCase)
  	=> 'battlenet.com.cn',
  	_ => 'battle.net' }}/" />
  	<Setting PropertyName="Region" ParameterName="region" Type="String" Required="false" DefaultValue="US"
  	Description="The preferred Battle.net region (by default, 'US')" />
  	</Provider>

Their token request requires a Authorization header with the value of Bearer 'access_token', where the access_token should be a app_access_token. Here are the docs how to obtain them:

Obtain access tokens - Server API - Documentation - Lark Developer
Get custom app app_access_token - Server API - Documentation - Lark Developer

What a horrible/non-standard thing... I wanted to kill myself after reading the docs 🤣

Are we sure that "app access token" (or should we use a tenant access tenant, their docs says they are merging the two?!) doesn't expire? If it does expire, we won't be able to use a setting for that, as the provider would stop working after a short time 😭

* **Extract data from the token response**

They respond the token request with a non-standard JSON, so I extracted the error code and successful data in OpenIddictClientWebIntegrationHandlers.Exchange.MapNonStandardResponseParameters.

That works, but the manual mapping is a bit cumbersome. We'll probably want to have an UnwrapTokenResponse handler that is an equivalent of UnwrapUserinfoResponse to simplify things here 😄

[kevinchalet]

Question 2: How do we deal with it better?

We can't do anything on our side, sadly. Feishu/Lark will have to fix their broken implementation to improve things for users.

Edit: or maybe we could try an hybrid approach by not removing the state parameter even if we added to the redirect_uri? I.e don't call this line for Feishu?

openiddict-core/src/OpenIddict.Client.WebIntegration/OpenIddictClientWebIntegrationHandlers.cs

Line 1589 in 7194d58

context.Request.State = null;

[kevinchalet]

The docs referenced by AspNet.Security.OAuth.Feishu implementation deprecated and is not recommended. See Login Process - Developer Guides - Documentation - Feishu Open Platform (English avaliable).

OMG, that implementation was fairly standard! Why did they replace it by a completely non-standard thing?! Makes no sense at all...

Do we know if they plan to phase their deprecated APIs out?

Edit:

🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣 🤣

[gehongyan]

are they operated by the same organization?

Both of them are organized by ByteDance, but I am not familiar with their internal management. According to information on search engines, it seems that Lark was published to the Global market except for China mainland earlier. After half a year, they published the China mainland version with a new name - Feishu.

Based on my observations of the documentation and management console for both products, I am inclined to believe that they are the same product, just deployed in different regions to serve different regions. Hence, I prefer to provide a setting of Region (CN and Global) or Product (Feishu or Lark), instead of splitting them into two different providers at present.

If the provider is named Feishu, should we consider the CN version as the default? Conversely, if the Global version is the default, perhaps the provider should be named Lark instead.

Are we sure that "app access token" doesn't expire?

The app_access_token does expire.

should we use a tenant access tenant, their docs says they are merging the two

Yes, the docs say they are merging of them with a single tenant_access_token. At present, the API above returns the same values of these fields under my circumstances. The docs still explicitly declare it needs the app_access_token. Should we use the tenant one instead in advance?

If it does expire, we won't be able to use a setting for that, as the provider would stop working after a short time

It means that the setting should be a static value instead of a dynamic one. So, do we have a better way to support it? Or we are not able to achieve it at present.

We'll probably want to have an UnwrapTokenResponse handler that is an equivalent of UnwrapUserinfoResponse to simplify things here

If we can solve the app_access_token to continue this pull request, I will try to create a new handler to simplify the unwarp action.

[gehongyan]

or maybe we could try an hybrid approach by not removing the state parameter even if we added to the redirect_uri? I.e don't call this line for Feishu?

Nice, it works for both authorization and deny actions.

Ingenious workaround. 👍

[kevinchalet]

If the provider is named Feishu, should we consider the CN version as the default? Conversely, if the Global version is the default, perhaps the provider should be named Lark instead.

Since it seems Lark was released first, I guess it makes sense to name it Lark and use the global region?

So, do we have a better way to support it? Or we are not able to achieve it at present.

If we wanted to support that, we'd need to acquire that stupid app token before making the token request using an HttpClient...

Another option - my favorite to be honest - would be to ignore the deprecation and target the old - standard! - API that doesn't require any of the workarounds we need with the "new" version. If they ever decide to remove it, we'll still be able to migrate to the new one at a later date 😃

[gehongyan]

Do we know if they plan to phase their deprecated APIs out?

Maybe not, but they should leave the deprecated version still working without breaking legacy applications. The docs say that this deprecated version does not support the scope parameter. I checked the developer console, and found that they support tons of scopes to authorize a third-party app to invoke APIs. If we implement the deprecated version, perhaps user codes will only to get who authorized the OAuth flow without more modern abilities in the future.

---

For example, if user code applies these scopes:

The authorization page will be:

which reflects the scopes requested.

[kevinchalet]

Hum. What happens when targeting the old OAuth 2.0 endpoints (i.e without scope support)? Do you get an unrestricted access?

[gehongyan]

Hum. What happens when targeting the old OAuth 2.0 endpoints (i.e without scope support)? Do you get an unrestricted access?

Let me take some time to test it out.

[gehongyan]

It seems that the legacy version of OAuth authorization supports scopes 🚀

The authorization page URI is like:

https://passport.feishu.cn/suite/passport/oauth/authorize?client_id=cli_**************&redirect_uri=http%3A%2F%2Flocalhost%3A49152%2Fcallback%2Flogin%2Ffeishu&response_type=code&scope=contact%3Auser.phone%3Areadonly%20contact%3Auser.email%3Areadonly&state=ODrV********************uK6e_iE

---

Before I authorized the app with email and mobile phone numbers, the user info API responded with:

If I authorized the app of those scopes:

The authorization page displayed what scopes the app requested:

And redirection works:

Now, the same user info API responded with the phone number and email:

The authorization detail in my account also shows these authorized scopes:

---

However, when users deny the authorization demand, the legacy OAuth doesn't redirect back to the redirct_uri.

[kevinchalet]

It seems that the legacy version of OAuth authorization supports scopes 🚀

🤣

In this case, let's use the legacy API: if they ever remove it, we'll update both the aspnet-contrib and OpenIddict providers, but in the meantime, the standard OAuth 2.0 API seems to be a much better choice 😄

However, when users deny the authorization demand, the legacy OAuth doesn't redirect back to the redirct_uri.

It's quite frequent. I guess it's a reasonable compromise 😃

Did you have a chance to test the Lark flavor and see if scopes also work with the legacy version?

[gehongyan]

In this case, let's use the legacy API: if they ever remove it, we'll update both the aspnet-contrib and OpenIddict providers, but in the meantime, the standard OAuth 2.0 API seems to be a much better choice 😄

Nice, I will update the codes to apply these suggestions.

It's quite frequent. I guess it's a reasonable compromise 😃

So we have to accept that denying does not work because the legacy version is a better choice from an overall perspective. 👍

Did you have a chance to test the Lark flavor and see if scopes also work with the legacy version?

Yes, I tested Lark as well, scopes of which also work with the legacy version.

[gehongyan]

(If I use the Feishu client_id and client_secret to create authorization demands on Lark endpoints:

The page shows my Feishu authentication credentials do not have permissions for the app created by my Lark account. Now, I may reasonably assume that their data are not completely separated and isolated.

[kevinchalet]

Merged, thanks for your contribution! 🎉

[gehongyan]

Thanks for your help! ❤️