check preventAccessingMissingAttributes user_id default owned

[gpibarra]

According to this article, it is recommended to use these checks to avoid errors during the development of an application in Laravel.
https://planetscale.com/blog/laravels-safety-mechanisms

When trying to add \Illuminate\Database\Eloquent\Model::preventAccessingMissingAttributes(); an exception is thrown when trying to check some permission of a specific model and it does not have an owned callback or column defined.
When undefined it tries to look up the attribute ({model}_id, for example user_id). In the vast majority of models that attribute (column) does not exist and the isOwnedVia function returns false, but activating the check option, when trying to access user_id an exception is thrown because the attribute does not exist.
This PR checks if the attribute exists only when the attribute has not been defined correctly (ie default).

[JosephSilber]

Can you explain this a little more? Why do you have owned abilities that are owned via a column that doesn't exist?

Are you saying you have the column on some models, and you're granting a blanket ability to all models, and you're relying that it'll only apply to the models that actually have that column?

[gpibarra]

Sorry for my English.

According to the documentation
https://github.com/JosephSilber/bouncer#allowing-a-user-or-role-to-own-a-model
and
https://github.com/JosephSilber/bouncer#ownership

When checking permission, the owner check is always run (method isOwnedVia), whether or not the column exists in the model.
I am doing a check on a model that does NOT have an owner, so the user is compared with the user_id column, which is null in the model because the column does not exist.

But according to the article mentioned, it can be risky since the attribute is null, it may be because:

• the model does not have the column (the attribute does not exist)
• the model does have the column but the value is null (the attribute exists)
• the model has the column but it was not hydrated correctly (the attribute does not exist)
  Accessing non-existent attributes can be risky, so throwing an exception is recommended.

[JosephSilber]

Oh I see. We're always calling it, regardless of whether you set it it up that way.

I now see the need for this change 👍

[JosephSilber]

Why do we need to add $isDefaultAttribute to this check, and pass it all the way down? It makes the whole thing messy. Couldn't we just always do this array_key_exists check?

---

Oh I see now. You do want it to throw when it's not there 🤔

But do you really? Imagine this piece of code:

Bouncer::allow($user)->toOwnEverything();

Would you want it to throw for all models that don't have a user_id column. I'd think not.

[JosephSilber]

I wish there were a better solution to this. $model->getAttributes() is quite expensive.

Maybe we could instead toggle the preventAccessingMissingAttributes back and forth before and after checking 🤔

[gpibarra]

That would also be nice.

Anyway, it's something I thought. I think if you want to implement it in some way that you think is best (I mean the entire PR), I'll just close this PR. If this PR served to show this behavior, I consider myself satisfied.

[JosephSilber]

The only thing I'm not sure about is the global nature of it. The setting in the Eloquent\Model class sets a property on static::, which I guess means you can override it per model type. Not sure how switching it back and forth could work 🤔

[davisriska]

Maybe its possible to just change this to return $authority->getKey() == $model->getAttributeValue($attribute);?
This gets around the missing attribute exception. Atleast im looking at laravel 10 and this "fixes" the issue.

Im only trying to get rid of the barrage of Missing attribute logs im getting, so no idea if this impacts any other functionality as im not using ownedVia functionality.

[lrljoe]

Maybe I'm being dense, but you could just create the mapping for isOwnedVia on those models where user_id is missing and have it do something else to determine ownership or just return true if you don't care about ownership of that model