docusign / docusign-esign-csharp-client

The Official Docusign C# Client Library used to interact with the eSignature REST API. Send, sign, and approve documents using this client.
https://developers.docusign.com/docs/esign-rest-api/sdks/csharp/
MIT License
129 stars 159 forks source link

JWT authentication is frustratingly difficult to get right due to lack of documentation knowledge and poor class design #353

Open bbarry opened 2 years ago

bbarry commented 2 years ago

I have a method which works in demo mode:


private async Task<(string AccountId, ApiClient ApiClient)> GetAccountIdAndSetApiClientUsingJwt(bool isDemoMode, CancellationToken token)
{
    // TODO: cache authToken for timespan related to authToken.expires_in?
    // how to determine baseurl for that token?
    try
    {
        var apiClient = isDemoMode ? new ApiClient(DocusignApiUrl) : new ApiClient();
        var scopes = new List<string>
        {
            "signature",
            "impersonation",
        };
        var (clientId, impersonatedUserId, key) = await EnvironmentConfiguration.GetDocusignCredentialInfo(token);
        var url = isDemoMode ? DocusignAuthServer : DocusignAuthServerProd;
        apiClient.SetOAuthBasePath(url);

        // TODO: use async call instead when available? https://github.com/docusign/docusign-esign-csharp-client/issues/294
        var authToken = apiClient.RequestJWTUserToken(clientId, impersonatedUserId, url, key, 1, scopes);

        // TODO: use async call instead when available? https://github.com/docusign/docusign-esign-csharp-client/issues/294
        var userInfo = apiClient.GetUserInfo(authToken.access_token);
        var acct = userInfo.Accounts.FirstOrDefault(); // why? what do I do if there is more than one?
        if (acct == null)
        {
            throw new Exception("The user does not have access to account");
        }

        apiClient.Configuration.AccessToken = authToken.access_token;
        return (acct.AccountId, apiClient);
    }
    catch (Exception e)
    {
        throw new DetailedServerException(StatusCodes.Status503ServiceUnavailable, "Service Unavailable: Docusign", e);
    }
}

This code contains a defect that is nontrivial to find: https://github.com/docusign/docusign-esign-csharp-client/blob/ae6ffc00c8a38ba2d67ad012e2f87d7769eda27a/test/Recipes/CoreRecipes.cs#L612

It might be able to be in 2 ways:

instead of:

    apiClient.Configuration.AccessToken = authToken.access_token;
    return (acct.AccountId, apiClient);

use:

    apiClient = new ApiClient(item.BaseUri + "/restapi");
    apiClient.Configuration.AccessToken = authToken.access_token;
    return (acct.AccountId, apiClient);

or (maybe? looks like it should work reading the source I think):

    apiClient.SetBasePath(item.BaseUri + "/restapi");
    apiClient.Configuration.AccessToken = authToken.access_token;
    return (acct.AccountId, apiClient);

but in any case:

  1. it is unclear why we need to do this; eventually a user might stumble upon https://developers.docusign.com/docs/esign-rest-api/sdk-tools/csharp/auth/ but until then all bets are off
  2. demo mode works when it is clearly broken still
  3. docusign call support suggests strange things like setting the base path on the initial ApiClient contstructor, but you have to use "na1.docusign.net" because the account call only works there...
  4. the magic string "/restapi" means nothing to any of us

It shouldn't be this hard to get a configured ApiClient that is ready to use.

To fix it I am requesting an ApiAuthenticationClient which would exist to produce authenticated api client instances:

public class ApiAuthenticationClient {
    public ApiAuthenticationClient() { ... }
    public ApiAuthenticationClient(bool demoMode) { ... }
    public OAuth.OAuthToken RequestJWTUserToken(...) {...}
    public Task<OAuth.OAuthToken> RequestJWTUserTokenAsync(...) {...}
    public OAuth.UserInfo GetUserInfo(...) {...}
    public Task<OAuth.UserInfo> GetUserInfoAsync(...) {...}
    public ApiClient CreateApiClient(OAuthToken accessToken, OAuth.UserInfo.Account account)
}

These methods on the existing ApiClient should be deprecated and eventually removed and documentation would be updated (and interlinked better between this repository and developers.docusign.com). Along with some information about how we might want to cache these details so that we don't make these requests every time we use ApiClient (surely there is some rule at docusign regarding how often an account's account id or BaseUrl changes and we can probably cache the token for a bit also).

HobbyProjects commented 2 years ago

Thank you for your feedback. We will soon be restructuring the JWT auth helper methods and we will alleviate the issues that you're seeing above long with them.

Again, thank you for providing these suggestions.

luke-atp commented 2 years ago

Has there been any progress on improving the authentication in the C# SDK?

I have been having similar issues setting of a proof-of-concept for a client. I have been able to get the QuickStart application to work, but the use-cases don't cover my scenario.

I would like to utilize a process that does not involve user consent during the process. The current documentation says I should utilize the "JSON Web Token (JWT) Grant authentication" method. This however requires that the application "impersonate" a user. Granting consent is overly complicated, though. I have to setup an SSO domain, which really limits my ability to do a simple proof-of-concept.

I might suggest a Client Credentials Flow. This follows the standards better. This would allow for the SDK to take advantage of IdentityModel project.

The advantage here, is that the developer no longer needs to keep track of the JWT or how to refresh it. This could be configured in the application Startup.

It might look something like:

// Startup.cs
...
services.AddAccessTokenManagement(options =>
{
  options.Client.Clients.Add("docusignclient", new ClientCredentialsTokenRequest
  {
    Address = "https://...",
    ClientId = "clientId",
    ClientSecret = "secret",
    Scope = "signature" // all scopes required
  });
});
...
services.AddHttpClient<ISignatureClient, SignatureClient>(client =>
{
    client.BaseAddress = new Uri(baseUrl);
})
.AddClientAccessTokenHandler("docusignclient");

The SignatureClient could then be injected in the application where needed. The code is no longer complicated by token management. I would assume the process of creating an ApiClient to get the JWT token, then creating another ApiClient to create the EnvelopesApi could be simplified to having separate HttpClients that expose the different API functionality. Any other configuration that is required by the clients could also be configured in Startup (e.g., ReturnUrl, AuthenticationMethod, etc.).

Finally, I would like to suggest that the other helper classes be updated to use more string typed definitions. Currently all of the properties are strings. It would be nice if properties such as Status or AuthenticationMethod were Enums. For tabs it would be nice if x and y coordinates were ints. etc.