Skip to content

Don't re-parse Authorization header in logout handlers, and audit revoke result #27

@Loule95450

Description

@Loule95450

Context

/auth/logout in src/routes/auth.ts:382-399 runs the bearerAuth middleware (so req.auth is populated) and also re-parses the Authorization header to call sessions.revoke(token).

Problem / Observation

The re-parse logic at line 386-390 is dead defensiveness — by the time we reach the handler, the middleware has already validated the same header. But it has a real failure mode: a malformed Authorization: Bearer (trailing whitespace) could resolve in the middleware (.trim() happens in bearerAuth) but the handler's header.slice(...).trim() on the same string is fine. The bigger issue: the handler swallows the revoke result. If sessions.revoke returns false (e.g. the session was just purged by the background timer between middleware lookup and handler execution), the audit log records logout and the client thinks logout succeeded.

This is mostly cosmetic, but the duplicated parsing + ignored return value is exactly the shape of code that hides bugs.

Proposed approach

  • Stash the resolved tokenHash on req.auth in the middleware (src/middleware/auth.ts), or pass it through a request-scoped key.
  • In the handler, call sessions.revokeByHash(...). Log the audit event only if changes > 0 or always but include the result in metadata.
  • Same change for /admin/auth/logout (admin.ts:256-262) which has the same shape.

Acceptance criteria

  • Logout handler no longer re-parses the Authorization header
  • Revoke return value is honored or audited
  • /admin/auth/logout receives the same treatment

Metadata

Metadata

Assignees

No one assigned

    Labels

    P3Low prioritybugSomething isn't workingsecuritySecurity-related issue

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions