adding Multi-factor authentication for nodejs #19
Conversation
There was a problem hiding this comment.
Looks good! I added a few minor comments.
For consistency's sake, could you also:
- change the folder name from
multi-factor-authenticationtoMulti-Factor-Authentication - add an overview README.md inside the "PhoneNumbers" directory with a link to your example (see the one for Fax as an example: https://github.com/signalwire/guides/tree/main/Fax)
| @@ -0,0 +1,5 @@ | |||
| PROJECT_ID=<project-id> | |||
| API_TOKEN=<api-token> | |||
| SPACE_NAME=<space-name> | |||
There was a problem hiding this comment.
For consistency we usually use SPACE_URL instead of SPACE_NAME, could you update this?
Also for consistency, the SPACE_URL usually contains the full url, for example daniele.signalwire.com instead of just daniele.
There was a problem hiding this comment.
made changes to the SPACE_NAME to SPACE_URL
| This guide will use the Express Framework to create a simple web application that uses the SignalWire MFA API to authenticate a user via sms or voice call. | ||
|
|
There was a problem hiding this comment.
Is there already a guide in the developer portal?
If so, you could add a "📖 Read the full guide" link, see here for an example:
https://github.com/signalwire/guides/blob/main/Video/Simple%20Video%20Demo/README.md
There was a problem hiding this comment.
Added the pre-draft url from readme
| "@signalwire/js": "^3.10.2", | ||
| "@signalwire/node": "^2.4.0", |
There was a problem hiding this comment.
Are these packages used? Could they be removed?
There was a problem hiding this comment.
Removed unwanted packages in the package.json
This pull request is for a guide that shows how to use signalwire 2fa api in NodeJs
Currently the SMS feature isn't working perfectly but there is a ticket with the engineering team https://github.com/signalwire/cloud-support/issues/1925