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

docs: add a PKCE authentication with Supabase example #2302

Merged

Conversation

morlinbrot
Copy link
Contributor

Closes #1217

Adds an example on how to implement the PKCE authentication flow with Supabase.

I included a link to a repo of mine holding the code to a fully fleshed out example app as a more comprehensive point of reference, hope that is ok. In fact, I was thinking of deploying that app on Deno Deploy and add it to the Showcase section, lmk if that would be a good idea.

@morlinbrot morlinbrot force-pushed the pkce-with-supabase-example branch from 71740f9 to 7267bfa Compare February 8, 2024 17:53
Copy link
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

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

This is a great tutorial so far!

docs/latest/examples/authentication-with-supabase.md Outdated Show resolved Hide resolved
docs/latest/examples/authentication-with-supabase.md Outdated Show resolved Hide resolved
docs/latest/examples/authentication-with-supabase.md Outdated Show resolved Hide resolved
docs/latest/examples/authentication-with-supabase.md Outdated Show resolved Hide resolved
docs/latest/examples/authentication-with-supabase.md Outdated Show resolved Hide resolved
docs/latest/examples/authentication-with-supabase.md Outdated Show resolved Hide resolved
docs/latest/examples/authentication-with-supabase.md Outdated Show resolved Hide resolved
docs/latest/examples/authentication-with-supabase.md Outdated Show resolved Hide resolved
docs/latest/examples/authentication-with-supabase.md Outdated Show resolved Hide resolved
@morlinbrot
Copy link
Contributor Author

This is a great tutorial so far!

Thanks! Also for the prompt and thorough review. I'll be able to work on this tomorrow but before that, I have a couple of questions regarding the --env CLI option.

In my local testing, it does not seem to have any effect. In fact, I am suddenly able to run the app with neither using @std/dotenv nor the --env flag. I do have the -A option specified, which includes --allow-env, but that should not enable actual reading of .env files at runtime, right? What am I not seeing here?

In general, for educational content like this, should I include the most precise run command deno run -r --unstable --allow-env --allow-read --allow-write --allow-run --allow-net --watch=static/,routes/ dev.ts or is it fine to use the -A flag to make it shorter?

@iuioiua
Copy link
Contributor

iuioiua commented Feb 12, 2024

In my local testing, it does not seem to have any effect. In fact, I am suddenly able to run the app with neither using @std/dotenv nor the --env flag. I do have the -A option specified, which includes --allow-env, but that should not enable actual reading of .env files at runtime, right? What am I not seeing here?

You understand correctly. --allow-env allows environment variables to be read and written but doesn't read .env files. --env does that, though. If you're using neither load() from std/dotenv or --env, I'd expect your example to fail somewhere. Try applying this and see if the issue persists. Also, just making sure, are you indeed using a .env file or setting environment variables in the CLI?

In general, for educational content like this, should I include the most precise run command deno run -r --unstable --allow-env --allow-read --allow-write --allow-run --allow-net --watch=static/,routes/ dev.ts or is it fine to use the -A flag to make it shorter?

Using specific permission flags instead of -A, in this case, is better as it's more explicit to the reader as to what permissions are required for this workflow.

@morlinbrot morlinbrot requested a review from iuioiua February 12, 2024 14:34
Copy link
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

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

A couple more nits.

docs/latest/examples/authentication-with-supabase.md Outdated Show resolved Hide resolved
docs/latest/examples/authentication-with-supabase.md Outdated Show resolved Hide resolved
@iuioiua
Copy link
Contributor

iuioiua commented Feb 12, 2024

@thorwebdev, feel free to review if you'd like.

@morlinbrot morlinbrot force-pushed the pkce-with-supabase-example branch from dd7d6d8 to cd3f3c2 Compare February 13, 2024 11:33
@morlinbrot morlinbrot requested a review from iuioiua February 13, 2024 11:34
Copy link
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you.

@morlinbrot
Copy link
Contributor Author

Quick ping to see if this is still on the radar. Is there anything I can do to move this forward?

@iuioiua
Copy link
Contributor

iuioiua commented Mar 5, 2024

Just awaiting Marvin's review 🙂

@iuioiua
Copy link
Contributor

iuioiua commented Mar 16, 2024

PTAL @marvinhagemeister

Copy link
Collaborator

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

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

This is great, thanks for adding this 👍

@marvinhagemeister marvinhagemeister merged commit c9ec6bb into denoland:main Mar 27, 2024
7 checks passed
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.

proposal(docs): add an example of how to work with Supabase Auth
3 participants