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

[BUGFIX] Adapt GoTo plugin for oauth2 authentication #7380

Merged
merged 2 commits into from May 2, 2019

Conversation

Projects
4 participants
@FlorianWessels
Copy link
Contributor

commented Apr 2, 2019

Q A
Bug fix? Y
New feature? N
Automated tests included? Y
Related user documentation PR URL N
Related developer documentation PR URL N
Issues addressed (#s or URLs) #5736
BC breaks? N
Deprecations? N

Description:

Migrate GoTo authentication to new RFC 6749 compliant OAuth Proxy.

@npracht npracht added this to the 2.15.2 milestone Apr 2, 2019

@npracht npracht added this to Ready to Test (first time) in Mautic 2 Apr 4, 2019

@npracht

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

@FlorianWessels Can you check what's wrong with travis please ?

@FlorianWessels

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

Yeah, I will do that.

@npracht

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

@johbuch can you confirm that with this new version of the GoTo API, everything is working properly ?

@npracht npracht requested a review from johbuch Apr 10, 2019

@johbuch

This comment has been minimized.

Copy link

commented Apr 10, 2019

I tested to set it up on mautibox. I can confirm that i could authorised the app without any problem.
Then I could find new filters in segments but I could not properly test the sync because we need a specific cronjob to run.
But before (without this PR), we could not set up the sync because of the Oauth1. But this seems to be fixed

@npracht npracht moved this from Ready to Test (first time) to Ready to Test (confirmation) in Mautic 2 Apr 11, 2019

@johbuch
Copy link

left a comment

It works now
👍

Mautic 2 automation moved this from Ready to Test (confirmation) to Ready to Test (first time) Apr 24, 2019

@npracht
Copy link
Member

left a comment

This is working perfectly good job. @kuzmany could you please give a look at the code before merge ? Otherwise the update is perfect.

@npracht npracht moved this from Ready to Test (first time) to Ready to Commit (passed testing) in Mautic 2 Apr 24, 2019

@kuzmany

This comment has been minimized.

Copy link
Contributor

commented Apr 24, 2019

Code looks good.
What about backward compatibility? Works after upgrade?

@npracht

This comment has been minimized.

Copy link
Member

commented Apr 24, 2019

Yes you just need to re-auth.
But without it, it won't work anymore....

@kuzmany

This comment has been minimized.

Copy link
Contributor

commented Apr 24, 2019

Now I got it from issue #5736
Better in future write that to description. Code it's legit.
Do you use it in production?

@npracht

This comment has been minimized.

Copy link
Member

commented Apr 24, 2019

Yes we do !

@kuzmany kuzmany merged commit 812aee9 into mautic:staging May 2, 2019

2 checks passed

Scrutinizer Analysis: 1 new issues, 4 updated code elements – Tests: passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

Mautic 2 automation moved this from Ready to Commit (passed testing) to Merged May 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.