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

Admin route (soft) view access #96

Merged
merged 1 commit into from
Mar 26, 2024
Merged

Admin route (soft) view access #96

merged 1 commit into from
Mar 26, 2024

Conversation

carletex
Copy link
Contributor

Let's soft-protect the /admin route. Data is not secret anyway, but let's now allow /admin direct access.

As we discussed on #18 and #17, if we wanted full protection, we could do SIWE + JWT or signed reads. We might want to consider it in the future.

@carletex carletex requested a review from technophile-04 March 26, 2024 10:10
Copy link

vercel bot commented Mar 26, 2024

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

Name Status Preview Updated (UTC)
grants-bg ✅ Ready (Inspect) Visit Preview Mar 26, 2024 10:10am

Copy link
Member

@technophile-04 technophile-04 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice !! Also what if maybe add :

  useEffect(() => {
    if (!address) router.push("/");
  }, [address, router]);

PS: We need use useEffect, if we do if (!address) router.push("/"); directly at page level it gives red warning


Also, incase if we want to redirect user to homepage onError added #96 (comment), but yup even current solution makes sense !! So please feel free to merge it as it 🙌

@carletex
Copy link
Contributor Author

  useEffect(() => {
    if (!address) router.push("/");
  }, [address, router]);

Hey @technophile-04 , the problem with this is that address might be undefined while the hook is "connecting". It won't happen if you are already on the page and navigate to /admin, but it happens if you go directly to /admin... so you get redirected haha

image

I'll be interested to know how to fix this for other occasions (isConnecting doesn't seem to work)


Merging this for now... since I think we'll be changing it soon haha (Austin is more inclined to do a SIWE + validation). I'll open an issue.

@carletex carletex merged commit 97a3ede into main Mar 26, 2024
3 checks passed
@carletex carletex deleted the soft-protect-admin-route branch March 26, 2024 15:12
@technophile-04
Copy link
Member

but it happens if you go directly to /admin... so you get redirected haha

Ohhhhh yes!! my bad, missed it completely sorry :(

(isConnecting doesn't seem to work)

Yeah :(, I think since we are using our custom solution useAutoConnect hook for auto connecting I think because of this wagmi isn't aware about its status.

Umm not sure what's the best solution is maybe very hacky solution : exposing some kindof isLoading/isConnecting state from useAutoConnect hook

But yup this should be solved in Wagm v2 though 🙌

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