aspnet-contrib / AspNet.Security.OAuth.Providers

OAuth 2.0 social authentication providers for ASP.NET Core
Apache License 2.0
2.38k stars 538 forks source link

About Alipay Provider #520

Closed LeaFrock closed 3 years ago

LeaFrock commented 3 years ago

Is your feature request related to a problem? Please describe.

I notice that the PR #387 has been closed. Alipay(of Alibaba) is a quite common external provider for Chinese users, which is similar to QQ or WeChat, so I think it’s better to add alipay provider here.

Describe the solution you'd like

The difference between Alipay provider and others is that it uses an unified gateway endpoint with many self-defined request params and it currently provides a dedicated SDK for .NET. So I want to make sure if I can rely on the SDK which is maintained actively by the official. The SDK provides two benefits: make develop work simpler(for example, the way to get value of 'sign' by RSA2) and more maintainable for the future (when outh-apis change, you need to upgrade the SDK only).

Describe alternatives you've considered

Also, I'm able to give my own implementation according to the document of Alipay without the help of official SDK, but I'm not sure I have the time to keep it work in the future.

Additional context Dingding Talk(another production of Alibaba) is similar to this situation, but I'm not interested about that because it's not very popular as an external provider.

kinosang commented 3 years ago

387 was closed due to inactivity and merge conflicts. if you'd like to contribute, you can fork and create a new pull request.

LeaFrock commented 3 years ago

@kinosang Well, I know it. In fact, I prefer to implement the solution based on the official SDK which I think it's easier to maintain and more flexiable for changes of future. That's why I don't fork the closed PR and ask for the answer of whether I can put the SDK in as a Nuget reference of alipay provider.

martincostello commented 3 years ago

There were also unresolved comments regarding the implementation due to its non-standard nature with regards to OAuth.

If, even with an official SDK, that's still the case then @kevinchalet may not want to accept such a provider into the repo.

LeaFrock commented 3 years ago

@martincostello I will take time to check it when I'm free.

