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

feat: Improve error message if an API Key is passed in place of an Auth Token #637

Closed
wants to merge 2 commits into from

Conversation

ducsuus
Copy link

@ducsuus ducsuus commented Dec 16, 2020

Fixes

Fixes #600

At present, if an auth token is provided instead of an API key the error 'accountSid must start with AC' is issued. This suggests that it is not possible to use API keys in place of auth tokens with twilio-node, when in fact API keys can be used but require an additional optional paramater to be used.

The present error messages encourages poor practice as it indiciates that API keys cannot be used in place of auth tokens.

This change implements a new error message that explains to developer the requirements in order to use API keys in the event that a API key SID is provided (whcih starts with "SK").

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

…tor in place of Auth Token

At present, if an auth token is provided instead of an API key the error 'accountSid must start with AC' is issued. This suggests that it is not possible to use API keys in place of auth tokens with twilio-node, when in fact API keys can be used but require an additional optional paramater to be used.

The present error messages encourages poor practice as it indiciates that API keys cannot be used in place of auth tokens.

This change implements a new error message that explains to developer the requirements in order to use API keys in the event that a API key SID is provided (whcih starts with "SK").
@@ -137,6 +137,11 @@ function Twilio(username, password, opts) {
}

if (!this.accountSid.startsWith('AC')) {

if (this.accountSid.startsWith('SK')) {
Copy link
Contributor

@thinkingserious thinkingserious Dec 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a test to verify this functionality?

We will be making this change internally, since this file is generated.

Copy link
Contributor

@thinkingserious thinkingserious left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR @ducsuus!

@eshanholtz
Copy link
Contributor

Closing until PR feedback is addressed.

@eshanholtz eshanholtz closed this Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong validation on accountSid when using API Sid/Api Key to initialize the client
3 participants