From b9f971b295be6c9a711c964e3d0795406b9f3670 Mon Sep 17 00:00:00 2001 From: Paul-Christian Volkmer Date: Wed, 5 Apr 2023 17:33:16 +0200 Subject: Issue #24: Erste Implementierung eines PermissionEvaluators Diese erste Implementierung wertet die Zugriffsberechtigung auf den Personenstamm aus, in dem der Patient oder eines der Formulare zum Patienten gehört. --- pom.xml | 7 ++ src/main/java/DNPM/config/PluginConfiguration.java | 7 ++ src/main/java/DNPM/security/PermissionType.java | 6 ++ .../PersonPoolBasedPermissionEvaluator.java | 78 ++++++++++++++++++++++ 4 files changed, 98 insertions(+) create mode 100644 src/main/java/DNPM/security/PermissionType.java create mode 100644 src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java diff --git a/pom.xml b/pom.xml index cd7d802..0f7ffdb 100644 --- a/pom.xml +++ b/pom.xml @@ -14,6 +14,7 @@ UTF-8 4.3.8.RELEASE + 4.2.2.RELEASE @@ -55,6 +56,12 @@ ${spring-version} provided + + org.springframework.security + spring-security-core + ${spring-security-version} + provided + org.springframework.data spring-data-jpa diff --git a/src/main/java/DNPM/config/PluginConfiguration.java b/src/main/java/DNPM/config/PluginConfiguration.java index 30d8fb3..654d4c4 100644 --- a/src/main/java/DNPM/config/PluginConfiguration.java +++ b/src/main/java/DNPM/config/PluginConfiguration.java @@ -1,6 +1,7 @@ package DNPM.config; import DNPM.database.SettingsRepository; +import DNPM.security.PersonPoolBasedPermissionEvaluator; import DNPM.services.*; import DNPM.services.consent.ConsentManagerServiceFactory; import DNPM.services.mtb.DefaultMtbService; @@ -12,6 +13,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.ComponentScan; import org.springframework.context.annotation.Configuration; import org.springframework.data.jpa.repository.config.EnableJpaRepositories; +import org.springframework.security.access.PermissionEvaluator; import javax.sql.DataSource; @@ -25,6 +27,11 @@ import javax.sql.DataSource; @EnableJpaRepositories(basePackages = "DNPM.database") public class PluginConfiguration { + @Bean + public PermissionEvaluator personBasedPermissionEvaluator(final DataSource dataSource) { + return new PersonPoolBasedPermissionEvaluator(dataSource); + } + @Bean public FormService formService(final DataSource dataSource) { return new DefaultFormService(dataSource); diff --git a/src/main/java/DNPM/security/PermissionType.java b/src/main/java/DNPM/security/PermissionType.java new file mode 100644 index 0000000..50a0bd3 --- /dev/null +++ b/src/main/java/DNPM/security/PermissionType.java @@ -0,0 +1,6 @@ +package DNPM.security; + +public enum PermissionType { + READ, + WRITE +} diff --git a/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java b/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java new file mode 100644 index 0000000..766cc70 --- /dev/null +++ b/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java @@ -0,0 +1,78 @@ +package DNPM.security; + +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 org.springframework.security.core.Authentication; +import org.springframework.security.core.userdetails.UserDetails; + +import javax.sql.DataSource; +import java.io.Serializable; +import java.util.List; + +/** + * Permission-Evaluator zur Auswertung der Berechtigung auf Objekte aufgrund der Personenstammberechtigung + */ +public class PersonPoolBasedPermissionEvaluator implements PermissionEvaluator { + + private final JdbcTemplate jdbcTemplate; + + public PersonPoolBasedPermissionEvaluator(final DataSource dataSource) { + this.jdbcTemplate = new JdbcTemplate(dataSource); + } + + /** + * Auswertung der Zugriffsberechtigung für authentifizierten Benutzer auf Zielobjekt mit angeforderter Berechtigung. + * @param authentication Das Authentication Objekt + * @param targetObject Das Zielobjekt + * @param permissionType Die angeforderte Berechtigung + * @return Gibt true zurück, wenn der Benutzer die Berechtigung hat + */ + @Override + public boolean hasPermission(Authentication authentication, Object targetObject, Object permissionType) { + if (permissionType instanceof PermissionType) { + if (targetObject instanceof Patient) { + return getPersonPoolIdsForPermission(authentication, (PermissionType)permissionType) + .contains(((Patient)targetObject).getPersonPoolCode()); + } else if (targetObject instanceof Procedure) { + return getPersonPoolIdsForPermission(authentication, (PermissionType)permissionType) + .contains(((Procedure)targetObject).getPatient().getPersonPoolCode()); + } + } + return false; + } + + /** + * Auswertung nicht anhand der ID möglich. Gibt immer false zurück. + * @param authentication Authentication-Object + * @param targetId ID des Objekts + * @param s + * @param o + * @return Gibt immer false zurück + */ + @Override + public boolean hasPermission(Authentication authentication, Serializable targetId, String s, Object o) { + return false; + } + + private 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.WRITE == permissionType) { + sql += " AND personenstamm_zugriff.bearbeiten "; + } + + var userDetails = (UserDetails)authentication.getPrincipal(); + + return jdbcTemplate + .query(sql, new Object[]{userDetails.getUsername()}, (rs, rowNum) -> rs.getString("id")); + } + + +} -- cgit v1.2.3 From dfbcf3186e6974ef71dfa77b2148f9a8bfe2ce42 Mon Sep 17 00:00:00 2001 From: Paul-Christian Volkmer Date: Wed, 5 Apr 2023 17:39:42 +0200 Subject: Issue #24: Verwende Berechtigung READ und READ_WRITE Schreibberechtigung bedeutet gleichzeitig Berechtigung den Eintrag zu lesen. --- src/main/java/DNPM/security/PermissionType.java | 2 +- src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/DNPM/security/PermissionType.java b/src/main/java/DNPM/security/PermissionType.java index 50a0bd3..1539aea 100644 --- a/src/main/java/DNPM/security/PermissionType.java +++ b/src/main/java/DNPM/security/PermissionType.java @@ -2,5 +2,5 @@ package DNPM.security; public enum PermissionType { READ, - WRITE + READ_WRITE } diff --git a/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java b/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java index 766cc70..4d895e4 100644 --- a/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java +++ b/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java @@ -64,7 +64,7 @@ public class PersonPoolBasedPermissionEvaluator implements PermissionEvaluator { " JOIN personenstamm p on personenstamm_zugriff.personenstamm_id = p.id " + " WHERE a.login = ? AND a.aktiv AND a.anmelden_moeglich "; - if (PermissionType.WRITE == permissionType) { + if (PermissionType.READ_WRITE == permissionType) { sql += " AND personenstamm_zugriff.bearbeiten "; } -- cgit v1.2.3 From e0dba6f4ee1550c55e2765adeabf334200984543 Mon Sep 17 00:00:00 2001 From: Paul-Christian Volkmer Date: Thu, 6 Apr 2023 14:42:35 +0200 Subject: Issue #24: Füge Annotationen und Spring-AOP Aspect zum Absichern von Methodenaufrufen hinzu --- pom.xml | 6 ++ src/main/java/DNPM/config/PluginConfiguration.java | 9 +-- .../IllegalSecuredObjectAccessException.java | 13 ++++ .../PersonPoolBasedPermissionEvaluator.java | 4 +- src/main/java/DNPM/security/PersonPoolSecured.java | 14 ++++ .../DNPM/security/PersonPoolSecuredResult.java | 14 ++++ src/main/java/DNPM/security/SecurityAspects.java | 74 ++++++++++++++++++++++ 7 files changed, 125 insertions(+), 9 deletions(-) create mode 100644 src/main/java/DNPM/security/IllegalSecuredObjectAccessException.java create mode 100644 src/main/java/DNPM/security/PersonPoolSecured.java create mode 100644 src/main/java/DNPM/security/PersonPoolSecuredResult.java create mode 100644 src/main/java/DNPM/security/SecurityAspects.java diff --git a/pom.xml b/pom.xml index 0f7ffdb..27c0e85 100644 --- a/pom.xml +++ b/pom.xml @@ -101,6 +101,12 @@ + + org.springframework + spring-test + ${spring-version} + test + org.junit.jupiter junit-jupiter-engine diff --git a/src/main/java/DNPM/config/PluginConfiguration.java b/src/main/java/DNPM/config/PluginConfiguration.java index 654d4c4..0042fe4 100644 --- a/src/main/java/DNPM/config/PluginConfiguration.java +++ b/src/main/java/DNPM/config/PluginConfiguration.java @@ -1,7 +1,6 @@ package DNPM.config; import DNPM.database.SettingsRepository; -import DNPM.security.PersonPoolBasedPermissionEvaluator; import DNPM.services.*; import DNPM.services.consent.ConsentManagerServiceFactory; import DNPM.services.mtb.DefaultMtbService; @@ -13,7 +12,6 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.ComponentScan; import org.springframework.context.annotation.Configuration; import org.springframework.data.jpa.repository.config.EnableJpaRepositories; -import org.springframework.security.access.PermissionEvaluator; import javax.sql.DataSource; @@ -23,15 +21,10 @@ import javax.sql.DataSource; * @since 0.0.2 */ @Configuration -@ComponentScan(basePackages = "DNPM.analyzer") +@ComponentScan(basePackages = { "DNPM.analyzer", "DNPM.security" }) @EnableJpaRepositories(basePackages = "DNPM.database") public class PluginConfiguration { - @Bean - public PermissionEvaluator personBasedPermissionEvaluator(final DataSource dataSource) { - return new PersonPoolBasedPermissionEvaluator(dataSource); - } - @Bean public FormService formService(final DataSource dataSource) { return new DefaultFormService(dataSource); diff --git a/src/main/java/DNPM/security/IllegalSecuredObjectAccessException.java b/src/main/java/DNPM/security/IllegalSecuredObjectAccessException.java new file mode 100644 index 0000000..542b604 --- /dev/null +++ b/src/main/java/DNPM/security/IllegalSecuredObjectAccessException.java @@ -0,0 +1,13 @@ +package DNPM.security; + +public class IllegalSecuredObjectAccessException extends RuntimeException { + + public IllegalSecuredObjectAccessException() { + super(); + } + + public IllegalSecuredObjectAccessException(String message) { + super(message); + } + +} diff --git a/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java b/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java index 4d895e4..2eac69c 100644 --- a/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java +++ b/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java @@ -6,6 +6,7 @@ import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.security.access.PermissionEvaluator; 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; @@ -14,6 +15,7 @@ import java.util.List; /** * Permission-Evaluator zur Auswertung der Berechtigung auf Objekte aufgrund der Personenstammberechtigung */ +@Component public class PersonPoolBasedPermissionEvaluator implements PermissionEvaluator { private final JdbcTemplate jdbcTemplate; @@ -71,7 +73,7 @@ public class PersonPoolBasedPermissionEvaluator implements PermissionEvaluator { var userDetails = (UserDetails)authentication.getPrincipal(); return jdbcTemplate - .query(sql, new Object[]{userDetails.getUsername()}, (rs, rowNum) -> rs.getString("id")); + .query(sql, new Object[]{userDetails.getUsername()}, (rs, rowNum) -> rs.getString("kennung")); } diff --git a/src/main/java/DNPM/security/PersonPoolSecured.java b/src/main/java/DNPM/security/PersonPoolSecured.java new file mode 100644 index 0000000..cac0f78 --- /dev/null +++ b/src/main/java/DNPM/security/PersonPoolSecured.java @@ -0,0 +1,14 @@ +package DNPM.security; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.METHOD) +public @interface PersonPoolSecured { + + PermissionType value() default PermissionType.READ_WRITE; + +} diff --git a/src/main/java/DNPM/security/PersonPoolSecuredResult.java b/src/main/java/DNPM/security/PersonPoolSecuredResult.java new file mode 100644 index 0000000..0ca8edf --- /dev/null +++ b/src/main/java/DNPM/security/PersonPoolSecuredResult.java @@ -0,0 +1,14 @@ +package DNPM.security; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.METHOD) +public @interface PersonPoolSecuredResult { + + PermissionType value() default PermissionType.READ_WRITE; + +} diff --git a/src/main/java/DNPM/security/SecurityAspects.java b/src/main/java/DNPM/security/SecurityAspects.java new file mode 100644 index 0000000..a1fcd3f --- /dev/null +++ b/src/main/java/DNPM/security/SecurityAspects.java @@ -0,0 +1,74 @@ +package DNPM.security; + +import de.itc.onkostar.api.Patient; +import de.itc.onkostar.api.Procedure; +import org.aspectj.lang.JoinPoint; +import org.aspectj.lang.annotation.AfterReturning; +import org.aspectj.lang.annotation.Aspect; +import org.aspectj.lang.annotation.Before; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.stereotype.Component; + +import java.util.Arrays; + +@Component +@Aspect +public class SecurityAspects { + + private final Logger logger = LoggerFactory.getLogger(this.getClass()); + + private final PersonPoolBasedPermissionEvaluator permissionEvaluator; + + public SecurityAspects(PersonPoolBasedPermissionEvaluator permissionEvaluator) { + this.permissionEvaluator = permissionEvaluator; + } + + @AfterReturning(value = "@annotation(PersonPoolSecuredResult) ", returning = "patient") + public void afterPatient(Patient patient) { + if ( + null != patient + && ! permissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(), patient, PermissionType.READ_WRITE) + ) { + logger.warn("Rückgabe von Patient blockiert: {}", patient.getId()); + throw new IllegalSecuredObjectAccessException(); + } + } + + @AfterReturning(value = "@annotation(PersonPoolSecuredResult)", returning = "procedure") + public void afterProcedure(Procedure procedure) { + if ( + null != procedure + && ! permissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(), procedure, PermissionType.READ_WRITE) + ) { + logger.warn("Rückgabe von Prozedur blockiert: {}", procedure.getId()); + throw new IllegalSecuredObjectAccessException(); + } + } + + @Before(value = "@annotation(PersonPoolSecured)") + public void beforePatient(JoinPoint jp) { + Arrays.stream(jp.getArgs()) + .filter(arg -> arg instanceof Patient) + .forEach(patient -> { + if (! permissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(), patient, PermissionType.READ_WRITE)) { + logger.warn("Zugriff auf Patient blockiert: {}", ((Patient)patient).getId()); + throw new IllegalSecuredObjectAccessException(); + } + }); + } + + @Before(value = "@annotation(PersonPoolSecured)") + public void beforeProcedure(JoinPoint jp) { + Arrays.stream(jp.getArgs()) + .filter(arg -> arg instanceof Procedure) + .forEach(procedure -> { + if (! permissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(), procedure, PermissionType.READ_WRITE)) { + logger.warn("Zugriff auf Prozedur blockiert: {}", ((Procedure)procedure).getId()); + throw new IllegalSecuredObjectAccessException(); + } + }); + } + +} -- cgit v1.2.3 From 07ff2aa316f243e4f997635dbf02a506671eb856 Mon Sep 17 00:00:00 2001 From: Paul-Christian Volkmer Date: Sun, 9 Apr 2023 13:19:38 +0200 Subject: Issue #24: Füge Unit Tests für SecurityAspect hinzu Diese Tests verwenden ein manuell erstelltes Proxy, wie es in OS automatisch verwendet wird, und prüfen dann entsprechende Methodenaufrufe. --- pom.xml | 6 + .../java/DNPM/security/SecurityAspectsTest.java | 164 +++++++++++++++++++++ 2 files changed, 170 insertions(+) create mode 100644 src/test/java/DNPM/security/SecurityAspectsTest.java diff --git a/pom.xml b/pom.xml index 27c0e85..1c5feaa 100644 --- a/pom.xml +++ b/pom.xml @@ -137,6 +137,12 @@ 4.11.0 test + + org.aspectj + aspectjweaver + 1.7.1 + test + ca.uhn.hapi hapi-base diff --git a/src/test/java/DNPM/security/SecurityAspectsTest.java b/src/test/java/DNPM/security/SecurityAspectsTest.java new file mode 100644 index 0000000..8416b8a --- /dev/null +++ b/src/test/java/DNPM/security/SecurityAspectsTest.java @@ -0,0 +1,164 @@ +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.aop.aspectj.annotation.AspectJProxyFactory; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +@ExtendWith(MockitoExtension.class) +class SecurityAspectsTest { + + private DummyClass dummyClass; + + private IOnkostarApi onkostarApi; + + private PersonPoolBasedPermissionEvaluator permissionEvaluator; + + @BeforeEach + void setup( + @Mock IOnkostarApi onkostarApi, + @Mock PersonPoolBasedPermissionEvaluator permissionEvaluator + ) { + this.onkostarApi = onkostarApi; + this.permissionEvaluator = permissionEvaluator; + + // Create proxied instance of DummyClass as done within Onkostar using Spring AOP + var dummyClass = new DummyClass(onkostarApi); + AspectJProxyFactory factory = new AspectJProxyFactory(dummyClass); + SecurityAspects securityAspects = new SecurityAspects(this.permissionEvaluator); + factory.addAspect(securityAspects); + this.dummyClass = factory.getProxy(); + } + + @Test + void testShouldPreventSecuredMethodCallWithPatientParam() { + when(this.permissionEvaluator.hasPermission(any(), any(Patient.class), any(PermissionType.class))) + .thenReturn(false); + + var exception = assertThrows( + Exception.class, + () -> this.dummyClass.methodWithPatientParam(new Patient(onkostarApi)) + ); + assertThat(exception).isExactlyInstanceOf(IllegalSecuredObjectAccessException.class); + } + + @Test + void testShouldAllowSecuredMethodCallWithPatientParam() { + when(this.permissionEvaluator.hasPermission(any(), any(Patient.class), any(PermissionType.class))) + .thenReturn(true); + + this.dummyClass.methodWithPatientParam(new Patient(onkostarApi)); + + verify(onkostarApi, times(1)).savePatient(any(Patient.class)); + } + + @Test + void testShouldPreventSecuredMethodCallWithProcedureParam() { + when(this.permissionEvaluator.hasPermission(any(), any(Procedure.class), any(PermissionType.class))) + .thenReturn(false); + + var exception = assertThrows( + Exception.class, + () -> this.dummyClass.methodWithProcedureParam(new Procedure(onkostarApi)) + ); + assertThat(exception).isExactlyInstanceOf(IllegalSecuredObjectAccessException.class); + } + + @Test + void testShouldAllowSecuredMethodCallWithProcedureParam() throws Exception { + when(this.permissionEvaluator.hasPermission(any(), any(Procedure.class), any(PermissionType.class))) + .thenReturn(true); + + this.dummyClass.methodWithProcedureParam(new Procedure(onkostarApi)); + + verify(onkostarApi, times(1)).saveProcedure(any(Procedure.class), anyBoolean()); + } + + @Test + void testShouldPreventSecuredMethodCallWithPatientReturnValue() { + when(this.permissionEvaluator.hasPermission(any(), any(Patient.class), any(PermissionType.class))) + .thenReturn(false); + + var exception = assertThrows( + Exception.class, + () -> this.dummyClass.methodWithPatientReturnValue(1) + ); + assertThat(exception).isExactlyInstanceOf(IllegalSecuredObjectAccessException.class); + } + + @Test + void testShouldAllowSecuredMethodCallWithPatientReturnValue() { + when(this.permissionEvaluator.hasPermission(any(), any(Patient.class), any(PermissionType.class))) + .thenReturn(true); + + var actual = this.dummyClass.methodWithPatientReturnValue(1); + + assertThat(actual).isNotNull(); + } + + @Test + void testShouldPreventSecuredMethodCallWithProcedureReturnValue() { + when(this.permissionEvaluator.hasPermission(any(), any(Procedure.class), any(PermissionType.class))) + .thenReturn(false); + + var exception = assertThrows( + Exception.class, + () -> this.dummyClass.methodWithProcedureReturnValue(1) + ); + assertThat(exception).isExactlyInstanceOf(IllegalSecuredObjectAccessException.class); + } + + @Test + void testShouldAllowSecuredMethodCallWithProcedureReturnValue() { + when(this.permissionEvaluator.hasPermission(any(), any(Procedure.class), any(PermissionType.class))) + .thenReturn(true); + + var actual = this.dummyClass.methodWithProcedureReturnValue(1); + + assertThat(actual).isNotNull(); + } + +} + +class DummyClass { + + private final IOnkostarApi onkostarApi; + + DummyClass(final IOnkostarApi onkostarApi) { + this.onkostarApi = onkostarApi; + } + + @PersonPoolSecured + public void methodWithPatientParam(Patient patient) { + this.onkostarApi.savePatient(patient); + } + + @PersonPoolSecured + public void methodWithProcedureParam(Procedure procedure) throws Exception { + this.onkostarApi.saveProcedure(procedure, false); + } + + @PersonPoolSecuredResult + public Patient methodWithPatientReturnValue(int id) { + var patient = new Patient(this.onkostarApi); + patient.setId(id); + return patient; + } + + @PersonPoolSecuredResult + public Procedure methodWithProcedureReturnValue(int id) { + var procedure = new Procedure(this.onkostarApi); + procedure.setId(id); + return procedure; + } +} -- cgit v1.2.3 From b56ff9e0d8a4efc71803e1eb435848c8bb42844c Mon Sep 17 00:00:00 2001 From: Paul-Christian Volkmer Date: Sun, 9 Apr 2023 14:01:14 +0200 Subject: Issue #24: Ermögliche Berechtigungsprüfung anhand ID und Klassennamen --- .../PersonPoolBasedPermissionEvaluator.java | 47 ++++++++++++++++++---- 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java b/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java index 2eac69c..0762dc9 100644 --- a/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java +++ b/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java @@ -1,5 +1,6 @@ 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; @@ -18,9 +19,12 @@ import java.util.List; @Component public class PersonPoolBasedPermissionEvaluator implements PermissionEvaluator { + private final IOnkostarApi onkostarApi; + private final JdbcTemplate jdbcTemplate; - public PersonPoolBasedPermissionEvaluator(final DataSource dataSource) { + public PersonPoolBasedPermissionEvaluator(final IOnkostarApi onkostarApi, final DataSource dataSource) { + this.onkostarApi = onkostarApi; this.jdbcTemplate = new JdbcTemplate(dataSource); } @@ -46,19 +50,48 @@ public class PersonPoolBasedPermissionEvaluator implements PermissionEvaluator { } /** - * Auswertung nicht anhand der ID möglich. Gibt immer false zurück. + * Auswertung anhand der ID und des Namens des Zielobjekts. * @param authentication Authentication-Object * @param targetId ID des Objekts - * @param s - * @param o - * @return Gibt immer false zurück + * @param targetType Name der Zielobjektklasse + * @param permissionType Die angeforderte Berechtigung + * @return Gibt true zurück, wenn der Benutzer die Berechtigung hat */ @Override - public boolean hasPermission(Authentication authentication, Serializable targetId, String s, Object o) { + public boolean hasPermission(Authentication authentication, Serializable targetId, String targetType, Object permissionType) { + if (targetId instanceof Integer) { + var personPoolCode = getPersonPoolCode((int)targetId, targetType); + if (null != personPoolCode && permissionType instanceof PermissionType) { + return getPersonPoolIdsForPermission(authentication, (PermissionType) permissionType).contains(personPoolCode); + } + } return false; } - private List getPersonPoolIdsForPermission(Authentication authentication, PermissionType permissionType) { + private String getPersonPoolCode(int id, String type) { + Patient patient = null; + switch (type) { + case "Patient": + patient = onkostarApi.getPatient(id); + break; + case "Procedure": + var procedure = onkostarApi.getProcedure(id); + if (null != procedure) { + patient = procedure.getPatient(); + } + break; + default: + break; + } + + if (null != patient) { + return patient.getPersonPoolCode(); + } + + 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 " + -- cgit v1.2.3 From 2495d851fcaa49ea61db2ce5c9a96f31b800014c Mon Sep 17 00:00:00 2001 From: Paul-Christian Volkmer Date: Mon, 10 Apr 2023 12:24:35 +0200 Subject: Issue #24: Ermögliche Berechtigungsprüfung anhand Formularnamen Diese Berechtigungsprüfung erlaubt immer den Zugriff auf Patienten, jedoch nur auf die Prozeduren, die explizit für die Benutzergruppe des Benutzers freigegeben wurde. --- .../security/FormBasedPermissionEvaluator.java | 97 ++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 src/main/java/DNPM/security/FormBasedPermissionEvaluator.java diff --git a/src/main/java/DNPM/security/FormBasedPermissionEvaluator.java b/src/main/java/DNPM/security/FormBasedPermissionEvaluator.java new file mode 100644 index 0000000..73937af --- /dev/null +++ b/src/main/java/DNPM/security/FormBasedPermissionEvaluator.java @@ -0,0 +1,97 @@ +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 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 + */ +@Component +public class FormBasedPermissionEvaluator implements PermissionEvaluator { + + private final IOnkostarApi onkostarApi; + + private final JdbcTemplate jdbcTemplate; + + public FormBasedPermissionEvaluator(final IOnkostarApi onkostarApi, final DataSource dataSource) { + this.onkostarApi = onkostarApi; + this.jdbcTemplate = new JdbcTemplate(dataSource); + } + + /** + * Auswertung der Zugriffsberechtigung für authentifizierten Benutzer auf Zielobjekt mit angeforderter Berechtigung. + * Zugriff auf Objekte vom Typ "Patient" wird immer gewährt. + * + * @param authentication Das Authentication Objekt + * @param targetObject Das Zielobjekt + * @param permissionType Die angeforderte Berechtigung + * @return Gibt true zurück, wenn der Benutzer die Berechtigung hat + */ + @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()); + } + } + return false; + } + + /** + * Auswertung anhand der ID und des Namens des Zielobjekts. + * Zugriff auf Objekte vom Typ "Patient" wird immer gewährt. + * + * @param authentication Authentication-Object + * @param targetId ID des Objekts + * @param targetType Name der Zielobjektklasse + * @param permissionType Die angeforderte Berechtigung + * @return Gibt true zurück, wenn der Benutzer die Berechtigung hat + */ + @Override + public boolean hasPermission(Authentication authentication, Serializable targetId, String targetType, Object permissionType) { + if (targetId instanceof Integer) { + if ("Patient".equals(targetType)) { + return true; + } + var procedure = this.onkostarApi.getProcedure((int)targetId); + if (null != procedure) { + return getFormNamesForPermission(authentication, (PermissionType) permissionType).contains(procedure.getFormName()); + } + } + return false; + } + + 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")); + } + + +} -- cgit v1.2.3 From 44396ff04a24088ac9fb2cab270036a9a983944f Mon Sep 17 00:00:00 2001 From: Paul-Christian Volkmer Date: Mon, 10 Apr 2023 13:09:54 +0200 Subject: Issue #24: Füge PermissionEvaluator zur Gesamtprüfung der Berechtigung hinzu Dieser PermissionEvaluator delegiert die einzelnen Prüfungen an PermissionEvaluatoren welche `AbstractDelegatedPermissionEvaluator` erweitern. Nur, wenn all diese PermissionEvaluatoren die Berechtigung erfolgreich geprüft haben, gibt dieser PermissionEvaluator ein positives Prüfungsergebnis zurück. --- .../AbstractDelegatedPermissionEvaluator.java | 26 +++++ .../DelegatingDataBasedPermissionEvaluator.java | 56 ++++++++++ .../security/FormBasedPermissionEvaluator.java | 13 +-- .../PersonPoolBasedPermissionEvaluator.java | 31 ++---- ...DelegatingDataBasedPermissionEvaluatorTest.java | 122 +++++++++++++++++++++ 5 files changed, 217 insertions(+), 31 deletions(-) create mode 100644 src/main/java/DNPM/security/AbstractDelegatedPermissionEvaluator.java create mode 100644 src/main/java/DNPM/security/DelegatingDataBasedPermissionEvaluator.java create mode 100644 src/test/java/DNPM/security/DelegatingDataBasedPermissionEvaluatorTest.java diff --git a/src/main/java/DNPM/security/AbstractDelegatedPermissionEvaluator.java b/src/main/java/DNPM/security/AbstractDelegatedPermissionEvaluator.java new file mode 100644 index 0000000..9d5f1ad --- /dev/null +++ b/src/main/java/DNPM/security/AbstractDelegatedPermissionEvaluator.java @@ -0,0 +1,26 @@ +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(); + + protected static final String PROCEDURE = Procedure.class.getSimpleName(); + + protected final IOnkostarApi onkostarApi; + + protected final JdbcTemplate jdbcTemplate; + + protected AbstractDelegatedPermissionEvaluator(final IOnkostarApi onkostarApi, final DataSource dataSource) { + this.onkostarApi = onkostarApi; + this.jdbcTemplate = new JdbcTemplate(dataSource); + } + +} diff --git a/src/main/java/DNPM/security/DelegatingDataBasedPermissionEvaluator.java b/src/main/java/DNPM/security/DelegatingDataBasedPermissionEvaluator.java new file mode 100644 index 0000000..d8ca92e --- /dev/null +++ b/src/main/java/DNPM/security/DelegatingDataBasedPermissionEvaluator.java @@ -0,0 +1,56 @@ +package DNPM.security; + +import org.springframework.security.access.PermissionEvaluator; +import org.springframework.security.core.Authentication; +import org.springframework.stereotype.Component; + +import java.io.Serializable; +import java.util.List; + +/** + * PermissionEvaluator zur Gesamtprüfung der Zugriffsberechtigung. + * Die konkrete Berechtigungsprüfung wird an die nachgelagerten PermissionEvaluatoren delegiert, + * welche jeweils einzeln dem Zugriff zustimmen müssen. + */ +@Component +public class DelegatingDataBasedPermissionEvaluator implements PermissionEvaluator { + + private final List permissionEvaluators; + + public DelegatingDataBasedPermissionEvaluator(final List permissionEvaluators) { + this.permissionEvaluators = permissionEvaluators; + } + + /** + * Auswertung der Zugriffsberechtigung für authentifizierten Benutzer auf Zielobjekt mit angeforderter Berechtigung. + * Hierbei wird die Berechtigungsprüfung an alle nachgelagerten PermissionEvaluatoren delegiert. + * Alle müssen dem Zugriff zustimmen. + * + * @param authentication Das Authentication Objekt + * @param targetObject Das Zielobjekt + * @param permissionType Die angeforderte Berechtigung + * @return Gibt true zurück, wenn der Benutzer die Berechtigung hat + */ + @Override + public boolean hasPermission(Authentication authentication, Object targetObject, Object permissionType) { + return permissionEvaluators.stream() + .allMatch(permissionEvaluator -> permissionEvaluator.hasPermission(authentication, targetObject, permissionType)); + } + + /** + * Auswertung anhand der ID und des Namens des Zielobjekts. + * Hierbei wird die Berechtigungsprüfung an alle nachgelagerten PermissionEvaluatoren delegiert. + * Alle müssen dem Zugriff zustimmen. + * + * @param authentication Authentication-Object + * @param targetId ID des Objekts + * @param targetType Name der Zielobjektklasse + * @param permissionType Die angeforderte Berechtigung + * @return Gibt true zurück, wenn der Benutzer die Berechtigung hat + */ + @Override + public boolean hasPermission(Authentication authentication, Serializable targetId, String targetType, Object permissionType) { + return permissionEvaluators.stream() + .allMatch(permissionEvaluator -> permissionEvaluator.hasPermission(authentication, targetId, targetType, permissionType)); + } +} diff --git a/src/main/java/DNPM/security/FormBasedPermissionEvaluator.java b/src/main/java/DNPM/security/FormBasedPermissionEvaluator.java index 73937af..4ba19dc 100644 --- a/src/main/java/DNPM/security/FormBasedPermissionEvaluator.java +++ b/src/main/java/DNPM/security/FormBasedPermissionEvaluator.java @@ -3,8 +3,6 @@ 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 org.springframework.security.core.Authentication; import org.springframework.security.core.userdetails.UserDetails; import org.springframework.stereotype.Component; @@ -17,15 +15,10 @@ import java.util.List; * Permission-Evaluator zur Auswertung der Berechtigung auf Objekte aufgrund der Formularberechtigung */ @Component -public class FormBasedPermissionEvaluator implements PermissionEvaluator { - - private final IOnkostarApi onkostarApi; - - private final JdbcTemplate jdbcTemplate; +public class FormBasedPermissionEvaluator extends AbstractDelegatedPermissionEvaluator { public FormBasedPermissionEvaluator(final IOnkostarApi onkostarApi, final DataSource dataSource) { - this.onkostarApi = onkostarApi; - this.jdbcTemplate = new JdbcTemplate(dataSource); + super(onkostarApi, dataSource); } /** @@ -63,7 +56,7 @@ public class FormBasedPermissionEvaluator implements PermissionEvaluator { @Override public boolean hasPermission(Authentication authentication, Serializable targetId, String targetType, Object permissionType) { if (targetId instanceof Integer) { - if ("Patient".equals(targetType)) { + if (PATIENT.equals(targetType)) { return true; } var procedure = this.onkostarApi.getProcedure((int)targetId); diff --git a/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java b/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java index 0762dc9..21cdca1 100644 --- a/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java +++ b/src/main/java/DNPM/security/PersonPoolBasedPermissionEvaluator.java @@ -3,8 +3,6 @@ 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 org.springframework.security.core.Authentication; import org.springframework.security.core.userdetails.UserDetails; import org.springframework.stereotype.Component; @@ -17,15 +15,10 @@ import java.util.List; * Permission-Evaluator zur Auswertung der Berechtigung auf Objekte aufgrund der Personenstammberechtigung */ @Component -public class PersonPoolBasedPermissionEvaluator implements PermissionEvaluator { - - private final IOnkostarApi onkostarApi; - - private final JdbcTemplate jdbcTemplate; +public class PersonPoolBasedPermissionEvaluator extends AbstractDelegatedPermissionEvaluator { public PersonPoolBasedPermissionEvaluator(final IOnkostarApi onkostarApi, final DataSource dataSource) { - this.onkostarApi = onkostarApi; - this.jdbcTemplate = new JdbcTemplate(dataSource); + super(onkostarApi, dataSource); } /** @@ -70,18 +63,14 @@ public class PersonPoolBasedPermissionEvaluator implements PermissionEvaluator { private String getPersonPoolCode(int id, String type) { Patient patient = null; - switch (type) { - case "Patient": - patient = onkostarApi.getPatient(id); - break; - case "Procedure": - var procedure = onkostarApi.getProcedure(id); - if (null != procedure) { - patient = procedure.getPatient(); - } - break; - default: - break; + + if (PATIENT.equals(type)) { + patient = onkostarApi.getPatient(id); + } else if (PROCEDURE.equals(type)) { + var procedure = onkostarApi.getProcedure(id); + if (null != procedure) { + patient = procedure.getPatient(); + } } if (null != patient) { diff --git a/src/test/java/DNPM/security/DelegatingDataBasedPermissionEvaluatorTest.java b/src/test/java/DNPM/security/DelegatingDataBasedPermissionEvaluatorTest.java new file mode 100644 index 0000000..1d8ecf8 --- /dev/null +++ b/src/test/java/DNPM/security/DelegatingDataBasedPermissionEvaluatorTest.java @@ -0,0 +1,122 @@ +package DNPM.security; + +import de.itc.onkostar.api.IOnkostarApi; +import de.itc.onkostar.api.Patient; +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 org.springframework.security.core.GrantedAuthority; + +import java.util.Collection; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class DelegatingDataBasedPermissionEvaluatorTest { + + private IOnkostarApi onkostarApi; + + private PersonPoolBasedPermissionEvaluator personPoolBasedPermissionEvaluator; + + private FormBasedPermissionEvaluator formBasedPermissionEvaluator; + + private DelegatingDataBasedPermissionEvaluator delegatingDataBasedPermissionEvaluator; + + @BeforeEach + void setup( + @Mock IOnkostarApi onkostarApi, + @Mock PersonPoolBasedPermissionEvaluator personPoolBasedPermissionEvaluator, + @Mock FormBasedPermissionEvaluator formBasedPermissionEvaluator + ) { + this.onkostarApi = onkostarApi; + this.personPoolBasedPermissionEvaluator = personPoolBasedPermissionEvaluator; + this.formBasedPermissionEvaluator = formBasedPermissionEvaluator; + + this.delegatingDataBasedPermissionEvaluator = new DelegatingDataBasedPermissionEvaluator( + List.of(personPoolBasedPermissionEvaluator, formBasedPermissionEvaluator) + ); + } + + @Test + void testShouldGrantPermissionIfAllDelegatedPermissionEvaluatorsGrantsAccessByObject() { + when(personPoolBasedPermissionEvaluator.hasPermission(any(), any(Patient.class), any(PermissionType.class))).thenReturn(true); + when(formBasedPermissionEvaluator.hasPermission(any(), any(Patient.class), any(PermissionType.class))).thenReturn(true); + + var actual = delegatingDataBasedPermissionEvaluator.hasPermission(new DummyAuthentication(), new Patient(this.onkostarApi), PermissionType.READ); + + assertThat(actual).isTrue(); + } + + @Test + void testShouldGrantPermissionIfAllDelegatedPermissionEvaluatorsGrantsAccessByIdAndType() { + when(personPoolBasedPermissionEvaluator.hasPermission(any(), anyInt(), anyString(), any(PermissionType.class))).thenReturn(true); + when(formBasedPermissionEvaluator.hasPermission(any(), anyInt(), anyString(), any(PermissionType.class))).thenReturn(true); + + var actual = delegatingDataBasedPermissionEvaluator.hasPermission(new DummyAuthentication(), 123, "Patient", PermissionType.READ); + + assertThat(actual).isTrue(); + } + + @Test + void testShouldDenyPermissionIfAtLeastOneDelegatedPermissionEvaluatorsDeniesAccessByObject() { + when(personPoolBasedPermissionEvaluator.hasPermission(any(), any(Patient.class), any(PermissionType.class))).thenReturn(true); + when(formBasedPermissionEvaluator.hasPermission(any(), any(Patient.class), any(PermissionType.class))).thenReturn(false); + + var actual = delegatingDataBasedPermissionEvaluator.hasPermission(new DummyAuthentication(), new Patient(this.onkostarApi), PermissionType.READ); + + assertThat(actual).isFalse(); + } + + @Test + void testShouldDenyPermissionIfAtLeastOneDelegatedPermissionEvaluatorsDeniesAccessByIdAndType() { + when(personPoolBasedPermissionEvaluator.hasPermission(any(), anyInt(), anyString(), any(PermissionType.class))).thenReturn(false); + + var actual = delegatingDataBasedPermissionEvaluator.hasPermission(new DummyAuthentication(), 123, "Patient", PermissionType.READ); + + assertThat(actual).isFalse(); + } + +} + +class DummyAuthentication implements Authentication { + @Override + public String getName() { + return "dummy"; + } + + @Override + public Collection getAuthorities() { + return null; + } + + @Override + public Object getCredentials() { + return null; + } + + @Override + public Object getDetails() { + return null; + } + + @Override + public Object getPrincipal() { + return null; + } + + @Override + public boolean isAuthenticated() { + return false; + } + + @Override + public void setAuthenticated(boolean b) throws IllegalArgumentException { + + } +} \ No newline at end of file -- cgit v1.2.3 From 5b9b12afc9ed29d005442b3a18a45b9a3104ad84 Mon Sep 17 00:00:00 2001 From: Paul-Christian Volkmer Date: Mon, 10 Apr 2023 14:26:35 +0200 Subject: Issue #24: Extrahiere Service mit Datenbankanfragen --- .../AbstractDelegatedPermissionEvaluator.java | 9 +- .../security/FormBasedPermissionEvaluator.java | 49 ++----- .../PersonPoolBasedPermissionEvaluator.java | 35 +---- src/main/java/DNPM/security/SecurityService.java | 60 ++++++++ .../security/FormBasedPermissionEvaluatorTest.java | 112 +++++++++++++++ .../PersonPoolBasedPermissionEvaluatorTest.java | 156 +++++++++++++++++++++ 6 files changed, 347 insertions(+), 74 deletions(-) create mode 100644 src/main/java/DNPM/security/SecurityService.java create mode 100644 src/test/java/DNPM/security/FormBasedPermissionEvaluatorTest.java create mode 100644 src/test/java/DNPM/security/PersonPoolBasedPermissionEvaluatorTest.java 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; - } - - 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")); + return true; } - } 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 -- cgit v1.2.3 From f2dc5b014d68fa61bacd5f9928eedd0c4c882070 Mon Sep 17 00:00:00 2001 From: Paul-Christian Volkmer Date: Mon, 10 Apr 2023 14:56:15 +0200 Subject: Issue #24: Annotationen für formularbasierte Berechtigungsprüfung --- .../DNPM/security/FormBasedSecurityAspects.java | 51 +++++++ src/main/java/DNPM/security/FormSecured.java | 14 ++ src/main/java/DNPM/security/FormSecuredResult.java | 14 ++ .../security/PersonPoolBasedSecurityAspects.java | 74 ++++++++++ src/main/java/DNPM/security/SecurityAspects.java | 74 ---------- .../security/FormBasedSecurityAspectsTest.java | 132 +++++++++++++++++ .../PersonPoolBasedSecurityAspectsTest.java | 164 +++++++++++++++++++++ .../java/DNPM/security/SecurityAspectsTest.java | 164 --------------------- 8 files changed, 449 insertions(+), 238 deletions(-) create mode 100644 src/main/java/DNPM/security/FormBasedSecurityAspects.java create mode 100644 src/main/java/DNPM/security/FormSecured.java create mode 100644 src/main/java/DNPM/security/FormSecuredResult.java create mode 100644 src/main/java/DNPM/security/PersonPoolBasedSecurityAspects.java delete mode 100644 src/main/java/DNPM/security/SecurityAspects.java create mode 100644 src/test/java/DNPM/security/FormBasedSecurityAspectsTest.java create mode 100644 src/test/java/DNPM/security/PersonPoolBasedSecurityAspectsTest.java delete mode 100644 src/test/java/DNPM/security/SecurityAspectsTest.java diff --git a/src/main/java/DNPM/security/FormBasedSecurityAspects.java b/src/main/java/DNPM/security/FormBasedSecurityAspects.java new file mode 100644 index 0000000..3dea944 --- /dev/null +++ b/src/main/java/DNPM/security/FormBasedSecurityAspects.java @@ -0,0 +1,51 @@ +package DNPM.security; + +import de.itc.onkostar.api.Procedure; +import org.aspectj.lang.JoinPoint; +import org.aspectj.lang.annotation.AfterReturning; +import org.aspectj.lang.annotation.Aspect; +import org.aspectj.lang.annotation.Before; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.stereotype.Component; + +import java.util.Arrays; + +@Component +@Aspect +public class FormBasedSecurityAspects { + + private final Logger logger = LoggerFactory.getLogger(this.getClass()); + + private final FormBasedPermissionEvaluator permissionEvaluator; + + public FormBasedSecurityAspects( + final FormBasedPermissionEvaluator permissionEvaluator + ) { + this.permissionEvaluator = permissionEvaluator; + } + + @AfterReturning(value = "@annotation(FormSecuredResult)", returning = "procedure") + public void afterProcedureFormBased(Procedure procedure) { + if ( + null != procedure + && ! permissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(), procedure, PermissionType.READ_WRITE) + ) { + logger.warn("Rückgabe von Prozedur blockiert: {}", procedure.getId()); + throw new IllegalSecuredObjectAccessException(); + } + } + + @Before(value = "@annotation(FormSecured)") + public void beforeProcedureFormBased(JoinPoint jp) { + Arrays.stream(jp.getArgs()) + .filter(arg -> arg instanceof Procedure) + .forEach(procedure -> { + if (! permissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(), procedure, PermissionType.READ_WRITE)) { + logger.warn("Zugriff auf Prozedur blockiert: {}", ((Procedure)procedure).getId()); + throw new IllegalSecuredObjectAccessException(); + } + }); + } +} diff --git a/src/main/java/DNPM/security/FormSecured.java b/src/main/java/DNPM/security/FormSecured.java new file mode 100644 index 0000000..2e12667 --- /dev/null +++ b/src/main/java/DNPM/security/FormSecured.java @@ -0,0 +1,14 @@ +package DNPM.security; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.METHOD) +public @interface FormSecured { + + PermissionType value() default PermissionType.READ_WRITE; + +} diff --git a/src/main/java/DNPM/security/FormSecuredResult.java b/src/main/java/DNPM/security/FormSecuredResult.java new file mode 100644 index 0000000..ccfbd24 --- /dev/null +++ b/src/main/java/DNPM/security/FormSecuredResult.java @@ -0,0 +1,14 @@ +package DNPM.security; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.METHOD) +public @interface FormSecuredResult { + + PermissionType value() default PermissionType.READ_WRITE; + +} diff --git a/src/main/java/DNPM/security/PersonPoolBasedSecurityAspects.java b/src/main/java/DNPM/security/PersonPoolBasedSecurityAspects.java new file mode 100644 index 0000000..37c313f --- /dev/null +++ b/src/main/java/DNPM/security/PersonPoolBasedSecurityAspects.java @@ -0,0 +1,74 @@ +package DNPM.security; + +import de.itc.onkostar.api.Patient; +import de.itc.onkostar.api.Procedure; +import org.aspectj.lang.JoinPoint; +import org.aspectj.lang.annotation.AfterReturning; +import org.aspectj.lang.annotation.Aspect; +import org.aspectj.lang.annotation.Before; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.stereotype.Component; + +import java.util.Arrays; + +@Component +@Aspect +public class PersonPoolBasedSecurityAspects { + + private final Logger logger = LoggerFactory.getLogger(this.getClass()); + + private final PersonPoolBasedPermissionEvaluator permissionEvaluator; + + public PersonPoolBasedSecurityAspects(PersonPoolBasedPermissionEvaluator permissionEvaluator) { + this.permissionEvaluator = permissionEvaluator; + } + + @AfterReturning(value = "@annotation(PersonPoolSecuredResult) ", returning = "patient") + public void afterPatient(Patient patient) { + if ( + null != patient + && ! permissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(), patient, PermissionType.READ_WRITE) + ) { + logger.warn("Rückgabe von Patient blockiert: {}", patient.getId()); + throw new IllegalSecuredObjectAccessException(); + } + } + + @AfterReturning(value = "@annotation(PersonPoolSecuredResult)", returning = "procedure") + public void afterProcedure(Procedure procedure) { + if ( + null != procedure + && ! permissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(), procedure, PermissionType.READ_WRITE) + ) { + logger.warn("Rückgabe von Prozedur blockiert: {}", procedure.getId()); + throw new IllegalSecuredObjectAccessException(); + } + } + + @Before(value = "@annotation(PersonPoolSecured)") + public void beforePatient(JoinPoint jp) { + Arrays.stream(jp.getArgs()) + .filter(arg -> arg instanceof Patient) + .forEach(patient -> { + if (! permissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(), patient, PermissionType.READ_WRITE)) { + logger.warn("Zugriff auf Patient blockiert: {}", ((Patient)patient).getId()); + throw new IllegalSecuredObjectAccessException(); + } + }); + } + + @Before(value = "@annotation(PersonPoolSecured)") + public void beforeProcedure(JoinPoint jp) { + Arrays.stream(jp.getArgs()) + .filter(arg -> arg instanceof Procedure) + .forEach(procedure -> { + if (! permissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(), procedure, PermissionType.READ_WRITE)) { + logger.warn("Zugriff auf Prozedur blockiert: {}", ((Procedure)procedure).getId()); + throw new IllegalSecuredObjectAccessException(); + } + }); + } + +} diff --git a/src/main/java/DNPM/security/SecurityAspects.java b/src/main/java/DNPM/security/SecurityAspects.java deleted file mode 100644 index a1fcd3f..0000000 --- a/src/main/java/DNPM/security/SecurityAspects.java +++ /dev/null @@ -1,74 +0,0 @@ -package DNPM.security; - -import de.itc.onkostar.api.Patient; -import de.itc.onkostar.api.Procedure; -import org.aspectj.lang.JoinPoint; -import org.aspectj.lang.annotation.AfterReturning; -import org.aspectj.lang.annotation.Aspect; -import org.aspectj.lang.annotation.Before; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.security.core.context.SecurityContextHolder; -import org.springframework.stereotype.Component; - -import java.util.Arrays; - -@Component -@Aspect -public class SecurityAspects { - - private final Logger logger = LoggerFactory.getLogger(this.getClass()); - - private final PersonPoolBasedPermissionEvaluator permissionEvaluator; - - public SecurityAspects(PersonPoolBasedPermissionEvaluator permissionEvaluator) { - this.permissionEvaluator = permissionEvaluator; - } - - @AfterReturning(value = "@annotation(PersonPoolSecuredResult) ", returning = "patient") - public void afterPatient(Patient patient) { - if ( - null != patient - && ! permissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(), patient, PermissionType.READ_WRITE) - ) { - logger.warn("Rückgabe von Patient blockiert: {}", patient.getId()); - throw new IllegalSecuredObjectAccessException(); - } - } - - @AfterReturning(value = "@annotation(PersonPoolSecuredResult)", returning = "procedure") - public void afterProcedure(Procedure procedure) { - if ( - null != procedure - && ! permissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(), procedure, PermissionType.READ_WRITE) - ) { - logger.warn("Rückgabe von Prozedur blockiert: {}", procedure.getId()); - throw new IllegalSecuredObjectAccessException(); - } - } - - @Before(value = "@annotation(PersonPoolSecured)") - public void beforePatient(JoinPoint jp) { - Arrays.stream(jp.getArgs()) - .filter(arg -> arg instanceof Patient) - .forEach(patient -> { - if (! permissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(), patient, PermissionType.READ_WRITE)) { - logger.warn("Zugriff auf Patient blockiert: {}", ((Patient)patient).getId()); - throw new IllegalSecuredObjectAccessException(); - } - }); - } - - @Before(value = "@annotation(PersonPoolSecured)") - public void beforeProcedure(JoinPoint jp) { - Arrays.stream(jp.getArgs()) - .filter(arg -> arg instanceof Procedure) - .forEach(procedure -> { - if (! permissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(), procedure, PermissionType.READ_WRITE)) { - logger.warn("Zugriff auf Prozedur blockiert: {}", ((Procedure)procedure).getId()); - throw new IllegalSecuredObjectAccessException(); - } - }); - } - -} diff --git a/src/test/java/DNPM/security/FormBasedSecurityAspectsTest.java b/src/test/java/DNPM/security/FormBasedSecurityAspectsTest.java new file mode 100644 index 0000000..a7ae32c --- /dev/null +++ b/src/test/java/DNPM/security/FormBasedSecurityAspectsTest.java @@ -0,0 +1,132 @@ +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.aop.aspectj.annotation.AspectJProxyFactory; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +@ExtendWith(MockitoExtension.class) +class FormBasedSecurityAspectsTest { + + private DummyClass dummyClass; + + private IOnkostarApi onkostarApi; + + private FormBasedPermissionEvaluator permissionEvaluator; + + @BeforeEach + void setup( + @Mock IOnkostarApi onkostarApi, + @Mock FormBasedPermissionEvaluator permissionEvaluator + ) { + this.onkostarApi = onkostarApi; + this.permissionEvaluator = permissionEvaluator; + + // Create proxied instance of DummyClass as done within Onkostar using Spring AOP + var dummyClass = new DummyClass(onkostarApi); + AspectJProxyFactory factory = new AspectJProxyFactory(dummyClass); + FormBasedSecurityAspects securityAspects = new FormBasedSecurityAspects(this.permissionEvaluator); + factory.addAspect(securityAspects); + this.dummyClass = factory.getProxy(); + } + + @Test + void testShouldAllowSecuredMethodCallWithPatientParam() { + this.dummyClass.methodWithPatientParam(new Patient(onkostarApi)); + verify(onkostarApi, times(1)).savePatient(any(Patient.class)); + } + + @Test + void testShouldPreventSecuredMethodCallWithProcedureParam() { + when(this.permissionEvaluator.hasPermission(any(), any(Procedure.class), any(PermissionType.class))) + .thenReturn(false); + + var exception = assertThrows( + Exception.class, + () -> this.dummyClass.methodWithProcedureParam(new Procedure(onkostarApi)) + ); + assertThat(exception).isExactlyInstanceOf(IllegalSecuredObjectAccessException.class); + } + + @Test + void testShouldAllowSecuredMethodCallWithProcedureParam() throws Exception { + when(this.permissionEvaluator.hasPermission(any(), any(Procedure.class), any(PermissionType.class))) + .thenReturn(true); + + this.dummyClass.methodWithProcedureParam(new Procedure(onkostarApi)); + + verify(onkostarApi, times(1)).saveProcedure(any(Procedure.class), anyBoolean()); + } + + @Test + void testShouldAllowSecuredMethodCallWithPatientReturnValue() { + var actual = this.dummyClass.methodWithPatientReturnValue(1); + assertThat(actual).isNotNull(); + } + + @Test + void testShouldPreventSecuredMethodCallWithProcedureReturnValue() { + when(this.permissionEvaluator.hasPermission(any(), any(Procedure.class), any(PermissionType.class))) + .thenReturn(false); + + var exception = assertThrows( + Exception.class, + () -> this.dummyClass.methodWithProcedureReturnValue(1) + ); + assertThat(exception).isExactlyInstanceOf(IllegalSecuredObjectAccessException.class); + } + + @Test + void testShouldAllowSecuredMethodCallWithProcedureReturnValue() { + when(this.permissionEvaluator.hasPermission(any(), any(Procedure.class), any(PermissionType.class))) + .thenReturn(true); + + var actual = this.dummyClass.methodWithProcedureReturnValue(1); + + assertThat(actual).isNotNull(); + } + + private static class DummyClass { + + private final IOnkostarApi onkostarApi; + + DummyClass(final IOnkostarApi onkostarApi) { + this.onkostarApi = onkostarApi; + } + + @FormSecured + public void methodWithPatientParam(Patient patient) { + this.onkostarApi.savePatient(patient); + } + + @FormSecured + public void methodWithProcedureParam(Procedure procedure) throws Exception { + this.onkostarApi.saveProcedure(procedure, false); + } + + @FormSecuredResult + public Patient methodWithPatientReturnValue(int id) { + var patient = new Patient(this.onkostarApi); + patient.setId(id); + return patient; + } + + @FormSecuredResult + public Procedure methodWithProcedureReturnValue(int id) { + var procedure = new Procedure(this.onkostarApi); + procedure.setId(id); + return procedure; + } + } + +} diff --git a/src/test/java/DNPM/security/PersonPoolBasedSecurityAspectsTest.java b/src/test/java/DNPM/security/PersonPoolBasedSecurityAspectsTest.java new file mode 100644 index 0000000..b20127e --- /dev/null +++ b/src/test/java/DNPM/security/PersonPoolBasedSecurityAspectsTest.java @@ -0,0 +1,164 @@ +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.aop.aspectj.annotation.AspectJProxyFactory; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +@ExtendWith(MockitoExtension.class) +class PersonPoolBasedSecurityAspectsTest { + + private DummyClass dummyClass; + + private IOnkostarApi onkostarApi; + + private PersonPoolBasedPermissionEvaluator permissionEvaluator; + + @BeforeEach + void setup( + @Mock IOnkostarApi onkostarApi, + @Mock PersonPoolBasedPermissionEvaluator permissionEvaluator + ) { + this.onkostarApi = onkostarApi; + this.permissionEvaluator = permissionEvaluator; + + // Create proxied instance of DummyClass as done within Onkostar using Spring AOP + var dummyClass = new DummyClass(onkostarApi); + AspectJProxyFactory factory = new AspectJProxyFactory(dummyClass); + PersonPoolBasedSecurityAspects securityAspects = new PersonPoolBasedSecurityAspects(this.permissionEvaluator); + factory.addAspect(securityAspects); + this.dummyClass = factory.getProxy(); + } + + @Test + void testShouldPreventSecuredMethodCallWithPatientParam() { + when(this.permissionEvaluator.hasPermission(any(), any(Patient.class), any(PermissionType.class))) + .thenReturn(false); + + var exception = assertThrows( + Exception.class, + () -> this.dummyClass.methodWithPatientParam(new Patient(onkostarApi)) + ); + assertThat(exception).isExactlyInstanceOf(IllegalSecuredObjectAccessException.class); + } + + @Test + void testShouldAllowSecuredMethodCallWithPatientParam() { + when(this.permissionEvaluator.hasPermission(any(), any(Patient.class), any(PermissionType.class))) + .thenReturn(true); + + this.dummyClass.methodWithPatientParam(new Patient(onkostarApi)); + + verify(onkostarApi, times(1)).savePatient(any(Patient.class)); + } + + @Test + void testShouldPreventSecuredMethodCallWithProcedureParam() { + when(this.permissionEvaluator.hasPermission(any(), any(Procedure.class), any(PermissionType.class))) + .thenReturn(false); + + var exception = assertThrows( + Exception.class, + () -> this.dummyClass.methodWithProcedureParam(new Procedure(onkostarApi)) + ); + assertThat(exception).isExactlyInstanceOf(IllegalSecuredObjectAccessException.class); + } + + @Test + void testShouldAllowSecuredMethodCallWithProcedureParam() throws Exception { + when(this.permissionEvaluator.hasPermission(any(), any(Procedure.class), any(PermissionType.class))) + .thenReturn(true); + + this.dummyClass.methodWithProcedureParam(new Procedure(onkostarApi)); + + verify(onkostarApi, times(1)).saveProcedure(any(Procedure.class), anyBoolean()); + } + + @Test + void testShouldPreventSecuredMethodCallWithPatientReturnValue() { + when(this.permissionEvaluator.hasPermission(any(), any(Patient.class), any(PermissionType.class))) + .thenReturn(false); + + var exception = assertThrows( + Exception.class, + () -> this.dummyClass.methodWithPatientReturnValue(1) + ); + assertThat(exception).isExactlyInstanceOf(IllegalSecuredObjectAccessException.class); + } + + @Test + void testShouldAllowSecuredMethodCallWithPatientReturnValue() { + when(this.permissionEvaluator.hasPermission(any(), any(Patient.class), any(PermissionType.class))) + .thenReturn(true); + + var actual = this.dummyClass.methodWithPatientReturnValue(1); + + assertThat(actual).isNotNull(); + } + + @Test + void testShouldPreventSecuredMethodCallWithProcedureReturnValue() { + when(this.permissionEvaluator.hasPermission(any(), any(Procedure.class), any(PermissionType.class))) + .thenReturn(false); + + var exception = assertThrows( + Exception.class, + () -> this.dummyClass.methodWithProcedureReturnValue(1) + ); + assertThat(exception).isExactlyInstanceOf(IllegalSecuredObjectAccessException.class); + } + + @Test + void testShouldAllowSecuredMethodCallWithProcedureReturnValue() { + when(this.permissionEvaluator.hasPermission(any(), any(Procedure.class), any(PermissionType.class))) + .thenReturn(true); + + var actual = this.dummyClass.methodWithProcedureReturnValue(1); + + assertThat(actual).isNotNull(); + } + + private static class DummyClass { + + private final IOnkostarApi onkostarApi; + + DummyClass(final IOnkostarApi onkostarApi) { + this.onkostarApi = onkostarApi; + } + + @PersonPoolSecured + public void methodWithPatientParam(Patient patient) { + this.onkostarApi.savePatient(patient); + } + + @PersonPoolSecured + public void methodWithProcedureParam(Procedure procedure) throws Exception { + this.onkostarApi.saveProcedure(procedure, false); + } + + @PersonPoolSecuredResult + public Patient methodWithPatientReturnValue(int id) { + var patient = new Patient(this.onkostarApi); + patient.setId(id); + return patient; + } + + @PersonPoolSecuredResult + public Procedure methodWithProcedureReturnValue(int id) { + var procedure = new Procedure(this.onkostarApi); + procedure.setId(id); + return procedure; + } + } + +} diff --git a/src/test/java/DNPM/security/SecurityAspectsTest.java b/src/test/java/DNPM/security/SecurityAspectsTest.java deleted file mode 100644 index 8416b8a..0000000 --- a/src/test/java/DNPM/security/SecurityAspectsTest.java +++ /dev/null @@ -1,164 +0,0 @@ -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.aop.aspectj.annotation.AspectJProxyFactory; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.*; - -@ExtendWith(MockitoExtension.class) -class SecurityAspectsTest { - - private DummyClass dummyClass; - - private IOnkostarApi onkostarApi; - - private PersonPoolBasedPermissionEvaluator permissionEvaluator; - - @BeforeEach - void setup( - @Mock IOnkostarApi onkostarApi, - @Mock PersonPoolBasedPermissionEvaluator permissionEvaluator - ) { - this.onkostarApi = onkostarApi; - this.permissionEvaluator = permissionEvaluator; - - // Create proxied instance of DummyClass as done within Onkostar using Spring AOP - var dummyClass = new DummyClass(onkostarApi); - AspectJProxyFactory factory = new AspectJProxyFactory(dummyClass); - SecurityAspects securityAspects = new SecurityAspects(this.permissionEvaluator); - factory.addAspect(securityAspects); - this.dummyClass = factory.getProxy(); - } - - @Test - void testShouldPreventSecuredMethodCallWithPatientParam() { - when(this.permissionEvaluator.hasPermission(any(), any(Patient.class), any(PermissionType.class))) - .thenReturn(false); - - var exception = assertThrows( - Exception.class, - () -> this.dummyClass.methodWithPatientParam(new Patient(onkostarApi)) - ); - assertThat(exception).isExactlyInstanceOf(IllegalSecuredObjectAccessException.class); - } - - @Test - void testShouldAllowSecuredMethodCallWithPatientParam() { - when(this.permissionEvaluator.hasPermission(any(), any(Patient.class), any(PermissionType.class))) - .thenReturn(true); - - this.dummyClass.methodWithPatientParam(new Patient(onkostarApi)); - - verify(onkostarApi, times(1)).savePatient(any(Patient.class)); - } - - @Test - void testShouldPreventSecuredMethodCallWithProcedureParam() { - when(this.permissionEvaluator.hasPermission(any(), any(Procedure.class), any(PermissionType.class))) - .thenReturn(false); - - var exception = assertThrows( - Exception.class, - () -> this.dummyClass.methodWithProcedureParam(new Procedure(onkostarApi)) - ); - assertThat(exception).isExactlyInstanceOf(IllegalSecuredObjectAccessException.class); - } - - @Test - void testShouldAllowSecuredMethodCallWithProcedureParam() throws Exception { - when(this.permissionEvaluator.hasPermission(any(), any(Procedure.class), any(PermissionType.class))) - .thenReturn(true); - - this.dummyClass.methodWithProcedureParam(new Procedure(onkostarApi)); - - verify(onkostarApi, times(1)).saveProcedure(any(Procedure.class), anyBoolean()); - } - - @Test - void testShouldPreventSecuredMethodCallWithPatientReturnValue() { - when(this.permissionEvaluator.hasPermission(any(), any(Patient.class), any(PermissionType.class))) - .thenReturn(false); - - var exception = assertThrows( - Exception.class, - () -> this.dummyClass.methodWithPatientReturnValue(1) - ); - assertThat(exception).isExactlyInstanceOf(IllegalSecuredObjectAccessException.class); - } - - @Test - void testShouldAllowSecuredMethodCallWithPatientReturnValue() { - when(this.permissionEvaluator.hasPermission(any(), any(Patient.class), any(PermissionType.class))) - .thenReturn(true); - - var actual = this.dummyClass.methodWithPatientReturnValue(1); - - assertThat(actual).isNotNull(); - } - - @Test - void testShouldPreventSecuredMethodCallWithProcedureReturnValue() { - when(this.permissionEvaluator.hasPermission(any(), any(Procedure.class), any(PermissionType.class))) - .thenReturn(false); - - var exception = assertThrows( - Exception.class, - () -> this.dummyClass.methodWithProcedureReturnValue(1) - ); - assertThat(exception).isExactlyInstanceOf(IllegalSecuredObjectAccessException.class); - } - - @Test - void testShouldAllowSecuredMethodCallWithProcedureReturnValue() { - when(this.permissionEvaluator.hasPermission(any(), any(Procedure.class), any(PermissionType.class))) - .thenReturn(true); - - var actual = this.dummyClass.methodWithProcedureReturnValue(1); - - assertThat(actual).isNotNull(); - } - -} - -class DummyClass { - - private final IOnkostarApi onkostarApi; - - DummyClass(final IOnkostarApi onkostarApi) { - this.onkostarApi = onkostarApi; - } - - @PersonPoolSecured - public void methodWithPatientParam(Patient patient) { - this.onkostarApi.savePatient(patient); - } - - @PersonPoolSecured - public void methodWithProcedureParam(Procedure procedure) throws Exception { - this.onkostarApi.saveProcedure(procedure, false); - } - - @PersonPoolSecuredResult - public Patient methodWithPatientReturnValue(int id) { - var patient = new Patient(this.onkostarApi); - patient.setId(id); - return patient; - } - - @PersonPoolSecuredResult - public Procedure methodWithProcedureReturnValue(int id) { - var procedure = new Procedure(this.onkostarApi); - procedure.setId(id); - return procedure; - } -} -- cgit v1.2.3