-
Notifications
You must be signed in to change notification settings - Fork 65
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
Adds Login functionality #23
Conversation
✅ Deploy Preview for kpop-stack ready! 🔨 Explore the source changes: 2a2da4f 🔍 Inspect the deploy log: https://app.netlify.com/sites/kpop-stack/deploys/6234dd0842bbf40008f1973d 😎 Browse the preview: https://deploy-preview-23--kpop-stack.netlify.app |
@@ -35,7 +35,7 @@ export const action: ActionFunction = async ({ request }) => { | |||
return createUserSession({ | |||
request, | |||
userId: user.id, | |||
remember: true, |
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 needed this for a hot second to test something but no longer necessary
@@ -9,7 +9,7 @@ export const sessionStorage = createCookieSessionStorage({ | |||
path: "/", | |||
sameSite: "lax", | |||
secrets: ["SUPERSECRET_SECRET"], | |||
secure: false, |
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.
This would be more appropriate for a production application so I needed to change this. I had to verify some testing before locally to see how this reacted
const hashedPassword = await bcrypt.hash(password, 10); | ||
|
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 mentioned this in the PR but Supabase hashes our pass word already when we surface through the supabase.auth
commands so I feel like we don't need to actually hash!
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.
this works great, woohoo!
Summary
We have a working login now 🙌🏾. This will allow users to send their credentials in and then redirect them to the stubbed out
/notes
page. This does not perform any sorts of visual errors so this is fully happy path code for now.Technical Detail
Very very similar to the #16 in terms of logic. The biggest things we did differently is instead make a request to Supabase so we can make sure that we successfully have a user and then we return the profile for that user.
Outline of flow:
/login
page. This will first run the loader function to determining if we have a user session already if not it will pass through with the rest of the rendering./notes
page__session
One thing I came across is that we shouldn't actually hash the passwords on our end because Supabase performs that work already for us with its auth client (see here). This will make the user code much nicer for folks.
side note: We will want to add form validations for #17 so like I said only happy path so far.
Manual Testing
**If you don't already have an account**
/join
, use a valid email and a 6 character password.To test sign in, (I have created a user with
[email protected]
andprince
):/login
and be prompted to log in with your user name and password/notes
page with a stubbed out notes HTML ofNotes scaffold
.Relevant Documentation and Issues