-
Notifications
You must be signed in to change notification settings - Fork 12
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
OAuth2 support #18
base: master
Are you sure you want to change the base?
OAuth2 support #18
Conversation
To give a bit more of context: we've been working with @kuzmany to enhance the Zapier plugin. Today, it is using basic auth which is.... not nice it terms of security. |
Can this be done without the BC break? If we release this then the Zapier users will have to go and reauthenticate all their Zaps. Ideally, it would be great if the integration would still support basic auth for existing Zaps and Oauth2 for new ones. |
Or if not possible, create a V2 plugin, i've seen this behavior often. |
@escopecz I think It works like you describe. @npracht Can you send me example of V2 plugin. Because what I've read from docs, they allow just one public version for one brand. |
Then that means they changed their policy (which can be understandable). |
@npracht Based on docs, not broken for them. If you use Zap for Mautic Zapier 2.16 then you stay on that version forever. You can migrate that user in dev platform, but not necessary. |
We can have Zapier users on 2 versions simultaneously for short period of time. We can even announce (I think, never done that) that hey will have to reauthenticate if they want to migrate to the newer version. And schedule the old version to disappear after 6 months or so. But there is still the need for hundreds of users to do some manual work with low value (from basic user standpoint) they gain for it. I would like to avoid that if possible. This may be another topic to poll the community about. |
Let do it like that. @escopecz there is a valuable reason: security. Basic Auth should not exists. |
@kuzmany we'll also need an update to https://github.com/mautic/mautic-zapier/blob/master/CHANGELOG.md for the release. And the version bump in https://github.com/mautic/mautic-zapier/blob/master/package.json#L3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also update Readme with how to test now? For example how to generate the access token?
I think that it would be best if we were to have a v2 of the app, and make it clear to folks when we are going to deprecate the v1. That way it does not immediately break things but allows people the time to transition over to the new version. Does that make sense? |
polling sample in Zap includes fields not found in latest task history ; extra keys: "dateModified", "lastActive", "modifiedBy", "modifiedByUser" (T006)
for PHPStorm
@escopecz when I build it, test passed (without it we were not able to upload it ). |
This PR brings OAuth2 support
Documentation: mautic/documentation#375