Add signup custom confirmation redirect url #6
Conversation
There was a problem hiding this comment.
Firstly, apologies for the delay in reviewing this. I seem to be the only active maintainer of this repo and I've been too busy to look at it until now.
Secondly, thank you for your contribution - it's always great to see! ❤️
Relating to the proposed changes, I have three comments:
- I was unable to get the redirect to work when testing - if you were able to use this feature successfully, could you provide a little more info about how you did that? E.g. I bumped the gotrue version to v2.157.0, set
GOTRUE_URI_ALLOW_LISTto"**", and passed a redirect URL ofhttps://google.com/, but the server seemed to respond as usual, with no sign of the expected redirect - I would expect to see an integration test for this feature before merging, e.g. by adding a new case to
signup_test.go. This is important to ensure we keep coverage high. There are plenty of example tests, so the burden of adding a test case shouldn't be too high - One minor comment on naming that I think we can improve
Apart from those comments, the approach generally looks sound. With a suitable integration test, I think we could get this in
| Phone string `json:"phone,omitempty"` | ||
| Password string `json:"password,omitempty"` | ||
| Data map[string]interface{} `json:"data,omitempty"` | ||
| ConfirmationRedirectUrl string `json:"-"` |
There was a problem hiding this comment.
I think RedirectToURL would be a preferable name here - more directly describes what the URL is for, and uses Go's preferred casing (i.e. Url → URL)
| @@ -28,6 +28,15 @@ func (c *Client) Signup(req types.SignupRequest) (*types.SignupResponse, error) | |||
| return nil, err | |||
| } | |||
|
|
|||
| if req.ConfirmationRedirectUrl != "" { | |||
There was a problem hiding this comment.
I would expect to see updates to the integration tests to exercise this new branch and verify the response includes the expected redirect before this can be merged
| r.URL.RawQuery = q.Encode() | ||
|
|
||
| // Set up a client that will not follow the redirect. | ||
| c.client = noRedirClient(c.client) |
There was a problem hiding this comment.
I can see that the JS client appears to be forwarding a redirect_to parameter like this, but In my testing, I was unable to get the server to actually return a redirect here. Were you able to successfully make use of this redirect param?
What kind of change does this PR introduce?
Feature.
What is the current behavior?
The current signup method does not allow to add a custom redirect url after user signup confirmation.
What is the new behavior?
When the user clicks the confirm email link he gets in his email inbox, he can now be redirected to the custom url passed in this new feature. As long as this url is allowed in the Supabase Authentication Redirect URLs list (https://supabase.com/docs/guides/auth/redirect-urls).
Additional context
The JS supabase library uses @supabase/auth-js, the implementation of the signUp method is here:
As seen in the code, in order to support the signup custom redirect url the only thing you need to do is add a query param to the url like this: https://.supabase.co/auth/v1/signup?redirect_to=