Open ozanmakes opened 1 year ago
This isn't addressing any issue as you've just added a new prop that does exactly the same as the existing redirectTo
prop, it's even making use of the redirectTo
prop internally. I have outlined in the issue you mentioned about how these components work https://github.com/supabase/auth-ui/issues/77#issuecomment-1632239423.
Hi @silentworks, thanks for responding to my PR. I do use the view
prop but I'm having trouble implementing a good flow for reset password. The problem I have is, <Auth />
component only takes one redirectTo param and doesn't have a callback to update this when the view changes.
/register
or /login
page, and I render either <Auth view="sign_in" redirectTo="www.example.com/dashboard" />
or <Auth view="sign_up" redirectTo="www.example.com/dashboard" />
. redirectTo
is set to www.example.com/dashboard
to redirect the user to the dashboard after logging in.redirectTo="www.example.com/dashboard"
link. www.example.com/dashboard
instead of "set new password" page which is very confusing.If I understand this correctly, some Supabase documentation suggest listening to PASSWORD_RECOVERY
event so that the /dashboard
page the user gets redirected to then can send the user to the "set new password" page. But in my app this event doesn't seem to be getting fired.
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.
This isn't addressing any issue as you've just added a new prop that does exactly the same as the existing
redirectTo
prop, it's even making use of theredirectTo
prop internally. I have outlined in the issue you mentioned about how these components work #77 (comment).
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 redirectTo
. https://github.com/supabase/auth-ui/pull/223/files#diff-112c28ac025a8ffe2a93903f96d912b0f922cbf226dc9637dc32323939cec8d4R177 As I've stated you should be using these as separate view
's rather than using the mega <Auth />
component.
@levity the components are fully functional, documentation might be what's missing the most. I linked to a repo in my comment on issue #77 which shows how you should be handling the password reset flow.
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 &next=/account/update-password
:
<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, resetPasswordRedirectTo
, so that when the user uses the internal links of the Auth component to go to Forgotten Password view, they end up with the right redirectTo
parameter.
What you've shown is that to implement this flow correctly you have to do two things:
showLinks={false}
and render all the links yourself.redirectTo
parameter.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.
@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
redirectTo
. https://github.com/supabase/auth-ui/pull/223/files#diff-112c28ac025a8ffe2a93903f96d912b0f922cbf226dc9637dc32323939cec8d4R177 As I've stated you should be using these as separateview
's rather than using the mega<Auth />
component.
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 🙏
If I understand this correctly, some Supabase documentation suggest listening to
PASSWORD_RECOVERY
event so that the /dashboard page the user gets redirected to then can send the user to the "set new password" page. But in my app this event doesn't seem to be getting fired.
This fits in with my experience as well. The default email has a link sending me to https://{REDACTED}.supabase.co/auth/v1/verify?token=pkce_{REDACTED}&type=recovery&redirect_to=http://localhost:3000/auth/callback
. When my browser navigates to that URL, I receive a 303
response with a Location
header of http://localhost:3000/auth/callback?code={REDACTED}
. At this point, it seems that just the SIGNED_IN
and INITIAL_SESSION
events fire.
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 /auth/callback
route. Allowing a user to provide a separate resetPasswordRedirectTo
is a fine idea.
Alternatively, I could imagine a world where supabase.auth.resetPasswordForEmail()
accepted a queryParams
to specify custom flags to ease their ability to parse callback requests without setting up separate routes. Of course, doing this would also mean that the Auth
component in this codebase would need to provide a standard queryparam for password resets and then inform users that they need to prepare to handle that case in their callback route. This sounds like a bit more work on Supabase's side.
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 PASSWORD_RECOVERY
event so perhaps that might help as well. We will try on our end as well when a slot frees up.
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, gotrue-js@2.62.2
and supabase-js@2.39.3
, but I am still not seeing PASSWORD_RECOVERY
event get fired when I follow the password reset link from the email in Inbucket.
I tried to follow the code in the linked PR and added some logs to GoTrueClient. In my case the changed function, verifyOtp
, doesn't seem to be called. I instead get the SIGNED_IN
event from this line with redirectType === null
.
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: https://github.com/supabase/auth-ui/issues/77#issuecomment-1343198673
What is the new behavior?
This prop allows you to redirect users to a page where they can set their new password.