slakbal / gotowebinar

Laravel GoToWebinar API wrapper
MIT License
16 stars 12 forks source link

OAuth v2 #4

Closed upwebdesign closed 5 years ago

upwebdesign commented 6 years ago

@slakbal I just received this today from GoTo Developer Center:

Thank you for using the GoTo Developer Center. We informed you earlier in the year about LogMeIn GoTo Developer Center and API enforcement of the strict OAuth 2.0 authentication protocol to improve security of our API program. This change requires your action.

Support for the old protocol has been extended until January 31, 2019 to give you time to test and update your code to use the new authentication services available under h‌ttps://api.getgo.com/oauth/v2.

Do you have any intentions of building out the OAuth 2.0 auth type like you stated in your config file?

upwebdesign commented 6 years ago

@slakbal It appears this may not be as difficult as originally thought. Updates to some fields and possibly the response with the access token:

image

image

slakbal commented 6 years ago

@upwebdesign hey there. Yeah I have a project running on this so still need to get to that sooner than later, but struggling a bit with finding time. Will see if I can make some time soon to tackle it. If you have already some ideas of how to implement it, it would help with the time issues :)

upwebdesign commented 6 years ago

@slakbal I asked this question via LogMeIn email support:

After reviewing the migration guide it appears Direct Login will still be available to use for the GoToWebinar API after Jan 31st 2019. Is this correct?

And their answer:

Correct. The Direct Login method will still be available. A curl example of Direct Login for v2 auth is shown on the Migration Guide

The migration, should you stick with Direct Login, looks to be fairly straight forward. This is great news from a time perspective. I will continue to assess.

slakbal commented 5 years ago

Cool... yeah will start to look at migrating everything to v2 auth soon.

upwebdesign commented 5 years ago

@slakbal I just want to make sure you are aware the Legacy OAuth authentication APIs will stop working October 1st, 2019. This is from an email I received Jan 30th.

Sample curl post for Direct Login with oAuth v2:

curl -X POST \
  'https://api.getgo.com/oauth/v2/token' \
  -H 'Authorization: Basic {Base64 Encoded client_id and client_secret}' \
  -H 'Content-Type: application/x-www-form-urlencoded' \
  -d 'grant_type=password&username={username}&password={password}'

Documentation: https://goto-developer.logmeininc.com/oauth-migration-guide

Let me know if you have questions. I will see if I can find time to get a pull request created for this update.

GalBrigitta commented 5 years ago

+"error": "OAuth API v1 has been decommissioned as announced in February 2018, please switch to OAuth API v2 documented here: https://goto-developer.logmeininc.com/oauth-migration-guide. Please reach out to Support (developer-support@logmein.com) with questions. Hy, I also get this error. I guess the path in the DirectLogin.php, but what else should be changed?

GalBrigitta commented 5 years ago

Oh I see now this was merged, but I installed with composer and it is not updated.

upwebdesign commented 5 years ago

@GalBrigitta it sounds like you required dev-master. If you want to use oAuth V1 (until V2 is stable) you need to require the package or specify a release.

composer require slakbal/gotowebinar composer require slakbal/gotowebinar:^1.0

upwebdesign commented 5 years ago

@slakbal I see there have been changes to the code in master, have you confirmed these updates work? I will perform some tests myself as well. Once we confirm, we should release a new version. Thoughts?

upwebdesign commented 5 years ago

@slakbal Seems the current dev-master has "orchestra/testbench": "~3.7" required and that particular version requires "laravel/framework": "~5.7.14". This is going to be a problem for those that have slightly older versions of Laravel. We are using L5.6 at the moment and I cannot install dev-master. This is not bleeding edge, but I feel like your package should support at least down to L5.6.

Proposed solution is to require "orchestra/testbench": "~3.6.6", which for now keeps us < 3.7

GalBrigitta commented 5 years ago

@upwebdesign a new release would be great, I can also help with testing. Anyway I reported an issue, do you think you could help?

upwebdesign commented 5 years ago

@GalBrigitta I am not the owner of this project nor do I have the ability to publish releases. We can create forks and provide pull requests.

slakbal commented 5 years ago

@GalBrigitta @upwebdesign - since the last week I've been working on a new release with OAuth2 and will soon start to commit to the dev branch.

slakbal commented 5 years ago

@slakbal I just want to make sure you are aware the Legacy OAuth authentication APIs will stop working October 1st, 2019. This is from an email I received Jan 30th.

Sample curl post for Direct Login with oAuth v2:

curl -X POST \
  'https://api.getgo.com/oauth/v2/token' \
  -H 'Authorization: Basic {Base64 Encoded client_id and client_secret}' \
  -H 'Content-Type: application/x-www-form-urlencoded' \
  -d 'grant_type=password&username={username}&password={password}'

Documentation: https://goto-developer.logmeininc.com/oauth-migration-guide

Let me know if you have questions. I will see if I can find time to get a pull request created for this update.

Hey @upwebdesign - on the above, can you please check if the access_token (bearer token) that you receive back from the direct login method actually works in the v2 API via the documentation? It doesn't want to work on my side. You can test if it work directly in the documentation. Maybe I'm doing something wrong. Please let me know, since I want to avoid the standard OAuth redirect login and approval flow and want to keep the API integration fully automated and transparent.

Thanks!

slakbal commented 5 years ago

