N21-1826 fixes roles mismatch (#4858)

* fixes possible data constellation of concatenated teacherRoles with own ldap role attribute (non-group)
hpi-schul-cloud · Mar 22, 2024 · c9be0d2 · c9be0d2
1 parent fed624c
commit c9be0d2
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 10 deletions.
diff --git 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
@@ -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');
 			});