-
Notifications
You must be signed in to change notification settings - Fork 128
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
Add resetPasswordRedirectTo prop to Auth component #223
base: main
Are you sure you want to change the base?
Add resetPasswordRedirectTo prop to Auth component #223
Conversation
This prop allows you to redirect users to a page where they can set their new password. Previously, it would behave as if the users just logged in.
This isn't addressing any issue as you've just added a new prop that does exactly the same as the existing |
Hi @silentworks, thanks for responding to my PR. I do use the
If I understand this correctly, some Supabase documentation suggest listening to And besides that, I would like the users to end up in the "set new password" page right away after following the link in the email they receive. This PR adds a new prop that only is used for ForgottenPassword view. You mentioned that this new prop "does exactly the same as the existing redirectTo prop". Could you please describe how to use redirectTo prop the same way? Please let me know if my understanding or implementation of this flow is wrong, and how to implement this correctly. If there's a public repo implementing a traditional reset password flow please let me know. |
I'm not sure why you're saying this isn't addressing any issue. Before this change, those Auth components can't be configured so that password recovery works correctly; after this change, they can. Even if those components are intended to be reference implementations, and people should not be using them directly as you state in #77, it would be nice if they were fully functional. Or perhaps they should be removed entirely? 🤷 |
@osener if you take a look at the repo in the issue I linked it has the password reset flow implemented in it. Also take a look at your code again, all you did was pass an alias to the |
I think I understand why your example works and my code doesn't. It looks like the key is the custom email template with the query param <a href="{{ .SiteURL }}/auth/confirm?token_hash={{ .TokenHash }}&type=recovery&next=/account/update-password">Reset Password</a> What I've been trying to do is to pass a different parameter, What you've shown is that to implement this flow correctly you have to do two things:
Neither the uselessness of the Auth component nor the requirement to supply custom email templates is at all obvious reading Supabase documentation, so I hope you understand the confusion. And shipping a broken Auth component that has all this internal links with no possibility to implement a working password reset flow is quite misleading as it does allow user to reset their password. |
The point we're trying to make is that the "mega Auth component" isn't fully functional. In the absence of other documentation, it is the documentation. And it's confusing to people (see #77 again for evidence of this) that it appears to Just Work -- it has a functional link that switches to the Forgot Password view -- but you can't actually configure that view correctly through the mega Auth component. So it just makes sense that the "mega Auth component" should either be removed or improved. This PR seems like a good step in that direction to me. After all, since you're saying we should be using separate views, then this "mega Auth component" is just example code. What's the downside of adding this change? Since it's just example code, there's no need to preserve "API cleanliness" or anything like that. The only other alternative I see is to add documentation that says, effectively, "Don't be misled by this mega Auth component! It doesn't fully work and you shouldn't use it." That seems... less straightforward... than just making it work. |
Anyway, thanks for all your patience and consideration while reading this feedback @silentworks 🙏 |
@@ -173,7 +174,7 @@ function Auth({ | |||
appearance={appearance} | |||
supabaseClient={supabaseClient} | |||
setAuthView={setAuthView} | |||
redirectTo={redirectTo} | |||
redirectTo={resetPasswordRedirectTo ?? redirectTo} |
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.
Shouldn't this be designed to specify the redirect URL based on the view
state of the Auth component?
redirectTo={
view === 'forgotten_password'
? resetPasswordRedirectTo ?? redirectTo
: redirectTo
}
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.
Ah, I now see that this is already under the case VIEWS.FORGOTTEN_PASSWORD:
condition, thus there's no need to check view
... Disregard.
This fits in with my experience as well. The default email has a link sending me to I'm fine with the reset password flow being pretty much akin to a magic link flow, but where I send the user to a reset form as soon as they're signed-in. However, I don't seem to have any contextual information available when handling the Alternatively, I could imagine a world where |
Reopening so that we can review this when available. |
I am also experiencing this issue. Can we just pass the type parameter that is sent to supabase back to the client so we can parse this out in /auth/callback and decide what to do? |
Hey team, An update on this thread - could y'all try updating the Supabase version like in this PR and see you can listen on the event? There was a fix while back which helps emit the As an additional aside we'll likely have someone from the frontend team look into Next.js 13+ forgot password flow. Thanks everyone for your patience with us. |
Hi @J0, thanks for revisiting this issue. I made sure I am on latest versions, I tried to follow the code in the linked PR and added some logs to GoTrueClient. In my case the changed function, In case you also don't have the reset password flow working, I hope the info above will help you debug the issue. If you do get it working and the problem is on my side somehow, I'd appreciate some example that shows how it is supposed to work. Thanks! |
@ozanmakes Thanks for trying it out, we'll get back to you shortly |
What kind of change does this PR introduce?
feature
What is the current behavior?
Previously, it would behave as if the users just logged in.
In my app
PASSWORD_RECOVERY
event doesn't fire after following the password reset link, at least locally. I want to make sure users end up in the right page to set their new password.You can find a relevant discussion in this issue: #77 (comment)
What is the new behavior?
This prop allows you to redirect users to a page where they can set their new password.