refactored ugly and risky code in role filter / removed Role.isIndependent()

This commit is contained in:
Michael Hoennig 2019-05-10 17:37:36 +02:00
parent a2b90b0a36
commit 3143f27b6c
3 changed files with 24 additions and 44 deletions

View File

@ -59,8 +59,10 @@ abstract class JSonAccessFilter<T> {
*/
Set<Role> getLoginUserRoles() {
final Set<Role> 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<Role> rolesOnThis = getId() != null ? getLoginUserDirectRolesFor(dto.getClass(), getId()) : EMPTY_SET;

View File

@ -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<String> 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<String> 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
*/

View File

@ -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