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

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

Open
szymonhernik opened this issue Mar 7, 2024 · 15 comments · May be fixed by #313
Open

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

szymonhernik opened this issue Mar 7, 2024 · 15 comments · May be fixed by #313

Comments

@szymonhernik
Copy link

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
Copy link

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
Copy link
Contributor

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:

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();

@szymonhernik
Copy link
Author

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
Copy link
Contributor

chriscarrollsmith commented Mar 8, 2024

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
Copy link
Author

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
Copy link
Contributor

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.

@chriscarrollsmith chriscarrollsmith linked a pull request Mar 10, 2024 that will close this issue
@thorwebdev
Copy link
Collaborator

@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
Copy link
Author

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
Copy link
Contributor

chriscarrollsmith commented Mar 11, 2024

@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
Copy link
Contributor

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
Copy link
Author

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

@chriscarrollsmith
Copy link
Contributor

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
Copy link
Contributor

k-thornton commented May 12, 2024

@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
Copy link
Author

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
Copy link

genru commented May 17, 2024

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"

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 a pull request may close this issue.

6 participants