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

Clarify use of API keys #531

Open
hultqvist opened this issue Jul 31, 2020 · 6 comments
Open

Clarify use of API keys #531

hultqvist opened this issue Jul 31, 2020 · 6 comments
Assignees
Labels
status: work in progress Twilio or the community is in the process of implementing type: community enhancement feature request not on Twilio's roadmap

Comments

@hultqvist
Copy link

The Twilio console for API keys states:

You can use API Keys to authenticate to the REST API using basic auth, with user=KeySid and password=KeySecret.

This made me think I could use code like this:

//public static void Init(string username, string password);
TwilioClient.Init(keySID, keySecret);

But that didn't work.
This one did:

// Summary:
//     Initialize base client with separate account SID
//public static void Init(string username, string password, string accountSid);
TwilioClient.Init(keySID, keySecret, accountSID);

Have I understood it correctly in that the first Init only works with Account SID and the other Init is intended for API keys?

If so I think you should update the API signature to reflect this.

In the first Init, rename the parameters to accountSID and authToken.

In the second Init, rename the parameters to keySID, keySecret and remove the comment in the summary about "separate account SID" since that made me think you can use one account credential to init against another account(which doesn't make sense)

/// <summary>
/// Initialize base client with username and password
/// </summary>
/// <param name="username">Auth username</param>
/// <param name="password">Auth password</param>
public static void Init(string username, string password)
{
SetUsername(username);
SetPassword(password);
}
/// <summary>
/// Initialize base client with separate account SID
/// </summary>
/// <param name="username">Auth username</param>
/// <param name="password">Auth password</param>
/// <param name="accountSid">Account SID to use</param>
public static void Init(string username, string password, string accountSid)
{
SetUsername(username);
SetPassword(password);
SetAccountSid(accountSid);
}

@childish-sambino
Copy link
Contributor

This is a larger Twilio issue regarding auth/identity. Let me explain.

Twilio.Rest.Api.* resources require an Account SID; it's part of the URL path. Other Twilio.Rest.* resources do not.

So if you're using accountSid-authToken creds, you can access all the resources. If you're using apiKeySid-keySecret creds, you can access non-Twilio.Rest.Api,* resources. But, you'll also need to provide the accountSid if you're to access Twilio.Rest.Api.* resources.

I'm not sure what's the best way to update the docs, unless just sticking that explanation in the class docs would suffice.

@hultqvist
Copy link
Author

hultqvist commented Aug 10, 2020

That explains the problem.

Would it be of interest to have the library track whether one is using Account or Key tokens?

When calling Twilio.Rest.Api the library could warn if it's missing credentials rather than trying to use the KeySID as account key and get a 404 response from the server.
Example with TokenResource

I think you have a specific prefixes for account and key SID so that would be detectable even without modifying the Init methods.
https://www.twilio.com/docs/glossary/what-is-a-sid

This is my suggestion, if you think it's valuable I can create a PR for it.

  1. In TwilioClient.Init only set the AccountSid property if the username parameter has the "AC" prefix.
  2. Throw an exception when the AccountSid is used but not set.

@eshanholtz
Copy link
Contributor

You are welcome to submit a PR with the proposed improvements and we will add it to our backlog for review.

@eshanholtz eshanholtz added type: community enhancement feature request not on Twilio's roadmap and removed type: getting started question while getting started labels Aug 10, 2020
@childish-sambino childish-sambino added status: work in progress Twilio or the community is in the process of implementing and removed status: waiting for feedback waiting for feedback from the submitter labels Aug 13, 2020
@gagandeepp
Copy link

I am interested to contribute on this,please assign this issue to me

@hultqvist
Copy link
Author

Hi @gagandeepp ,
I've already submitted a pull request, see above, though they would prefer some tests to validate the changes.
I haven't had time to look into that yet so feel free to help me there.

@AshleyAsh90
Copy link

This is a larger Twilio issue regarding auth/identity. Let me explain.

Twilio.Rest.Api.* resources require an Account SID; it's part of the URL path. Other Twilio.Rest.* resources do not.

So if you're using accountSid-authToken creds, you can access all the resources. If you're using apiKeySid-keySecret creds, you can access non-Twilio.Rest.Api,* resources. But, you'll also need to provide the accountSid if you're to access Twilio.Rest.Api.* resources.

I'm not sure what's the best way to update the docs, unless just sticking that explanation in the class docs would suffice.

Most of the docs only mention that we use the API Keys as username and password (see - https://www.twilio.com/docs/usage/requests-to-twilio#api-keys) This is misleading especially when requests respond with an error code of 90002 with status 400. Could we also please improve on the error code supplied?
This is what I see as an exception messgae when I supply the ApiKey as userName and APISecret as password

Twilio: Error occurred in ApiCall. Message: AccountSid yyyyyyyyy is invalid Twilio Error: 90002 -https://www.twilio.com/docs/errors/90002

And error 90002 is for your product Flex and doesn't seem to make sense for this scenario.

Could we improve this part of twilio doc if that seems like a reasonable suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: work in progress Twilio or the community is in the process of implementing type: community enhancement feature request not on Twilio's roadmap
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants