Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
bbarry opened this issue Oct 22, 2021 · 2 comments
Assignees

Comments

@bbarry
Copy link

bbarry commented Oct 22, 2021

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:

apiClient = new ApiClient(item.BaseUri + "/restapi");

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
Copy link
Contributor

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
Copy link

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.

@garg-mudit garg-mudit self-assigned this Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants