Laravel の FormRequest を使うとき、多くの場合は rules() にばかり意識が向きます。
一方で authorize() は、とりあえず return true; のままにしてしまうことも多いのではないかと思います。
自分もそうしていたのですが、最近コードレビューで
各 Request クラスで
authorize()がreturn true;のままになっていないか確認してください
という指摘を受けました。
最初は
「認可はコントローラー側でやっているし問題ないのでは？」
と思ったのですが、話を整理してみると authorize() の役割をちゃんと理解していなかったことに気づきました。
その内容を覚え書きとしてまとめておきます。
authorize() の役割
Laravel の FormRequest には大きく2つの役割があります。
| メソッド | 役割 |
|---|---|
rules() |
入力値のバリデーション |
authorize() |
その操作を実行する権限があるか |
つまり
- 入力の形式が正しいか →
rules() - このユーザーがその操作をしてよいか →
authorize()
という分担です。
authorize() が false を返すと、Laravel はコントローラーに入る前にそのリクエストを拒否します。
return true; の意味
ここが今回一番大きな気づきでした。
authorize() で
return true;
と書くのは、「あとでコントローラーで認可します」という意味ではありません。
本当にそのまま 「このリクエストは誰でも通してよい」 という意味になります。
よくあるパターン
例えば、次のようなコード構成になっているケースです。
Request
class UpdatePostRequest extends FormRequest
{
public function authorize(): bool
{
return true;
}
}
Controller
class PostController extends Controller
{
public function update(UpdatePostRequest $request, Post $post)
{
if ($request->user()->id !== $post->user_id) {
abort(403);
}
// 更新処理
}
}
この場合、実際の認可チェックはコントローラーに書かれています。
authorize() は何もしていません。
ここで問題になるのは、もし将来この認可チェックが削除された場合です。
authorize() が true のままなので、
誰でも更新できる状態になります。
つまり authorize() は最後の砦として機能していません。
なぜ authorize() を明示的に書いたほうがよいのか
1. 認可ロジックの場所が分かりやすくなる
認可チェックがコントローラーのあちこちに散らばると、
- この操作は誰ができるのか
- どこでチェックしているのか
が分かりにくくなります。
一方で FormRequest に書かれていれば、
「このリクエストはどんな条件で許可されるのか」
が一目で分かります。
2. コントローラーがシンプルになる
コントローラーの役割は、
- リクエストを受ける
- 処理を呼ぶ
- レスポンスを返す
という流れを管理することです。
認可ロジックまで全部書いてしまうと、コントローラーが肥大化します。
authorize() に寄せることで、コントローラーはシンプルになります。
3. セキュリティの保険になる
コントローラーの認可チェックが
- 削除された
- 書き忘れた
- 新しいメソッドで忘れた
といった場合でも、authorize() が防波堤になります。
authorize() に書く例
例えば「自分の投稿だけ更新できる」というルールなら、次のように書けます。
class UpdatePostRequest extends FormRequest
{
public function authorize(): bool
{
$post = $this->route('post');
return $this->user()->id === $post->user_id;
}
}
こうしておくと、 このリクエストは「自分の投稿だけ更新できる」 というルールが Request クラスを見るだけで分かります。
Policy を使う場合
Laravel の Policy を使っている場合は、さらに分かりやすく書けます。
class UpdatePostRequest extends FormRequest
{
public function authorize(): bool
{
return $this->user()->can('update', $this->route('post'));
}
}
コードレビューで言われた「確認すべきこと」
今回レビューで言われていた
authorize()がreturn true;のままになっていないか確認する
という指摘は、単に true を探せという意味ではありません。
確認すべきポイントは次の3つです。
1. Request の authorize() が無条件 true になっていないか
まず FormRequest を継承したクラスを確認します。
public function authorize(): bool
{
return true;
}
となっているものが対象です。
2. コントローラーに認可ロジックが書かれていないか
次に、その Request を使っているコントローラーを確認します。
例えば次のようなコードです。
if ($request->user()->id !== $post->user_id) {
abort(403);
}
または
$this->authorize(...)
$user->can(...)
などです。
3. そのロジックを authorize() に書けないか検討する
コントローラーで行っている認可チェックを、
authorize()に移す- Policy を呼ぶ
などの形に整理します。
まとめ
今回のレビュー指摘を一言でまとめると、
authorize() = return true; は「未実装」ではなく「全員許可」
ということでした。
コントローラーで認可をしているからといって、
authorize() を空にしてよい理由にはなりません。
むしろ
- 認可ロジックが散らばる
- チェック漏れのリスクが上がる
- Request の意味が薄れる
といった問題が出てきます。
FormRequest を作るときは
rules()だけでなくauthorize()にも意味のある条件を書く
という点を意識したほうが良さそうです。