From 63bd60239732533f5e829e14df1a42a2dbab8184 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 23 Apr 2019 08:31:26 +0200 Subject: [PATCH] JSonDeserializerWithAccessFilter with working access rights validation --- .../service/accessfilter/AccessFor.java | 2 +- .../JSonDeserializerWithAccessFilter.java | 69 +++++++++-- .../service/accessfilter/SelfId.java | 17 +++ .../hsadminng/service/dto/CustomerDTO.java | 9 +- .../service/util/ReflectionUtil.java | 9 ++ .../rest/errors/BadRequestAlertException.java | 20 ++-- .../web/rest/errors/ExceptionTranslator.java | 2 +- src/main/webapp/i18n/de/custom-error.json | 4 +- src/main/webapp/i18n/en/custom-error.json | 4 +- ...nDeserializerWithAccessFilterUnitTest.java | 107 +++++++++++++++--- 10 files changed, 198 insertions(+), 45 deletions(-) create mode 100644 src/main/java/org/hostsharing/hsadminng/service/accessfilter/SelfId.java diff --git a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/AccessFor.java b/src/main/java/org/hostsharing/hsadminng/service/accessfilter/AccessFor.java index d25b5c87..852f6138 100644 --- a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/AccessFor.java +++ b/src/main/java/org/hostsharing/hsadminng/service/accessfilter/AccessFor.java @@ -3,9 +3,9 @@ package org.hostsharing.hsadminng.service.accessfilter; import java.lang.annotation.*; +@Documented @Target({ElementType.FIELD, ElementType.TYPE_USE}) @Retention(RetentionPolicy.RUNTIME) -@Documented public @interface AccessFor { Role[] init() default Role.NOBODY; diff --git a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializerWithAccessFilter.java b/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializerWithAccessFilter.java index e9c25ae9..13e3fbdd 100644 --- a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializerWithAccessFilter.java +++ b/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializerWithAccessFilter.java @@ -7,9 +7,13 @@ import com.fasterxml.jackson.databind.node.IntNode; import com.fasterxml.jackson.databind.node.LongNode; import com.fasterxml.jackson.databind.node.TextNode; import org.apache.commons.lang3.NotImplementedException; +import org.hostsharing.hsadminng.security.SecurityUtils; import org.hostsharing.hsadminng.service.util.ReflectionUtil; +import org.hostsharing.hsadminng.web.rest.errors.BadRequestAlertException; import java.lang.reflect.Field; +import java.util.HashSet; +import java.util.Set; import static org.hostsharing.hsadminng.service.util.ReflectionUtil.unchecked; @@ -17,6 +21,8 @@ public class JSonDeserializerWithAccessFilter { private final T dto; private final TreeNode treeNode; + private final Set modifiedFields = new HashSet<>(); + private Field selfIdField = null; public JSonDeserializerWithAccessFilter(final JsonParser jsonParser, final DeserializationContext deserializationContext, Class dtoClass) { this.treeNode = unchecked(() -> jsonParser.getCodec().readTree(jsonParser)); @@ -30,35 +36,82 @@ public class JSonDeserializerWithAccessFilter { final Field field = dto.getClass().getDeclaredField(fieldName); final Object value = readValue(treeNode, field); writeValue(dto, field, value); + markAsModified(field); } catch (NoSuchFieldException e) { throw new RuntimeException("setting field " + fieldName + " failed", e); } }); + + modifiedFields.forEach(this::checkAccess); return dto; } private Object readValue(final TreeNode treeNode, final Field field) { final TreeNode fieldNode = treeNode.get(field.getName()); if (fieldNode instanceof TextNode) { - return ((TextNode)fieldNode).asText(); + return ((TextNode) fieldNode).asText(); } else if (fieldNode instanceof IntNode) { - return ((IntNode)fieldNode).asInt(); + return ((IntNode) fieldNode).asInt(); } else if (fieldNode instanceof LongNode) { - return ((LongNode)fieldNode).asLong(); + return ((LongNode) fieldNode).asLong(); } else { throw new NotImplementedException("property type not yet implemented: " + field); } } private void writeValue(final T dto, final Field field, final Object value) { - if ( field.getType().isAssignableFrom(value.getClass()) ) { + if (field.getType().isAssignableFrom(value.getClass())) { ReflectionUtil.setValue(dto, field, value); - } else if (Integer.class.isAssignableFrom(field.getType()) || int.class.isAssignableFrom(field.getType())) { - ReflectionUtil.setValue(dto, field, ((Number)value).intValue()); + } else if (Integer.class.isAssignableFrom(field.getType()) || int.class.isAssignableFrom(field.getType())) { + ReflectionUtil.setValue(dto, field, ((Number) value).intValue()); } else if (Long.class.isAssignableFrom(field.getType()) || long.class.isAssignableFrom(field.getType())) { - ReflectionUtil.setValue(dto, field, ((Number)value).longValue()); - } else { + ReflectionUtil.setValue(dto, field, ((Number) value).longValue()); + } else { throw new NotImplementedException("property type not yet implemented: " + field); } } + + private void markAsModified(final Field field) { + modifiedFields.add(field); + } + + private Object getId() { + if (selfIdField == null) { + return null; + } + return ReflectionUtil.getValue(dto, selfIdField); + } + + private void checkAccess(final Field field) { + if ( !rememberSelfIdField(field) ) { + if (getId() == null) { + if (!getLoginUserRole().isAllowedToInit(field)) { + throw new BadRequestAlertException("Initialization of field prohibited for current user", toDisplay(field), "initializationProhibited"); + } + } else if (getId() != null) { + if (!getLoginUserRole().isAllowedToUpdate(field)) { + throw new BadRequestAlertException("Update of field prohibited for current user", toDisplay(field), "updateProhibited"); + } + } + } + } + + private boolean rememberSelfIdField(final Field field) { + if ( field.isAnnotationPresent(SelfId.class) ) { + if ( selfIdField != null ) { + throw new AssertionError("multiple " + SelfId.class + " detected in " + field.getDeclaringClass().getSimpleName() ); + } + selfIdField = field; + return true; + } + return false; + } + + private String toDisplay(final Field field) { + return field.getDeclaringClass().getSimpleName() + "." + field.getName(); + } + + private Role getLoginUserRole() { + return SecurityUtils.getCurrentUserLogin().map(u -> Role.valueOf(u.toUpperCase())).orElse(Role.ANYBODY); + } } diff --git a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/SelfId.java b/src/main/java/org/hostsharing/hsadminng/service/accessfilter/SelfId.java new file mode 100644 index 00000000..8d3a352d --- /dev/null +++ b/src/main/java/org/hostsharing/hsadminng/service/accessfilter/SelfId.java @@ -0,0 +1,17 @@ +package org.hostsharing.hsadminng.service.accessfilter; + +import java.lang.annotation.*; + +/** + * Used to mark a field within a DTO as containing the id of a field, + * it's needed to identify an existing entity for update functions. + * Initialization and update rights have no meaning for such fields, + * its initialized automatically and never updated. + * + * @see AccessFor + */ +@Documented +@Target({ElementType.FIELD}) +@Retention(RetentionPolicy.RUNTIME) +public @interface SelfId { +} diff --git a/src/main/java/org/hostsharing/hsadminng/service/dto/CustomerDTO.java b/src/main/java/org/hostsharing/hsadminng/service/dto/CustomerDTO.java index cdec25f3..28a84d2a 100644 --- a/src/main/java/org/hostsharing/hsadminng/service/dto/CustomerDTO.java +++ b/src/main/java/org/hostsharing/hsadminng/service/dto/CustomerDTO.java @@ -2,17 +2,11 @@ package org.hostsharing.hsadminng.service.dto; import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.core.TreeNode; import com.fasterxml.jackson.databind.DeserializationContext; import com.fasterxml.jackson.databind.JsonDeserializer; import com.fasterxml.jackson.databind.JsonSerializer; import com.fasterxml.jackson.databind.SerializerProvider; -import com.fasterxml.jackson.databind.node.IntNode; -import com.fasterxml.jackson.databind.node.TextNode; -import org.hostsharing.hsadminng.service.accessfilter.AccessFor; -import org.hostsharing.hsadminng.service.accessfilter.JSonDeserializerWithAccessFilter; -import org.hostsharing.hsadminng.service.accessfilter.JSonSerializerWithAccessFilter; -import org.hostsharing.hsadminng.service.accessfilter.Role; +import org.hostsharing.hsadminng.service.accessfilter.*; import org.springframework.boot.jackson.JsonComponent; import javax.validation.constraints.*; @@ -25,6 +19,7 @@ import java.util.Objects; */ public class CustomerDTO implements Serializable { + @SelfId @AccessFor(read = Role.ANY_CUSTOMER_USER) private Long id; diff --git a/src/main/java/org/hostsharing/hsadminng/service/util/ReflectionUtil.java b/src/main/java/org/hostsharing/hsadminng/service/util/ReflectionUtil.java index 9492a28c..0bcabc78 100644 --- a/src/main/java/org/hostsharing/hsadminng/service/util/ReflectionUtil.java +++ b/src/main/java/org/hostsharing/hsadminng/service/util/ReflectionUtil.java @@ -26,6 +26,15 @@ public class ReflectionUtil { } } + public static Object getValue(T dto, Field field) { + try { + field.setAccessible(true); + return field.get(dto); + } catch (IllegalAccessException e) { + throw new RuntimeException(e); + } + } + @FunctionalInterface public interface ThrowingSupplier { T get() throws Exception; diff --git a/src/main/java/org/hostsharing/hsadminng/web/rest/errors/BadRequestAlertException.java b/src/main/java/org/hostsharing/hsadminng/web/rest/errors/BadRequestAlertException.java index 0548500f..f395377a 100644 --- a/src/main/java/org/hostsharing/hsadminng/web/rest/errors/BadRequestAlertException.java +++ b/src/main/java/org/hostsharing/hsadminng/web/rest/errors/BadRequestAlertException.java @@ -11,32 +11,32 @@ public class BadRequestAlertException extends AbstractThrowableProblem { private static final long serialVersionUID = 1L; - private final String entityName; + private final String param; private final String errorKey; - public BadRequestAlertException(String defaultMessage, String entityName, String errorKey) { - this(ErrorConstants.DEFAULT_TYPE, defaultMessage, entityName, errorKey); + public BadRequestAlertException(String defaultMessage, String param, String errorKey) { + this(ErrorConstants.DEFAULT_TYPE, defaultMessage, param, errorKey); } - public BadRequestAlertException(URI type, String defaultMessage, String entityName, String errorKey) { - super(type, defaultMessage, Status.BAD_REQUEST, null, null, null, getAlertParameters(entityName, errorKey)); - this.entityName = entityName; + public BadRequestAlertException(URI type, String defaultMessage, String param, String errorKey) { + super(type, defaultMessage, Status.BAD_REQUEST, null, null, null, getAlertParameters(param, errorKey)); + this.param = param; this.errorKey = errorKey; } - public String getEntityName() { - return entityName; + public String getParam() { + return param; } public String getErrorKey() { return errorKey; } - private static Map getAlertParameters(String entityName, String errorKey) { + private static Map getAlertParameters(String param, String errorKey) { Map parameters = new HashMap<>(); parameters.put("message", "error." + errorKey); - parameters.put("params", entityName); + parameters.put("params", param); return parameters; } } diff --git a/src/main/java/org/hostsharing/hsadminng/web/rest/errors/ExceptionTranslator.java b/src/main/java/org/hostsharing/hsadminng/web/rest/errors/ExceptionTranslator.java index d810090c..ea414e84 100644 --- a/src/main/java/org/hostsharing/hsadminng/web/rest/errors/ExceptionTranslator.java +++ b/src/main/java/org/hostsharing/hsadminng/web/rest/errors/ExceptionTranslator.java @@ -104,7 +104,7 @@ public class ExceptionTranslator implements ProblemHandling { @ExceptionHandler public ResponseEntity handleBadRequestAlertException(BadRequestAlertException ex, NativeWebRequest request) { - return create(ex, request, HeaderUtil.createFailureAlert(ex.getEntityName(), ex.getErrorKey(), ex.getMessage())); + return create(ex, request, HeaderUtil.createFailureAlert(ex.getParam(), ex.getErrorKey(), ex.getMessage())); } @ExceptionHandler diff --git a/src/main/webapp/i18n/de/custom-error.json b/src/main/webapp/i18n/de/custom-error.json index 11be4363..89c3fffc 100644 --- a/src/main/webapp/i18n/de/custom-error.json +++ b/src/main/webapp/i18n/de/custom-error.json @@ -5,6 +5,8 @@ "shareTransactionImmutable": "Transaktionen mit Geschäftsanteilen sind unveränderlich", "membershipNotDeletable": "Mitgliedschaft kann nicht gelöscht werden, setze stattdessen das 'untilDate'", "untilDateMustBeAfterSinceDate": "Mitgliedshafts-Austrittsdatum muss nach dem Beitrittsdatum liegen", - "anotherUncancelledMembershipExists": "Nur eine einzige ungekündigte Mitgliedschaft pro Kunde ist zulässig" + "anotherUncancelledMembershipExists": "Nur eine einzige ungekündigte Mitgliedschaft pro Kunde ist zulässig", + "initializationProhibited": "Initialisierung des Feldes unzulässig", + "updateProhibited": "Aktualisierung des Feldes unzulässig" } } diff --git a/src/main/webapp/i18n/en/custom-error.json b/src/main/webapp/i18n/en/custom-error.json index 145dfcd1..13d9463a 100644 --- a/src/main/webapp/i18n/en/custom-error.json +++ b/src/main/webapp/i18n/en/custom-error.json @@ -5,6 +5,8 @@ "shareTransactionImmutable": "Share transactions are immutable", "membershipNotDeletable": "Membership cannot be deleted, instead set 'untilDate'", "untilDateMustBeAfterSinceDate": "Membership until date must be after since date", - "anotherUncancelledMembershipExists": "Only a single uncancelled membership allowed per customer" + "anotherUncancelledMembershipExists": "Only a single uncancelled membership allowed per customer", + "initializationProhibited": "Initialization of the field prohibited", + "updateProhibited": "Update of the field prohibited" } } diff --git a/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializerWithAccessFilterUnitTest.java b/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializerWithAccessFilterUnitTest.java index 2e3bcaa4..2b131490 100644 --- a/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializerWithAccessFilterUnitTest.java +++ b/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializerWithAccessFilterUnitTest.java @@ -1,14 +1,12 @@ package org.hostsharing.hsadminng.service.accessfilter; -import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.annotation.JsonTypeId; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.core.ObjectCodec; import com.fasterxml.jackson.core.TreeNode; -import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import org.apache.commons.lang3.NotImplementedException; import org.apache.commons.lang3.tuple.ImmutablePair; -import org.hostsharing.hsadminng.service.dto.CustomerDTO; +import org.hostsharing.hsadminng.web.rest.errors.BadRequestAlertException; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -22,9 +20,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.catchThrowable; import static org.hostsharing.hsadminng.service.accessfilter.MockSecurityContext.givenLoginUserWithRole; import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; +@SuppressWarnings("ALL") public class JSonDeserializerWithAccessFilterUnitTest { @Rule @@ -40,7 +37,7 @@ public class JSonDeserializerWithAccessFilterUnitTest { public TreeNode treeNode; @Before - public void init() throws IOException { + public void init() { givenLoginUserWithRole(Role.ANY_CUSTOMER_USER); given(jsonParser.getCodec()).willReturn(codec); @@ -82,21 +79,81 @@ public class JSonDeserializerWithAccessFilterUnitTest { assertThat(actualDto.openLongField).isEqualTo(1234L); } - // --- fixture code below --- + @Test + public void shouldDeserializeStringFieldIfRequiredRoleIsCoveredByUser() throws IOException { + // given + givenLoginUserWithRole(Role.FINANCIAL_CONTACT); + givenJSonTree(asJSon(ImmutablePair.of("restrictedField", "Restricted String Value"))); - private String asJSon(final ImmutablePair... properties) { + // when + GivenDto actualDto = new JSonDeserializerWithAccessFilter<>(jsonParser, null, GivenDto.class).deserialize(); + + // then + assertThat(actualDto.restrictedField).isEqualTo("Restricted String Value"); + } + + @Test + public void shouldInitializeFieldIfRequiredRoleIsNotCoveredByUser() throws IOException { + // given + givenLoginUserWithRole(Role.ANY_CUSTOMER_USER); + givenJSonTree(asJSon(ImmutablePair.of("restrictedField", "Restricted String Value"))); + + // when + Throwable exception = catchThrowable(() -> new JSonDeserializerWithAccessFilter<>(jsonParser, null, GivenDto.class).deserialize()); + + // then + assertThat(exception).isInstanceOfSatisfying(BadRequestAlertException.class, badRequestAlertException -> { + assertThat(badRequestAlertException.getParam()).isEqualTo("GivenDto.restrictedField"); + assertThat(badRequestAlertException.getErrorKey()).isEqualTo("initializationProhibited"); + }); + } + + @Test + public void shouldUpdateFieldIfRequiredRoleIsNotCoveredByUser() throws IOException { + // given + givenLoginUserWithRole(Role.ANY_CUSTOMER_USER); + givenJSonTree(asJSon( + ImmutablePair.of("id", 1234L), + ImmutablePair.of("restrictedField", "Restricted String Value"))); + + // when + Throwable exception = catchThrowable(() -> new JSonDeserializerWithAccessFilter<>(jsonParser, null, GivenDto.class).deserialize()); + + // then + assertThat(exception).isInstanceOfSatisfying(BadRequestAlertException.class, badRequestAlertException -> { + assertThat(badRequestAlertException.getParam()).isEqualTo("GivenDto.restrictedField"); + assertThat(badRequestAlertException.getErrorKey()).isEqualTo("updateProhibited"); + }); + } + + @Test + public void should() throws IOException { + // given + givenJSonTree(asJSon(ImmutablePair.of("restrictedField", "Restricted String Value"))); + + // when + Throwable exception = catchThrowable(() -> new JSonDeserializerWithAccessFilter<>(jsonParser, null, GivenDtoWithMultipleSelfId.class).deserialize()); + + // then + assertThat(exception).isInstanceOf(AssertionError.class).hasMessageContaining("xx"); + } + + // --- only fixture code below --- + + @SafeVarargs + private final String asJSon(final ImmutablePair... properties) { final StringBuilder json = new StringBuilder(); - for ( ImmutablePair prop: properties ) { + for (ImmutablePair prop : properties) { json.append(inQuotes(prop.left)); json.append(": "); - if ( prop.right instanceof Number ) { + if (prop.right instanceof Number) { json.append(prop.right); } else { json.append(inQuotes(prop.right)); } json.append(",\n"); } - return "{\n" + json.substring(0, json.length()-2) + "\n}"; + return "{\n" + json.substring(0, json.length() - 2) + "\n}"; } private void givenJSonTree(String givenJSon) throws IOException { @@ -108,16 +165,34 @@ public class JSonDeserializerWithAccessFilterUnitTest { } public static class GivenDto { - @AccessFor(update = {Role.TECHNICAL_CONTACT, Role.FINANCIAL_CONTACT}) + + @SelfId + @AccessFor(read = Role.ANY_CUSTOMER_USER) + Long id; + + @AccessFor(init = {Role.TECHNICAL_CONTACT, Role.FINANCIAL_CONTACT}, update = {Role.TECHNICAL_CONTACT, Role.FINANCIAL_CONTACT}) String restrictedField; - @AccessFor(update = Role.ANYBODY) + @AccessFor(init = Role.ANYBODY, update = Role.ANYBODY) String openStringField; - @AccessFor(update = Role.ANYBODY) + @AccessFor(init = Role.ANYBODY, update = Role.ANYBODY) Integer openIntegerField; - @AccessFor(update = Role.ANYBODY) + @AccessFor(init = Role.ANYBODY, update = Role.ANYBODY) Long openLongField; } + + + public static class GivenDtoWithMultipleSelfId { + + @SelfId + @AccessFor(read = Role.ANY_CUSTOMER_USER) + Long id; + + @SelfId + @AccessFor(read = Role.ANY_CUSTOMER_USER) + Long id2; + + } }