Is it necessary to do a check if the file belongs to the user?

Users in my app can upload images for different resources. I configured aws s3 to store files of my users. The question is now do I need to check if a certain file belongs to the user, so only a user who owns the file can view it. Or is this not necessary because I am using a s3 private bucket? This is what my fileupload looks like. The form is only visible to users to which the data belongs. I did that with policies.
FileUpload::make('files')
->multiple()
->visibility('private')
->disk('s3')
->directory('rentals')
->imageEditor()
->maxSize(10)
->maxFiles(10)
->openable()
->previewable(true)
->storeFileNamesIn('file_names')
->columnSpan(['default' => 2]),
FileUpload::make('files')
->multiple()
->visibility('private')
->disk('s3')
->directory('rentals')
->imageEditor()
->maxSize(10)
->maxFiles(10)
->openable()
->previewable(true)
->storeFileNamesIn('file_names')
->columnSpan(['default' => 2]),
8 Replies
Dennis Koch
Dennis Koch2mo ago
Private bucket just means that you cannot publicly access it. If you want access control on a file level you’d need to implement this yourself.
TheNastyPasty
TheNastyPasty2mo ago
Maybe you can give me a hint how that could look like please. Need to make sure sensitive data of my users is not exposed. What I mean is logged in user should not be able to see files of another logged in user. I really need to make sure only the creator/owner of a file can see it. What I dont understand is the private file is loaded into the users Filupload field, how is it possible for another user to access this file?
awcodes
awcodes2mo ago
You could use the directory() modifier to be based on the user id. That way it will only ever show files upload to a directory in the bucket inside the user’s directory. Otherwise you’d need an ACL on the Aws side for each user of the system which just isn’t practical.
Dennis Koch
Dennis Koch2mo ago
loaded into the users FileUpload field
How do you make sure that nobody else can see that upload field? If you have permissions in place so that nobody can access the whole page/field then you nicht be fine.
TheNastyPasty
TheNastyPasty2mo ago
I am doing it like so for my resource forms. https://filamentphp.com/docs/3.x/panels/resources/getting-started#authorization This is a policy I am using in my app to make sure only the user who created the record can view/update or delete it.
class RentalPolicy
{
/**
* Determine whether the user can view any models.
*/
public function viewAny(User $user): bool
{
return true;
}

/**
* Determine whether the user can view the model.
*/
public function view(User $user, Rental $rental): bool
{
return $user->id === $rental->user_id;
}

/**
* Determine whether the user can create models.
*/
public function create(User $user): bool
{
return true;
}

/**
* Determine whether the user can update the model.
*/
public function update(User $user, Rental $rental): bool
{
return $user->id === $rental->user_id;
}

/**
* Determine whether the user can delete the model.
*/
public function delete(User $user, Rental $rental): bool
{
return $user->id === $rental->user_id;
}

/**
* Determine whether the user can restore the model.
*/
public function restore(User $user, Rental $rental): bool
{
//
}

/**
* Determine whether the user can permanently delete the model.
*/
public function forceDelete(User $user, Rental $rental): bool
{
//
}
}
class RentalPolicy
{
/**
* Determine whether the user can view any models.
*/
public function viewAny(User $user): bool
{
return true;
}

/**
* Determine whether the user can view the model.
*/
public function view(User $user, Rental $rental): bool
{
return $user->id === $rental->user_id;
}

/**
* Determine whether the user can create models.
*/
public function create(User $user): bool
{
return true;
}

/**
* Determine whether the user can update the model.
*/
public function update(User $user, Rental $rental): bool
{
return $user->id === $rental->user_id;
}

/**
* Determine whether the user can delete the model.
*/
public function delete(User $user, Rental $rental): bool
{
return $user->id === $rental->user_id;
}

/**
* Determine whether the user can restore the model.
*/
public function restore(User $user, Rental $rental): bool
{
//
}

/**
* Determine whether the user can permanently delete the model.
*/
public function forceDelete(User $user, Rental $rental): bool
{
//
}
}
Hope that is good enough.
TheNastyPasty
TheNastyPasty2mo ago
I can't get it why is that safe? Is it not possible to guess the id of a other user?
awcodes
awcodes2mo ago
You can use UUID’s as the id for your users.
TheNastyPasty
TheNastyPasty2mo ago
I think the way with policies is more safe, I think will do that