REST API: 401/403 semantics inverted — `error-unauthorized` means permission denied, `error-invalid-user` means unauthenticated

[ggazzo]

Summary

In the REST API, the HTTP status codes for authentication and authorization failures are inverted relative to their meaning:

• "You are not logged in" is reported with error-invalid-user, which falls through to API.v1.failure() → HTTP 400.
• "You are logged in but not allowed" is reported with error-unauthorized / error-not-authorized; error-unauthorized is mapped to HTTP 401 once the 9.0.0 breaking-change flag turns on.

So the only error code promoted to 401 is one that never means "unauthenticated", and the condition that genuinely is 401 answers 400. Three endpoints already return 401 for authorization failures today, independent of the flag.

This matters beyond tidiness: a client cannot infer session state from the status code. Any client-side handling of "session expired" (redirect to login, clear stored credentials) built on a 401 will fire on permission denials instead, and will miss the real unauthenticated case.

Census

Counted over apps/meteor/server, apps/meteor/app, apps/meteor/ee (specs excluded), classifying each throw by the guard that precedes it.

Actual intent	Error code	Sites	Status today	Status in 9.0.0
Not authenticated (no session)	error-invalid-user	134	400	400
Not authenticated (no session)	error-action-not-allowed, error-not-allowed	3	400	400
Authenticated, no permission	error-not-authorized	27	400	400
Authenticated, no permission	error-unauthorized	4	403	401

error-invalid-user has 244 occurrences in total; the 134 above are the subset guarded by a missing-session check (if (!this.userId), if (!Meteor.userId())). The rest use the same code for "that user does not exist", so the code carries two unrelated meanings.

Every error-unauthorized / error-not-authorized throw is an authorization check

No site among the 31 means "unauthenticated":

server/lib/users/setUserAvatar.ts:47              !hasPermissionAsync('edit-other-user-avatar')
server/meteor-methods/users/resetAvatar.ts:32     !canEditOtherUserAvatar
server/meteor-methods/platform/cloud.ts           10 permission checks
server/meteor-methods/rooms/archiveRoom.ts:38     permission check
server/meteor-methods/rooms/unarchiveRoom.ts:33   permission check
server/meteor-methods/rooms/getRoomJoinCode.ts:27 permission check
server/meteor-methods/messages/deleteFileMessage.ts:51   !Upload.canDeleteFile()
server/meteor-methods/auth/createToken.ts:16,20   secret mismatch, 'user-generate-access-token'
server/meteor-methods/auth/crowd.ts:28,60         permission check
server/api/lib/rooms.ts:27,68,113                 permission scope
server/api/v1/omnichannel/lib/inquiries.ts:28     department not allowed for agent
server/slashcommands/archiveroom/server.ts:57     permission check
server/slashcommands/unarchiveroom/server.ts:56   permission check
server/bridges/slack/removeChannelLinks.ts:25     permission check
server/lib/omnichannel/closeLivechatRoom.ts:39    permission check
ee/server/api/ldap.ts:36                          permission check
ee/server/lib/omnichannel/business-hour/lib/business-hour.ts:14   permission check

One partial exception: server/api/v1/middlewares/authenticationHono.ts:42 throws error-unauthorized for 'Users must have a username', which is closer to an unusable account than to a permission denial.

Where it goes wrong

1. The 9.0.0 mapping promotes permission denials to 401

apps/meteor/server/api/ApiClass.ts:900-906

case 'unauthorized':
case 'error-unauthorized':
    if (applyBreakingChanges) {
        return api.unauthorized(typeof e === 'string' ? e : e.message);   // 401
    }
    return api.forbidden(typeof e === 'string' ? e : e.message);          // 403

applyBreakingChanges = shouldBreakInVersion('9.0.0') (ApiClass.ts:63), so this is inert on 8.x and active from 9.0.0.

The case pair assumes error-unauthorized = unauthenticated and error-forbidden = no permission. The census above shows the codebase does not use it that way. The permission middleware disagrees with the switch as well — for the very same condition it answers 403 while embedding the code in the message (server/api/v1/middlewares/permissions.ts:28,47,51):

API.v1.forbidden('User does not have the permissions required for this action [error-unauthorized]')

Result in 9.0.0: the same permission denial returns 403 when the check lives in the middleware and 401 when it lives in the handler.

2. error-not-authorized is never mapped

