N21-1563 ldap-sync-multi-teacher-cn (#4679)

* N21-1563 adds possibility to have multiple cns for teacher role in systems ldapConfig.providerOptions.roleAttributeNameMapping
hpi-schul-cloud · Jan 8, 2024 · 59a64f7 · 59a64f7
1 parent 6e99df3
commit 59a64f7
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 12 deletions.
diff --git a/ansible/roles/schulcloud-server-init/templates/configmap_file_init.yml.j2 b/ansible/roles/schulcloud-server-init/templates/configmap_file_init.yml.j2
@@ -68,7 +68,7 @@ data:
           },
           "roleAttributeNameMapping" : {
             "roleStudent" : "cn=ROLE_STUDENT,ou=roles,o=schoolOne0,dc=de,dc=example,dc=org",
-            "roleTeacher" : "cn=ROLE_TEACHER,ou=roles,o=schoolOne0,dc=de,dc=example,dc=org",
+            "roleTeacher": "cn=ROLE_TEACHER,ou=roles,o=schoolOne0,dc=de,dc=example,dc=org;;cn=ROLE_SUBSTITUTE_TEACHER,ou=roles,o=schoolOne0,dc=de,dc=example,dc=org",
             "roleAdmin" : "cn=ROLE_ADMIN,ou=roles,o=schoolOne0,dc=de,dc=example,dc=org",
             "roleNoSc" : "cn=ROLE_NBC_EXCLUDE,ou=roles,o=schoolOne0,dc=de,dc=example,dc=org"
           },
@@ -116,7 +116,7 @@ data:
           },
           "roleAttributeNameMapping" : {
             "roleStudent" : "cn=ROLE_STUDENT,ou=roles,o=schoolOne1,dc=de,dc=example,dc=org",
-            "roleTeacher" : "cn=ROLE_TEACHER,ou=roles,o=schoolOne1,dc=de,dc=example,dc=org",
+            "roleTeacher": "cn=ROLE_TEACHER,ou=roles,o=schoolOne0,dc=de,dc=example,dc=org;;cn=ROLE_SUBSTITUTE_TEACHER,ou=roles,o=schoolOne0,dc=de,dc=example,dc=org",
             "roleAdmin" : "cn=ROLE_ADMIN,ou=roles,o=schoolOne1,dc=de,dc=example,dc=org",
             "roleNoSc" : "cn=ROLE_NBC_EXCLUDE,ou=roles,o=schoolOne1,dc=de,dc=example,dc=org"
           },

diff --git a/src/services/ldap/strategies/general.js b/src/services/ldap/strategies/general.js
@@ -67,6 +67,7 @@ class GeneralLDAPStrategy extends AbstractLDAPStrategy {
 			);
 		}
 
+		const splittedTeacherRoles = roleAttributeNameMapping.roleTeacher.split(';;');
 		const results = [];
 		ldapUsers.forEach((obj) => {
 			const roles = [];
@@ -77,9 +78,11 @@ class GeneralLDAPStrategy extends AbstractLDAPStrategy {
 				if (obj.memberOf.includes(roleAttributeNameMapping.roleStudent)) {
 					roles.push('student');
 				}
-				if (obj.memberOf.includes(roleAttributeNameMapping.roleTeacher)) {
-					roles.push('teacher');
-				}
+				splittedTeacherRoles.forEach((role) => {
+					if (obj.memberOf.includes(role)) {
+						roles.push('teacher');
+					}
+				});
 				if (obj.memberOf.includes(roleAttributeNameMapping.roleAdmin)) {
 					roles.push('administrator');
 				}
@@ -90,9 +93,11 @@ class GeneralLDAPStrategy extends AbstractLDAPStrategy {
 				if (obj[userAttributeNameMapping.role] === roleAttributeNameMapping.roleStudent) {
 					roles.push('student');
 				}
-				if (obj[userAttributeNameMapping.role] === roleAttributeNameMapping.roleTeacher) {
-					roles.push('teacher');
-				}
+				splittedTeacherRoles.forEach((role) => {
+					if (obj[userAttributeNameMapping.role].includes(role)) {
+						roles.push('teacher');
+					}
+				});
 				if (obj[userAttributeNameMapping.role] === roleAttributeNameMapping.roleAdmin) {
 					roles.push('administrator');
 				}

diff --git a/test/services/ldap/strategies/general.test.js b/test/services/ldap/strategies/general.test.js
@@ -23,7 +23,8 @@ 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',
+			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',
 			roleAdmin: 'cn=ROLE_ADMIN,ou=roles,o=school0,dc=de,dc=example,dc=org',
 		},
 		classAttributeNameMapping: {
@@ -137,6 +138,18 @@ describe('GeneralLDAPStrategy', () => {
 						mail: '[email protected]',
 						memberOf: 'cn=ROLE_ADMIN,ou=roles,o=school0,dc=de,dc=example,dc=org',
 					},
+					{
+						dn: 'uid=herr.anwalt,ou=users,o=school0,dc=de,dc=example,dc=org',
+						givenName: 'Herr',
+						sn: 'Anwalt',
+						uid: 'herr.anwalt',
+						uuid: 'ZDg0Y2ZlMjMtZGYwNi00MWNjLTg3YmUtZjI3NjA1NDJhY2Y4',
+						mail: '[email protected]',
+						memberOf: [
+							'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',
+						],
+					},
 				]),
 			};
 		}
@@ -150,7 +163,7 @@ describe('GeneralLDAPStrategy', () => {
 		it('should return all users', async () => {
 			const instance = new GeneralLDAPStrategy(app, mockLDAPConfig);
 			const users = await instance.getUsers();
-			expect(users.length).to.equal(4);
+			expect(users.length).to.equal(5);
 		});
 
 		it('should follow the internal interface', async () => {
@@ -178,11 +191,15 @@ describe('GeneralLDAPStrategy', () => {
 		});
 
 		it('should assign roles based on specific group memberships for group role type', async () => {
-			const [student1, student2, teacher, admin] = await new GeneralLDAPStrategy(app, mockLDAPConfig).getUsers();
+			const [student1, student2, teacher, admin, teacher2] = await new GeneralLDAPStrategy(
+				app,
+				mockLDAPConfig
+			).getUsers();
 			expect(student1.roles).to.include('student');
 			expect(student2.roles).to.include('student');
 			expect(teacher.roles).to.include('teacher');
 			expect(admin.roles).to.include('administrator');
+			expect(teacher2.roles).to.include('teacher');
 		});
 
 		it('should assign roles based on specific group memberships for non-group role type', async () => {
@@ -220,7 +237,7 @@ describe('GeneralLDAPStrategy', () => {
 						}),
 						createLDAPUserResult({
 							givenName: '',
-							memberOf: mockLDAPConfig.providerOptions.roleAttributeNameMapping.roleTeacher,
+							memberOf: mockLDAPConfig.providerOptions.roleAttributeNameMapping.roleTeacher.split(';;')[0],
 						}),
 						createLDAPUserResult({
 							givenName: '',