Skip to content

CWE-285: Improper Authorization - JavaScript

Overview

In Express and Node.js applications, improper authorization usually happens because a route handler performs a privileged operation without any authorization middleware in front of it, or because the only check present confirms the request is authenticated (a valid session or JWT) without confirming the caller is permitted to do what the request asks. Authentication and authorization are separate questions - a valid token proves who the caller is, not what they are allowed to do - and both need their own check. Authorization should run in middleware attached before the route handler, not as an afterthought inside it, so a new route cannot go live without going through the same check as every other route.

Primary Defence: Attach role- or permission-checking middleware directly to each route or router group (router.delete('/users/:id', requireRole('admin'), deleteUser)), reading the caller's role or permissions from the verified token or session (req.user), never from the request body or query string. For endpoints that return or modify a resource, put the owner in the query rather than comparing it after the row is loaded - Order.findOne({ _id: id, userId: req.user.id }) - so the constraint applies to the collection route as well as the single-resource one.

Common Vulnerable Patterns

Route Handler With No Authorization Middleware

const router = express.Router();

// VULNERABLE - no authentication or authorization middleware; anyone can
// call this route
router.delete('/users/:id', async (req, res) => {
  await User.findByIdAndDelete(req.params.id);
  res.sendStatus(204);
});

Why this is vulnerable: Without any middleware in the route definition, deleteUser runs for every request regardless of who sent it. There is nothing here that even confirms the caller is logged in, let alone permitted to delete users.

Authentication Checked, Authorization Skipped

// VULNERABLE - authenticate() confirms a valid session, but nothing
// checks whether this user is allowed to view billing data
router.get('/admin/billing', authenticate, async (req, res) => {
  const invoices = await Invoice.find({});
  res.json(invoices);
});

Why this is vulnerable: authenticate proves req.user exists and the token is valid. It says nothing about role or permission, so any logged-in user - not just administrators - can reach an endpoint that should be admin-only.

Trusting a Client-Supplied Role

// VULNERABLE - role comes from the request body, which the caller controls
router.post('/users', authenticate, async (req, res) => {
  const user = await User.create({
    username: req.body.username,
    role: req.body.role,  // attacker sets this to "admin"
  });
  res.status(201).json(user);
});

Why this is vulnerable: Reading role from req.body instead of deriving it from the authenticated session lets any caller grant themselves whatever role the schema accepts, regardless of any role checks elsewhere in the application.

Object-Level Check Missing (IDOR)

// VULNERABLE - proves the caller is logged in, but never checks that this
// order belongs to them
router.get('/orders/:id', authenticate, async (req, res) => {
  const order = await Order.findById(req.params.id);
  if (!order) return res.sendStatus(404);
  res.json(order);
});

Why this is vulnerable: Any authenticated user can view any order by changing the id in the URL, because the handler never compares order.userId to req.user.id.

The Collection Endpoint Beside the Protected One

// The detail route is correctly scoped
router.get('/orders/:id', authenticate, async (req, res) => {
  const order = await Order.findOne({ _id: req.params.id, userId: req.user.id });
  if (!order) return res.sendStatus(404);
  res.json(order);
});

// VULNERABLE - the list route beside it returns every order in the
// collection to any authenticated caller
router.get('/orders', authenticate, async (req, res) => {
  const orders = await Order.find({});
  res.json(orders);
});

Why this is vulnerable: An ownership check needs a resource to check against, and a collection route names none - so the check that guards /orders/:id has nothing to attach to on /orders and is simply absent. The correctly-protected detail route above it is what lets this survive review: the ownership logic is visibly present in the file. It is also the cheaper attack, needing no ID guessing at all - one request returns the table.

Middleware Registered After the Route It Should Guard

const adminRouter = express.Router();

adminRouter.get('/billing', getBilling);

// VULNERABLE - registered after the route, so it never runs for it
adminRouter.use(authenticate, requireRole('admin'));

app.use('/admin', adminRouter);

Why this is vulnerable: Express matches layers in registration order, so a use() added below a route is only reached by requests that fall past that route - which a matching request never does. There is no warning and nothing in the route definition looks wrong; the middleware is present, correct, and unreachable. Measured on Express 5.2.1 and Node 24.3, GET /admin/billing returned 200 with the billing body against this arrangement, and 401 once the use() moved above the route. The same happens one level up when a global app.use(authenticate) is added after app.use('/admin', adminRouter), which also returned 200.

Secure Patterns

Role-Check Middleware Attached to the Route

function requireRole(...roles) {
  return (req, res, next) => {
    if (!req.user) return res.sendStatus(401);
    if (!roles.includes(req.user.role)) return res.sendStatus(403);
    next();
  };
}

const router = express.Router();

// SECURE - authentication and role check both run as middleware before
// the handler executes
router.get('/admin/billing', authenticate, requireRole('admin'), async (req, res) => {
  const invoices = await Invoice.find({});
  res.json(invoices);
});

router.delete('/users/:id', authenticate, requireRole('admin'), async (req, res) => {
  await User.findByIdAndDelete(req.params.id);
  res.sendStatus(204);
});

Why this works: requireRole() reads the role from req.user, which is populated by authenticate from the verified token or session - never from data the caller supplies directly. Because the check is middleware declared in the route definition itself, there is no code path that reaches the handler without first passing through authenticate and requireRole.

Object-Level Authorization in the Query

// SECURE - ownership is part of the filter, so a non-owner and a
// nonexistent ID are the same miss and produce the same response
function scopeToCaller(req) {
  return req.user.role === 'admin' ? {} : { userId: req.user.id };
}

router.get('/orders/:id', authenticate, async (req, res) => {
  const order = await Order.findOne({ _id: req.params.id, ...scopeToCaller(req) });
  if (!order) return res.sendStatus(404);
  res.json(order);
});

