spatie / laravel-permission

Associate users with roles and permissions
https://spatie.be/docs/laravel-permission
MIT License
12.21k stars 1.78k forks source link

not all but some users returning no roles or permissions, and noticing 9223372036854775807 issue on model_id in debug #2700

Open matthewstick opened 4 months ago

matthewstick commented 4 months ago

Description

Was running Laravel 5, spatie/laravel-permission 2.28 - no issues Migrated to Laravel 10, Spatie 6.9.0 - issues with some users.

I have many users where it works fine after the update.

But I have one user where it worked for years, except now it does not. Their user is returning an empty array for the roles, and also no permissions.

I tried adjusting the settings and giving it others roles and max permissions, no dice.

I tried making a brand new user, with the exact same permissions, and the brand new users works great.

Cache has been cleared many times.

I've spent days debugging Spatie vendor packages. The only thing I'm noticing in the bad query, the model_id gets blown out of the water and becomes 9223372036854775807. Wheras normally it returns something more sane, like the second example.

My gut says cache, but I've disabled the cache in any way I can think of.

BAD QUERY select roles.*, model_has_roles.model_id as pivot_model_id, model_has_roles.role_id as pivot_role_id, model_has_roles.model_type as pivot_model_type from roles inner join model_has_roles on roles.id = model_has_roles.role_id where model_has_roles.model_id in (9223372036854775807) and model_has_roles.model_type = 'App\User';

GOOD QUERY select roles.*, model_has_roles.model_id as pivot_model_id, model_has_roles.role_id as pivot_role_id, model_has_roles.model_type as pivot_model_type from roles inner join model_has_roles on roles.id = model_has_roles.role_id where model_has_roles.model_id in (9) and model_has_roles.model_type = 'App\User';

IDeas

Steps To Reproduce

1. 2. 3. ...

Example Application

No response

Version of spatie/laravel-permission package:

6.9

Version of laravel/framework package:

10

PHP version:

8.1

Database engine and version:

No response

OS: Windows/Mac/Linux version:

No response

drbyte commented 4 months ago

Curiously, 9223372036854775807 is the maximum value for a long number in various languages.

(9223372036854775807 is 2^63 - 1, ie: a 64-bit number)

What is the db field type for the id in the users table? Have you converted to/from uuid with this project?

Is there any attribute or accessor that might translate a smaller integer value to some very large value by virtue of some type-conversion taking place? (including converting to a string or to an exponent value, whether intended or not)

matthewstick commented 4 months ago

Curiously, 9223372036854775807 is the maximum value for a long number in various languages.

What is the db field type for the id in the users table? Have you converted to/from uuid with this project?

Hi. They'd all been using uuid since the beginning about 7 or 8 years ago. They are all char(36).

There's about 500 active users. I ran a report, and 31 of them had "no roles" show up. They were working on Spatie 2/Laravel 5. They stopped working Spatie 6/Laravel 10. It also stopped working Spatie 5, but I upgraded to Spatie 6 and it also doesnt work there.

I agree the issue seems to be the model_id is blowing out the value as a number, hnce the 9223372036854775807. The only issue is that these 500 or so active users all use the same tables.

For example, a user has 99e62127-c1d9-49c9-9566-22c678240447 as their id. No roles. So what I do is find and replace that value with some random new uuid aross the db, such as 2b6a148f-6d46-4d60-a6bd-d07aac131292. Then suddenly the user is reporting roles and is working again.

So my "fix" is to replace the uuid's on these 31 users that returned no roles. But I'd love to get to the bottom of it. There's nothing anywhere in the code, because once I find and replace the ID, the user suddenly works. This makes me thing it's cache.

I tried a million versions of cache clear:

php artisan cache:clear php artisan config:cache php artisan cache:forget spatie.permission.cache php artisan cache:forget laravelspatie.permission.cache php artisan config:clear php artisan cache:forget spatie.permission.cache php artisan permission:cache-reset

All work, except the last one. It says... php artisan permission:cache-reset Unable to flush cache.

I've disabled cache entirely. I've tried on 4 different server builds (local vagrant, dev, staging, prod). All servers have the same issue. So feels like... not a cache?

It's completely crazy. Maybe the problem will go away forever when I cycle the UUIDs. However, maybe it will come back. Would love some insight in case it's a bigger issue. I've gone deep into the vendor files trying to debug this.. and I just see nothing, other than it totally works once I swap out UUIDs.

Thanks in advance.

drbyte commented 2 months ago

Hi. Apologies for the delayed reply. I suspect you've already sorted out a fix in the meantime. But I wanted to follow up anyway.

While considering this issue awhile back and again now, I did wonder whether the v6 changes to better support UUID within permission/role models was a culprit (worried that it was pulling in a string as an int by mistake). But you said it happened with v5 as well.

Then I wondered if your schema for model_has_roles.model_id might be of the wrong type, and inherited from way back? How does it compare to the current schema in the migration stub?

matthewstick commented 2 months ago

hi @drbyte Thanks!

I could not fix it and instead gave up. The IDs seemed perfectly valid, just 30 or so users out of 800 were affected. I made a script to find and replace the 30 IDs with a new ID in the database. Makes no sense. No code was changed. Literally I swapped one formerly good v6 ID with a new v6 ID. And then the bad accounts worked. This is why I suspect some kind of caching issue, but as I mentioned I killed all possible ways that I knew of where caching occured.

So.... gremlins? :)

drbyte commented 2 months ago

I do follow your logic for suspecting a caching issue (the isolation of just certain users affected, and the fact that a db-wide search-replace corrects it, and would break any stored caches for the replaced id).

The reason I didn't focus on cache is because the query in question is doing a lookup of roles for a user, using the roles relationship, but it's passing the model-id to that query because it's been obtained from someplace else, ie: from the User model. So strictly speaking, the bad model ID isn't coming from this package, but from the User model.

Of course, the temptation is to blame code, which is still possible, but one would think that if it's code-related then it would affect everyone, not just 6% of users.

Still, considering the User model, it makes me wonder:

Again, I realize these "should" affect "everyone", not just 6% of records. Unless there's conditional code loops for certain records.

Naturally, I'm curious whether these 31 Users have any pattern unique to them. I'm sure you already explored that, but i post it here for those who might read this discussion.

It could still be a cache thing, but probably not the cache of this package.

Clearly you've dug through a zillion possibilities before arriving at the last-resort of search/replace of raw data. I'd probably end up doing the same, as a last resort. And like you, I'd find myself concerned that it might still "bite" again if the root cause isn't identified.

And it's hard to reverse-engineer it in order to reproduce the problem on-demand, because it seems data-related and there was no pattern reported about that data.

drbyte commented 2 months ago

More on the UUID vs long thing: In https://github.com/laravel/framework/discussions/48933#discussioncomment-7495617 it is pointed out that the posted model code does not contain protected $keyType = 'string'; for (part of) implementing Laravel's UUID support (could just use the HasUuids trait instead though).

Also related: https://github.com/laravel/framework/issues/26594

And these are more about big unsigned int vs int: https://github.com/laravel/framework/issues/18676 https://github.com/laravel/framework/issues/23022