Jericho / ZoomNet

.NET client library for the Zoom.us REST API v2
MIT License
69 stars 46 forks source link

OAuthConnectionInfo has partial support for RefreshToken only construction #253

Closed cdmdotnet closed 1 year ago

cdmdotnet commented 1 year ago

If the Null argument check is removed, then OAuthConnectionInfo already has full support to use a provided RefreshToken to get an AccessToken. However this does not work currently as there is a null reference check.

Jericho commented 1 year ago

Regarding how I found this works, I did a quick step through/step over to by pass the null reference check as normal OAuth flows allow for saving just the refreshtoken... accesstokens are supposed to be transient, so it just made sense that it would work.

You are absolutely correct. The access token is indeed transient (expires after 60 minutes) and ZoomNet should absolutely allow developers to omit it when setting up the OAuth connection. The code in OAuthConnectionInfo constructor is able to deal with a null token but the "null parameter check" prevents that from happening. Quite the contradiction! Removing the "null parameter check" as you have done is the right thing to do.

While we are on this topic, here's something else I would like to debate with you: if you look at the code samples in my readme you'll see that I instruct developers to save BOTH the refresh token and the access token. I am now second guessing this recommendation. Maybe I should recommend that only the refresh token is necessary? What do you think?

cdmdotnet commented 1 year ago

I'd say to only use the access token in the method/class, not save/store it in a DB etc like you would with the refresh token.

Jericho commented 1 year ago

I'll merge your PR and re-word my recommendation in readme to make it clearer that only the refresh token should be preserved.

Jericho commented 1 year ago

:tada: This issue has been resolved in version 0.54.0 :tada:

The release is available on:

Your GitReleaseManager bot :package::rocket: