# Laravel application rules

Applies when: writing or reviewing day-to-day Laravel — controllers, models, policies,
requests. The framework will let you build all of this badly and stay green in CI.

## Service layer & architecture

### A service class earns its existence or it does not exist

The test is not "is this business logic". The test is **would a second caller want this**. A
`UserService` wrapping `User::create()` in a method called `createUser()` has added a file,
an import, and a place for logic to hide, in exchange for nothing. Delete it.

It earns its keep when the operation runs from more than one entry point — an HTTP controller
and an Artisan command and a queued job — or when it coordinates collaborators that must move
together. Two callers is the threshold; one caller is a controller method with extra steps.
The failure mode of premature extraction is not verbosity but misdirection: the next engineer
reads the controller, finds a one-line delegation, opens the service, finds another one-line
delegation, and nothing is where it appears to be.

### Where logic actually belongs

- **Validation** → form request. Not the controller, not the model.
- **Authorization** → policy. Not an `if` in the controller.
- **A single write with a couple of side effects** → the controller is fine. Say it out loud.
- **Multi-model coordination, or reused across entry points** → an action or service class.
- **Data shaping for the response** → an API resource, not array building in the controller.

An invokable action — one public `handle()`, one job — beats a service with eleven methods.
`App\Actions\Billing\ChargeSubscription` states its job in its filename. `BillingService`
states nothing, and accumulates everything tangentially about billing until nobody will touch it.

### Transaction boundaries belong to the caller, not the model

The rule: **the transaction wraps the unit of business work, and nothing that can block.**

```php
// Wrong — third-party call holds row locks for someone else's timeout
DB::transaction(function () use ($order) {
    $order->update(['status' => 'paid']);
    Http::post('https://erp.example.com/orders', $order->toArray());
    Mail::to($order->user)->send(new OrderPaid($order));
});

// Right — commit, then react
$order = DB::transaction(fn () => tap($order)->update(['status' => 'paid']));
SyncOrderToErp::dispatch($order);
```

Under load the first version exhausts the connection pool and every request queues behind a
remote system you do not control. Dispatching a job *inside* the transaction has the opposite
failure: a worker picks it up before the commit lands and fails on a row that does not exist
yet. Set `after_commit` on the queue connection. Model events (`created`, `saved`) fire
inside the transaction too, so anything they dispatch inherits the same bug.

### When not to abstract

Do not put a repository over Eloquent — it is already the abstraction, and wrapping it costs
scopes, eager loading and every relation method in exchange for a database swap you will
never perform. Do not write an interface with one implementation and no test double needing it.

## Security

### Mass assignment: `$guarded = []` is a decision, not a default

`$fillable` is a whitelist and should read like one. The concrete failure is a `users` table
with `is_admin`, an `update(request()->all())`, and a request body carrying `is_admin=1`. No
exception and no log line — privilege escalation looks exactly like a successful save.

```php
$user->update($request->all());        // wrong
$user->update($request->validated());  // right
```

`validated()` returns only keys that have rules, which makes the form request the single
place the writable surface is defined. Calling `$request->all()` *after* validating is the
bug that survives review: validation ran, and the whole payload still went through.
`Model::preventSilentlyDiscardingAttributes()` in a service provider turns a related class
of silent failure into an exception locally and in CI.

### Forgetting `authorize()` fails open

The most damaging Laravel security bug, because it is invisible: a policy exists, is correct,
is unit tested — and the controller never calls it. Every test passes; the endpoint is open.

```php
public function update(UpdatePostRequest $request, Post $post)
{
    // Missing: $this->authorize('update', $post);
    $post->update($request->validated()); // any authenticated user, any post
}
```

Make it structural rather than remembered: `authorizeResource()` in the controller
constructor, `Route::resource(...)->middleware('can:update,post')`, or the check inside the
form request's own `authorize()`. Then write the test asserting the *wrong* user gets 403 —
one asserting the owner gets 200 proves nothing about authorization.

### Raw expressions are the only SQL injection route left

Query builder bindings are safe. These are not:

```php
// Wrong — all three interpolate
DB::table('posts')->whereRaw("title LIKE '%{$term}%'");
Post::orderByRaw("created_at {$request->direction}");
DB::select("select * from posts where author = '{$id}'");

// Right — values bound, structure allowlisted
DB::table('posts')->whereRaw('title LIKE ?', ["%{$term}%"]);
$column = in_array($request->sort, ['created_at', 'title'], true) ? $request->sort : 'created_at';
Post::orderBy($column, $request->direction === 'asc' ? 'asc' : 'desc');
```

