enBloc-org / kindly

GNU General Public License v3.0
12 stars 16 forks source link

#98/password recovery #268

Open OlenaReukova opened 1 week ago

OlenaReukova commented 1 week ago

Linked Issues:

Closes #98

Password Recovery Functionality

Description: This PR introduces password recovery functionality to the application. It includes a page where users can request a password reset link and a reset password page where they can set a new password. The password recovery process is integrated with Supabase for authentication management.

Changes:

Login Page:

Added Forgot Password link.

Forgot Password Page:

Created a new page at app/(auth)/login/forgot-password/page.tsx.

This page includes a form where users can enter their email address to request a password reset link. On form submission, the email address is sent to Supabase’s resetPasswordForEmail method. Displays success and error messages based on the outcome of the request.

Reset Password Page:

Created a new page at app/(auth)/login/reset-password/page.tsx.

This page includes a form where users can enter their new password. On form submission, the new password is updated using Supabase’s updateUser method. Displays success and error messages based on the outcome of the update. Supabase Client Utility:

Confirm Page:

Created a new page at app/(auth)/login/confirm/page.tsx. This page includes a message: Reset link has been sent to your email address.

Testing:

Manually tested the password recovery flow: Requested a password reset link. Received the reset link via email.

Test failed Clicked on the reset link and successfully set a new password. Verified error handling by entering invalid email addresses and invalid passwords.

Screenshots:

Login Page:

Added Forgot Password link.

link forgot password

Forgot Password Page:

forgot password page

Confirm Page:

confirm message

Reset link sent on email:

email link

Reset Password Page:

reset password page
netlify[bot] commented 1 week ago

Deploy request for cool-creponne-3e1272 pending review.

Visit the deploys page to approve it

Name Link
Latest commit 5abba0e5e5b10d060f1734b4f07e36b5471bb3e3
OlenaReukova commented 1 week ago

Hi There is an issue with the reset link testing on the localhost:3000.

Expected result: When the user clicks the reset link, he should be redirected to the local host at http://localhost:3000/login/reset-password.

Actual result: When the user clicks the reset link, he is redirected to the Netlify host at https://cool-creponne-3e1272.netlify.app/?code=...

This is probably an issue related to the Redirect URLs section in the Supabase authentication settings. I have tried hardcoding the URL to http://localhost:3000/login/reset-password, but the issue persists.

camelPhonso commented 1 week ago

Hey Olena,

Can you just confirm how you created this email template? - email template

I assume you edited this in the Supabase Studio URL right? It's just that when I try to test it I get an unstyled email instead on my InBucket url instead Screenshot 2024-06-21 at 14 36 30

But yours looks like it was opened directly from the email app, so I think we might be testing in different environments and that might be why when you press 'reset' you get sent to the 'Staging' preview

OlenaReukova commented 4 days ago

Hi @camelPhonso, I changed .env variables to local supabase. I tested reset link and it does not work as expected. Here's a breakdown of the problem:

Steps to Reproduce: Enter a valid email Click "Confirm Reset" on the Forgot Password page.

1 email input

Confirmation message on the screen

2 confirmation message

Open the Inbucket inbox and click the received reset password link.

inbucket reset link redirection home register page

Instead of being redirected to http://localhost:3000/login/reset-password, the link directs me to log in page http://127.0.0.1:3000/?code=89c2d864-808d-4c29-a9be-4dced754c03d.

Expected Behavior:

Clicking the reset link in the email should redirect the user to the http://localhost:3000/login/reset-password page where they can enter their new password.

mnixo commented 2 days ago

A couple of things I found on this one:

  1. Going directly to http://localhost:3000/login/reset-password is showing an Unhandled Runtime Error for me. If I use 'use server' instead of ('use server') here, I can see the page without issues.
  2. The redirectTo parameter is being set (here) based on the value of headers().get('origin') (here) which is showing up empty for me. My headers() object does not contain an entry for origin.
  3. I also found this discussion: Supabase will use the default URL (http://localhost:3000/) for redirects unless it is specifically configured to allow any other URL. Maybe you'll need to modify the Supabase config somehow to allow the redirect to http://localhost:3000/login/reset-password.
camelPhonso commented 2 days ago

Ok so this one is turning out all kinds of weird.

I came across pretty much the same as @mnixo and following the official documentation plus some use cases I spotted from other people I tried to confirm that the setup in the remote project is adequate. These are our redirect options: Screenshot 2024-06-27 at 09 12 29 So by all means the redirect should be working fine.

Then I tried going into the branch and hardcoding the redirect link as such:

    const { error } = await supabase.auth.resetPasswordForEmail(email, {
      redirectTo: `https://localhost:3000/login/reset-password`,
    });

The weird one is that I don't get an error or anything I just consistently get redirected to this link instead http://127.0.0.1:3000/?code=***

I've also tried:

The call for .resetPasswordForEmail() also seems fine 🤔 so I'm not really sure what's causing this at the moment.

My best hope at the moment is trying to spot where and why the link in our reset email is showing up different to what we sent in the function call, but so far I'm coming up empty 🙃

mnixo commented 2 days ago

A few other things:

These are our redirect options

Assuming that this is from a Supabase instance in Netlify, it shouldn't have impact in this scenario. Here we'd need to configure the local Supabase instance somehow, maybe through this config option. The entire debug loop should be happening locally. Only after the merge I would expect the deployed instance(s) config to be updated.

Then I tried going into the branch and hardcoding the redirect link as such

Keep in mind the http vs https differences in URLs. These will make a difference in these rules.

The weird one is that I don't get an error or anything I just consistently get redirected to this link instead http://127.0.0.1:3000/?code=***

This is the URL that gets resolved after clicking the email link, right? We might want to pay closer attention to the URL on the email link first (before the browser resolves it) as that's the one with the redirect_to=http://127.0.0.1:3000/ that needs to be fixed.

mnixo commented 1 day ago

With this set of changes I got it to redirect successfully. I guess you'd still need to make sure to get the origin from somewhere else.

https://github.com/enBloc-org/kindly/assets/4223240/c63ad881-e94a-4fe8-9dec-07b1f0a60e4c

camelPhonso commented 1 day ago

Assuming that this is from a Supabase instance in Netlify, it shouldn't have impact in this scenario. Here we'd need to configure the local Supabase instance somehow, maybe through this config option. The entire debug loop should be happening locally. Only after the merge I would expect the deployed instance(s) config to be updated.

Yes, really good catch! And searching a little about config.toml on the official docs I found this page which we should probably add a note about on our own docs.

@OlenaReukova once you adjust the config.toml file on your PR could you also add a small paragraph on BEFORE_YOUR_FIRST_ISSUE.md where we give directions on setting up supabase?