From c9be0d2f58da95f23e3ef211ada74d9bf1510ce0 Mon Sep 17 00:00:00 2001
From: agnisa-cap <arne.gnisa@capgemini.com>
Date: Thu, 21 Mar 2024 17:09:22 +0100
Subject: [PATCH] N21-1826 fixes roles mismatch (#4858)

* fixes possible data constellation of concatenated teacherRoles with own ldap role attribute (non-group)
---
 src/services/ldap/strategies/general.js       |  8 +++-----
 test/services/ldap/strategies/general.test.js | 13 ++++++++-----
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/services/ldap/strategies/general.js b/src/services/ldap/strategies/general.js
index fa48dc2c4ff..e95a5487feb 100644
--- a/src/services/ldap/strategies/general.js
+++ b/src/services/ldap/strategies/general.js
@@ -97,11 +97,9 @@ class GeneralLDAPStrategy extends AbstractLDAPStrategy {
 				if (userRole === roleAttributeNameMapping.roleStudent) {
 					roles.push('student');
 				}
-				splittedTeacherRoles.forEach((role) => {
-					if (userRole.includes(role)) {
-						roles.push('teacher');
-					}
-				});
+				if (splittedTeacherRoles.includes(userRole)) {
+					roles.push('teacher');
+				}
 				if (userRole === roleAttributeNameMapping.roleAdmin) {
 					roles.push('administrator');
 				}
diff --git a/test/services/ldap/strategies/general.test.js b/test/services/ldap/strategies/general.test.js
index 9154e32aaa5..0c08eadef21 100644
--- a/test/services/ldap/strategies/general.test.js
+++ b/test/services/ldap/strategies/general.test.js
@@ -6,6 +6,8 @@ const appPromise = require('../../../../src/app');
 const AbstractLDAPStrategy = require('../../../../src/services/ldap/strategies/interface');
 const GeneralLDAPStrategy = require('../../../../src/services/ldap/strategies/general');
 
+const teacherRole1 = 'cn=ROLE_TEACHER,ou=roles,o=school0,dc=de,dc=example,dc=org';
+const teacherRole2 = 'cn=OTHER_TEACHERS,ou=roles,o=school0,dc=de,dc=example,dc=org';
 const mockLDAPConfig = {
 	url: 'ldaps://foo.bar:636',
 	rootPath: 'o=school0,dc=de,dc=example,dc=org',
@@ -23,8 +25,7 @@ const mockLDAPConfig = {
 		},
 		roleAttributeNameMapping: {
 			roleStudent: 'cn=ROLE_STUDENT,ou=roles,o=school0,dc=de,dc=example,dc=org',
-			roleTeacher:
-				'cn=ROLE_TEACHER,ou=roles,o=school0,dc=de,dc=example,dc=org;;cn=OTHER_TEACHERS,ou=roles,o=school0,dc=de,dc=example,dc=org',
+			roleTeacher: `${teacherRole1};;${teacherRole2}`,
 			roleAdmin: 'cn=ROLE_ADMIN,ou=roles,o=school0,dc=de,dc=example,dc=org',
 		},
 		classAttributeNameMapping: {
@@ -214,13 +215,15 @@ describe('GeneralLDAPStrategy', () => {
 					get: () => {},
 					searchCollection: sinon.fake.resolves([
 						createLDAPUserResult({ role: mockLDAPConfig.providerOptions.roleAttributeNameMapping.roleStudent }),
-						createLDAPUserResult({ role: mockLDAPConfig.providerOptions.roleAttributeNameMapping.roleTeacher }),
+						createLDAPUserResult({ role: teacherRole1 }),
+						createLDAPUserResult({ role: teacherRole2 }),
 						createLDAPUserResult({ role: mockLDAPConfig.providerOptions.roleAttributeNameMapping.roleAdmin }),
 					]),
 				});
-				const [student, teacher, admin] = await new GeneralLDAPStrategy(app, ldapConfig).getUsers();
+				const [student, teacher1, teacher2, admin] = await new GeneralLDAPStrategy(app, ldapConfig).getUsers();
 				expect(student.roles).to.include('student');
-				expect(teacher.roles).to.include('teacher');
+				expect(teacher1.roles).to.include('teacher');
+				expect(teacher2.roles).to.include('teacher');
 				expect(admin.roles).to.include('administrator');
 			});