Skip to content
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

feat: Add saveSession option in goture_client signUp method #1053

Closed
wants to merge 1 commit into from

Conversation

htoopyaelinn56
Copy link

What kind of change does this PR introduce?

Feature

What is the current behavior?

When the user successfully signup, goture_client will automatically saves the user session.

What is the new behavior?

Ability to set the saveSession option in case we don't want to save the user's session after signup for some app that the user have to login after successful signup.

@htoopyaelinn56 htoopyaelinn56 changed the title feat: saveSession option in goture_client signUp method feat: Add saveSession option in goture_client signUp method Oct 4, 2024
@dshukertjr
Copy link
Member

for some app that the user have to login after successful signup.

Why would one want to add extra steps for the users? I guess it's not the same, but you can set the autoRefreshToken to false if you don't want the session to be refreshed once the token expires.

await Supabase.initialize(
  url: supabaseUrl,
  anonKey: supabaseKey,
  debug: false,
  authOptions: FlutterAuthClientOptions(
    autoRefreshToken: false,
  ),
);

@htoopyaelinn56
Copy link
Author

Why would one want to add extra steps for the users?

I think it is just about app behavior. Most apps including GitHub asks a user to login again after successful sign up.

autoRefreshToken will not do this behavior I guess.

@dshukertjr
Copy link
Member

@htoopyaelinn56 Hmm, why do you want the user to sign in again when the user is already signed in? There is no real benefit to having them sign out right after they sign up. If you really want to do this today, you could just call signOut() right after you complete the signUp() call, but I really don't see why one would want to do that.

@htoopyaelinn56
Copy link
Author

This is about applications' business logic other than benefits. As I mentioned, some apps including even GitHub asks a user to login again after the registration. Justing calling signOut() after signUp() might not be a good practice, it could be better not saving user's session at the first place for those who don't want to save the session of the user and want the user to re-login after signup. I set the newly added parameter saveSession to true so it still works as expected as before without breaking change.

@dshukertjr
Copy link
Member

Sorry, my comment here stands and I'm closing this.

@dshukertjr dshukertjr closed this Oct 4, 2024
@awalias
Copy link
Member

awalias commented Oct 4, 2024

hey @htoopyaelinn56 . I can see the use-case for this but I think it's important that it happens on the server and not on the client (since a malicious actor could still access the session token in the API response even if you decide to drop it in the app).

Currently to my knowledge the session token is not returned when awaiting email signup confirmation.

Which signup mechanism are you using? and do you use email confirmations?

@awalias
Copy link
Member

awalias commented Oct 4, 2024

@kangmingtay I have a question. If SECURITY_UPDATE_PASSWORD_REQUIRE_REAUTHENTICATION is set to true, on sign up - will the user be required to re-authenticate? or does this config only affect password resets?

If it doesn't affect signups (for cases where MAILER_AUTOCONFIRM is true), perhaps it could be useful functionality 🤔. Although it may have to be a new config, to give more granular control.

@awalias awalias reopened this Oct 4, 2024
@awalias awalias closed this Oct 4, 2024
@htoopyaelinn56
Copy link
Author

Hi @awalias. Thanks for the response. I'm using email auth and set auto confirmation to true. We have the existing auth service and use supabase auth just to access database. Sorry if my use-case is a bit complex.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants