dropbox-sdk-dotnet icon indicating copy to clipboard operation
dropbox-sdk-dotnet copied to clipboard

Should a revoked refresh token result in an AuthException?

Open jonsagara opened this issue 3 years ago • 5 comments

Describe the bug

In testing a revoked refresh token used when calling Files.ListFolderAsync, the Dropbox SDK threw a generic HttpRequestException with status 400 Bad Request. There is no indication of what failed or why.

Here is the sample code that I ran in LINQPad:

var dropbox = new DropboxClient(
    oauth2RefreshToken: "my revoked refresh token",
    appKey: "my app key",
    appSecret: "my app secret"
    );

var listFolderResult = await dropbox.Files.ListFolderAsync("");

Here is the stack trace of the HttpRequestException from the request to https://api.dropbox.com/oauth2/token:

   at System.Net.Http.HttpResponseMessage.EnsureSuccessStatusCode()
   at Dropbox.Api.DropboxRequestHandler.RefreshAccessToken(String[] scopeList)
   at Dropbox.Api.DropboxRequestHandler.CheckAndRefreshAccessToken()
   at Dropbox.Api.DropboxRequestHandler.RequestJsonStringWithRetry(String host, String routeName, String auth, RouteStyle routeStyle, String requestArg, Stream body)
   at Dropbox.Api.DropboxRequestHandler.Dropbox.Api.Stone.ITransport.SendRpcRequestAsync[TRequest,TResponse,TError](TRequest request, String host, String route, String auth, IEncoder`1 requestEncoder, IDecoder`1 responseDecoder, IDecoder`1 errorDecoder)
   at UserQuery.Main(), line 10

Here is the raw response returned by Fiddler:

{
    "error_description": "refresh token is invalid or revoked",
    "error": "invalid_grant"
}

If I make a similar request using an old-style long-lived access token, I get an AuthException whose Message is invalid_access_token/.... This I can use to alert the user that my app can no longer communicate with Dropbox on their behalf.

Here is the sample code:

var dropbox = new DropboxClient(oauth2Token: "my revoked access token");

var listFolderResult = await dropbox.Files.ListFolderAsync("");

Here is the stack trace of the AuthException from the request to https://api.dropboxapi.com/2/files/list_folder:

   at Dropbox.Api.DropboxRequestHandler.RequestJsonString(String host, String routeName, String auth, RouteStyle routeStyle, String requestArg, Stream body)
   at Dropbox.Api.DropboxRequestHandler.RequestJsonStringWithRetry(String host, String routeName, String auth, RouteStyle routeStyle, String requestArg, Stream body)
   at Dropbox.Api.DropboxRequestHandler.RequestJsonStringWithRetry(String host, String routeName, String auth, RouteStyle routeStyle, String requestArg, Stream body)
   at Dropbox.Api.DropboxRequestHandler.Dropbox.Api.Stone.ITransport.SendRpcRequestAsync[TRequest,TResponse,TError](TRequest request, String host, String route, String auth, IEncoder`1 requestEncoder, IDecoder`1 responseDecoder, IDecoder`1 errorDecoder)
   at UserQuery.Main(), line 10

Here is the raw response returned by Fiddler:

{
    "error_summary": "invalid_access_token/...",
    "error": {
        ".tag": "invalid_access_token"
    }
}

To Reproduce

  1. Obtain a short-lived refresh token by having the user authorize the application via OAuth.
  2. Have the user disconnect the app from Connected Apps in their Dropbox.com settings.
  3. Use the refresh token as described in the code snippet above to try to list folders.

Expected Behavior

When trying to use a revoked refresh token, I expect the SDK to throw an AuthException telling me that the refresh token is invalid or has been revoked.

Actual Behavior

The SDK throws a generic HttpRequestException with no details as to what caused the failure.

I believe the issue is in DropboxRequestHandler.cs in the RefreshAccessToken method:

https://github.com/dropbox/dropbox-sdk-dotnet/blob/dbd3a37bfb2e33411314cbd168fd0ef370d168b5/dropbox-sdk-dotnet/Dropbox.Api/DropboxRequestHandler.cs#L275-L300

At line 279, it handles an Unauthorized response, but it doesn't handle the 400 Bad Request returned by the API. The subsequent call to response.EnsureSuccessStatusCode(); on line 288 causes the generic HttpRequestException to be thrown.

Would it be possible to add error handling before line 288 to throw an AuthException if it detects an invalid or revoked refresh token?

Versions

  • What version of the SDK are you using? Dropbox.Api 6.4.0
  • What version of the language are you using? C# 9.0
  • What platform are you using? (if applicable)
    • ASP.NET Core 5.0
    • .NET SDK 5.0.202
    • Windows 10 / whatever version of Windows Azure App Service uses

Thank you,

Jon

jonsagara avatar May 04 '21 21:05 jonsagara

Thanks for the detailed write up! I'm raising this with the team to see if we can fix up this behavior.

greg-db avatar May 05 '21 15:05 greg-db

Another vote for this. Without being able to inform users that the link is broken, you can end up in a scenario where things like auto backups and/or syncing silently stop working, potentially resulting in data loss for the user.

bellissimo avatar May 17 '21 09:05 bellissimo

@bellissimo Thanks for the feedback!

greg-db avatar May 17 '21 13:05 greg-db

Hi all, has there been further discussion on how to proceed with this?

jonsagara avatar Jun 01 '21 21:06 jonsagara

@jonsagara This is still open with engineering, but I don't have an update on it yet. I'll follow up here once I do.

greg-db avatar Jun 02 '21 17:06 greg-db