Authorize ability to access/update specific model attributes

[egeriis]

We're in the process of updating our API to use the newest version of this lib. Really great work with the recent updates!

One thing we're in doubt about now though, is how we would go about whitelisting attributes for updating based on which user has been authorized.

It doesn't seem like the dirty attributes are available in the Authorizer. But maybe the correct way would be to check for unauthorized change of attributes via the Hydrator? If that's the case, I'm not sure how to throw an error, in the case that I would like to accept some changed attributes, but return an error to tell which ones didn't update.

Do you have any insight into this?

[lindyhopchris]

In terms of concerns, this definitely is an authorization concern rather than a hydration concern. So the solution is for the AuthorizerInterface to receive the resource object that the client has sent in the canUpdate method. It makes sense to make the change so I can do that.

Changing the interface will technically be a breaking change, so I'll have to release a v0.5 version with the change in it. I'll see if I can get to that tomorrow or over the weekend.

PS: Glad you like the updates! I'm a lot happier with where the library is now and finding the implementation a lot cleaner to use in applications, so it's good to hear you like the updates too.

[egeriis]

To add some context, we previously made our own CrudController to extend our model controllers from. These updates that you've made completely eliminates the need for this and we feel like our API is way more structured now. Just to add some praise 🙏🏻

Thank you for considering and accepting this request :)

[lindyhopchris]

Yeah, I had a CrudController as well on the previous version, which is what has led to this update! Plus I'm a huge fan of composition and separation of concerns, which is where the separate unit classes has come from.

Something has occurred to me on this request, which is to point out that the authorization needs to occur before validation - this is logically the correct order and is the same order in which the Laravel FormRequest does things.

However, I think it'd be sensible for me to change the request so that it does things in the following steps:

1. Check that what the client has sent conforms to the JSON API spec - i.e. the content is structurally and semantically correct.
2. Check with the authorizer that the request can happen
3. Run "logical" validation - this is when the Laravel attribute validators get called and relationship validators.

This at least means that in the authorizer you can interact with the resource knowing that it does conform to the JSON API spec - e.g. you can do $resource->getAttributes() knowing that it won't throw an exception. But what you can't rely on is that attribute foo is a string if that is in your validation rule for your attributes.

To me that makes sense in terms of if the client hasn't sent a valid JSON API request, it can't be authorized. But logical validation shouldn't occur until after authorization because validation messages might reveal something that a client isn't authorized to know.

Just wondering if you agree/disagree with any of that?

[egeriis]

I reckon that's the way to do it. My only concern would be the ease with which a check for unauthorized attributes in request payload would be tedious and "difficult" to implement.

If it could be as simple as having a getter which returns whitelisted attributes, that'd be my dream scenario I think.

Otherwise I believe you're spot on and got every aspect in your description.

[lindyhopchris]

Yep, if you do $resource->getAttributes() you get an instance of StandardObjectInterface:
https://github.com/cloudcreativity/json-api/blob/master/src/Contracts/Object/StandardObjectInterface.php

So you can use getMany($keys) to get attribute values if they exist (so the returned array will be empty if they don't exist). However, it might be better to use hasAny($keys) which returns a boolean if any of the keys exist.

[egeriis]

I would still have to process that array and compare the values to the persisted values. Considering the use case where a client are allowed to pass all attributes, as long as they are not changed. But maybe that's too application specific, so I would just build that logic myself. Any input?

[lindyhopchris]

Yeah, I think at the moment it's too application specific. A resource's attribute values can be anything - including complex sub-objects, arrays, etc. So it'd be quite difficult for a generic library to know how to diff the two.

If you're trying to enforce immutability of values, then there are multiple strategies to do this. Diffing in the authorizer is one. The others would be to validate that the provided value matches the existing value. You can do that as you have access to the existing record when creating your attribute rules:

protected function attributeRules($record = null)
{
    return [
        'foo' => is_null($record) ?
            'string:between:1,40' : "in:{$record->foo}"
    ];
}

There's limitations (the in rule won't be usable if the current value has a comma , in it). I've contributed to this Laravel issue that's related to validating immutability (worth a read):
laravel/ideas#84

The other strategy is just not to hydrate values that aren't changeable. E.g. in the hydrator if foo can only be set if the record is new:

protected function hydrateAttributes(StandardObjectInterface $attributes, $record)
{
    $keys = ['bar', 'baz'];

    if (!$record->exists) {
        $keys[] = 'foo';
    }

    $record->fill($attributes->getMany($keys));
}

The limitation of that is the client gets no feedback that it has tried to change an immutable attribute.

It depends on your application as to which approach is going to fit your needs.

[egeriis]

I'd rather do a comparison in the authorizer then to throw an error to the client. I think that a change of an unauthorized attribute would be a programming error, thus it would be more beneficial to respond with an error rather than setting the allowed attributes and not respond with an error.

Great feedback and ideas, thanks!

[lindyhopchris]

@egeriis this authorizer change is implemented on the dev branch of the next version. For installation instructions, see issue #13

[lindyhopchris]

Closing as this is available via Componser on the v0.5.x-dev release