Column and direction names **cannot be bound** — bindings are for values only, so anything
structural from user input needs an allowlist. A sortable-column parameter piped into
`orderByRaw()` is the version of this that ships, because it reads as UI plumbing, not a query.

### File uploads

The client-supplied filename and MIME header are attacker-controlled. `mimes:pdf` validates
against guessed content; `mimetypes:` trusts the reported header — use `mimes`. Never build
the stored path from `getClientOriginalName()`; a name like `../../.env` or `x.php` is a free
write primitive. Let Laravel name it:

```php
$request->file('doc')->storeAs('docs', $file->getClientOriginalName()); // wrong
$path = $request->file('doc')->store('docs', 'private');                // right
```

Uploads belong on a **non-public disk** unless they are genuinely public assets, served via a
controller that runs the policy first. A file on the `public` disk is authorized by whoever
guesses the URL — `storage:link` plus user documents is a data leak with a friendly UI.

### Signed URLs, secrets, rate limiting

- `URL::temporarySignedRoute()` for anything emailed. Without an expiry, a link forwarded once
  is permanent access — and the `signed` middleware is what verifies it. Forget the
  middleware and the signature is decorative.
- `config()` in application code, `env()` only inside `config/`. After `config:cache` every
  `env()` call outside config files returns `null` in production, and null secrets fail as
  "unauthenticated" — which reads like a credentials problem for hours.
- Throttle login, registration, password reset and token endpoints by **credential, not just
  IP**: `Limit::perMinute(5)->by($request->input('email').$request->ip())`. IP-only limiting
  is defeated by any botnet; email-only lets an attacker lock out a real user. Throttling the
  `api` group at 60/min is not auth protection — it is 60 password guesses a minute, forever.

## Comments & documentation

### Comment the why; the what is already there

Code states what it does. A comment restating it is a second copy that drifts:

```php
// Wrong — a second copy of the line below, and it rots
// Check if the user is active
if ($user->status === Status::Active) {

// Right — a constraint the code cannot state
// Registry rejects renewals inside the 5-day pre-expiry lock window, so we queue
// them until it clears rather than failing the customer's request. See #482.
if ($domain->expiresWithin(days: 5)) {
```

The durable test: **would this comment still be true after a refactor?** Why-comments survive
because the constraint outlives the implementation. What-comments describe the current lines
and become lies the first time someone edits them without reading below. Three things always
earn one: an external system's non-obvious behaviour, a workaround that looks wrong (a
redundant check, a sleep, an ordering dependency), and a deliberate omission — code that
looks wrong and is correct gets "fixed" eventually unless a comment says why not.

### PHPDoc where the type system cannot reach

PHP 8.4 types cover parameters and returns. Annotate only what they cannot express — array
shapes and generic collection members are the whole value:

```php
/**
 * @param  array{sku: string, qty: int, meta?: array<string, string>}  $line
 * @return Collection<int, Invoice>
 */
public function invoicesFor(array $line): Collection
```

`@param string $sku` next to `string $sku` is noise, and so is `@return void` on a typed
method. If the docblock only repeats the signature, delete the docblock.

## Code organisation

### Form requests are the boundary, not a validation dumping ground

A controller doing `$request->validate([...])` inline gets copied to the API controller,
diverges, and now two endpoints accept different shapes into the same table. Form requests
give you one shape, `authorize()`, and `prepareForValidation()` for normalisation before the
rules run. Database-touching rules (`Rule::unique(...)->ignore($id)`) belong there too.

### Model bloat, and what to do instead

A model is a table's shape, its relations, its casts and its scopes. Everything else has a
better home:

- Repeated `where` chains → **query scopes**. Without them five controllers each define
  "active" slightly differently, and a status column added later gets fixed in four of five.
- Status strings → **backed enums**, cast on the model. `'pending'` scattered as a literal is
  one typo from a record in a state no query matches.
- Anything that orchestrates other models, sends mail, or calls an API → action class.

Casts belong on the model; accessors that run a query do not — that is the N+1 that never
appears in the controller.

### Facade-heavy code is untestable code

Facades resolve from the container at call time, so a class using six of them has six
dependencies its constructor denies having. `Http::fake()`, `Queue::fake()` and `Bus::fake()`
are a fine middle ground in tests. The real line is *unfakeable* coupling: a class reaching
for `Auth::user()` deep in a calculation cannot run from a queued job or a console command,
and that restriction surfaces only the day someone tries. Pass the user in.

---
MIT licensed. Written by Smit Desai — <https://laravel.org.in>