-
Notifications
You must be signed in to change notification settings - Fork 59
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
Handle API Key and API Secret properly #34
Comments
I prefer if we encourage teas to use API Key and Secret. That's a better/safer way to be using credentials anyway since you can rotate them easily, and if you leak an API Key, you can just revoke it. Why are you saying if you have APIKey/Secret things might not work normally? I would say the reverse, if they are using accountSid/authToken, we should warn them to use apiKey/Secret! |
I understand that we want to encourage use of API Key and Secret. But for Functions we also need:
I'd be happy to add an optional prompt to ask for an API Key and Secret when initialising a project with Will new Functions continue to have |
Like @philnash said, some APIs require an Account SID in the API path. The Twilio CLI has that aspect so we could bridge that functionality.
|
This has been solved for now by using |
Right now when a project is set-up using the Twilio CLI we are actually setting the API Key and API Secret that the Twilio CLI is using as ACCOUNT_SID and AUTH_TOKEN in the
.env
file. That works for deployment and interacting with the Twilio Serverless API because it doesn't require the knowledge of the Account SID.However, it causes two problems.
context.getTwilioClient()
to get a Twilio client this is going to fail for some APIs that require the knowledge of the Account SID (SMS, Calls etc.)protected
webhooks because we need the auth token for that which we don't have in that situation.Why do we use the Auth Token in the first place?
In the legacy Functions environment we have
ACCOUNT_SID
andAUTH_TOKEN
available on thecontext
object. This enabled the use ofcontext.getTwilioClient()
but people could also access these variables directly. Therefore we are setting them upon project creation in your.env
file.By default these are also read for other interactions with the Serverless API for consistency. For deploy and other commands we introduced two flags
--auth-token
and--account-sid
to override the ones that are read from the.env
file.When the Serverless Plugin was built, this introduced another complexity. The logic that is being used right now is the following:
serverless:init
is called we'll passusername
andpassword
of the Twilio Client in the CLI tocreate-twilio-function
. This will write them asACCOUNT_SID
andAUTH_TOKEN
into.env
.serverless:start
bothACCOUNT_SID
andAUTH_TOKEN
are being read and made available oncontext
argument inside Function executionsi) if a specific project has been passed with
-p
or--project
we will use thatusername
andpassword
to interact with the API.ii) if the user passed in
--account-sid
and--auth-token
we'll use those credentialsiii) if the user as
ACCOUNT_SID
andAUTH_TOKEN
in the.env
file we'll use thoseiv) otherwise we'll fallback to the default credentials of the Twilio CLI that are under
username
andpassword
.Right now my train of thought here is:
--account-sid
and--auth-token
for API-related commands to--username
and--password
for consistency and deprecate the other onesACCOUNT_SID
andAUTH_TOKEN
and set them tousername
andpassword
essentially. Meaning these could be API Key and API Secretstart
command thatclient.getTwilioClient()
andprotected
routes might not operate as expected and that if they want to make this more realistic, they have to change to Account SID and Auth Token.Alternatively we can fix
client.getTwilioClient()
by detecting increate-twilio-function
already that we are setting an API Secret not an Auth Token and instead setACCOUNT_SID
to the actual account SID and introduceTWILIO_API_KEY
andTWILIO_API_SECRET
instead and use that in creating thegetTwilioClient()
locally but then we will have the issue thatcontext.AUTH_TOKEN
will be available in the deployment but not locally. And since that's a sensitive piece of information, I wouldn't want to surprise the user that it exists.Would love to have some input from you folks on how to solve this problem.
@philnash @dprothero
The text was updated successfully, but these errors were encountered: