chore(update): Update passkeys-backend to use verify instead of comms #607
Conversation
| const createServiceURL = `${API_URL}`; | ||
|
|
||
| try { | ||
| const APIResponse = await axios.post(createServiceURL, data, { |
There was a problem hiding this comment.
Do we need to include documentation in the README to explain why and how to use this?
package-lock.json
Outdated
| "zod": "^3.24.4" | ||
| } | ||
| }, | ||
| "mcp-server/node_modules/@twilio-labs/serverless-runtime-types": { |
There was a problem hiding this comment.
Are these changes needed?
passkeys-backend/README.md
Outdated
| | `ACCOUNT_SID` | Find in the [console](https://www.twilio.com/console) | Yes | | ||
| | `AUTH_TOKEN` | Find in the [console](https://www.twilio.com/console) | Yes | | ||
| | `ANDROID_APP_KEYS` | The domain of the Android identity providers hash | No | | ||
| | `NAMESPACE` | UUID for generating deterministic UUIDs with the uuid library for username conversion | No | |
There was a problem hiding this comment.
If this is not required, why is it needed? Can the function manage it by itself?
There was a problem hiding this comment.
This should be required, change it right now
| @@ -0,0 +1,8 @@ | |||
| const origins = (context) => { | |||
| const { DOMAIN_NAME } = context; | |||
| return [`https://${DOMAIN_NAME}`, 'android:apk-key-hash:{base64_hash}']; | |||
There was a problem hiding this comment.
Is this for Android & Web? What about iOS?
There was a problem hiding this comment.
iOS only need the apple-app-site-association, it uses backend domain as its origin
| if (!keys || keys.trim() === '""') return []; | ||
| return keys.split(','); | ||
| }; | ||
| const uuidIdentity = v5(event.username, NAMESPACE); |
There was a problem hiding this comment.
Could it be possible to use something already included as library (to prevent adding a new library)? Could it be possible to use a hash of the value?
There was a problem hiding this comment.
The API specifies a UUID format, it reject the request if this specification is not fulfil
passkeys-backend/.env.example
Outdated
| # format: text | ||
| # required: false | ||
| ANDROID_APP_KEYS= | ||
| NAMESPACE= |
There was a problem hiding this comment.
What is the purpose of this value? What will be the format of the value? Could there be a default value?
There was a problem hiding this comment.
The purpose is to create a uuid based on the username, this is described in the readme, I think the UUID as is described is self explanatory, but yes we can add a default value
|
|
||
| exports.handler = async (context, event, callback) => { | ||
| const { DOMAIN_NAME, API_URL, ANDROID_APP_KEYS } = context; | ||
| const { API_URL, SERVICE_SID, NAMESPACE } = context; |
There was a problem hiding this comment.
Do you need the NAMESPACE?
| # format: list(text) | ||
| # description: [Optional] SID of the service created in Twilio verify | ||
| # format: sid | ||
| # required: false |
There was a problem hiding this comment.
I think if I set this to true the code exchange will take it as a mandatory when creating the function, but this is created with one of the endpoints
| }; | ||
|
|
||
| const challengeURL = `${API_URL}/Verifications`; | ||
| const challengeURL = `${API_URL}/${SERVICE_SID}/Passkeys/Challenges`; |
There was a problem hiding this comment.
do we have a sense of when these endpoints will be available in the Node Helper Library?
| const createServiceURL = `${API_URL}`; | ||
|
|
||
| try { | ||
| const APIResponse = await axios.post(createServiceURL, data, { |
There was a problem hiding this comment.
this should already be available in the Node Helper Library - I vote we add that as an import instead of using axios.
Description
Migration from using the comms API to Verify API
Checklist
npm testlocally and it passed without errors.Related issues