set up basic auth with username & password #13
Conversation
mrkarimoff
commented
Jun 5, 2024
•
mrkarimoff
commented
- Set up basic Auth with username & password
- Set up Shadcn UI and use the components to style the sign-in & sign-up pages
- Implement page protection that redirects to the sign-in page when unauthenticated
| <form action={formAction}> | ||
| {formState.error && ( | ||
| <div className="text-red-500 text-center text-sm"> | ||
| {formState.error} |
There was a problem hiding this comment.
Can we just display error.message here?
There was a problem hiding this comment.
This is not a javascript Error object, this is a custom error data being returned from the server action and its type is string, so I was showing it there, I followed this video to implement that way: https://youtu.be/1cBhgW2vtZE?si=Mlt80egIM4cXhxav
if you have seen JSON object as an error message that's because I was returning it as a string from the action. I fixed it and now formState.error is always an error message. Sorry if it was confusing to see objects rendered on the page as an error. I'm open to any suggestions if there is a better way to handle errors. I didn't spend much time on this as we are just prototyping so
Commit: 9f7eb59
| userId: text("user_id") | ||
| .notNull() | ||
| .references(() => user.id), | ||
| expiresAt: timestamp("expires_at", { |
There was a problem hiding this comment.
Do you think the other date fields (on protocols, etc) should also use timestamp?
There was a problem hiding this comment.
This is a complicated question - date doesn't store any time information. Most of the time we should use timestamp if there's any chance that time information will also be important. I don't think there are any scenarios where we only care about it down to the day level of resolution.
There was a problem hiding this comment.
I used timestamp because it was used in the Lucia docs.
| <form action={formAction}> | ||
| {formState.error && ( | ||
| <div className="text-red-500 text-center text-sm"> | ||
| {formState.error} |
There was a problem hiding this comment.
Same comment as SignUpForm - can this be error.message
There was a problem hiding this comment.
Answered it here: #13 (comment)
| const initialState = { error: null, success: false }; | ||
| const [formState, formAction] = useFormState(signup, initialState); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
I think a better way to handle redirect would be to:
- create a utility function to check for session
- Use it to redirect at the page level once there is a session
This is the approach we take in Fresco.
Also, the signup action logs the user in, so theoretically we would want this to redirect to /. There's no need for the user to log in again. Implementing in the suggested way above should take care of this though.
There was a problem hiding this comment.
Resolved with this b309354
Thank you for the suggestion. You're right it's much better that way
next.config.mjs
Outdated
| const nextConfig = {}; | ||
| const nextConfig = { | ||
| experimental: { | ||
| serverComponentsExternalPackages: ["@node-rs/argon2"], |
There was a problem hiding this comment.
As far as I know, this is included in nextjs automatically now: vercel/next.js#63850
Also, be aware of this: pilcrowonpaper/oslo#69
oslo is removing the password module, which is the reason for this dependency.
There was a problem hiding this comment.
Okay, saw the issue on vercel and removed the config: 70c82d3
There was a problem hiding this comment.
Added back the config because we don't have the latest changes from Next yet
…olve conflict with main
