Blog
Logged In Isn't the Same as Allowed
- software-engineering
- laravel
- typescript
- security
Here's a bug that passes every demo. Log in, open /invoices/1042, see your invoice. Works great.
Now change the URL to /invoices/1043.
If you see someone else's invoice, the app checked that you were logged in. It never checked whether that invoice was yours. Those are two different questions, and a lot of code only asks the first one.
Two questions, not one
Authentication: who are you?
Authorization: are you allowed to do this, to this specific thing?
The auth middleware answers the first one. It has no idea what an invoice is. Wrapping a route group in auth and calling it secure is like checking IDs at the front door of an apartment building and leaving every unit unlocked.
This bug has a name, by the way. Insecure direct object reference, or IDOR. OWASP files it under broken access control, which was number one on their 2021 Top 10.
The bug looks completely normal
public function show(Invoice $invoice)
{
return view('invoices.show', compact('invoice'));
}Route model binding found the invoice by ID. Nothing here asks whose it is. Same thing in Node:
app.get('/invoices/:id', requireAuth, async (req, res) => {
const invoice = await prisma.invoice.findUnique({ where: { id: req.params.id } });
res.json(invoice);
});requireAuth passed, so the request is "authenticated." The query still returns any invoice in the table.
Fix one: scope the query
The simplest fix is to never look the record up globally. Look it up through the user.
public function show(Request $request, string $id)
{
$invoice = $request->user()->invoices()->findOrFail($id);
return view('invoices.show', compact('invoice'));
}If the invoice belongs to someone else, it doesn't exist as far as this query is concerned. findOrFail throws, Laravel returns a 404, and nobody learns whether that ID is real.
Same move with Prisma:
const invoice = await prisma.invoice.findFirst({
where: { id: req.params.id, userId: req.user.id },
});
if (!invoice) return res.sendStatus(404);Ownership goes in the where clause, not in an if after you already loaded the row.
Writes work the same way:
$request->user()->invoices()->create($request->validated());The user_id comes from the session, not the request body. If user_id is in your validation rules, someone will eventually send a different one.
Fix two: policies, when it's more than ownership
Real rules get messier. Team members can view. Only the owner can delete. Paid invoices can't be edited at all. That logic belongs in one place, and in Laravel that place is a policy.
class InvoicePolicy
{
public function update(User $user, Invoice $invoice): bool
{
return $user->team_id === $invoice->team_id
&& $invoice->status !== 'paid';
}
}Then check it before you do anything:
public function update(UpdateInvoiceRequest $request, Invoice $invoice)
{
Gate::authorize('update', $invoice);
$invoice->update($request->validated());
}Or right on the route:
Route::put('/invoices/{invoice}', [InvoiceController::class, 'update'])
->can('update', 'invoice');Gate::authorize throws a 403 when the policy says no. If you'd rather not confirm the record exists at all, have the policy return Response::denyAsNotFound() instead of false.
Nested routes only half check themselves
Route::scopeBindings()->group(function () {
Route::get('/teams/{team}/projects/{project}', [ProjectController::class, 'show']);
});With scoped bindings, Laravel resolves {project} through {team}, so a project from another team 404s. Good. But that only proves the two IDs belong to each other. It doesn't prove either one belongs to you. You still need to check the user is on that team.
Hiding the button is not a permission check
{canDelete && <button onClick={() => deleteInvoice(invoice.id)}>Delete</button>}That's good UX. It's not security. The endpoint behind that button is still there for anyone with a session cookie and curl.
Next.js server actions make this easy to forget, because they look like a function call instead of a route. They're still reachable with a plain POST request, and anyone can call them with whatever arguments they want.
'use server';
export async function deleteInvoice(id: string) {
const user = await getCurrentUser(); // however your app reads the session
if (!user) throw new Error('Unauthorized');
const { count } = await prisma.invoice.deleteMany({
where: { id, userId: user.id },
});
if (count === 0) throw new Error('Not found');
}With ownership in the where, the wrong user deletes nothing.
UUIDs don't count
Switching from 1043 to a UUID makes IDs hard to guess. That's nice. It's not access control.
IDs leak. They end up in emails, logs, browser history, screenshots, and support tickets. The day someone has a real ID, the only thing between them and the record is your check. Make sure there is one.
Lists leak too
Detail pages get the attention, but index routes, search, exports, and autocomplete endpoints return records too.
Invoice::where('status', 'overdue')->get(); // everyone's overdue invoices
$request->user()->invoices()->where('status', 'overdue')->get(); // yoursIf every query in a feature should be scoped to an account or team, a global scope can enforce it so nobody has to remember. Just know it's there, especially in queue jobs and console commands where nobody is logged in.
Write the test that changes the ID
This is the test that catches it, and it takes a minute to write:
public function test_users_cannot_view_someone_elses_invoice(): void
{
$invoice = Invoice::factory()->create();
$this->actingAs(User::factory()->create())
->get("/invoices/{$invoice->id}")
->assertNotFound();
}Use assertForbidden() instead if you went the policy route. Either way, one of these per route that takes an ID. Boring, fast, and it catches the kind of bug that ends up in a breach notice.
The habit
Every time a route, action, or job takes an ID from outside, ask two things: whose is this, and where did I check?
- Look records up through the user or team, not globally.
- Put rules beyond plain ownership in a policy, and call it on every route that touches the record.
- Treat hidden buttons and unguessable IDs as nice extras, not checks.
- Write the test where a second user tries the first user's ID.
Being logged in just means you know who's asking. It doesn't mean the answer is yes.