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 8a29f645..7d76f6b3 100644 --- a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JSonAccessFilter.java +++ b/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JSonAccessFilter.java @@ -59,8 +59,10 @@ abstract class JSonAccessFilter { */ Set getLoginUserRoles() { final Set independentRoles = Arrays.stream(Role.values()) - // TODO mhoennig: ugly and risky filter condition => refactor! - .filter(role -> role.isIndependent() && SecurityUtils.isCurrentUserInRole(role.asAuthority())) + .filter( + role -> role.getAuthority() + .map(authority -> SecurityUtils.isCurrentUserInRole(authority)) + .orElse(false)) .collect(Collectors.toSet()); final Set rolesOnThis = getId() != null ? getLoginUserDirectRolesFor(dto.getClass(), getId()) : EMPTY_SET; diff --git a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/Role.java b/src/main/java/org/hostsharing/hsadminng/service/accessfilter/Role.java index 0c98c322..742d9ed9 100644 --- a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/Role.java +++ b/src/main/java/org/hostsharing/hsadminng/service/accessfilter/Role.java @@ -6,6 +6,7 @@ import static com.google.common.base.Verify.verify; import org.hostsharing.hsadminng.security.AuthoritiesConstants; import java.lang.reflect.Field; +import java.util.Optional; /** * These enum values are on the one hand used to define the minimum role required to grant access to resources, @@ -94,21 +95,21 @@ public enum Role { IGNORED; private final Integer level; - private final String authority; + private final Optional authority; Role(final int level, final String authority) { this.level = level; - this.authority = authority; + this.authority = Optional.of(authority); } Role(final int level) { this.level = level; - this.authority = AuthoritiesConstants.USER; + this.authority = Optional.empty(); } Role() { this.level = null; - this.authority = null; + this.authority = Optional.empty(); } /** @@ -124,7 +125,12 @@ public enum Role { return updateAccessFor.length == 1 && updateAccessFor[0].isIgnored(); } - public String asAuthority() { + /** + * @return the independent authority related 1:1 to this Role or empty if no independent authority is related 1:1 + * + * @see AuthoritiesConstants + */ + public Optional getAuthority() { return authority; } @@ -135,13 +141,6 @@ public enum Role { return this == Role.IGNORED; } - /** - * @return true if this role is independent of a target object, false otherwise. - */ - public boolean isIndependent() { - return this != NOBODY && (this == ANYBODY || covers(Role.SUPPORTER)); - } - /** * @return the role with the broadest access rights */ diff --git a/src/test/java/org/hostsharing/hsadminng/service/accessfilter/RoleUnitTest.java b/src/test/java/org/hostsharing/hsadminng/service/accessfilter/RoleUnitTest.java index 9fe65dae..74d6f195 100644 --- a/src/test/java/org/hostsharing/hsadminng/service/accessfilter/RoleUnitTest.java +++ b/src/test/java/org/hostsharing/hsadminng/service/accessfilter/RoleUnitTest.java @@ -4,6 +4,8 @@ package org.hostsharing.hsadminng.service.accessfilter; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.ThrowableAssert.catchThrowable; +import org.hostsharing.hsadminng.security.AuthoritiesConstants; + import com.google.common.base.VerifyException; import org.junit.Test; @@ -95,22 +97,6 @@ public class RoleUnitTest { assertThat(catchThrowable(() -> Role.HOSTMASTER.coversAny((Role[]) null))).isInstanceOf(VerifyException.class); } - @Test - public void isNdependend() { - assertThat(Role.NOBODY.isIndependent()).isFalse(); - - assertThat(Role.HOSTMASTER.isIndependent()).isTrue(); - assertThat(Role.ADMIN.isIndependent()).isTrue(); - assertThat(Role.SUPPORTER.isIndependent()).isTrue(); - - assertThat(Role.CONTRACTUAL_CONTACT.isIndependent()).isFalse(); - assertThat(Role.FINANCIAL_CONTACT.isIndependent()).isFalse(); - assertThat(Role.ACTUAL_CUSTOMER_USER.isIndependent()).isFalse(); - assertThat(Role.ANY_CUSTOMER_USER.isIndependent()).isFalse(); - - assertThat(Role.ANYBODY.isIndependent()).isTrue(); - } - @Test public void isIgnored() { for (Role role : Role.values()) { @@ -131,20 +117,13 @@ public class RoleUnitTest { } @Test - public void isIndependent() { - assertThat(Role.HOSTMASTER.isIndependent()).isTrue(); - assertThat(Role.SUPPORTER.isIndependent()).isTrue(); - - assertThat(Role.CONTRACTUAL_CONTACT.isIndependent()).isFalse(); - assertThat(Role.ANY_CUSTOMER_USER.isIndependent()).isFalse(); - } - - @Test - public void asAuthority() { - assertThat(Role.HOSTMASTER.asAuthority()).isEqualTo("ROLE_HOSTMASTER"); - assertThat(Role.ADMIN.asAuthority()).isEqualTo("ROLE_ADMIN"); - assertThat(Role.SUPPORTER.asAuthority()).isEqualTo("ROLE_SUPPORTER"); - assertThat(Role.CONTRACTUAL_CONTACT.asAuthority()).isEqualTo("ROLE_USER"); + public void getAuthority() { + assertThat(Role.NOBODY.getAuthority()).isEmpty(); + assertThat(Role.HOSTMASTER.getAuthority()).hasValue(AuthoritiesConstants.HOSTMASTER); + assertThat(Role.ADMIN.getAuthority()).hasValue(AuthoritiesConstants.ADMIN); + assertThat(Role.SUPPORTER.getAuthority()).hasValue(AuthoritiesConstants.SUPPORTER); + assertThat(Role.CONTRACTUAL_CONTACT.getAuthority()).isEmpty(); + assertThat(Role.ANYBODY.getAuthority()).hasValue(AuthoritiesConstants.ANONYMOUS); } @Test