due to its non-standard nature with regards to OAuth`

Emmm... I agree with you in some degree. But I think Alipay is still based on the process of OAuth, and I should be able to handler the non-standard part at OAuthHandler<T>.

BTW, I hope to promote this wonderful repository to more Chinese developers through my blog. Because Alipay is as popular as WeChat in China, I think Alipay provider should join here as much as possible.

kevinchalet commented 3 years ago

@martincostello depending on a SDK, even an official one, doesn't seem ideal to me.

The original PR had a few crypto helpers needed to deal with the non-standard Alipay mess, but at least, it didn't depend on external code. Maybe it's the price to pay if we want to support Alipay?

@LeaFrock I hope we'll eventually see one standard-compliant OAuth 2.0 implementation operated by a Chinese company 😃 I'm really starting to think not respecting standards is a cultural thing in China 🤣

jijiechen commented 3 years ago

@kevinchalet

I hope we'll eventually see one standard-compliant OAuth 2.0 implementation operated by a Chinese company 😃 I'm really starting to think not respecting standards is a cultural thing in China

It's really impolite for you to speech like this. It's true that we are seeing these non standard-compliant implementations from many service providers, including many large entities. However, let's keep the conversation within the original technical thread and do not extend it unnecessarily.

And @martincostello just reacted a thumbs-up for the message. I hope you are aware that you are hurting a kind author who wanted to contribute to this project.

kevinchalet commented 3 years ago

It's really impolite for you to speech like this.

Don't take it personally, it's just a fact: the most horrible hacks we had to introduce are for providers operated by Chinese companies. If you don't like this situation, feel free to contact these companies to encourage them to respect standards.

martincostello commented 3 years ago

@jijiechen My thumbs up was simply to acknowledge the comment about whether or not using an SDK was better than custom code.

kinosang commented 3 years ago

@kevinchalet

Though many Chinese companies operate OAuth providers, some of them, such as Alipay and WeChat, do not want to make it open nor free (libre).

There're many Chinese companies follow the standards, for e.g., Baidu, Weibo, and QQ (the new implementation #509 by @LeaFrock).

Back to the topic, I Disagree to add any provider based on a SDK, it's dirty and not easy to maintain as a part of the repository.

jijiechen commented 3 years ago

Well, by following your theory, one may tell the situation could become more horrible when those Chinese companies become standard makers some day.

There must be a reason for them to break the standards. It could be they didn't learn quite well, another reason could be there can be something missing from the standards so they had to customize or extend the standards to meet their needs. I acknowledge that it's a reality that these non-standardized implementations do exist, but please don't try to joke this can be a culture thing in China. Because there can be such implementations everywhere.

Let's go back to the original topic. Unfortunately, when a product is widely used by a massive population in the world, it becomes a standard automatically. I think this is the reason why did @LeaFrock raise his idea.

kinosang commented 3 years ago

But I don't think it's bad to add non-standard providers with some hacks and something else. Apple OAuth Provider also needs hacks to work cuz it does not follow the standards either.

kevinchalet commented 3 years ago

There're many Chinese companies follow the standards, for e.g., Baidu, Weibo, and QQ (the new implementation #509 by @LeaFrock).

@kinosang well, not sure those are good examples: these 3 providers use non-standard scopes representations (comma-separated instead of space-separated). And QQ is likely the most horrible one: it forces us to send GET token requests - the spec requires using POST - with a non-standard fmt parameter.

Well, by following your theory, one may tell the situation could become more horrible when those Chinese companies become standard makers some day.

Definitely not. The more people/organizations/countries participate to the elaboration of standards, the better they are. I'm all for developers of all origins - China included - to contribute to OAuth 2.0 and OIDC.

There must be a reason for them to break the standards. It could be they didn't learn quite well, another reason could be there can be something missing from the standards so they had to customize or extend the standards to meet their needs.

@jijiechen standards are like rules, breaking them is pretty much always a bad thing (and in the examples we mentioned so far, I can't think of any valid reason not to respect them: things like GET token requests are just horrible practices).

Because there can be such implementations everywhere.

Definitely. Should these providers be French (or anything else, really), I would have absolutely no problem saying it could be a French cultural thing 🤣

But I don't think it's bad to add non-standard providers with some hacks and something else.

Introducing workaround for non-standard behaviors means we need to maintain additional code to support them, which makes everyone's life more painful.

Apple OAuth Provider also needs hacks to work cuz it does not follow the standards either.

Certainly. But at least, they listened to some of the criticism made by the OAuth 2.0/OIDC community and fixed their implementation (e.g they introduced standard OIDC discovery support). None of the providers we have mentioned in this topic have been willing to fix their issues and that's what I have a problem with.

kevinchalet commented 3 years ago

@jijiechen BTW, since you work for Tencent, why not helping us by contacting the WeChat/Weixin folks in charge of the OAuth 2.0 integration to fix things like limited state support, for which we had to introduce an ugly workaround?

kinosang commented 3 years ago

@kevinchalet

these 3 providers use non-standard scopes representations (coma-separated instead of space-separated). And QQ is likely the most horrible one: it forces us to send GET token requests - the spec requires using POST - with a non-standard fmt parameter.

my mistake. they are not standard providers. the three providers existed before the OAuth 2.0 specs released. they are not up to date, but they try to follow the standard (e.g. QQ accept a new parameter to return JSON instead of JSONP).

I mean,

but please don't try to joke this can be a culture thing in China -- by @jijiechen

But I do not agree with There must be a reason for them to break the standards. Developers hate the non-standard implementations, but those companies do not care about the feedback. Again, they are not open nor libre, they create "open platform" for enclosure.

jijiechen commented 3 years ago

Thanks for your reply, and it's rather valuable. In terms of the hacky implementations, I agree that maintaining an SDK with all those hacks included is painful. I don't expect the idea by the author could be accepted by this repo. As open source practitioners, we muchly benefited from plenty of standards, so we always try to let people know how important it is to align with existing standards to keep themselves secure and up-to-date with the extended eco-systems. I think it's obviously true that there are more and more Chinese developers actively make their contributions in OSS communities. So the situation will definitely become better in the near future. However, many existing non-standard-compliant systems had been widely adopted and they are large which means we can't expect them to change quickly.

jijiechen commented 3 years ago

@kevinchalet

BTW, since you work for Tencent, why not helping us by contacting the WeChat/Weixin folks in charge of the OAuth 2.0 integration to fix things like limited state support, for which we had to introduce an ugly workaround?

Yes, good idea. I'm happy to deliver the message.

kevinchalet commented 3 years ago

@jijiechen thanks, much appreciated.

Pain points you'll probably want to discuss with them:

jijiechen commented 3 years ago

@kinosang

those companies do not care about the feedback

It depends. It terms of OAuth2 implementations, I would rather think the real reason can be very simple as the original developers in the team who implemented those hacky services added their opinionated "salt" to their own OAuth services. I know many guys trying to make variants from standards to keep themselves different so that they'll believe to be more secure or more self-controllable. Mostly, the giant companies will not pay any attention to things as small as a few lines of code. After a period of time, when their hacky services were published to the public, the effort to fix could be non-trivial.

kinosang commented 3 years ago

@jijiechen Do Not create your own "smart" hacks about security, even though you're a giant company. The standards are audited and verified by the community.

安全领域, 勿造轮子

Mostly, the giant companies will not pay any attention to things as small as a few lines of code.

I don't think so. The giant companies have project managers, code reviewing and QA. Some of the "variants" are as designed (for enclosure I believe).

I know there're also many remained issues, and change the issues cannot be done quickly. I hope the situation will definitely become better in the near future as you said.

LeaFrock commented 3 years ago

I don't want to cause a quarrel. I'm just asking whether I can contribute what I want. If the members think it's not suitable, what I need to do is just closing this issue.

I'm really starting to think not respecting standards is a cultural thing in China 🤣

TBH I feel a little bit offended, though I guess you're just joking @kevinchalet . It would be a good humor if you say ... is a cultural thing in French since you're a French man.

Everyone here knows the good of standards. As @jijiechen and @kinosang said, most of non-stardard situations are due to historical reasons instead of deliberate desire to fight against the world. The Apple or Facebook also have the same problem. If you think it's acceptable that Apple does its changes, you may also need to realize that Chinese companies are also not blind or deaf, though maybe you have no touch with their productions. As my recent PR tells, QQ changed the response format to JSON, of course it's not perfect, but it is reasonable at this current.

This repo makes me happy at first because it's a stronghold of OAuth providers. My original intention is just to make it more comprehensive and more acceptable for Chinese developers. I'll turn back to delivery your suggestions to Alipay SDK developers.

At last, I hope we have no prejudice against each other. We never give up chasing ideals, but we also have to deal with reality 😅 . Let's find common and save differences, and don't waste time on the argument.

kevinchalet commented 3 years ago

It depends. It terms of OAuth2 implementations, I would rather think the real reason can be very simple as the original developers in the team who implemented those hacky services added their opinionated "salt" to their own OAuth services. I know many guys trying to make variants from standards to keep themselves different so that they'll believe to be more secure or more self-controllable.

I've been maintaining my own OAuth 2.0/OIDC server implementations for almost 7 years now, so I have a strong experience of what "opinionated" means in both camps (servers and clients). The OAuth 2.0 base specification - one of the clearest and most pleasant specs I have ever dealt with - leaves many aspects up to the implementers (e.g the access/refresh token formats are freely chosen by the server and are opaque for the clients and they can change at any time).

... but here, we're not talking about rules broken to increase security (e.g in my OpenIddict implementation, response_type=token authorization requests were not allowed for confidential clients to prevent downgrade attacks). We're talking about clear violations of the protocol for no tangible reason (e.g by forcing folks to use GET token requests, chances are high a few tokens end up in the logs, as the query string is generally logged by servers...).

The problem is that none of these providers seems to realize this hurts interoperability. All these issues have been reported countless times and nothing has changed: yet most of the issues could be fixed in a non-breaking way.

It would be a good humor if you say ... is a cultural thing in French since you're a French man.

👇🏻

Definitely. Should these providers be French (or anything else, really), I would have absolutely no problem saying it could be a French cultural thing 🤣

https://github.com/aspnet-contrib/AspNet.Security.OAuth.Providers/issues/520#issuecomment-773307266

kevinchalet commented 3 years ago

Anyway, back to the initial topic: @LeaFrock would you be interested in reviving https://github.com/aspnet-contrib/AspNet.Security.OAuth.Providers/pull/387/files?

LeaFrock commented 3 years ago

@kevinchalet That's OK. I'll handle it when I'm free (and may take some time).

martincostello commented 3 years ago

Thanks everyone for your contributions.

The Alipay provider is now available from NuGet.org: https://www.nuget.org/packages/AspNet.Security.OAuth.Alipay/5.0.2