diff --git a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JSonAccessFilter.java b/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JSonAccessFilter.java index de844c19..db364afe 100644 --- a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JSonAccessFilter.java +++ b/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JSonAccessFilter.java @@ -82,13 +82,16 @@ abstract class JSonAccessFilter { final Class parentDtoClass = ReflectionUtil. determineGenericInterfaceParameter(parentDtoLoader, rawType, 0); final Long parentId = ReflectionUtil.getValue(dto, parentIdField); + if (parentId == null) { + return emptySet(); + } final Set rolesOnParent = getLoginUserDirectRolesFor(parentDtoClass, parentId); final Object parentEntity = loadDto(parentDtoLoader, parentId); return union(rolesOnParent, getLoginUserRoleOnAncestorIfHigher(parentEntity)); } - private Set getLoginUserDirectRolesFor(final Class dtoClass, final Long id) { + private Set getLoginUserDirectRolesFor(final Class dtoClass, final long id) { if (!SecurityUtils.isAuthenticated()) { return emptySet(); } 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 ba188b3c..8398f01a 100644 --- a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JsonDeserializerWithAccessFilter.java +++ b/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JsonDeserializerWithAccessFilter.java @@ -199,38 +199,47 @@ public abstract class JsonDeserializerWithAccessFilter private void checkAccessToWrittenFields(final T currentDto) { updatingFields.forEach( field -> { - // TODO this ugly code needs cleanup if (!field.equals(selfIdField)) { final Set roles = getLoginUserRoles(); if (isInitAccess()) { - if (!isAllowedToInit(roles, field)) { - if (!field.equals(parentIdField)) { - throw new BadRequestAlertException( - "Initialization of field " + toDisplay(field) - + " prohibited for current user role(s): " - + Joiner.on("+").join(roles), - toDisplay(field), - "initializationProhibited"); - } else { - throw new BadRequestAlertException( - "Referencing field " + toDisplay(field) - + " prohibited for current user role(s): " - + Joiner.on("+").join(roles), - toDisplay(field), - "referencingProhibited"); - } - } - } else if (!Role.toBeIgnoredForUpdates(field) && !isAllowedToUpdate(getLoginUserRoles(), field)) { - throw new BadRequestAlertException( - "Update of field " + toDisplay(field) + " prohibited for current user role(s): " - + Joiner.on("+").join(roles), - toDisplay(field), - "updateProhibited"); + validateInitAccess(field, roles); + } else { + validateUpdateAccess(field, roles); } } }); } + private void validateInitAccess(Field field, Set roles) { + if (!Role.toBeIgnoredForUpdates(field) && !isAllowedToInit(roles, field)) { + if (!field.equals(parentIdField)) { + throw new BadRequestAlertException( + "Initialization of field " + toDisplay(field) + + " prohibited for current user role(s): " + + Joiner.on("+").join(roles), + toDisplay(field), + "initializationProhibited"); + } else { + throw new BadRequestAlertException( + "Referencing field " + toDisplay(field) + + " prohibited for current user role(s): " + + Joiner.on("+").join(roles), + toDisplay(field), + "referencingProhibited"); + } + } + } + + private void validateUpdateAccess(Field field, Set roles) { + if (!Role.toBeIgnoredForUpdates(field) && !isAllowedToUpdate(getLoginUserRoles(), field)) { + throw new BadRequestAlertException( + "Update of field " + toDisplay(field) + " prohibited for current user role(s): " + + Joiner.on("+").join(roles), + toDisplay(field), + "updateProhibited"); + } + } + private boolean isAllowedToInit(final Set roles, final Field field) { for (Role role : roles) { if (role.isAllowedToInit(field)) { diff --git a/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonAccessFilterTestFixture.java b/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonAccessFilterTestFixture.java index 24095f01..dac62ce0 100644 --- a/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonAccessFilterTestFixture.java +++ b/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonAccessFilterTestFixture.java @@ -106,6 +106,9 @@ public class JSonAccessFilterTestFixture { @AccessFor(init = ANYBODY, update = ANYBODY, read = ANYBODY) int[] openArrayField; + + @AccessFor(init = IGNORED, update = IGNORED, read = ANYBODY) + String displayLabel; } static abstract class GivenService implements IdToDtoResolver { diff --git a/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializationWithAccessFilterUnitTest.java b/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializationWithAccessFilterUnitTest.java index af7dc287..23d3d0d2 100644 --- a/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializationWithAccessFilterUnitTest.java +++ b/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializationWithAccessFilterUnitTest.java @@ -119,7 +119,7 @@ public class JSonDeserializationWithAccessFilterUnitTest { ImmutablePair.of("openStringField", null))); // when - final GivenDto actualDto = deserializerForGivenDto().deserialize(jsonParser, null); + final GivenDto actualDto = deserializerFor(GivenDto.class).deserialize(jsonParser, null); // then assertThat(actualDto.openStringField).isNull(); @@ -135,7 +135,7 @@ public class JSonDeserializationWithAccessFilterUnitTest { ImmutablePair.of("openStringField", "String Value"))); // when - final GivenDto actualDto = deserializerForGivenDto().deserialize(jsonParser, null); + final GivenDto actualDto = deserializerFor(GivenDto.class).deserialize(jsonParser, null); // then assertThat(actualDto.openStringField).isEqualTo("String Value"); @@ -152,7 +152,7 @@ public class JSonDeserializationWithAccessFilterUnitTest { // when // @formatter:off - final GivenDto actualDto = deserializerForGivenDto().deserialize(jsonParser, null);; + final GivenDto actualDto = deserializerFor(GivenDto.class).deserialize(jsonParser, null);; // @formatter:on // then @@ -170,7 +170,7 @@ public class JSonDeserializationWithAccessFilterUnitTest { ImmutablePair.of("restrictedBigDecimalField", SOME_BIG_DECIMAL_WITH_ANOTHER_SCALE))); // when - final GivenDto actualDto = deserializerForGivenDto().deserialize(jsonParser, null); + final GivenDto actualDto = deserializerFor(GivenDto.class).deserialize(jsonParser, null); ; // then @@ -201,7 +201,7 @@ public class JSonDeserializationWithAccessFilterUnitTest { ImmutablePair.of("openEnumField", TestEnum.GREEN))); // when - final GivenDto actualDto = deserializerForGivenDto().deserialize(jsonParser, null); + final GivenDto actualDto = deserializerFor(GivenDto.class).deserialize(jsonParser, null); ; // then @@ -227,7 +227,7 @@ public class JSonDeserializationWithAccessFilterUnitTest { ImmutablePair.of("openArrayField", Arrays.asList(11, 22, 33)))); // when - Throwable exception = catchThrowable(() -> deserializerForGivenDto().deserialize(jsonParser, null)); + Throwable exception = catchThrowable(() -> deserializerFor(GivenDto.class).deserialize(jsonParser, null)); // then assertThat(exception).isInstanceOf(NotImplementedException.class); @@ -245,7 +245,7 @@ public class JSonDeserializationWithAccessFilterUnitTest { ImmutablePair.of("restrictedField", "update value of restricted field"))); // when - final GivenDto actualDto = deserializerForGivenDto().deserialize(jsonParser, null); + final GivenDto actualDto = deserializerFor(GivenDto.class).deserialize(jsonParser, null); // then assertThat(actualDto.restrictedField).isEqualTo("update value of restricted field"); @@ -263,7 +263,7 @@ public class JSonDeserializationWithAccessFilterUnitTest { ImmutablePair.of("restrictedField", "initial value of restricted field"))); // when - final GivenDto actualDto = deserializerForGivenDto().deserialize(jsonParser, null); + final GivenDto actualDto = deserializerFor(GivenDto.class).deserialize(jsonParser, null); // then assertThat(actualDto.restrictedField).isEqualTo("initial value of restricted field"); @@ -280,7 +280,7 @@ public class JSonDeserializationWithAccessFilterUnitTest { ImmutablePair.of("restrictedField", "updated value of restricted field"))); // when - final Throwable exception = catchThrowable(() -> deserializerForGivenDto().deserialize(jsonParser, null)); + final Throwable exception = catchThrowable(() -> deserializerFor(GivenDto.class).deserialize(jsonParser, null)); // then assertThat(exception).isInstanceOfSatisfying(BadRequestAlertException.class, badRequestAlertException -> { @@ -300,7 +300,7 @@ public class JSonDeserializationWithAccessFilterUnitTest { ImmutablePair.of("restrictedField", "another value of restricted field"))); // when - final Throwable exception = catchThrowable(() -> deserializerForGivenDto().deserialize(jsonParser, null)); + final Throwable exception = catchThrowable(() -> deserializerFor(GivenDto.class).deserialize(jsonParser, null)); // then assertThat(exception).isInstanceOfSatisfying(BadRequestAlertException.class, badRequestAlertException -> { @@ -320,7 +320,7 @@ public class JSonDeserializationWithAccessFilterUnitTest { // when Throwable exception = catchThrowable( - () -> deserializerForGivenChildDto().deserialize(jsonParser, null)); + () -> deserializerFor(GivenChildDto.class).deserialize(jsonParser, null)); // then assertThat(exception).isInstanceOfSatisfying(BadRequestAlertException.class, badRequestAlertException -> { @@ -339,7 +339,7 @@ public class JSonDeserializationWithAccessFilterUnitTest { ImmutablePair.of("parentId", 1234L))); // when - final GivenChildDto actualDto = deserializerForGivenChildDto().deserialize(jsonParser, null); + final GivenChildDto actualDto = deserializerFor(GivenChildDto.class).deserialize(jsonParser, null); ; // then @@ -359,7 +359,7 @@ public class JSonDeserializationWithAccessFilterUnitTest { // when final Throwable exception = catchThrowable( - () -> deserializerForGivenDto().deserialize(jsonParser, null)); + () -> deserializerFor(GivenDto.class).deserialize(jsonParser, null)); // then assertThat(exception).isInstanceOfSatisfying(BadRequestAlertException.class, badRequestAlertException -> { @@ -375,7 +375,7 @@ public class JSonDeserializationWithAccessFilterUnitTest { // when final Throwable exception = catchThrowable( - () -> deserializerForGivenDtoWithMultipleSelfId().deserialize(jsonParser, null)); + () -> deserializerFor(GivenDtoWithMultipleSelfId.class).deserialize(jsonParser, null)); // then assertThat(exception).isInstanceOf(AssertionError.class) @@ -390,7 +390,7 @@ public class JSonDeserializationWithAccessFilterUnitTest { // when final Throwable exception = catchThrowable( - () -> deserializerForGivenDtoWithUnknownFieldType().deserialize(jsonParser, null)); + () -> deserializerFor(GivenDtoWithUnknownFieldType.class).deserialize(jsonParser, null)); // then assertThat(exception).isInstanceOf(NotImplementedException.class) @@ -407,7 +407,7 @@ public class JSonDeserializationWithAccessFilterUnitTest { // when final Throwable exception = catchThrowable( - () -> deserializerForGivenDto().deserialize(jsonParser, null)); + () -> deserializerFor(GivenDto.class).deserialize(jsonParser, null)); // then assertThat(exception).isInstanceOfSatisfying(BadRequestAlertException.class, (exc) -> { @@ -417,6 +417,38 @@ public class JSonDeserializationWithAccessFilterUnitTest { }); } + @Test + public void shouldIgnorePropertyToIgnoreForInit() throws IOException { + // given + securityContext.havingAuthenticatedUser().withAuthority(AuthoritiesConstants.ADMIN); + givenJSonTree( + asJSon( + ImmutablePair.of("displayLabel", "Some Value"))); + + // when + deserializerFor(GivenDto.class).deserialize(jsonParser, null); + final Throwable exception = catchThrowable(() -> deserializerFor(GivenDto.class).deserialize(jsonParser, null)); + + // then + assertThat(exception).isNull(); + } + + @Test + public void shouldIgnorePropertyToIgnoreForUpdate() throws IOException { + // given + securityContext.havingAuthenticatedUser().withAuthority(AuthoritiesConstants.ADMIN); + givenJSonTree( + asJSon( + ImmutablePair.of("id", 1234L), + ImmutablePair.of("displayLabel", "Some Value"))); + + // when + final Throwable exception = catchThrowable(() -> deserializerFor(GivenDto.class).deserialize(jsonParser, null)); + + // then + assertThat(exception).isNull(); + } + // --- only fixture code below --- private void givenJSonTree(String givenJSon) throws IOException { @@ -425,28 +457,41 @@ public class JSonDeserializationWithAccessFilterUnitTest { given(codec.readTree(jsonParser)).willReturn(new ObjectMapper().readTree(givenJSon)); } - // We need specialied factories for the deserializer subclasses so that the generic type can be accessed via reflection. + // We need specialied factories for the deserializer subclasses + // so that the generic type can be accessed via reflection. // And it's down here to keep the ugly formatting out of the test cases. + // The trick with the unused ...-parameterr might look strange but the reason is + // that I wanted the concrete class to be navigable in the tests and + // multiple deserializer(Class<...Dto>) methods would have the same erasure + // and adding the type the method name, is redundant for the reader. - public JsonDeserializerWithAccessFilter deserializerForGivenDto() throws IOException { + public JsonDeserializerWithAccessFilter deserializerFor( + final Class clazz, + final GivenDto... qualifier) { return new JsonDeserializerWithAccessFilter(ctx, userRoleAssignmentService) { // no need to overload any method here }; } - public JsonDeserializerWithAccessFilter deserializerForGivenChildDto() throws IOException { + public JsonDeserializerWithAccessFilter deserializerFor( + final Class clazz, + final GivenChildDto... qualifier) { return new JsonDeserializerWithAccessFilter(ctx, userRoleAssignmentService) { // no need to overload any method here }; } - private JsonDeserializer deserializerForGivenDtoWithMultipleSelfId() { + private JsonDeserializer deserializerFor( + final Class clazz, + final GivenDtoWithMultipleSelfId... qualifier) { return new JsonDeserializerWithAccessFilter(ctx, userRoleAssignmentService) { // no need to overload any method here }; } - private JsonDeserializer deserializerForGivenDtoWithUnknownFieldType() { + private JsonDeserializer deserializerFor( + final Class clazz, + final GivenDtoWithUnknownFieldType... qualifier) { return new JsonDeserializerWithAccessFilter(ctx, userRoleAssignmentService) { // no need to overload any method here };