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

Implement Zoho OAuth provider #910

Merged

Conversation

denis-goncharenko
Copy link
Contributor

The implementation allows users to authenticate using their Zoho account credentials.

@kevinchalet
Copy link
Member

kevinchalet commented Jul 4, 2024

Did you try with a user account created in a non-US region?

I just added this provider to OpenIddict and it was fairly painful to support that, as you need to make the token and userinfo endpoints dynamic (based on the location parameter returned from the authorization endpoint). Given it doesn't seem to be implemented here, it's likely it requires a bit more work to really work.

@kevinchalet kevinchalet marked this pull request as draft July 4, 2024 08:29
@denis-goncharenko
Copy link
Contributor Author

Did you try with a user account created in a non-US region?

I just added this provider to OpenIddict and it was fairly painful to support that, as you need to make the token and userinfo endpoints dynamic (based on the location parameter returned from the authorization endpoint). Given it doesn't seem to be implemented here, it's likely it requires a bit more work to really work.

Yes, I did. I have tested with almost all regions.

Each region that supports Zoho has its own domain (https://www.zoho.com/accounts/protocol/oauth/multi-dc.html). In my implementation, I configure all necessary endpoints in ZohoAuthenticationPostConfigureOptions.cs file and use hardcoded endpoints for selected region. Do you mean to use this endpoint (https://accounts.zoho.com/oauth/serverinfo) to receive locations or smth else?

@kevinchalet
Copy link
Member

In my implementation, I configure all necessary endpoints in ZohoAuthenticationPostConfigureOptions.cs file and use hardcoded endpoints for selected region.

But they are configured statically/globally and are used for all users. Yet, if your application is configured to use the US region and you try to log in with a EU account, you'll get an error during the token request since the client was expected to send that token request to the EU token endpoint and not to the US token endpoint.

@kevinchalet
Copy link
Member

That (stupid) logic is explained here: https://www.zoho.com/accounts/protocol/oauth/multi-dc/client-authorization.html

@denis-goncharenko
Copy link
Contributor Author

Hey, @kevinchalet ! When you get a chance, could you take a look at this pull request?

Copy link
Member

@kevinchalet kevinchalet left a comment

Choose a reason for hiding this comment

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

Hey, @kevinchalet ! When you get a chance, could you take a look at this pull request?

Hey,

Sorry for the late reply, busy weekend 🤣

@denis-goncharenko denis-goncharenko marked this pull request as ready for review September 10, 2024 15:56
@martincostello martincostello added this to the 8.2.0 milestone Sep 12, 2024
@martincostello
Copy link
Member

Just to confirm, have you tested this works with the real service?

@denis-goncharenko
Copy link
Contributor Author

Just to confirm, have you tested this works with the real service?

yep, I just did one more test and it works as expected.

@martincostello martincostello merged commit 8b3e741 into aspnet-contrib:dev Sep 14, 2024
8 checks passed
@martincostello
Copy link
Member

Thanks again for your contribution. I'll look to do a 8.2.0 release for this and Docusign at some point this weekend.

@martincostello
Copy link
Member

The Docusign and Zoho providers are now both available from NuGet.org as part of the 8.2.0 release - thanks again!

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

Successfully merging this pull request may close these issues.

3 participants