The dominant permission code (27 sites) is not handled by the switch and falls to default: → API.v1.failure() → HTTP 400, both today and in 9.0.0.

3. Three endpoints return 401 for authorization failures today

Location	Condition	Returns	Should be
server/api/v1/rooms.ts:1155	!hasPermissionAsync(user, 'view-broadcast-member-list', rid)	401	403
server/api/v1/rooms.ts:1301 (rooms.hide)	!canAccessRoomIdAsync(roomId, userId)	401	403
server/api/v1/rooms.ts:1704 (rooms.bannedUsers)	!canAccessRoomIdAsync(roomId, userId)	401	403

Their response schemas declare 401: validateUnauthorizedErrorResponse, so the published contract carries the mistake. Note also that rooms.ts:1147, in the same handler as line 1155, answers notFound for an access failure — three different treatments of the same class of condition in one file.

The only legitimate 401 producers are authenticationHono.ts:29 and permissions.ts:24 (both !user), and ApiClass.ts:1079,1095 (failed login).

4. Dead branch in the permission middleware

server/api/v1/middlewares/permissions.ts:45-52 — both sides of the if (applyBreakingChanges) are identical:

if (!hasPermission) {
    if (applyBreakingChanges) {
        const forbidden = API.v1.forbidden('User does not have the permissions required for this action [error-unauthorized]');
        return c.json(forbidden.body, forbidden.statusCode);
    }
    const failure = API.v1.forbidden('User does not have the permissions required for this action [error-unauthorized]');
    return c.json(failure.body, failure.statusCode);
}

The variable in the legacy branch is named failure, suggesting API.v1.failure was intended there and the if/else was left half-finished.

Why the DDP → REST migration makes this urgent

The 134 unauthenticated checks live almost entirely in apps/meteor/server/meteor-methods/. Called over DDP, the error code reaches the client verbatim and no status code is involved. As each method is migrated to a typed route, that same check starts answering HTTP 400 — indistinguishable from a validation error.

This already surfaced in the sendMessage / getReadReceipts migration (#40675), where a client-side handler that treated any REST 401 as an expired session logged out live sessions. That handler was reverted, and it cannot be reimplemented correctly until 401 has a single meaning.

Proposed fix, in order

1. ApiClass.ts:900-906 — map error-unauthorized and error-not-authorized to forbidden() (403). This removes 401 from permission denials before 9.0.0 activates it.
2. Introduce a dedicated code for the unauthenticated case (e.g. error-unauthenticated) mapped to 401, and switch the ~134 missing-session guards to it. A blanket remap of error-invalid-user is not viable because the code also means "user not found".
3. rooms.ts:1155,1301,1704 — return 403 and update the response schemas.
4. permissions.ts:45-52 — resolve the dead branch.

Steps 1 and 2 are breaking for API consumers (403 stays 403; the error-not-authorized cases move 400 → 403; missing-session cases move 400 → 401), so they belong in the 9.0.0 batch.

[coderabbitai]

🔗 Related PRs

#40704 - chore: migrate low-friction DDP callers to REST endpoints [merged]
#40728 - chore: migrate batch4 client DDP callers + add 6 REST endpoints [merged]
#40737 - chore: migrate OAuth services admin DDP methods to REST [merged]
#41367 - refactor(api): pass this.user to permission checks in REST endpoints [merged]
#41455 - chore: migrate web-ui-registration DDP callers to REST (resetPassword, checkRegistrationSecretURL) [open]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

• Create Plan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!

[ggazzo]

Tracked internally as ARCH-2297 (Tech Debt), scoped for the 9.0.0 breaking-change batch alongside ARCH-2166 (DDP → REST method migration).

[ggazzo]

Another asymmetry in the same family, found while writing tests for a migrated endpoint: the typed router reports a different error code depending on which validator rejects.

packages/http-router/src/Router.ts:

// 223 — query validator rejected
errorType: 'error-invalid-params',
// 248 — body validator rejected
errorType: 'invalid-params',

Same class of failure (request did not match the schema), same 400 status, two different errorType values that differ only by the error- prefix. So a client cannot match on one code, and a test asserting the wrong branch passes review and fails in CI — which is exactly what happened on #41593, where POST /v1/chat.readThread validates a body while the sibling GET suites validate queries.

Fold into the same pass: pick one form (the prefixed one, matching every other code in the codebase) and update the affected endpoint tests.