Skip to content
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

[WIP] Exploring SIWE + JWT #17

Closed
wants to merge 7 commits into from
Closed

[WIP] Exploring SIWE + JWT #17

wants to merge 7 commits into from

Conversation

carletex
Copy link
Contributor

@carletex carletex commented Feb 15, 2024

[WIP]

Exploring the option of SIWE + JWT for #13. There are a bunch of things to fix / add... but this is meant to be a POC.

  1. Created an Admin page
    image

image

  1. If you SIWE + are and admin => sent a request to a "protected" route to get all the grants (just to test that it works)
    image

I have a bunch of open questions:

  • If we do this... do we want to do this only for admins, or for everyone?
    • I kind of like people signing messages to submit grants. But for admins it's smoother to do it with JWT because they only sign once. But they are also used to sign everything in APP BG.
    • It might have too much overhead having both?
    • If we end NOT using SIWE +JWT... we could also protect the /admin route (making sign a message to get all the data). But Again, this data is not super secret. The important thing is to protect "writes". I could do a test with this in another PR next week.
  • API route vs Server actions: still not clear to me what/how should we use haha. Could we do everything with just server actions? For this POC I did it with API routes (more experienced with it)... but it might make more sense to do it one way (if possible). I see that server actions are not only for forms and also support auth?? (https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions-and-mutations#authentication-and-authorization)

Copy link

vercel bot commented Feb 15, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
grants-bg ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 16, 2024 10:13am

@carletex
Copy link
Contributor Author

@technophile-04

will love to hear your take on this (updated the OP comment with some questions). We can also have a call next week.

@technophile-04
Copy link
Member

Tysm Carlos, working great !

If we do this... do we want to do this only for admins, or for everyone?
...

I am kind of 49% on SIWE + JWT and 51% on letting admin every time he do write operation.
Reason for this is "The important thing is to protect "writes"". and also because of its simplicity and not handling the whole auth stuff and security issues it comes with

JWT + SIWE Admin signs for every write to DB
Advantages Improves UX for admin Unbreakable security (lol until admin's wallet is hacked)
Disadvantages If admin's JWT is leaked from localstorage, hacker can do random updates to user grants Bad UX for admin

we could also protect the /admin route (making sign a message to get all the data)

Yeah,Intially I was thinking this might be a bad option since whever admin refreshes the page he would need to sign again, But I think we could solve this by telling Admin to sign message(with expiryTimestamp) and store it inside localstorage and always send it while fetching getAllGrants similar to jwt. So we could use this method just to gurad read for admin.

But yup I am all in if we go with JWT + SIWE too !! Also we could add it to whole app and improve stuff like #8 (comment), also gurad /myGrants, ..... we could also even explore SIWE library + Next Auth since they seems to handle nonce etc too

API route vs Server actions: still not clear to me what/how should we use haha. Could we do everything with just server actions? For this POC I did it with API routes (more experienced with it)... but it might make more sense to do it one way (if possible). I see that server actions are not only for forms and also support auth??

Yeah we can do any write operation with it .... but doing some research there is as such no such good advantage of using server actions besides a bit improved DX(of not creating different routes files and collocation). So I think maybe we should stick to api routes for everything and we are also we versed with it and any which case we need to create some GET routes. Will create a PR to update submit form proposal to api routes 🙌

@carletex
Copy link
Contributor Author

Thanks @technophile-04 !! Super helpful.

Let me draft a PR next week with "Admin signs for every write to DB". I like the idea of signing some "admin reads" to and the timestamp.

@carletex carletex mentioned this pull request Feb 18, 2024
@carletex
Copy link
Contributor Author

Closing this in favor of #18

@carletex carletex closed this Feb 19, 2024
@carletex carletex mentioned this pull request Feb 19, 2024
@carletex carletex deleted the 13#auth-siwe-jwt branch March 6, 2024 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants