-
Notifications
You must be signed in to change notification settings - Fork 474
Sync examples #937
Sync examples #937
Conversation
|
Just realised I left "outgoingApplicationSid" in. Have updated it, but unsure how to update PR to the latest commit |
|
Thanks for adding this. It definitely seems necessary and helpful to get all of the Grants examples in one place! I'm not super-familiar with Sync yet, but I notice that the JavaScript example that exists in docs right now has a lot of information about Specifically, it has these comments:
// This is the most critical part of your backend code, as you must identify the user and (possibly)
// challenge them with some authentication scheme. To determine the identity, you might use:
// * A session datum consistently identifying one anonymous visitor,
// * A session key identifying a logged-in user
// * OAuth credentials identifying a logged-in user
// * A random username for all comers.
//and we see it added to the token and sent in the response separate from the token itself: token.identity = identity;
// Serialize the token to a JWT string and include it in a JSON response
response.send({
identity: identity,
token: token.toJwt()
});@anabenites Do you have any insight into whether we need to include these comments in the SyncGrant examples for this page? Is there more detailed information about how to authenticate a user's identity, or is this just a "you can do this if necessary" kind of thing? Also, @anabenites the SyncGrant can also take an |
|
hi @bld010 I asked and the |
|
@anabenites "outgoingApplicationSid" was a mistake. If you look, I did an additional commit after the PR to remove it. My mistake |
|
@deshartman oh! thanks for the information, I didn't see it. A question why are you passing |
|
@anabenites I just realised, I checked in the wrong code. I had meant to delete all the voice related grants and only add the Sync ones. My mistake. Sorry about this. Have not done PRs before, so still learning. I have completely reworked the examples, and re-uploaded. Hope my other language proposals are ok? |
|
Hi @deshartman Thanks for the changes 👍 . I added a couple of suggestions, once these are done, I will approve the PR. I tested the script locally and it is working. Thanks! |
|
@anabenites Changes done and pushed. Good suggestions! |
anabenites
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the changes, it looks good to me!
Propose the below code snippet to add to the docs here: https://www.twilio.com/docs/iam/access-tokens#
I'll also submit the WagTail request once code has been merged