search

prschmid / zoomus

Python client library for the Zoom.us REST API v1 and v2
Other
253 stars 133 forks source link

Add Server-to-Server OAuth Support without Breaking Current Implementation #341

Closed woodsbw closed 1 year ago

woodsbw commented 1 year ago

This adds a new top-level client class, ZoomClientServerToServerOauth, which can be used instead of ZoomClient to support Zoom's new Server-to-Server OAuth authentication method. Since both methods just resolve to bearer tokens, the class can inherit from zoomus.util.ApiClient with no issues and and benefit from all existing work, without changing the functionality for the ZoomClient class as all.

xezpeleta commented 1 year ago

Hi! Is this PR working? Any plan to merge it? Thanks to everybody contributing to this project!!!

regiscamimura commented 1 year ago

In my experience, yes, it works. Just take care with the refresh_token() things I mentioned above.

IMO we should break the current zoomus implementation though, because JWT is deprecated on zoom so it should be already broken. In other words, just update the current methods, and keep the names. More or less.

I have a fork from a fork of zoomus that should do that. I didn't open a PR against this repo because I didn't figure out how to do that (couldn't get along with the repo branching workflow and such), I own that to this repo though. And also for the fork that I forked (thank you https://github.com/mlhess) If someone can pick my fork and do that for us, it would be nice.

mlhess commented 1 year ago

I can close my PR that started this work, thank you @regiscamimura for picking it up.

prschmid commented 1 year ago

@mlhess @regiscamimura @xezpeleta @woodsbw Hi all!

First, massive apologies for not hopping on this sooner. I wasn't receiving the notifications on this and so completely dropped the ball. I personally haven't been using this library for my work/projects and so wasn't up to speed on the issues.

I am just about to open a branch based on the changes that @mlhess did in his fork that should get us all where we need to be. Since this is a breaking change, once that gets merged in, I'll bump the version number on the library so that everyone is aware of the update.

Once that is live, I'll also go about updating some of the dependencies so that they are not completely out of date.

prschmid commented 1 year ago

@mlhess @regiscamimura @xezpeleta @woodsbw Assuming I didn't mess anything up, version 1.2.0 of this library should now support the new authentication method. Please let me know if you run in to any issues. Closing this PR as this should now be solved as per #343.