Jericho / ZoomNet

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

Pass AccessToken to WithClientCredentials #309

Closed theit8514 closed 11 months ago

theit8514 commented 11 months ago

Please add the ability to pass an AccessToken and OnTokenRefreshed handler to WithClientCredentials so that tokens may be cached between executions (for example, short lived applications persisting tokens to disk/cache).

Jericho commented 11 months ago

You can use the WithRefreshToken method to pass client id, client secret, refresh token, access token and refresh handler.

WithRefreshToken(clientId, clientSecret, refreshToken, accessToken, onTokenRefreshed)

theit8514 commented 11 months ago

I was not able to get WithRefreshToken working since I don't have a refresh token and those methods check for null and also change the grant type to refresh_token when provided any value.

Jericho commented 11 months ago

I didn't think about the fact that the grant type would be incorrect for your scenario.

To advance this discussion, let me point out a few things:

  1. ZoomNet is reliant on Microsoft's HttpClient client for sending requests to the Zoom API which means that the same lifetime management considerations that apply to the HttpClient also apply to ZoomNet. I discuss this a little more in-depth here
  2. The main consideration with the HttpClient is that it should NOT be short-lived because it can lead to socket exhaustion.
  3. This means that, whenever possible, a long-lived ZoomNet instance is preferable.
  4. Given the fact that ZoomNet client is typically long-lived, I have built-in logic to persist the token in memory, reuse it as long as possible, renew it when it's about to expire, etc.
  5. What this means is that you typically don't even need to persist your token. You can simply let ZoomNet manage it for you. There are exceptions, of course, where you must preserve your token. After all, that's why ZoomNet allows you to specify a handler which is invoked whenever a new token is requested.

I am mentioning all this to make sure you know that it is recommended not to constantly instantiate new ZoomNet clients (because of the underlying HttpClient) and also that token management is built-in. You may have a scenario that I haven't thought about that requires you to manually manage your token. I would love to hear more about your scenario, and understand why you are persisting your tokens.

On a related note: there was a problem in the Zoom API with Server-to-Server OAuth until recently where requesting a token would invalidate a previously issued token. This was problematic in scenarios where you had multiple instances of your app and they all requested a token. The result was that instance 2 would request a token which would invalidate token 1, forcing instance 1 to request a new token, which would invalidate token 2, and so on... To overcome this limitation in the Zoom platform, I was working on an advanced token management infrastructure discussed here and proof-of-concept here but Zoom changed their logic (announced here). Requesting a new token no longer invalidates previous tokens, therefore I abandoned my advanced token management idea.

Are you persisting your token because of a similar consideration? If so, would my token management idea be beneficial to you?

In the end, the easiest solution might be to simply add string accessToken and OnTokenRefreshedDelegate onTokenRefreshed parameters to the WithClientCredentials (which is what you suggested in the first place!), but before doing that, I want to make sure I thoroughly understand your scenario.

Jericho commented 11 months ago

I didn't think about the fact that the grant type would be incorrect for your scenario.

I spent a little more time thinking about your scenario and it occurred to me that I misspoke: the grant type is NOT wrong in your scenario. ClientCredentials is the appropriate grant type when the first access token is requested (only the client id and the secret are provided) but when you subsequently request to renew a token, you must provide the "refresh token" and the grant type must be RefreshToken. The problem that I see is that you seem to be preserving the access token but you are not preserving the refresh token.

Please refer to the Connection using OAuth section in the readme where I provide code samples and where I emphasize how important it is to preserve the refresh token. I even explain why, typically, it is not necessary to preserve the access token:

The access token on the other hand does not need to be preserved because it is ephemeral (meaning it expires after 60 minutes). Even if you preserve it, it is very likely to be expired (and therefore useless) before the next time you need to instantiate an OAuthConnectionInfo.

In your scenario, it may be beneficial to preserve the access token (in addition to the refresh token of course). Therefore you can customize the code sample from the readme like so:

var clientId = "... your client ID ...";
var clientSecret = "... your client secret ...";
var refreshToken = Environment.GetEnvironmentVariable("ZOOM_OAUTH_REFRESHTOKEN", EnvironmentVariableTarget.User);
var accessToken = Environment.GetEnvironmentVariable("ZOOM_OAUTH_ACCESSTOKEN", EnvironmentVariableTarget.User);
var connectionInfo = OAuthConnectionInfo.WithRefreshToken(clientId, clientSecret, refreshToken,
    (newRefreshToken, newAccessToken) =>
    {
        Environment.SetEnvironmentVariable("ZOOM_OAUTH_REFRESHTOKEN", newRefreshToken, EnvironmentVariableTarget.User);
        Environment.SetEnvironmentVariable("ZOOM_OAUTH_ACCESSTOKEN", newAccessToken, EnvironmentVariableTarget.User);
    });
var zoomClient = new ZoomClient(connectionInfo);
theit8514 commented 11 months ago

I would love to hear more about your scenario, and understand why you are persisting your tokens.

