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

Support for Zendesk #2217

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Support for Zendesk #2217

wants to merge 1 commit into from

Conversation

mozts2005
Copy link

I have added Support for Zendesk. I am happy to help in testing validation as I have an account with Zendesk that is used for Open-source project testing.

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.

Thanks for your PR! ❤️

I am happy to help in testing validation as I have an account with Zendesk that is used for Open-source project testing.

Yes please! (I don't have a Zendesk account and won't be able to test it myself)

@kevinchalet
Copy link
Member

Hey @mozts2005,

If you have any question or need help to complete this PR, please let me know 😃

Cheers.

@mozts2005
Copy link
Author

I made the changes you requested.
I also added support for revocation of the token.
Please let me know if you would like to handle the revocation authentication differently. I did it this way as the same type of override is done for the Trovo userinfo endpoint authentication.

request.Headers.Authorization = new AuthenticationHeaderValue("OAuth",

But I was thinking that if you wanted this could be added as a new EndpointAuthMethod if you prefer.

Next, I added some mappings for to the UserInfo based on Zendesk endpoint results but if you would like I can remove them or add more.

@kevinchalet
Copy link
Member

Please let me know if you would like to handle the revocation authentication differently.

No that's fine 👍🏻

@@ -1362,6 +1362,9 @@ public ValueTask HandleAsync(ProcessAuthenticationContext context)

// Shopify returns the email address as a custom "associated_user/email" node in token responses:
ProviderTypes.Shopify => (string?) context.TokenResponse?["associated_user"]?["email"],

// Zendesk returns the emails address as a custom "user/email" node:
ProviderTypes.Zendesk => (string?) context.UserInfoResponse?["user"]?["email"],
Copy link
Member

Choose a reason for hiding this comment

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

Looks like the userinfo details are wrapped in a user node? If so, we'll need to unwrap the userinfo response so all the claims can be correctly mapped: https://documentation.openiddict.com/guides/contributing-a-new-web-provider#unwrap-userinfo-responses-if-necessary

Once done, you'll need to remove the ?["user"] part of these new lines.

@kevinchalet
Copy link
Member

Thanks. Could you please attach a screenshot of the returned claims (visible in the output generated by the console client sandbox)? Feel free to blur out personal/sensitive details.

Were you able to test everything? Revocation works fine?

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

Successfully merging this pull request may close these issues.

2 participants