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

Use server-side auth #71

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

troutowicz
Copy link
Contributor

@troutowicz troutowicz commented Feb 25, 2024

  • use server-side auth
  • remove supabase url and anon key login fields
  • create route /login
  • fix /login form
  • updated /login help link

With these changes, the supabase URL and anon key have been moved to env variables. Server-side auth opens up an easier path to refactoring supabase calls behind API routes.

Copy link

vercel bot commented Feb 25, 2024

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

Name Status Preview Comments Updated (UTC)
ad-deus ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 25, 2024 6:09pm

@troutowicz troutowicz changed the title Use server side auth Use server-side auth Feb 25, 2024
@adamcohenhillel
Copy link
Owner

hey @troutowicz ,

Appreciate the contribution a lot! but the client-side auth (+ client-side app entirely) and the user-configureable supabase URL and anon key was actually intentional, for the following reasons:

  1. Client-side: this is a web app, yes - but it is also a mobile app, converted using CapacitorJS. For capacitor to be able to run this Javascript as a native app as well, it needs to be all client-side.

  2. Supabase details: this is more of a "logic" decision. Because this is open-source-first project, where everyone can deploy their own Supabase backend, we wanted to let them use the frontend/client without running it themselves (i.e., going to https://adeus.ai and be able to use their own backend). This becomes more important with the native-mobile, where uploading a native mobile app to your phone is more complicated than just running a nextjs app. This way, people can just use the "official" deployed frontend on the web or on the app stores, with their own backend.

With that being said, I agree it is a bit messy and laggy. I am more than happy to see how we can improve it (I think @Jacksonmills was trying to make some edits to the hooks, etc) - but just bear in mind the constraints

What do you think?

@Jacksonmills
Copy link
Collaborator

Jacksonmills commented Feb 26, 2024

I talk about this a bit in this issue #19, but yeah as I have also learned about capacitorJS we need the entire app to be client only https://nextjs.org/docs/pages/building-your-application/deploying/static-exports#unsupported-features

@troutowicz would love your input on the issue I mentioned, I had some ideas about creating the client first then logging in, maybe we can refactor to make the client work a bit cleaner, would love your input/help if you'd like to

@troutowicz
Copy link
Contributor Author

troutowicz commented Feb 26, 2024

Hmm, these are indeed strict constraints. Fair. There will be another half of users that want something owned by them, with a backend, though. Perhaps there should be a couple offerings for the application.

From my perspective, I would never use the mobile apps. The web app is simple enough that I'd access through my deployed web app instance for mobile and desktop.

@troutowicz
Copy link
Contributor Author

@Jacksonmills regarding #19, this is an easy fix tweaking some local state. Will open a PR for you.

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.

None yet

3 participants