This application was originally designed to run several times an hour, but the business necessitated it run much faster than that. It would take significant effort to convert to a long lived service. I have not yet run into any socket limitations with it, but I am aware that I might at some point.

Luckily, I should only need to call the Zoom API infrequently, as it is only needed when the conditions are met. That being said, regenerating an access token each time I need one when I have the ability to persist it seems unfriendly.

ClientCredentials is the appropriate grant type when the first access token is requested (only the client id and the secret are provided) but when you subsequently request to renew a token, you must provide the "refresh token" and the grant type must be RefreshToken. The problem that I see is that you seem to be preserving the access token but you are not preserving the refresh token.

WithClientCredentials does not return a refresh token for me, only an access token. The RefreshToken property is an empty string in the options after a call to the API.

Jericho commented 11 months ago

WithClientCredentials does not return a refresh token for me, only an access token.

The only scenario that I am aware of where a refresh token is not returned by the Zoom API is when you use Sever-To-Server OAuth. All other scenarios require a refresh token because that's how you can refresh your access token when it expires after 60 minutes. My guess is that it is returned (you could validate this by inspecting the response returned from the Zoom API when a token is requested) but you are not made aware of it because ZoomNet's ConnectionInfo.WithClientCredentials does not allow you to specify a OnTokenRefreshed handler. Again, just a guess on my part.

Anyway, long story short, I propose adding an overload to the WithClientCredentials that will allow you to specify the refresh token and/or the access token and/or the OnTokenRefreshed handler. I also propose marking the WithRefreshToken as obsolete since it's logic would be duplicated in the new overloaded method. Finally, I propose adding the following paragraph to the Connection using OAuth section in the readme:

In the vast majority of cases, preserving only the refresh token is sufficient and there is no benefit to preserve the access token mainly because it expires every 60 minutes and therefore it is very likely to be expired before the next time you need to instantiate an OAuthConnectionInfo. However, there are some rare cases that can benefit from preserving the access token in addition to the refresh token. One example that comes to mind is when the ZoomClient in your app is short-lived and gets instantiated frequently (generally speaking, this should be avoided because it can lead to socket exhaustion). In this particular scenario, preserving the access token allows you to avoid continuously requesting a new token.

theit8514 commented 11 months ago

All other scenarios require a refresh token because that's how you can refresh your access token when it expires after 60 minutes.

From my experience, refresh tokens are only used to extend a user interaction (such as authorization code flows in the browser where the user logs in and authorizes the app). For a client credentials, you already have all the necessary information (client id and secret) to generate a new access token, so a refresh token is not needed.

Again, unless I'm missing some scope, I do not get a RefreshToken in the OAuthConnectionInfo when it generates a token: image

Request body:

grant_type: "client_credentials"

Response:

{"access_token":"eyJz...","token_type":"bearer","expires_in":3600,"scope":"..."}

I propose adding an overload to the WithClientCredentials that will allow you to specify the refresh token and/or the access token and/or the OnTokenRefreshed handler.

This new overload will need to not check for null of the RefreshToken. That would be the only difference between WithRefreshToken and WithClientCredentials if you add both fields. WithRefreshToken may still be useful for authorization code flow where you would not want a client credentials flow to accidentally be used. As I understand it, Zoom is using the authorization code flow to tie to a specific accountId, which is why you would want a refresh token so that you can extend that initial authorization.

Jericho commented 11 months ago

Took me a while but I think I am finally understanding what you've been saying all along: the refresh token is returned only when an authorization code is converted into an access token. This scenario occurs when a user authorizes an OAuth app. This refresh token can be subsequently used to extend the lifetime of the access token when it expires (after 60 minutes). However, there is no refresh token involved when you simply use client credentials to get an access token.

I was a little incredulous when I saw the response from the Zoom API you shared in your previous comment so I inspected the response from one of my own API calls and, sure enough, the response I received matches what you posted:

image

So let me clear the confusion that I caused by not properly distinguishing the "user interactive" scenario where we get a refresh token from the "client credentials" scenario where no refresh token is involved:

I realize it took me a few days to come to the conclusion you had already reached several days ago. Sorry for the delay and thanks for being patient with me!

I'll get this done over the weekend and you should have a new release by Monday morning.

Jericho commented 11 months ago

FYI: published a beta here. Let me know if you get a chance to test it.

theit8514 commented 11 months ago

FYI: published a beta here. Let me know if you get a chance to test it.

I don't have access to the feed you posted, but I pulled the code down and everything looks good on this end.

Sorry for the delay and thanks for being patient with me!

Not a problem! I appreciate you taking the time to look at it.

Jericho commented 11 months ago

Thanks for confirming. Will publish the final release over the weekend.

For future reference, I wrote instructions on how to access beta packages: https://github.com/Jericho/ZoomNet/discussions/311

Jericho commented 11 months ago

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

The release is available on:

Your GitReleaseManager bot :package::rocket: