vercel / nextjs-subscription-payments

Clone, deploy, and fully customize a SaaS subscription application with Next.js.
https://subscription-payments.vercel.app/
MIT License
5.77k stars 1.18k forks source link

updateName doesn't update the name in account page #311

Open szymonhernik opened 3 months ago

szymonhernik commented 3 months ago

simply, the updateName does update the full name in auth users but it's not reflected on the frontend as the value in display name is being taken from the users table. if you click on update name it says it's updated but on refresh it goes back to the first value.

in case i should provide more info let me know, thank you

hilyas786786 commented 3 months ago

Same issue, seems to be a problem with the handleRequest function in auth-helpers If you manually change it inside supabase, it will update it. Otherwise same issue as you.

chriscarrollsmith commented 3 months ago

It looks like @thorwebdev rolled back some updates to the database schema where I added stronger RLS and cascaded changes from the auth table to the users table. If he explains why he did this, maybe we can find a solution that restores this functionality while addressing his concerns.

The deleted migration file is here:

https://github.com/vercel/nextjs-subscription-payments/commit/80f65154d3572faf9f6493df8b0d4364d576b4bb

As an alternative to cascading the changes, it's also possible to manually set up a trigger with the following SQL code:

-- Function to handle updates to existing users
CREATE FUNCTION public.handle_update_user()
RETURNS TRIGGER AS $$
BEGIN
  UPDATE public.users
  SET full_name = NEW.raw_user_meta_data->>'full_name',
      avatar_url = NEW.raw_user_meta_data->>'avatar_url'
  WHERE id = NEW.id;

  RETURN NEW;
END;
$$ LANGUAGE plpgsql SECURITY DEFINER;

-- Trigger to invoke the function after any update on the auth.users table
CREATE TRIGGER on_auth_user_updated
AFTER UPDATE ON auth.users
FOR EACH ROW EXECUTE FUNCTION public.handle_update_user();
szymonhernik commented 3 months ago

Thank you for reaching out with both the commit and the alternative solution @chriscarrollsmith i have one question: is it a desired behavior that even when I update the display name in the auth users it gets overwritten when I sign out and in again? It resets to the original one from the provider. I decided to update public users table instead and treat that as the source of truth for the display name in the account page.

chriscarrollsmith commented 3 months ago

You mean the auth.users table is reverting to the original name, because it's getting the name from the third-party OAuth provider?

No, that's not desired behavior. Maybe your approach is the right one, though.

szymonhernik commented 3 months ago

when i change it with

auth.updateUser({
    data: { full_name: fullName }
 });

it edits the user_metadata correctly

