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

.changeset/real-mails-count.md

@@ -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

apps/meteor/ee/server/api/ldap.ts

@@ -29,7 +29,7 @@ API.v1.post(
 	},
 	async function action() {
 		if (!this.userId) {
-			throw new Error('error-invalid-user');
+			throw new Error('unauthorized');
 		}
 
 		if (!(await hasPermissionAsync(this.user, 'sync-auth-services-users'))) {

apps/meteor/server/api/ApiClass.ts

@@ -895,23 +895,27 @@ export class APIClass<TBasePath extends string = '', TOperations extends Record<
 							}
 						} catch (e: any) {
 							result = ((e: any) => {
-								switch (e.error) {
+								const errorKey = typeof e === 'string' ? e : e?.error ?? e?.message;
+								const errorMessage = typeof e === 'string'
+									? e
+									: e?.message || e?.reason || (typeof e?.error === 'string' ? e.error : undefined) || String(e);
+								switch (errorKey) {
 									case 'error-too-many-requests':
-										return api.tooManyRequests(typeof e === 'string' ? e : e.message);
+										return api.tooManyRequests(errorMessage);
 									case 'unauthorized':
+									case 'error-invalid-user':
+										return api.unauthorized(errorMessage);
 									case 'error-unauthorized':
-										if (applyBreakingChanges) {
-											return api.unauthorized(typeof e === 'string' ? e : e.message);
-										}
-										return api.forbidden(typeof e === 'string' ? e : e.message);
+									case 'error-not-authorized':
+										return api.forbidden(errorMessage);
 									case 'forbidden':
 									case 'error-forbidden':
 										if (applyBreakingChanges) {
-											return api.forbidden(typeof e === 'string' ? e : e.message);
+											return api.forbidden(errorMessage);
 										}
-										return api.failure(typeof e === 'string' ? e : e.message, e.error, process.env.TEST_MODE ? e.stack : undefined, e);
+										return api.failure(errorMessage, e?.error, process.env.TEST_MODE ? e?.stack : undefined, e);
 									default:
-										return api.failure(typeof e === 'string' ? e : e.message, e.error, process.env.TEST_MODE ? e.stack : undefined, e);
+										return api.failure(errorMessage, e?.error, process.env.TEST_MODE ? e?.stack : undefined, e);
 								}
 							})(e);
 						} finally {

apps/meteor/server/api/v1/ldap.ts

@@ -31,7 +31,7 @@ API.v1.post(
 	},
 	async function action() {
 		if (!this.userId) {
-			throw new Error('error-invalid-user');
+			throw new Error('unauthorized');
 		}
 
 		if (settings.get<boolean>('LDAP_Enable') !== true) {
@@ -65,7 +65,7 @@ API.v1.post(
 	},
 	async function action() {
 		if (!this.userId) {
-			throw new Error('error-invalid-user');
+			throw new Error('unauthorized');
 		}
 
 		if (settings.get<boolean>('LDAP_Enable') !== true) {

apps/meteor/server/api/v1/middlewares/permissions.ts

@@ -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();

apps/meteor/server/api/v1/rooms.ts

@@ -736,6 +736,7 @@ API.v1.get(
 			}),
 			400: validateBadRequestErrorResponse,
 			401: validateUnauthorizedErrorResponse,
+			403: validateForbiddenErrorResponse,
 		},
 	},
 	async function action() {
@@ -775,6 +776,7 @@ API.v1.get(
 			}),
 			400: validateBadRequestErrorResponse,
 			401: validateUnauthorizedErrorResponse,
+			403: validateForbiddenErrorResponse,
 		},
 	},
 	async function action() {
@@ -810,6 +812,7 @@ API.v1.get(
 			}),
 			400: validateBadRequestErrorResponse,
 			401: validateUnauthorizedErrorResponse,
+			403: validateForbiddenErrorResponse,
 		},
 	},
 	async function action() {
@@ -1136,6 +1139,7 @@ API.v1.get(
 			}),
 			400: validateBadRequestErrorResponse,
 			401: validateUnauthorizedErrorResponse,
+			403: validateForbiddenErrorResponse,
 			404: validateNotFoundErrorResponse,
 		},
 	},
@@ -1154,7 +1158,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();
 		}
 
 		// Ensures that role priorities for the specified room are synchronized correctly.
@@ -1294,13 +1298,14 @@ API.v1.post(
 			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 } });
@@ -1697,13 +1702,14 @@ export const roomEndpoints = API.v1
 			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);

docs/api-endpoint-migration.md

@@ -597,7 +597,7 @@ expect(res.body).to.have.property('errorType', 'invalid-params');
 expect(res.body).to.have.property('errorType', 'error-invalid-params');
 ```
 
-This only affects **query** parameter validation (GET/DELETE). Body parameter validation (POST/PUT) keeps `'invalid-params'`.
+This affects both **query** parameter validation (GET/DELETE) and **body** parameter validation (POST/PUT).
 
 ### Error message format changes
 
@@ -615,7 +615,7 @@ expect(res.body).to.have.property('error', "must have required property 'platfor
 
 When migrating an endpoint, search for its tests and update:
 
-1. `errorType` from `'invalid-params'` to `'error-invalid-params'` (for query params only)
+1. `errorType` from `'invalid-params'` to `'error-invalid-params'` for query and body params
 2. Remove `' [invalid-params]'` suffix from `error` message assertions
 3. Verify that status codes remain the same (400 for validation errors)
 

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

@@ -207,12 +207,12 @@ describe('Router', () => {
 			const invalidResponse = await request(app).post('/api/validate-body').send({ name: 'John' });
 
 			expect(invalidResponse.status).toBe(400);
-			expect(invalidResponse.body).toHaveProperty('errorType', 'invalid-params');
+			expect(invalidResponse.body).toHaveProperty('errorType', 'error-invalid-params');
 
 			const invalidTypeResponse = await request(app).post('/api/validate-body').send({ name: 'John', age: 'thirty' });
 
 			expect(invalidTypeResponse.status).toBe(400);
-			expect(invalidTypeResponse.body).toHaveProperty('errorType', 'invalid-params');
+			expect(invalidTypeResponse.body).toHaveProperty('errorType', 'error-invalid-params');
 		});
 
 		it('should validate response body in test mode', async () => {

packages/http-router/src/Router.ts

@@ -245,7 +245,7 @@ export class Router<
 					return c.json(
 						{
 							success: false,
-							errorType: 'invalid-params',
+							errorType: 'error-invalid-params',
 							error: validatorFn.errors?.map((error: any) => error.message).join('\n '),
 						},
 						400,