Hey @upwebdesign - on the above, can you please check if the access_token (bearer token) that you receive back from the direct login method actually works in the v2 API via the documentation? It doesn't want to work on my side. You can test if it work directly in the documentation. Maybe I'm doing something wrong. Please let me know, since I want to avoid the standard OAuth redirect login and approval flow and want to keep the API integration fully automated and transparent.

Thanks!

No worries, it was my damn developer account on the dev portal that isn't allowing authentication. the production account is working fine.

upwebdesign commented 5 years ago

@slakbal, thanks for getting back on the wagon with this. I have not had the time to dive in, but at some point I will need to make the switch myself. I am loaded with another project at the moment and trying to fit in time for the project that your package uses. Once I get there I will keep you informed of any findings and contribute all that I can at that time. Thanks again for the dialog.

upwebdesign commented 5 years ago

@slakbal, after reviewing the commits on master it appears develop is slightly behind. Please review pull request #6. I dont believe those commits made it to develop branch.

slakbal commented 5 years ago

@slakbal, after reviewing the commits on master it appears develop is slightly behind. Please review pull request #6. I dont believe those commits made it to develop branch.

I am doing a complete new development branch, will start to commit the new branch shortly.

upwebdesign commented 5 years ago

@slakbal when do you think you will have develop branch ready to be up on Github? I'd like to help to push this along.

@agmadt

agmadt commented 5 years ago

Hi @slakbal, I can help with the development if your hands is full, just let me know, I will gladly help..

slakbal commented 5 years ago

@upwebdesign @agmadt - thanks! I'm going to try and finish up the documentation tonight and the push the first version to the development branch. Then you all can jump in and help to polish :)

slakbal commented 5 years ago

@upwebdesign

With the new major version I'm experimenting with getting the API closer to fluent Laravel way:

Create Webinar:

$result = Webinar::subject('Some meeting')
                    ->description('Description of the meeting')
                    ->timeFromTo(Carbon::now()->addDays(2)->toW3cString(), Carbon::now()->addDays(2)->addHour()->toW3cString())
                    ->timezone('Europe/Amsterdam')
                    ->singleSession()
                    ->noEmailReminder()
                    ->noEmailConfirmation()
                    ->create();   

Update Webinar:

$result = Webinar::webinarKey(123456789)
                ->subject('Some new Subject')
                ->description('a new description of the meeting')
                ->update();

Delete Webinar:

$result = Webinar::webinarKey(123456789)
                ->sendCancellationEmails()
                ->delete();

Create Registrant:

$result = Registrant::webinarKey(123456789)
                ->firstName('Peter')
                ->lastName('Pan')
                ->email('Peter@pan.com')
                ->create();

What do you think of the above API?

upwebdesign commented 5 years ago

@slakbal this is looking really nice so far. I am inclined to do something like this when creating...

Create Registrant:

$result = Registrant::webinarKey(123456789)
                ->create([
                    'firstName' => 'Peter',
                    'firstName' => 'Pan',
                    'email' => 'Peter@pan.com'
                ]);

This gives a little more flexibility to add more fields without adding methods. This is just an observation. This is your package and you can decide which way you would like to go.

Flags such as:

->sendCancellationEmails()

These are really convenient and they explain what is happening very well.

I really like this:

Create Webinar:

$result = Webinar::subject('Some meeting')
                    ->description('Description of the meeting')
                    ->timeFromTo(Carbon::now()->addDays(2)->toW3cString(), Carbon::now()->addDays(2)->addHour()->toW3cString())
                    ->timezone('Europe/Amsterdam')
                    ->singleSession()
                    ->noEmailReminder()
                    ->noEmailConfirmation()
                    ->create();  

Alternative suggestion:

$result = Webinar::singleSession()
                    ->noEmailReminder()->noEmailConfirmation()
                    ->scheduleFromTo(Carbon::now()->addDays(2)->toW3cString(), Carbon::now()->addDays(2)->addHour()->toW3cString())
                    ->create([
                        'subject' => 'Some meeting',
                        'description' => 'Description of the meeting'
                        'timezone' => 'Europe/Amsterdam'
                    ]);  

Again, minor, and if the fields for GTW are not named subject or description, I like the method approach better as we map the data with more readable code.

Great work on this. Looking forward to playing around with it.

upwebdesign commented 5 years ago

@slakbal I just received the final notice from GTW. It is crunch time. Let me know where you are with the updates so we can review. As always, let me know if I can help. Thanks!

slakbal commented 5 years ago

@upwebdesign hey there, I've pushed the newest implementation, will tag only once we are happy with the stability. Will still be adding the sessions, etc. end-points to the implementation asap. Can you in the meantime have a look at it, test and if you find anything let me know, please.

slakbal commented 5 years ago

@upwebdesign hey Jesse, I've also pushed now Attendee and Session operations to the new implementation. Maybe you can test for me something on your side? When adding a registrant I get an API error when I want to delete it bu registranKey, the same also when I want to look up the registrant by registranKey. This also happens on the attendees side of things. I will also follow-up with Goto guys on the ticket I've raised for this. I've marked those issues with //todo in the code.

slakbal commented 5 years ago

@upwebdesign @agmadt @GalBrigitta - Going to close the issue for now since it is getting long. But you help with testing etc. will be appreciated. Especially around the issues on registrantKey. Thanks!

upwebdesign commented 5 years ago

@slakbal Im very busy at the moment to do new testing with the new code. I was able to use my PR to get oAuth working for now. I will reach out when I think I am ready for the new code.

slakbal commented 5 years ago

will reach out when I think I am ready for the new code.

100%