Authorisation belongs in two places

The bug class

Broken object-level authorisation — commonly IDOR — is the most boring serious vulnerability there is. A request is properly authenticated. The user is exactly who they claim to be. They pass an identifier for an object they don't own, and the API does the operation anyway.

PATCH /credentials/:id     # authenticated ✓   owned by caller? …unchecked
DELETE /conversations/:id  # authenticated ✓   owned by caller? …unchecked

No exploit chain, no clever payload. Change a number in a URL.

Why it accumulates

It is almost never a decision. It is an omission, and the conditions that produce it are ordinary:

• Authentication middleware feels like it covers this. A route behind an auth guard looks protected. It is — against anonymous access, which is a different problem.
• New endpoints get copied from old ones. If the template omits the ownership check, so does everything descended from it.
• Read endpoints get scrutiny; mutations sometimes don't. Several of the gaps here were on update and delete paths, where the consequence is worse.
• It cannot be found by testing your own account. Every manual test passes, because you only ever pass your own identifiers.

Enforcing at both layers

The fix applied ownership checks at the route layer and inside the service layer. That is deliberate duplication.

The route layer is where it's convenient — declarative, consistent, easy to read. But it's also one forgotten annotation away from being absent, and a new route added under deadline is exactly where that happens.

The service layer doesn't care who called it. A service method invoked from a controller, a background job, or a script enforces the same rule every time. It's the layer where the invariant is actually true rather than merely usually applied.

Where they were

Six subsystems, found by reading code rather than running a scanner: credentials, comments, conversations, opportunity applications, short profile pitches, and user skills. The pattern was consistent — resources belonging to a user, reachable by identifier, mutable through an endpoint that verified the session and stopped there.

A scanner would have struggled with these. Knowing that PATCH /skills/:id should be restricted to the skill's owner requires understanding the domain, not just the routes.

The adjacent problem: abuse of unauthenticated endpoints

One-time-password endpoints sit before authentication by definition, so ownership checks don't apply. They're also directly abusable — an unprotected OTP-send endpoint is a way to spend your SMS budget and harass arbitrary phone numbers.

A CAPTCHA challenge on that flow was the pragmatic answer: it doesn't identify the caller, it just makes automation expensive enough not to be worth it. Rate limiting alone is weak here because the attacker controls the distribution of source addresses.

How to stop reintroducing them

• Make ownership a parameter of the query, not a check after it. findOne({ _id: id, accountId: caller }) cannot be forgotten the way a subsequent if can — a missing owner returns nothing instead of returning someone else's row.
• Make ownership a parameter of the query, not a check after it. This is the single highest-leverage habit, because it fails closed:

  // Fails open — the check is a separate statement someone can omit.
  const skill = await skills.findById(id);
  if (skill.accountId !== caller) throw new ForbiddenError();
  await skills.update(id, patch);
  
  // Fails closed — ownership is part of the question being asked.
  const updated = await skills.findOneAndUpdate(
    { _id: id, accountId: caller },
    { $set: patch },
  );
  if (!updated) throw new NotFoundError();

  A missing owner clause in the second form returns nothing rather than someone else's row. Returning 404 rather than 403 is deliberate too: 403 confirms the object exists, which is free reconnaissance.

• Fix the template. If new endpoints are copied from an existing one, the one they copy needs to be correct.
• Test with two users. A test suite with a single fixture user can never catch this. Two users and one cross-request assertion catches all of it.
• Read mutations specifically during review. "Whose object is this, and where is that established?" is a question worth asking on every write path.

← All engineering notes