storage icon indicating copy to clipboard operation
storage copied to clipboard

Unable to delete `auth.users` row due to `objects_owner_fkey` FK constraint

Open liaujianjie opened this issue 3 years ago • 9 comments

Bug report

Describe the bug

When a user uploads an object to a bucket, the object's row in storage.objects has a column owner that has a FK constraint objects_owner_fkey to auth.users.id. However, it's not set up with on delete {cascade|set null}—which prevents the user from actually being deleted.

Attempting to delete a user with a storage.object referencing the user results in a FK constraint violation.

To Reproduce

  1. Create a user
  2. Authenticate as that user on the client
  3. Upload an object as that user on the client
  4. Delete that user via dashboard
  5. You'll get a FK constraint violation error:

    Deleting user failed: update or delete on table "users" violates foreign key constraint "objects_owner_fkey" on table "objects"

Expected behavior

Should be able to delete user whilst retaining the object in the database.

Suggested fix & temporary workaround

Add on delete set null to the objects_owner_fkey constraint:

alter table storage.objects
drop constraint objects_owner_fkey,
add constraint objects_owner_fkey
   foreign key (owner)
   references auth.users(id)
   on delete set null;

liaujianjie avatar Aug 11 '21 08:08 liaujianjie

Also as a side question, is it a good idea or good practice to run our own migrations on the auth and storage schemas?

liaujianjie avatar Aug 11 '21 08:08 liaujianjie

I believe the idea here is that they don't want to delete the row because then the file will forever be stored without any reference. Until they have a proper way of handling deleting files as well on user deleted I don't think this will come anytime soon.

k0shk0sh avatar Aug 11 '21 21:08 k0shk0sh

good practice to run our own migrations on the auth and storage schemas

No - it would not be a good idea to modify the auth or storage schema. If you need to extend the functionality it would be best to add your own schemas. (and you can create views if you want to "join" schemas)

kiwicopple avatar Aug 12 '21 03:08 kiwicopple

I believe the idea here is that they don't want to delete the row because then the file will forever be stored without any reference. Until they have a proper way of handling deleting files as well on user deleted I don't think this will come anytime soon.

If we use set null instead of cascade, the database will still maintain a reference to the underlying S3 object though. Would that be sufficient?

liaujianjie avatar Aug 12 '21 03:08 liaujianjie

good practice to run our own migrations on the auth and storage schemas

No - it would not be a good idea to modify the auth or storage schema. If you need to extend the functionality it would be best to add your own schemas. (and you can create views if you want to "join" schemas)

Then, what would be the recommended practice here? I can't delete all objects uploaded by the user to storage before deleting the user because some of the objects still have to be retained after a user deletion.

For example, user Z uploads an avatar image for his team and later deletes his account. In this scenario, we would expect that deleting user Z wouldn't also delete the avatar image he uploaded.

liaujianjie avatar Aug 12 '21 03:08 liaujianjie

Thats a good point Jian. I will add a trigger to make the owner as null in storage.objects when a row is deleted from auth.users.

inian avatar Aug 12 '21 10:08 inian

Hi @inian

As I understand, the solution to this is to add this SQL

drop constraint objects_owner_fkey,
add constraint objects_owner_fkey
   foreign key (owner)
   references auth.users(id)
   on delete set null;

Is that correct?

Additionally, can you share some insight into what is going on behind the scenes to create this restriction, and why it is in place?

mcewen87 avatar Aug 17 '21 02:08 mcewen87

I haven't tested it out on an actual project but that does seem like it would work @mcewen87. The constraint is in place so that each storage owner is linked to an auth user and cos owner field shouldn't be any UUID.

inian avatar Aug 17 '21 02:08 inian

I haven't tested it out on an actual project but that does seem like it would work @mcewen87. The constraint is in place so that each storage owner is linked to an auth user and cos owner field shouldn't be any UUID.

Thank you! I'll give it a try.

mcewen87 avatar Aug 17 '21 02:08 mcewen87

On my side, I had a "users" table with constraint users_id_fkey between my stripe table and "auth.users" table.

I simply added : alter table users drop constraint users_id_fkey, add constraint users_id_fkey foreign key (id) references auth.users(id) on delete cascade;

theodufort avatar Dec 10 '22 02:12 theodufort

Feel free to re-open when providing more info

fenos avatar Dec 15 '22 13:12 fenos

Feel free to re-open when providing more info

Why would this need more info? You cannot delete a user from auth if they have records associated in the objects table. It is a bit of a pain.

tonyneel avatar Dec 18 '22 17:12 tonyneel

Faced the same issue today, didn't help that when I tried to delete a user through the UI (inside the Authentication tab), the error message was: "Failed to delete user: Failed to delete user". Only stumbled upon the actual error when I tried deleting the user through the SQL editor.

hansfuchs avatar Jan 11 '23 13:01 hansfuchs

An example of this problem in production for us was when employees of a business updated image content within the application. However, we did not realize that the authentication object associated with the employee would become bound to that image asset, making it impossible to delete the authentication user afterwards. Although we were able to work around this issue, it would be helpful to have more control over this relationship when uploading assets, especially since we updated some signup logic.

workaround:
Before deleting a user, delete all files using the supabase client.

const deleteAllUsers = async () => {
  const {
    data: { users },
  } = await client.auth.admin.listUsers();
  for (let user of users) {
    const directory = `users/${user.id}`;
    const fileRes = await client.storage.from("images").list(directory);
    for (let file of fileRes?.data ?? []) {
      console.log({ file });
      await client.storage.from("images").remove([`${directory}/${file.name}`]);
    }

    const { error } = await client.auth.admin.deleteUser(user.id);
    if (error) {
      console.log(`client.auth.admin.deleteUser(${user.id}) failed.`, {
        error,
      });
      throw new Error(error.message);
    }
  }
};

convcha avatar Apr 17 '23 06:04 convcha

@kiwicopple @fenos We face the same issue. Since this is still open, what is your recommended best practice if we want to delete a user but keep their uploaded objects?

d-e-h-i-o avatar Aug 14 '23 12:08 d-e-h-i-o

I got the same issue now, after hours of debugging and testing every possible way, this ended up working:

  1. Select Storage from Table Editor image

  2. Change the column type of owner to text instead of uuid. image

  3. Set correct RLS Policies

  4. Voila! 🥳

HamedMP avatar Aug 29 '23 15:08 HamedMP

In my case, I want to delete all objects(photos uploaded by a user) when the user is deleted. So I fixed storage.objects table's owner column Cascade.

1 2

Now, user and objects can be deleted without error.

kazuooooo avatar Sep 11 '23 14:09 kazuooooo

We have removed the FK constraint now and we have a new column called owner_id and owner column is deprecated

inian avatar Dec 20 '23 13:12 inian