diff --git a/src/main/java/DNPM/security/AbstractDelegatedPermissionEvaluator.java b/src/main/java/DNPM/security/AbstractDelegatedPermissionEvaluator.java index 9d5f1ad..60e7ad2 100644 --- a/src/main/java/DNPM/security/AbstractDelegatedPermissionEvaluator.java +++ b/src/main/java/DNPM/security/AbstractDelegatedPermissionEvaluator.java @@ -3,11 +3,8 @@ package DNPM.security; import de.itc.onkostar.api.IOnkostarApi; import de.itc.onkostar.api.Patient; import de.itc.onkostar.api.Procedure; -import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.security.access.PermissionEvaluator; -import javax.sql.DataSource; - public abstract class AbstractDelegatedPermissionEvaluator implements PermissionEvaluator { protected static final String PATIENT = Patient.class.getSimpleName(); @@ -16,11 +13,11 @@ public abstract class AbstractDelegatedPermissionEvaluator implements Permission protected final IOnkostarApi onkostarApi; - protected final JdbcTemplate jdbcTemplate; + protected final SecurityService securityService; - protected AbstractDelegatedPermissionEvaluator(final IOnkostarApi onkostarApi, final DataSource dataSource) { + protected AbstractDelegatedPermissionEvaluator(final IOnkostarApi onkostarApi, final SecurityService securityService) { this.onkostarApi = onkostarApi; - this.jdbcTemplate = new JdbcTemplate(dataSource); + this.securityService = securityService; } } diff --git a/src/main/java/DNPM/security/FormBasedPermissionEvaluator.java b/src/main/java/DNPM/security/FormBasedPermissionEvaluator.java index 4ba19dc..912a19c 100644 --- a/src/main/java/DNPM/security/FormBasedPermissionEvaluator.java +++ b/src/main/java/DNPM/security/FormBasedPermissionEvaluator.java @@ -1,15 +1,11 @@ package DNPM.security; import de.itc.onkostar.api.IOnkostarApi; -import de.itc.onkostar.api.Patient; import de.itc.onkostar.api.Procedure; import org.springframework.security.core.Authentication; -import org.springframework.security.core.userdetails.UserDetails; import org.springframework.stereotype.Component; -import javax.sql.DataSource; import java.io.Serializable; -import java.util.List; /** * Permission-Evaluator zur Auswertung der Berechtigung auf Objekte aufgrund der Formularberechtigung @@ -17,8 +13,8 @@ import java.util.List; @Component public class FormBasedPermissionEvaluator extends AbstractDelegatedPermissionEvaluator { - public FormBasedPermissionEvaluator(final IOnkostarApi onkostarApi, final DataSource dataSource) { - super(onkostarApi, dataSource); + public FormBasedPermissionEvaluator(final IOnkostarApi onkostarApi, final SecurityService securityService) { + super(onkostarApi, securityService); } /** @@ -32,15 +28,11 @@ public class FormBasedPermissionEvaluator extends AbstractDelegatedPermissionEva */ @Override public boolean hasPermission(Authentication authentication, Object targetObject, Object permissionType) { - if (permissionType instanceof PermissionType) { - if (targetObject instanceof Patient) { - return true; - } else if (targetObject instanceof Procedure) { - return getFormNamesForPermission(authentication, (PermissionType)permissionType) - .contains(((Procedure)targetObject).getFormName()); - } + if (permissionType instanceof PermissionType && targetObject instanceof Procedure) { + return this.securityService.getFormNamesForPermission(authentication, (PermissionType)permissionType) + .contains(((Procedure)targetObject).getFormName()); } - return false; + return true; } /** @@ -55,36 +47,13 @@ public class FormBasedPermissionEvaluator extends AbstractDelegatedPermissionEva */ @Override public boolean hasPermission(Authentication authentication, Serializable targetId, String targetType, Object permissionType) { - if (targetId instanceof Integer) { - if (PATIENT.equals(targetType)) { - return true; - } + if (permissionType instanceof PermissionType && targetId instanceof Integer && PROCEDURE.equals(targetType)) { var procedure = this.onkostarApi.getProcedure((int)targetId); if (null != procedure) { - return getFormNamesForPermission(authentication, (PermissionType) permissionType).contains(procedure.getFormName()); + return this.securityService.getFormNamesForPermission(authentication, (PermissionType) permissionType).contains(procedure.getFormName()); } } - return false; + return true; } - List getFormNamesForPermission(Authentication authentication, PermissionType permissionType) { - - var sql = "SELECT df.name FROM formular_usergroup_zugriff " + - " JOIN data_form df ON formular_usergroup_zugriff.formular_id = df.id " + - " JOIN usergroup u ON formular_usergroup_zugriff.usergroup_id = u.id " + - " JOIN akteur_usergroup au ON u.id = au.usergroup_id " + - " JOIN akteur a on au.akteur_id = a.id " + - " WHERE a.login = ? AND a.aktiv AND a.anmelden_moeglich "; - - if (PermissionType.READ_WRITE == permissionType) { - sql += " AND formular_usergroup_zugriff.bearbeiten "; - } - - var userDetails = (UserDetails)authentication.getPrincipal(); - - return jdbcTemplate - .query(sql, new Object[]{userDetails.getUsername()}, (rs, rowNum) -> rs.getString("name")); - } - - } diff --git a/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java b/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java index 21cdca1..e3ba16e 100644 --- a/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java +++ b/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java @@ -4,12 +4,9 @@ import de.itc.onkostar.api.IOnkostarApi; import de.itc.onkostar.api.Patient; import de.itc.onkostar.api.Procedure; import org.springframework.security.core.Authentication; -import org.springframework.security.core.userdetails.UserDetails; import org.springframework.stereotype.Component; -import javax.sql.DataSource; import java.io.Serializable; -import java.util.List; /** * Permission-Evaluator zur Auswertung der Berechtigung auf Objekte aufgrund der Personenstammberechtigung @@ -17,8 +14,8 @@ import java.util.List; @Component public class PersonPoolBasedPermissionEvaluator extends AbstractDelegatedPermissionEvaluator { - public PersonPoolBasedPermissionEvaluator(final IOnkostarApi onkostarApi, final DataSource dataSource) { - super(onkostarApi, dataSource); + public PersonPoolBasedPermissionEvaluator(final IOnkostarApi onkostarApi, final SecurityService securityService) { + super(onkostarApi, securityService); } /** @@ -32,10 +29,10 @@ public class PersonPoolBasedPermissionEvaluator extends AbstractDelegatedPermiss public boolean hasPermission(Authentication authentication, Object targetObject, Object permissionType) { if (permissionType instanceof PermissionType) { if (targetObject instanceof Patient) { - return getPersonPoolIdsForPermission(authentication, (PermissionType)permissionType) + return this.securityService.getPersonPoolIdsForPermission(authentication, (PermissionType)permissionType) .contains(((Patient)targetObject).getPersonPoolCode()); } else if (targetObject instanceof Procedure) { - return getPersonPoolIdsForPermission(authentication, (PermissionType)permissionType) + return this.securityService.getPersonPoolIdsForPermission(authentication, (PermissionType)permissionType) .contains(((Procedure)targetObject).getPatient().getPersonPoolCode()); } } @@ -52,10 +49,10 @@ public class PersonPoolBasedPermissionEvaluator extends AbstractDelegatedPermiss */ @Override public boolean hasPermission(Authentication authentication, Serializable targetId, String targetType, Object permissionType) { - if (targetId instanceof Integer) { + if (targetId instanceof Integer && permissionType instanceof PermissionType) { var personPoolCode = getPersonPoolCode((int)targetId, targetType); - if (null != personPoolCode && permissionType instanceof PermissionType) { - return getPersonPoolIdsForPermission(authentication, (PermissionType) permissionType).contains(personPoolCode); + if (null != personPoolCode) { + return this.securityService.getPersonPoolIdsForPermission(authentication, (PermissionType) permissionType).contains(personPoolCode); } } return false; @@ -80,23 +77,5 @@ public class PersonPoolBasedPermissionEvaluator extends AbstractDelegatedPermiss return null; } - List getPersonPoolIdsForPermission(Authentication authentication, PermissionType permissionType) { - var sql = "SELECT p.kennung FROM personenstamm_zugriff " + - " JOIN usergroup u ON personenstamm_zugriff.benutzergruppe_id = u.id " + - " JOIN akteur_usergroup au ON u.id = au.usergroup_id " + - " JOIN akteur a ON au.akteur_id = a.id " + - " JOIN personenstamm p on personenstamm_zugriff.personenstamm_id = p.id " + - " WHERE a.login = ? AND a.aktiv AND a.anmelden_moeglich "; - - if (PermissionType.READ_WRITE == permissionType) { - sql += " AND personenstamm_zugriff.bearbeiten "; - } - - var userDetails = (UserDetails)authentication.getPrincipal(); - - return jdbcTemplate - .query(sql, new Object[]{userDetails.getUsername()}, (rs, rowNum) -> rs.getString("kennung")); - } - } diff --git a/src/main/java/DNPM/security/SecurityService.java b/src/main/java/DNPM/security/SecurityService.java new file mode 100644 index 0000000..479701f --- /dev/null +++ b/src/main/java/DNPM/security/SecurityService.java @@ -0,0 +1,60 @@ +package DNPM.security; + +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.userdetails.UserDetails; +import org.springframework.stereotype.Service; + +import javax.sql.DataSource; +import java.util.List; + +/** + * Service mit Methoden zum Feststellen von sicherheitsrelevanten Informationen eines Benutzers + */ +@Service +public class SecurityService { + + private final JdbcTemplate jdbcTemplate; + + public SecurityService(final DataSource dataSource) { + this.jdbcTemplate = new JdbcTemplate(dataSource); + } + + List getPersonPoolIdsForPermission(Authentication authentication, PermissionType permissionType) { + var sql = "SELECT p.kennung FROM personenstamm_zugriff " + + " JOIN usergroup u ON personenstamm_zugriff.benutzergruppe_id = u.id " + + " JOIN akteur_usergroup au ON u.id = au.usergroup_id " + + " JOIN akteur a ON au.akteur_id = a.id " + + " JOIN personenstamm p on personenstamm_zugriff.personenstamm_id = p.id " + + " WHERE a.login = ? AND a.aktiv AND a.anmelden_moeglich "; + + if (PermissionType.READ_WRITE == permissionType) { + sql += " AND personenstamm_zugriff.bearbeiten "; + } + + var userDetails = (UserDetails)authentication.getPrincipal(); + + return jdbcTemplate + .query(sql, new Object[]{userDetails.getUsername()}, (rs, rowNum) -> rs.getString("kennung")); + } + + List getFormNamesForPermission(Authentication authentication, PermissionType permissionType) { + + var sql = "SELECT df.name FROM formular_usergroup_zugriff " + + " JOIN data_form df ON formular_usergroup_zugriff.formular_id = df.id " + + " JOIN usergroup u ON formular_usergroup_zugriff.usergroup_id = u.id " + + " JOIN akteur_usergroup au ON u.id = au.usergroup_id " + + " JOIN akteur a on au.akteur_id = a.id " + + " WHERE a.login = ? AND a.aktiv AND a.anmelden_moeglich "; + + if (PermissionType.READ_WRITE == permissionType) { + sql += " AND formular_usergroup_zugriff.bearbeiten "; + } + + var userDetails = (UserDetails)authentication.getPrincipal(); + + return jdbcTemplate + .query(sql, new Object[]{userDetails.getUsername()}, (rs, rowNum) -> rs.getString("name")); + } + +} diff --git a/src/test/java/DNPM/security/FormBasedPermissionEvaluatorTest.java b/src/test/java/DNPM/security/FormBasedPermissionEvaluatorTest.java new file mode 100644 index 0000000..ca3d314 --- /dev/null +++ b/src/test/java/DNPM/security/FormBasedPermissionEvaluatorTest.java @@ -0,0 +1,112 @@ +package DNPM.security; + +import de.itc.onkostar.api.IOnkostarApi; +import de.itc.onkostar.api.Patient; +import de.itc.onkostar.api.Procedure; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.security.core.Authentication; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class FormBasedPermissionEvaluatorTest { + + private IOnkostarApi onkostarApi; + + private Authentication dummyAuthentication; + + private SecurityService securityService; + + private FormBasedPermissionEvaluator permissionEvaluator; + + @BeforeEach + void setup( + @Mock IOnkostarApi onkostarApi, + @Mock SecurityService securityService, + @Mock DummyAuthentication dummyAuthentication + ) { + this.onkostarApi = onkostarApi; + this.dummyAuthentication = dummyAuthentication; + this.securityService = securityService; + + this.permissionEvaluator = new FormBasedPermissionEvaluator( + onkostarApi, securityService + ); + } + + @Test + void testShouldGrantPermissionByProcedure() { + when(securityService.getFormNamesForPermission(any(Authentication.class), any(PermissionType.class))).thenReturn(List.of("OS.Form2", "OS.Form3", "OS.Form5")); + + var object = new Procedure(onkostarApi); + object.setFormName("OS.Form2"); + + var actual = permissionEvaluator.hasPermission(this.dummyAuthentication, object, PermissionType.READ); + assertThat(actual).isTrue(); + } + + @Test + void testShouldGrantPermissionByProcedureId() { + when(securityService.getFormNamesForPermission(any(Authentication.class), any(PermissionType.class))).thenReturn(List.of("OS.Form2", "OS.Form3", "OS.Form5")); + + doAnswer(invocationOnMock -> { + var object = new Procedure(onkostarApi); + object.setFormName("OS.Form2"); + return object; + }).when(onkostarApi).getProcedure(anyInt()); + + var actual = permissionEvaluator.hasPermission(this.dummyAuthentication, 123, PersonPoolBasedPermissionEvaluator.PROCEDURE, PermissionType.READ); + assertThat(actual).isTrue(); + } + + @Test + void testShouldDenyPermissionByProcedure() { + when(securityService.getFormNamesForPermission(any(Authentication.class), any(PermissionType.class))).thenReturn(List.of("OS.Form2", "OS.Form3", "OS.Form5")); + + var object = new Procedure(onkostarApi); + object.setFormName("OS.Form1"); + + var actual = permissionEvaluator.hasPermission(this.dummyAuthentication, object, PermissionType.READ); + assertThat(actual).isFalse(); + } + + @Test + void testShouldDenyPermissionByProcedureId() { + when(securityService.getFormNamesForPermission(any(Authentication.class), any(PermissionType.class))).thenReturn(List.of("OS.Form2", "OS.Form3", "OS.Form5")); + + doAnswer(invocationOnMock -> { + var object = new Procedure(onkostarApi); + object.setFormName("OS.Form1"); + return object; + }).when(onkostarApi).getProcedure(anyInt()); + + var actual = permissionEvaluator.hasPermission(this.dummyAuthentication, 123, PersonPoolBasedPermissionEvaluator.PROCEDURE, PermissionType.READ); + assertThat(actual).isFalse(); + } + + @Test + void testShouldVoteForPermissionToPatient() { + var object = new Patient(onkostarApi); + object.setPersonPoolCode("Pool1"); + + var actual = permissionEvaluator.hasPermission(this.dummyAuthentication, object, PermissionType.READ); + assertThat(actual).isTrue(); + } + + @Test + void testShouldVoteForPermissionToIdOfTypeProcedure() { + var actual = permissionEvaluator.hasPermission(this.dummyAuthentication, 123, FormBasedPermissionEvaluator.PATIENT, PermissionType.READ); + assertThat(actual).isTrue(); + } + +} diff --git a/src/test/java/DNPM/security/PersonPoolBasedPermissionEvaluatorTest.java b/src/test/java/DNPM/security/PersonPoolBasedPermissionEvaluatorTest.java new file mode 100644 index 0000000..a05f83a --- /dev/null +++ b/src/test/java/DNPM/security/PersonPoolBasedPermissionEvaluatorTest.java @@ -0,0 +1,156 @@ +package DNPM.security; + +import de.itc.onkostar.api.IOnkostarApi; +import de.itc.onkostar.api.Patient; +import de.itc.onkostar.api.Procedure; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.security.core.Authentication; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class PersonPoolBasedPermissionEvaluatorTest { + + private IOnkostarApi onkostarApi; + + private Authentication dummyAuthentication; + + private PersonPoolBasedPermissionEvaluator permissionEvaluator; + + @BeforeEach + void setup( + @Mock IOnkostarApi onkostarApi, + @Mock SecurityService securityService, + @Mock DummyAuthentication dummyAuthentication + ) { + this.onkostarApi = onkostarApi; + this.dummyAuthentication = dummyAuthentication; + + this.permissionEvaluator = new PersonPoolBasedPermissionEvaluator( + onkostarApi, securityService + ); + + when(securityService.getPersonPoolIdsForPermission(any(Authentication.class), any(PermissionType.class))).thenReturn(List.of("Pool2", "Pool3", "Pool5")); + } + + @Test + void testShouldGrantPermissionByPatientObject() { + var object = new Patient(onkostarApi); + object.setPersonPoolCode("Pool2"); + + var actual = permissionEvaluator.hasPermission(this.dummyAuthentication, object, PermissionType.READ); + + assertThat(actual).isTrue(); + } + + @Test + void testShouldGrantPermissionByPatientIdAndType() { + doAnswer(invocationOnMock -> { + var object = new Patient(onkostarApi); + object.setPersonPoolCode("Pool2"); + return object; + }).when(onkostarApi).getPatient(anyInt()); + + var actual = permissionEvaluator.hasPermission(this.dummyAuthentication, 123, PersonPoolBasedPermissionEvaluator.PATIENT, PermissionType.READ); + + assertThat(actual).isTrue(); + } + + @Test + void testShouldDenyPermissionByPatientObject() { + var object = new Patient(onkostarApi); + object.setPersonPoolCode("Pool1"); + + var actual = permissionEvaluator.hasPermission(this.dummyAuthentication, object, PermissionType.READ); + + assertThat(actual).isFalse(); + } + + @Test + void testShouldDenyPermissionByPatientIdAndType() { + doAnswer(invocationOnMock -> { + var object = new Patient(onkostarApi); + object.setPersonPoolCode("Pool1"); + return object; + }).when(onkostarApi).getPatient(anyInt()); + + var actual = permissionEvaluator.hasPermission(this.dummyAuthentication, 123, PersonPoolBasedPermissionEvaluator.PATIENT, PermissionType.READ); + + assertThat(actual).isFalse(); + } + + @Test + void testShouldGrantPermissionByProcedureObject() { + var patient = new Patient(onkostarApi); + patient.setPersonPoolCode("Pool2"); + + var object = new Procedure(onkostarApi); + object.setFormName("OS.Form1"); + object.setPatient(patient); + + var actual = permissionEvaluator.hasPermission(this.dummyAuthentication, object, PermissionType.READ); + + assertThat(actual).isTrue(); + } + + @Test + void testShouldGrantPermissionByProcedureIdAndType() { + doAnswer(invocationOnMock -> { + var patient = new Patient(onkostarApi); + patient.setPersonPoolCode("Pool2"); + + var object = new Procedure(onkostarApi); + object.setFormName("OS.Form1"); + object.setPatient(patient); + + return object; + }).when(onkostarApi).getProcedure(anyInt()); + + var actual = permissionEvaluator.hasPermission(this.dummyAuthentication, 456, PersonPoolBasedPermissionEvaluator.PROCEDURE, PermissionType.READ); + + assertThat(actual).isTrue(); + } + + @Test + void testShouldDenyPermissionByProcedureObject() { + var patient = new Patient(onkostarApi); + patient.setPersonPoolCode("Pool1"); + + var object = new Procedure(onkostarApi); + object.setFormName("OS.Form1"); + object.setPatient(patient); + + var actual = permissionEvaluator.hasPermission(this.dummyAuthentication, object, PermissionType.READ); + + assertThat(actual).isFalse(); + } + + @Test + void testShouldDenyPermissionByProcedureIdAndType() { + doAnswer(invocationOnMock -> { + var patient = new Patient(onkostarApi); + patient.setPersonPoolCode("Pool1"); + + var object = new Procedure(onkostarApi); + object.setFormName("OS.Form1"); + object.setPatient(patient); + + return object; + }).when(onkostarApi).getProcedure(anyInt()); + + var actual = permissionEvaluator.hasPermission(this.dummyAuthentication, 123, PersonPoolBasedPermissionEvaluator.PROCEDURE, PermissionType.READ); + + assertThat(actual).isFalse(); + } + +} \ No newline at end of file