Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[10.x] "Can" validation rule #47371

Merged
merged 5 commits into from
Jun 18, 2023
Merged

Conversation

stevebauman
Copy link
Contributor

@stevebauman stevebauman commented Jun 7, 2023

Description

This PR implements a new "can" validation rule (accessible via the Rule::can method).

It provides a way to authorize an ability for a given form field.

Usage

Given the following PostPolicy that ensures only admins can change a blog post's author:

class PostPolicy
{
    // ...
    
    public function updateAuthor(User $user, Post $post)
    {
        return $user->isAdmin();
    }
}

And a PostRequest that allows the user to change the author if it's present, we can validate that the user has the ability to do so using the Rule::can() method:

use App\Models\Post;

class PostRequest extends FormRequest
{
    public function rules()
    {
        return [
            'author' => Rule::can('update-author', Post::class, $this->route('post')),
            'title' => '...',
            'body' => '...',
        ];
    }
}

This allows us to move this validation from the controller and into the form request:

public function update(PostRequest $request, Post $post)
{
    // ...

-    if ($request->filled('author') && $request->user()->can('update-author', $post)) {
+    if ($request->filled('author')) {
        $post->author()->associate($request->author);
    }
}

Notes

The validation rule will always include the submitted field's value in the authorization method, but it will be pushed to the end when given more arguments, allowing developers to send their own values to authorization methods that will take precedence. This offers compatibility to raw Gate definitions, as well as model policies.

For the example below, we don't provide any additional arguments in the Rule::can() method after the ability, so the second argument in the gate definition's callback is the field's value:

Gate::define('update-something', function (User $user, $fieldValue) {
    // ...
});
public function rules()
{
    return [
        'field' => Rule::can('update-something'),
    ];
}

Using the same example, if we add new arguments into the Rule::can() method, the $fieldValue will be last:

public function rules()
{
    return [
        'field' => Rule::can('update-something', 'foo', 'bar'),
    ];
}
Gate::define('update-something', function (User $user, $foo, $bar, $fieldValue) {
    // ...
});

Let me know if you have any issues or concerns with this PR. No hard feelings on closure ❤️

Thanks for your time!

@johanrosenson
Copy link
Contributor

Does this not belong in the authorize method of the form request?

    /**
     * Determine if the user is authorized to make this request.
     */
    public function authorize() : bool
    {
        return $this->user()?->can('update-author', $this->route('post')) === true;
    }

@dennisprudlo
Copy link
Contributor

@johanrosenson This would prevent the whole request in case the user is not authorized to update the author. The Rule::can validation rule checks if the user may send a value for a specific field.

So User X may update a post (title and body) but may not update the author of the post – only admins are allowed to do that for example. Another way to achieve this would be to use a custom closure based validation rule or the prohibited rule.

@johanrosenson
Copy link
Contributor

@dennisprudlo true, I think it just feels weird for me to put authorization checks together with input validation.

@cosmastech
Copy link
Contributor

@dennisprudlo true, I think it just feels weird for me to put authorization checks together with input validation.

It's kind of a chicken/egg thing.

The request flow is authorize, then validate. But in order to authorize, you need to check that the input exists.

This can be handled in a Request's prepareForValidation() method... but you end up sort of doing the same work twice that way. We typically resolve the model and then merge it into the request's input. Then use our own "required|can:" rule.

There's no perfect solution for it.

@newtonjob
Copy link

Love this! Although this is how I'd currently achieve it, using the missing rule.

return [
    'author' => $this->user()->can('update-author', $this->post) ? '' : 'missing',
    ...
]

@taylorotwell taylorotwell merged commit c8097ef into laravel:10.x Jun 18, 2023
@stevebauman stevebauman deleted the can-validation-rule branch June 19, 2023 01:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants