Skip to content

fix(api)!: return 403 for authorization failures, reserve 401 for missing session #41658

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
surjeetkumar8006 wants to merge 2 commits into from
Closed

fix(api)!: return 403 for authorization failures, reserve 401 for missing session #41658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b7abb0
fix(api)!: return 403 for authorization failures, reserve 401 for mis…
surjeetkumar800 Aug 2, 2026
7b2e48c
chore: add changeset for PR #41658
surjeetkumar800 Aug 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/real-mails-count.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@rocket.chat/meteor": patch
"@rocket.chat/http-router": patch
---

fix(api)!: return 403 for authorization failures, reserve 401 for missing session
4 changes: 1 addition & 3 deletions apps/meteor/server/api/ApiClass.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -900,9 +900,7 @@ export class APIClass<TBasePath extends string = '', TOperations extends Record<
return api.tooManyRequests(typeof e === 'string' ? e : e.message);
case 'unauthorized':
case 'error-unauthorized':
if (applyBreakingChanges) {
return api.unauthorized(typeof e === 'string' ? e : e.message);
}
case 'error-not-authorized':
return api.forbidden(typeof e === 'string' ? e : e.message);
case 'forbidden':
case 'error-forbidden':
Expand Down
9 changes: 2 additions & 7 deletions apps/meteor/server/api/v1/middlewares/permissions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,8 @@ export const permissionsMiddleware =
}

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);
const forbidden = API.v1.forbidden('User does not have the permissions required for this action [error-unauthorized]');
return c.json(forbidden.body, forbidden.statusCode);
}

return next();
Expand Down
10 changes: 5 additions & 5 deletions apps/meteor/server/api/v1/rooms.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1154,7 +1154,7 @@ API.v1.get(
}

if (findResult.broadcast && !(await hasPermissionAsync(this.user, 'view-broadcast-member-list', findResult._id))) {
return API.v1.unauthorized();
return API.v1.forbidden();

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Register the new 403 response

For rooms.membersOrderedByRole, the response spec above this action still only registers 200/400/401/404. When this broadcast-room permission branch is hit under TEST_MODE, the typed router looks up a validator for status 403 and throws Missing response validator... instead of returning the intended forbidden response; it also leaves the generated route metadata without the new 403. Add 403: validateForbiddenErrorResponse to this endpoint's response map along with the behavior change.

Useful? React with 👍 / 👎.

}

// Ensures that role priorities for the specified room are synchronized correctly.
Expand Down Expand Up @@ -1293,14 +1293,14 @@ API.v1.post(
response: {
200: successResponseSchema,
400: validateBadRequestErrorResponse,
401: validateUnauthorizedErrorResponse,
403: validateForbiddenErrorResponse,
},
},
async function action() {
const { roomId } = this.bodyParams;

if (!(await canAccessRoomIdAsync(roomId, this.userId))) {
return API.v1.unauthorized();
return API.v1.forbidden();
}

const user = await Users.findOneById(this.userId, { projection: { _id: 1 } });
Expand Down Expand Up @@ -1696,14 +1696,14 @@ export const roomEndpoints = API.v1
query: isRoomsBannedUsersProps,
response: {
200: roomsBannedUsersResponseSchema,
401: validateUnauthorizedErrorResponse,
403: validateForbiddenErrorResponse,
},
},
async function action() {
const { roomId } = this.queryParams;

if (!(await canAccessRoomIdAsync(roomId, this.userId))) {
return API.v1.unauthorized();
return API.v1.forbidden();
}

const { offset, count } = await getPaginationItems(this.queryParams);
Expand Down
2 changes: 1 addition & 1 deletion packages/http-router/src/Router.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ export class Router<
return c.json(
{
success: false,
errorType: 'invalid-params',
errorType: 'error-invalid-params',

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Update body-validation expectations

When packages/http-router/src/Router.spec.ts runs, the two invalid body cases still assert errorType is invalid-params; this line now emits error-invalid-params, so the existing package test fails even though the behavior change is intentional. Please update those expectations in the same change, or preserve the old emitted value.

Useful? React with 👍 / 👎.

error: validatorFn.errors?.map((error: any) => error.message).join('\n '),
},
400,
Expand Down