filamentphp / filament

A collection of beautiful full-stack components for Laravel. The perfect starting point for your next app. Using Livewire, Alpine.js and Tailwind CSS.
https://filamentphp.com
MIT License
17.48k stars 2.73k forks source link

Table not respecting Global Scope after "X per page" change #1360

Closed BrendanTWhite closed 2 years ago

BrendanTWhite commented 2 years ago

Package

filament/tables

Package Version

v2.9.13

Laravel Version

v8.81.0

Livewire Version

v2.10.1

PHP Version

php 8.1.0

Bug description

I'm using a Laravel Global Scope to restrict users to only seeing their own records, not other users' records.

When I first go to a Filament resource table, it only lists records for the logged in user, which means the table is respecting the global scope.

But when I then change from eg 10 records to page to eg 25 records per page, it suddenly shows me records from every user, which means it's no longer respecting the global scope.

Steps to reproduce

1 - Create a Global Scope that restricts records. For example:

class JustMyFamilyScope implements Scope
{
    public function apply(Builder $builder, Model $model)
    {
        return $builder->where('family_id', '=', Auth::user()->family_id);
    }
}

2 - Add that Global Scope to a Laravel model, eg

    protected static function booted()
    {
        static::addGlobalScope(new JustMyFamilyScope);
    }

5 - Ensure there are some records for that model that the logged in user CAN see with that scope, and some that the user CANNOT see. In my example - ensure some records have family_id that matches the logged in user's family_id, and some records where it does not match.

4 - Create a Filament resource for that Laravel model, including a table.

5 - View that table in a browser. You'll only see the records for that user, as expected. In this example, we see only the 17 shops for this family.

image

6 - Change the number of records on the page, eg from 10 to 25. You'll suddenly see all records for all users, which means the Global Scope is no longer being used. In this example we see all 42 shops, even though most of them are for other families.

image

Relevant log output

No response

danharrin commented 2 years ago

Hey, this is weird. Does it work if you apply your global scope inside your Resource's getEloquentQuery()?

https://filamentadmin.com/docs/2.x/admin/resources#disabling-global-scopes

BrendanTWhite commented 2 years ago

Hi @danharrin

I added this to my ShopResource class, but it didn't make any difference - the problem still happened. :-(

    public static function getEloquentQuery(): Builder
    {
        return parent::getEloquentQuery()->withGlobalScope('family', JustMyFamilyScope::Class);
    }
BrendanTWhite commented 2 years ago

In case it's useful, my repo is on GitHub github.com/BrendanTWhite/morplees/tree/family_id_on_all_models (see the family_id_on_all_models branch).

BrendanTWhite commented 2 years ago

Also, you can see this in a test environment if you'd like to, [redacted] username [redacted] and password [redacted]

(By the way, I'm aware it's not usually good practice to put a password on a public forum, but in this case it's a password that I've never used (and will never use) anywhere else, and it's a test environment with no production data, and that user account will disappear as soon as I seed the database again.)

BrendanTWhite commented 2 years ago

Update - after a bit more testing, I've discovered that it's ANY change to the page that triggers the problem. So it's like this:

  1. User does Initial page load.
  2. Everything is perfect. GlobalScope is respected; only records for the user's family appear.
  3. User makes a Livewire change, eg change the 'records per page' dropdown or change the sort order or something.
  4. The page updates via Livewire to show the new number of records per page, or the new sort order or whatever, but the data is now wrong. GlobalScope is NOT respected, all records for all families appear.

This makes me wonder if perhaps it's a Livewire bug not a Filament bug...

danharrin commented 2 years ago

Could you perhaps try and replicate it with pure Livewire? Pass your records in at render() (as a parameter to the view() method), loop through the records using @foreach, then wire:click a button, and see if the records are still using the global scope?

ManojKiranA commented 2 years ago

will dig the issue and let you know what is actually causing the issue.

ManojKiranA commented 2 years ago

hey @BrendanTWhite can you provide me a Repo with minimal reproduction

BrendanTWhite commented 2 years ago

Hi @ManojKiranAppathurai , thanks for your kind offer. Yes I will create a minimal repo, but I won't be able to get to that for 12 or 24 hours.

BrendanTWhite commented 2 years ago

Hi @ManojKiranAppathurai (and cc to @danharrin )

I'm closing this issue, as I've found the problem and it isn't a bug in Filament (and it's not really a bug in Livewire either).

The problem was in my scope file. For example:

class DotComScope implements Scope
{
    public function apply(Builder $builder, Model $model)
    {
        if (Auth::hasUser()) {
            return $builder->where('email', 'LIKE', '%.com');
        } else {
            return $builder;
        }
    }
}

And the problem is that the initial load was a regular HTTP call, with the Auth user set, and so the scope restriction was used. But subsequent changes were Livewire updates, which are API calls, and apparently they don't set Auth user in the same way in API calls.

I've put together a minimal repo at https://github.com/BrendanTWhite/issue-1360 if you're interested in taking a look.

Anyway, this issue can be closed. I'll find some other way to tweak the scope to only take effect when the user is logged in.

Thanks, Brendan.

BrendanTWhite commented 2 years ago

Just for the record, I ended up changing my scope to:

    public function apply(Builder $builder, Model $model)
    {
        if(session()->has('family_id')) {
            $builder->where('family_id', '=', session(key: 'family_id'));            
        }
    }

... as per Kevin McKee’s Single Database multi-tenancy talk - and this seems to be working perfectly, assuming you remembered to set the session variable at login and remove it again at logout.

pmartelletti commented 1 year ago

Just for anyone having the same problem: this has solved it for me (in case you applied the scope in a middleware)

https://laravel-livewire.com/docs/2.x/security#persistent-middleware