twilio / twilio-csharp

Twilio C#/.NET Helper Library for .NET6+.
MIT License
670 stars 301 forks source link

Clarify use of API keys #531

Open hultqvist opened 4 years ago

hultqvist commented 4 years ago

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)

https://github.com/twilio/twilio-csharp/blob/165072fbf2fb5f77086bdb6224e77d6e8c4b17d0/src/Twilio/Twilio.cs#L20-L42

childish-sambino commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

gagandeepp commented 3 years ago

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

hultqvist commented 3 years ago

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 commented 3 years ago

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.