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

[8.x] Adds Response authorization to Form Requests #38489

Merged
merged 6 commits into from
Aug 23, 2021
Merged

[8.x] Adds Response authorization to Form Requests #38489

merged 6 commits into from
Aug 23, 2021

Conversation

DarkGhostHunter
Copy link
Contributor

@DarkGhostHunter DarkGhostHunter commented Aug 21, 2021

What?

Allows to use Response objects in the authorize() method of the Form Requests.

use Illuminate\Foundation\Http\FormRequest;
use Illuminate\Auth\Access\Response;

class CreatePostFormRequest extends FormRequest
{
    public function authorize()
    {
        return Response::deny('You are too cool to post.');
    }
}

Why?

Because otherwise you have to fallback to using the Gate or Policy manually, or authorize via Controller, which makes FormRequest authorization a moot point.

How?

When the passesAuthorization() method of the FormRequest is called, it will do two additional checks if the response from the authorize() method is not falsy: if it's a Response instance, call authorize().

BC?

None, as the authorize() is meant to return a bool anyway as the PHPDoc states.

@rodrigopedra
Copy link
Contributor

Isn't overridden the failedAuthorization() method meant to allow customizing the return message/exception?

@DarkGhostHunter
Copy link
Contributor Author

DarkGhostHunter commented Aug 22, 2021

Isn't overridden the failedAuthorization() method meant to allow customizing the return message/exception?

Yes, but then you're bound for one generic authorization message, which may not reflect why the authorization failed.

public function authorize()
{
    return $this->user()->isAdmin() || $this->user()->posts()->count() < 30;
}

public function failedAuthorization()
{
    return 'Well... you are not an admin, or you are over your post count'.
}

@rodrigopedra
Copy link
Contributor

Got it. Thanks for the heads up

@taylorotwell
Copy link
Member

It would be less of a breaking if you do not change the behavior at all for non Response results.

@DarkGhostHunter
Copy link
Contributor Author

It would be less of a breaking if you do not change the behavior at all for non Response results.

Yeah, was kind of a stretch for strings. I'll fix it once I get into my oven.

@taylorotwell taylorotwell merged commit b5300a1 into laravel:8.x Aug 23, 2021
victorvilella pushed a commit to cdsistemas/framework that referenced this pull request Oct 12, 2021
* Adds Response authorization to Form Requests.

* Style changes

* Removes string check to denying responses.

* Fixes tests by removing string check.

* Removed string authorization to denying response.
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.

None yet

3 participants