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

Update Zalo v4 api #972

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

Update Zalo v4 api #972

wants to merge 1 commit into from

Conversation

huyntt
Copy link

@huyntt huyntt commented Nov 10, 2024

Update Zalo v4 API enpoint.

@martincostello
Copy link
Member

  • What are the benefits of changing to the v4 endpoint?
  • Do you have a link to the v4 documentation?
  • Does the user need to add any new settings to their app or configure anything in the portal to use it?

If we accept this change, the tests will need updating as in their current state they'll fail due to the stub HTTP requests no longer matching.

@huyntt
Copy link
Author

huyntt commented Nov 11, 2024

Hi martincostello

The primary reason of change: without changes, current AspNet.Security.OAuth.Zalo fail to work.

There 2 changes are required to keep Zalo provider work properly.

  1. Zalo v3 API endpoint was removed ref Zalo API annoucement . This cause ZaloAuthenticationDefaults.cs does not work since Nov 2021. Require to update endpoint.
  2. Zalo API change request Get method to Post method and require add secret key in request header "secret_key: <your_secret_key>"
    ZaloAuthenticationHandler.cs

No additional settings are required from app side.

Reference doc:

@martincostello
Copy link
Member

Ok thanks, if it's completely broken with regards to the server side, then I guess there's no worries about breaking someone as they must have already been broken for a while 😅

Could you update the tests as required, then we can take the updates.

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

Successfully merging this pull request may close these issues.

2 participants