data {
  user: {
   ...
    app_metadata: { provider: 'github', providers: [Array] },
    user_metadata: {
    ...
      full_name: 'changed names',

but when i sign out and in with the provider again the user_metadata gets modified just by the signing in back to:

data {
  user: {
   ...
    app_metadata: { provider: 'github', providers: [Array] },
    user_metadata: {
    ...
      full_name: 'original_username',

from what i understand looking at what's happening when loging in with the provider is that the display name (full_name) gets overwritten on the action of signing in so i shouldn't try to change this value but the value in custom public users table to have the value up to date that doesn't get overwritten.

chriscarrollsmith commented 3 months ago

Yes, I see. I think you're right, then. We should be updating the public users table, not the auth table. And we shouldn't be cascading name changes from auth to public.

thorwebdev commented 3 months ago

@chriscarrollsmith sorry, the main issue with that was that you're allowing users access to modify admin tables like public.customers and public.subscriptions. These tables should never be able to be modified by users themselves. E.g. take this scenario, a user somehow finds out someone else's customer_id and then goes ahead and changes their customer id to the other in the customers table. Now the other customer will be paying for their subscriptions.

szymonhernik commented 3 months ago

Thanks for opening a PR @chriscarrollsmith. @thorwebdev Do you mean the PR @chriscarrollsmith just opened is not correct or the one you modified in the past (linked at the top by @chriscarrollsmith )? How about something like this?

export async function updateName(formData: FormData) {
  const fullName = String(formData.get('fullName')).trim();
  const supabase = createClient();

  // Retrieve the user's ID from the custom users table
  const { data: userDetails, error: userDetailsError } = await supabase
    .from('users')
    .select('id')
    .single();

  if (userDetailsError) {
    console.error('Failed to retrieve user details:', userDetailsError.message);
    return getErrorRedirect(
      '/account',
      'User details could not be retrieved.',
      userDetailsError.message
    );
  }

  // Update the name in the custom users table
  const { error: usersUpdateError } = await supabase
    .from('users')
    .update({ full_name: fullName })
    .match({ id: userDetails?.id })
    .single();

  if (usersUpdateError) {
    console.error('Failed to update users table:', usersUpdateError.message);
    return getErrorRedirect(
      '/account',
      'Users table update failed.',
      usersUpdateError.message
    );
  } else {
    return getStatusRedirect(
      '/account',
      'Success!',
      'Your name has been updated.'
    );
  }
}
chriscarrollsmith commented 3 months ago

@chriscarrollsmith sorry, the main issue with that was that you're allowing users access to modify admin tables like public.customers and public.subscriptions. These tables should never be able to be modified by users themselves. E.g. take this scenario, a user somehow finds out someone else's customer_id and then goes ahead and changes their customer id to the other in the customers table. Now the other customer will be paying for their subscriptions.

Thanks, @thorwebdev! Yes, once I looked more closely at this, I understood why you rolled it back. I don't think your scenario would work, because there's no way for users to alter their ID in the auth.users table, but they could abuse it in other ways, like by altering their own subscription. Not sure what I was thinking, lol. I will update my latest PR to fix the broken types.

chriscarrollsmith commented 3 months ago

Thanks for opening a PR @chriscarrollsmith. @thorwebdev Do you mean the PR @chriscarrollsmith just opened is not correct or the one you modified in the past (linked at the top by @chriscarrollsmith )? How about something like this?

export async function updateName(formData: FormData) {
  const fullName = String(formData.get('fullName')).trim();
  const supabase = createClient();

  // Retrieve the user's ID from the custom users table
  const { data: userDetails, error: userDetailsError } = await supabase
    .from('users')
    .select('id')
    .single();

  if (userDetailsError) {
    console.error('Failed to retrieve user details:', userDetailsError.message);
    return getErrorRedirect(
      '/account',
      'User details could not be retrieved.',
      userDetailsError.message
    );
  }

  // Update the name in the custom users table
  const { error: usersUpdateError } = await supabase
    .from('users')
    .update({ full_name: fullName })
    .match({ id: userDetails?.id })
    .single();

  if (usersUpdateError) {
    console.error('Failed to update users table:', usersUpdateError.message);
    return getErrorRedirect(
      '/account',
      'Users table update failed.',
      usersUpdateError.message
    );
  } else {
    return getStatusRedirect(
      '/account',
      'Success!',
      'Your name has been updated.'
    );
  }
}

He's talking about the past PR. The new PR I just opened should be okay.

szymonhernik commented 3 months ago

Alright! Thank you a lot @chriscarrollsmith ! Can I close this issue or the PR needs to be accepted first?

chriscarrollsmith commented 3 months ago

Alright! Thank you a lot @chriscarrollsmith ! Can I close this issue or the PR needs to be accepted first?

This issue will be automatically closed once the PR is accepted.

k-thornton commented 1 month ago

@szymonhernik's code above does work to update the public users table, but it bugs me that the user's name in auth metadata starts diverging from the one in the public/users table. I like @chriscarrollsmith's idea of using a trigger to keep these synchronized, but I'm wondering why even bother replicating the information across to the public users table at all?

Why not just delete that row from the public table, and in account/page.tsx, swap in <NameForm userName={user?.user_metadata.full_name ?? ''} />

szymonhernik commented 1 month ago

when i change it with

auth.updateUser({
    data: { full_name: fullName }
 });

it edits the user_metadata correctly

data {
  user: {
   ...
    app_metadata: { provider: 'github', providers: [Array] },
    user_metadata: {
    ...
      full_name: 'changed names',

but when i sign out and in with the provider again the user_metadata gets modified just by the signing in back to:

data {
  user: {
   ...
    app_metadata: { provider: 'github', providers: [Array] },
    user_metadata: {
    ...
      full_name: 'original_username',

from what i understand looking at what's happening when loging in with the provider is that the display name (full_name) gets overwritten on the action of signing in so i shouldn't try to change this value but the value in custom public users table to have the value up to date that doesn't get overwritten.

@k-thornton as in the previous comments, if you change it in the Auth table the name will be overwritten with the next sign in.

genru commented 1 month ago

It looks like @thorwebdev rolled back some updates to the database schema where I added stronger RLS and cascaded changes from the auth table to the users table. If he explains why he did this, we can find a solution that restores this functionality while addressing his concerns.

The deleted migration file is here:

80f6515

As an alternative to cascading the changes, it's also possible to manually set up a trigger with the following SQL code:

-- Function to handle updates to existing users
CREATE FUNCTION public.handle_update_user()
RETURNS TRIGGER AS $$
BEGIN
  UPDATE public.users
  SET full_name = NEW.raw_user_meta_data->>'full_name',
      avatar_url = NEW.raw_user_meta_data->>'avatar_url'
  WHERE id = NEW.id;

  RETURN NEW;
END;
$$ LANGUAGE plpgsql SECURITY DEFINER;

-- Trigger to invoke the function after any update on the auth.users table
CREATE TRIGGER on_auth_user_updated
AFTER UPDATE ON auth.users
FOR EACH ROW EXECUTE FUNCTION public.handle_update_user();

issue perfectly fixed, after running the above SQL code in supabase "SQL Editor"