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/SecurityAspects.java b/src/main/java/DNPM/security/PersonPoolBasedSecurityAspects.java similarity index 95% rename from src/main/java/DNPM/security/SecurityAspects.java rename to src/main/java/DNPM/security/PersonPoolBasedSecurityAspects.java index a1fcd3f..37c313f 100644 --- a/src/main/java/DNPM/security/SecurityAspects.java +++ b/src/main/java/DNPM/security/PersonPoolBasedSecurityAspects.java @@ -15,13 +15,13 @@ import java.util.Arrays; @Component @Aspect -public class SecurityAspects { +public class PersonPoolBasedSecurityAspects { private final Logger logger = LoggerFactory.getLogger(this.getClass()); private final PersonPoolBasedPermissionEvaluator permissionEvaluator; - public SecurityAspects(PersonPoolBasedPermissionEvaluator permissionEvaluator) { + public PersonPoolBasedSecurityAspects(PersonPoolBasedPermissionEvaluator permissionEvaluator) { this.permissionEvaluator = permissionEvaluator; } 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/SecurityAspectsTest.java b/src/test/java/DNPM/security/PersonPoolBasedSecurityAspectsTest.java similarity index 81% rename from src/test/java/DNPM/security/SecurityAspectsTest.java rename to src/test/java/DNPM/security/PersonPoolBasedSecurityAspectsTest.java index 8416b8a..b20127e 100644 --- a/src/test/java/DNPM/security/SecurityAspectsTest.java +++ b/src/test/java/DNPM/security/PersonPoolBasedSecurityAspectsTest.java @@ -16,7 +16,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; @ExtendWith(MockitoExtension.class) -class SecurityAspectsTest { +class PersonPoolBasedSecurityAspectsTest { private DummyClass dummyClass; @@ -35,7 +35,7 @@ class SecurityAspectsTest { // 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); + PersonPoolBasedSecurityAspects securityAspects = new PersonPoolBasedSecurityAspects(this.permissionEvaluator); factory.addAspect(securityAspects); this.dummyClass = factory.getProxy(); } @@ -128,37 +128,37 @@ class SecurityAspectsTest { assertThat(actual).isNotNull(); } -} + private static class DummyClass { -class DummyClass { + private final IOnkostarApi onkostarApi; - private final IOnkostarApi onkostarApi; + DummyClass(final IOnkostarApi onkostarApi) { + this.onkostarApi = onkostarApi; + } - DummyClass(final IOnkostarApi onkostarApi) { - this.onkostarApi = onkostarApi; - } - - @PersonPoolSecured - public void methodWithPatientParam(Patient patient) { + @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; + } } - @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; - } }