// The same scope reaches the collection route, where an ownership check
// applied to a loaded object has nothing to attach to
router.get('/orders', authenticate, async (req, res) => {
  const orders = await Order.find(scopeToCaller(req));
  res.json(orders);
});

Why this works: The ownership term is part of what is asked, not a check applied to the answer. The row is never loaded for a caller who may not have it, and scopeToCaller() constrains the collection route as well as the detail route - a find() that forgets it is visibly different from one that has it, whereas a missing post-load comparison on a list route looks like nothing at all.

Collapsing the two failures is deliberate. An order owned by someone else and an _id matching no document both produce null and the same 404, so walking :id cannot map which orders exist. The load-then-check alternative - findById() and then sendStatus(403) when the owner does not match - answers 404 for one and 403 for the other, and that gap is opened by adding the ownership check rather than closed by it.

Mongoose casts the string in userId to an ObjectId when building the filter, which is the other reason to prefer the query. A comparison written by hand does not get that: order.userId is an ObjectId and req.user.id a string from the token, so order.userId !== req.user.id is true even for the owner. That fails closed, which reads as safe and is still broken - it refuses the legitimate owner while every rejection test passes and the scanner sees a fixed finding. Where a post-load comparison is unavoidable, normalize both sides (String(a) === String(b) or order.userId.equals(req.user.id)) and assert the owner's request succeeds, not only that a stranger's fails.

Deriving Role From the Server, Not the Request

// SECURE - new user's role is a fixed default; req.body.role is ignored
router.post('/users', authenticate, requireRole('admin'), async (req, res) => {
  const user = await User.create({
    username: req.body.username,
    role: 'standard_user',
  });
  res.status(201).json(user);
});

Why this works: requireRole('admin') ensures only administrators can reach this endpoint, and the new user's role is set from a fixed value in server code rather than trusted from the request body - closing the privilege-escalation path even if an attacker adds a role field to the payload.

Router-Level Middleware for Consistent Coverage

// SECURE - every route under /admin requires authentication and the
// admin role, including routes added later
const adminRouter = express.Router();
adminRouter.use(authenticate, requireRole('admin'));

adminRouter.get('/billing', getBilling);
adminRouter.get('/users', listUsers);
adminRouter.delete('/users/:id', deleteUser);

app.use('/admin', adminRouter);

Why this works: Applying authenticate and requireRole('admin') with router.use() at the router level means every route mounted on adminRouter - present and future - inherits the same check automatically, instead of relying on each new route remembering to add it inline.

Framework-Specific Guidance

Express

  • Prefer attaching authorization middleware in the route definition (router.get(path, authenticate, requireRole(...), handler)) over checking req.user.role manually inside the handler body - it keeps the check visible in the route table and impossible to skip by accident.
  • Cover every HTTP verb on a resource path. A GET that leaks another user's data is exploited exactly as easily as an unprotected DELETE.
  • If using a permission library such as express-jwt-permissions or casl, define permissions centrally and reference them by name in route middleware, rather than re-deriving role logic in each handler.

NestJS

NestJS applications typically implement the same middleware pattern as declarative guards: a custom RolesGuard implementing CanActivate, applied with @UseGuards(AuthGuard, RolesGuard) and a @Roles('admin') decorator on the controller method. The guard runs before the handler and reads role/permission data from the authenticated request, not from the request body.

Testing

A re-scan sees the middleware in the route definition. It cannot tell whether the middleware is reachable, whether the check permits the people it should, or which of two denials a caller received. Drive these through the HTTP layer with supertest and two fixture users, alice and bob:

  • The owner still gets their own record. alice requesting her own order returns 200 with the order body. Put this first: a comparison between a Mongoose ObjectId and a token's string ID refuses the owner and passes every rejection assertion below unchanged.
  • The two denials are indistinguishable. alice requesting bob's order and alice requesting an ID that matches no document return the same status and the same body. A 403 for one and a 404 for the other enumerates the collection one request at a time.
  • The collection route is scoped. GET /orders as alice returns exactly her orders - assert the IDs and the length, not 200. An ownership check written for /orders/:id never runs here, so this route passes an object-level review while returning every document.
  • The middleware is reachable. Assert the 401 for an unauthenticated request against every route in the group, including ones added after the router.use() line. A use() registered below a route never runs for it, and the route definition gives no sign - measured on Express 5.2.1, that arrangement answered 200 with the protected body.
  • Every verb on the path is covered. Repeat the cross-owner request for GET, PUT, PATCH and DELETE on the same path. A requireRole added to one verb says nothing about the others, and the GET is what leaks the data.
  • Client-supplied privilege fields are ignored. POST a body carrying role, isAdmin or permissions set to an elevated value as an authorized caller, then read the persisted document and assert the field holds the server-side default. A response that omits the field is not evidence it was not written.

Common Pitfalls

  • Checking authentication and calling it authorization: authenticate middleware confirming a valid token is not the same as confirming the caller may perform this specific action - a route with only authenticate and no role/permission check is open to any logged-in user.
  • Scattering checks inline instead of using middleware: an if (req.user.role !== 'admin') written inside one handler is easy to forget when a new route is added later; middleware attached at the route or router level cannot be skipped by accident the same way.
  • Trusting role or permission fields in the request body: any value the client sends - including inside a JWT-shaped object the client itself constructed for a request body, not the verified token - must be treated as attacker-controlled. Only the verified, server-issued token or session is a trustworthy source for req.user.
  • Covering some HTTP verbs but not others on the same path: adding requireRole('admin') to a DELETE route while leaving the corresponding GET on the same resource unprotected still exposes the underlying data.

Additional Resources