-
Notifications
You must be signed in to change notification settings - Fork 10
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
write auth for farcaster and lens #1237
Conversation
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.
Good job figuring this stuff out, @benny-conn! Looks great to me. 🥇
autosocial/handler.go
Outdated
return | ||
} | ||
|
||
logger.For(c).Infof("checking farcaster approval for %s", in.SignerUUID) |
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.
Is SignerUUID
sensitive enough that we shouldn't log it?
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.
Yes!
# farcaster only supports ETH addresses currently so no need to specify a chain | ||
} | ||
|
||
input LensAuth { | ||
address: Address! | ||
signature: String @scrub |
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.
It might be helpful to document the withSigner
and signature
parameters so callers know what happens if they provide them. You can use """
to add documentation comments to the schema """like this"""
.
service/farcaster/neynar.go
Outdated
appFid := new(big.Int) | ||
appFid.SetString(appFidStr, 10) | ||
|
||
deadline := big.NewInt(time.Now().Unix() + 86400) |
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.
I think 86400
could benefit from being a constant. Personally, I like defining time constants in a way that makes the duration and units obvious, like:
const expirationSeconds = 60 * 60 * 24 // 24 hours
service/farcaster/neynar.go
Outdated
return nil, err | ||
} | ||
|
||
logger.For(ctx).Warnf("account %s", account.Address.Hex()) |
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.
Do we still need this?
defer tracing.FinishSpan(span) | ||
queue := env.GetString("AUTOSOCIAL_POLL_QUEUE") | ||
url := fmt.Sprintf("%s/checkFarcasterApproval?signer_uuid=%s&user_id=%s", env.GetString("AUTOSOCIAL_URL"), message.SignerUUID, message.UserID) | ||
return submitTask(ctx, client, queue, url, withTrace(span)) |
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.
What do you think the retry on this will look like? Seems like we'll want to try a bunch early on so we can be quick to update their account if they do the on-chain thing, but we probably want to back off significantly if they don't do the transaction right away.
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.
Sounds good to me. I imagine this will all be in the span of about 10 minutes as well.
Changes:
Farcaster is by far the more complex flow, here is how this PR works to accomplish authenticated farcaster connections:
withSigner
in the graphQL request that will have Neynar generate a "signer". A signer is useless until approved by the user later in this process but we don't want to expose thesigner_uuid
which is sort of like the "access token" in farcaster.FarcasterSocialAccount
type as well as the status of their approval. The frontend has to give this URL to the user.autosocial
service that will continuously check if the user approved the token. Once they do, it will update their status to approved.The last step is to match the "deadline" of the signature to whatever the cloud task total retry time is so that we don't run into a case where a user successfully approves of a token and we didn't poll for it. If this does happen, this isn't the end of the world, but might lead to us telling the user "you haven't approved of gallery to act on your behalf" when they in fact have. We could also remove the polling altogether but then the frontend would have no way of showing the user that they successfully connected.