Regression: Fix LDAP sync route (#23775)

Co-authored-by: Diego Sampaio <chinello@gmail.com>
RocketChat · Nov 23, 2021 · f9b2070 · f9b2070
1 parent 8ccbf9f
commit f9b2070
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 11 deletions.
diff --git a/.mocharc.api.js b/.mocharc.api.js
@@ -11,6 +11,7 @@ module.exports = {
 	file: 'tests/end-to-end/teardown.js',
 	spec: [
 		'tests/end-to-end/api/*.js',
+		'tests/end-to-end/api/*.ts',
 		'tests/end-to-end/apps/*.js',
 	],
 };
diff --git a/app/api/server/api.d.ts b/app/api/server/api.d.ts
@@ -43,19 +43,21 @@ type UnauthorizedResult<T> = {
 
 export type NonEnterpriseTwoFactorOptions = {
 	authRequired: true;
-	twoFactorRequiredNonEnterprise: true;
+	forceTwoFactorAuthenticationForNonEnterprise: true;
 	twoFactorRequired: true;
 	permissionsRequired?: string[];
 	twoFactorOptions: ITwoFactorOptions;
 }
 
 type Options = {
 	permissionsRequired?: string[];
-	twoFactorOptions?: ITwoFactorOptions;
-	twoFactorRequired?: boolean;
 	authRequired?: boolean;
-	twoFactorRequiredNonEnterprise?: true;
-};
+	forceTwoFactorAuthenticationForNonEnterprise?: boolean;
+} | {
+	authRequired: true;
+	twoFactorRequired: true;
+	twoFactorOptions?: ITwoFactorOptions;
+}
 
 type Request = {
 	method: 'GET' | 'POST' | 'PUT' | 'DELETE';

diff --git a/app/api/server/api.js b/app/api/server/api.js
@@ -273,6 +273,9 @@ export class APIClass extends Restivus {
 	}
 
 	processTwoFactor({ userId, request, invocation, options, connection }) {
+		if (!options.twoFactorRequired) {
+			return;
+		}
 		const code = request.headers['x-2fa-code'];
 		const method = request.headers['x-2fa-method'];
 
@@ -399,9 +402,7 @@ export class APIClass extends Restivus {
 						};
 						Accounts._setAccountData(connection.id, 'loginToken', this.token);
 
-						if (_options.twoFactorRequired) {
-							api.processTwoFactor({ userId: this.userId, request: this.request, invocation, options: _options, connection });
-						}
+						api.processTwoFactor({ userId: this.userId, request: this.request, invocation, options: _options, connection });
 
 						result = DDP._CurrentInvocation.withValue(invocation, () => Promise.await(originalAction.apply(this))) || API.v1.success();
 

diff --git a/ee/server/api/api.ts b/ee/server/api/api.ts
@@ -7,8 +7,8 @@ import { isEnterprise } from '../../app/license/server/license';
 
 export const isNonEnterpriseTwoFactorOptions = (options?: Options):
 	options is NonEnterpriseTwoFactorOptions => !!options
-	&& 'twoFactorRequiredNonEnterprise' in options
-	&& Boolean(options.twoFactorRequiredNonEnterprise);
+	&& 'forceTwoFactorAuthenticationForNonEnterprise' in options
+	&& Boolean(options.forceTwoFactorAuthenticationForNonEnterprise);
 
 API.v1.processTwoFactor = use(API.v1.processTwoFactor, function([params, ...context], next) {
 	if (isNonEnterpriseTwoFactorOptions(params.options) && !isEnterprise()) {

diff --git a/ee/server/api/ldap.ts b/ee/server/api/ldap.ts
@@ -3,7 +3,11 @@ import { settings } from '../../../app/settings/server';
 import { API } from '../../../app/api/server/api';
 import { LDAPEE } from '../sdk';
 
-API.v1.addRoute('ldap.syncNow', { authRequired: true, twoFactorRequiredNonEnterprise: true }, {
+API.v1.addRoute('ldap.syncNow', {
+	authRequired: true,
+	forceTwoFactorAuthenticationForNonEnterprise: true,
+	twoFactorRequired: true,
+}, {
 	async post() {
 		if (!this.userId) {
 			throw new Error('error-invalid-user');

diff --git a/tests/end-to-end/api/26-LDAP.ts b/tests/end-to-end/api/26-LDAP.ts
@@ -0,0 +1,24 @@
+import { expect } from 'chai';
+import type { Response } from 'supertest';
+
+import { getCredentials, api, request, credentials } from '../../data/api-data.js';
+
+describe('LDAP', function() {
+	this.retries(0);
+
+	before((done) => getCredentials(done));
+
+	describe('[/ldap.syncNow]', () => {
+		it('should throw an error containing totp-required error ', (done) => {
+			request.post(api('ldap.syncNow'))
+				.set(credentials)
+				.expect('Content-Type', 'application/json')
+				.expect(400)
+				.expect((res: Response) => {
+					expect(res.body).to.have.property('success', false);
+					expect(res.body).to.have.property('errorType', 'totp-required');
+				})
+				.end(done);
+		});
+	});
+});