From 1ad74907bd1247c1fad0414e947a51e967f042d0 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Fri, 19 Apr 2019 09:13:19 +0200 Subject: [PATCH] RoleUnitTest + special case FINANCIAL_CUSTOMER_CONTACT --- .../hsadminng/service/accessfilter/Role.java | 24 +++++- .../service/accessfilter/RoleUnitTest.java | 83 +++++++++++++++++++ 2 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 src/test/java/org/hostsharing/hsadminng/service/accessfilter/RoleUnitTest.java 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 b1b2d648..81ff1078 100644 --- a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/Role.java +++ b/src/main/java/org/hostsharing/hsadminng/service/accessfilter/Role.java @@ -7,6 +7,11 @@ import java.lang.reflect.Field; /** * These enum values are on the one hand used to define the minimum role required to grant access to resources, * but on the other hand also for the roles users can be assigned to. + * + * TODO: Maybe splitting it up into UserRole and RequiredRole would make it more clear? + * And maybe instead of a level, we could then add the comprised roles in the constructor? + * This could also be a better way to express that the financial contact has no rights to + * other users resources (see also ACTUAL_CUSTOMER_USEr vs. ANY_CUSTOMER_USER). */ public enum Role { /** @@ -45,17 +50,32 @@ public enum Role { /** * This role is for financial contacts of a customer, e.g. for accessing billing data. */ - FINANCIAL_CONTACT(22), + FINANCIAL_CONTACT(22) { + @Override + boolean covers(final Role role) { + if (role == ACTUAL_CUSTOMER_USER) { + return false; + } + return super.covers(role); + } + }, /** * This role is for technical contacts of a customer. */ TECHNICAL_CONTACT(22), + /** * Any user which belongs to a customer has at least this role. */ - ANY_CUSTOMER_USER(80), + ACTUAL_CUSTOMER_USER(80), + + /** + * Use this to grant rights to any user, also special function users who have no + * rights on other users resources. + */ + ANY_CUSTOMER_USER(89), /** * This role is meant to specify that a resources can be accessed by anybody, even without login. diff --git a/src/test/java/org/hostsharing/hsadminng/service/accessfilter/RoleUnitTest.java b/src/test/java/org/hostsharing/hsadminng/service/accessfilter/RoleUnitTest.java new file mode 100644 index 00000000..d1fe7481 --- /dev/null +++ b/src/test/java/org/hostsharing/hsadminng/service/accessfilter/RoleUnitTest.java @@ -0,0 +1,83 @@ +package org.hostsharing.hsadminng.service.accessfilter; + +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class RoleUnitTest { + + @Test + public void allUserRolesShouldCoverSameRequiredRole() { + assertThat(Role.HOSTMASTER.covers(Role.HOSTMASTER)).isTrue(); + assertThat(Role.ADMIN.covers(Role.ADMIN)).isTrue(); + assertThat(Role.SUPPORTER.covers(Role.SUPPORTER)).isTrue(); + + assertThat(Role.CONTRACTUAL_CONTACT.covers(Role.CONTRACTUAL_CONTACT)).isTrue(); + assertThat(Role.FINANCIAL_CONTACT.covers(Role.FINANCIAL_CONTACT)).isTrue(); + assertThat(Role.TECHNICAL_CONTACT.covers(Role.TECHNICAL_CONTACT)).isTrue(); + + + assertThat(Role.ACTUAL_CUSTOMER_USER.covers((Role.ACTUAL_CUSTOMER_USER))).isTrue(); + assertThat(Role.ANY_CUSTOMER_USER.covers((Role.ANY_CUSTOMER_USER))).isTrue(); + } + + @Test + public void lowerUserRolesShouldNotCoverHigherRequiredRoles() { + assertThat(Role.HOSTMASTER.covers(Role.NOBODY)).isFalse(); + assertThat(Role.ADMIN.covers(Role.HOSTMASTER)).isFalse(); + assertThat(Role.SUPPORTER.covers(Role.ADMIN)).isFalse(); + + assertThat(Role.ANY_CUSTOMER_CONTACT.covers(Role.SUPPORTER)).isFalse(); + assertThat(Role.CONTRACTUAL_CONTACT.covers(Role.ANY_CUSTOMER_CONTACT)).isFalse(); + assertThat(Role.FINANCIAL_CONTACT.covers(Role.CONTRACTUAL_CONTACT)).isFalse(); + assertThat(Role.FINANCIAL_CONTACT.covers(Role.TECHNICAL_CONTACT)).isFalse(); + assertThat(Role.TECHNICAL_CONTACT.covers(Role.CONTRACTUAL_CONTACT)).isFalse(); + assertThat(Role.TECHNICAL_CONTACT.covers(Role.FINANCIAL_CONTACT)).isFalse(); + + assertThat(Role.ACTUAL_CUSTOMER_USER.covers((Role.ANY_CUSTOMER_CONTACT))).isFalse(); + assertThat(Role.ACTUAL_CUSTOMER_USER.covers((Role.CONTRACTUAL_CONTACT))).isFalse(); + assertThat(Role.ACTUAL_CUSTOMER_USER.covers((Role.TECHNICAL_CONTACT))).isFalse(); + assertThat(Role.ACTUAL_CUSTOMER_USER.covers((Role.FINANCIAL_CONTACT))).isFalse(); + + assertThat(Role.ANY_CUSTOMER_USER.covers((Role.ACTUAL_CUSTOMER_USER))).isFalse(); + assertThat(Role.ANY_CUSTOMER_USER.covers((Role.ANY_CUSTOMER_CONTACT))).isFalse(); + assertThat(Role.ANY_CUSTOMER_USER.covers((Role.CONTRACTUAL_CONTACT))).isFalse(); + assertThat(Role.ANY_CUSTOMER_USER.covers((Role.TECHNICAL_CONTACT))).isFalse(); + assertThat(Role.ANY_CUSTOMER_USER.covers((Role.FINANCIAL_CONTACT))).isFalse(); + + assertThat(Role.ANYBODY.covers((Role.ANY_CUSTOMER_USER))).isFalse(); + } + + @Test + public void higherUserRolesShouldCoverLowerRequiredRoles() { + assertThat(Role.HOSTMASTER.covers(Role.SUPPORTER)).isTrue(); + assertThat(Role.ADMIN.covers(Role.SUPPORTER)).isTrue(); + + assertThat(Role.SUPPORTER.covers(Role.ANY_CUSTOMER_CONTACT)).isTrue(); + + assertThat(Role.ANY_CUSTOMER_CONTACT.covers(Role.CONTRACTUAL_CONTACT)).isTrue(); + assertThat(Role.CONTRACTUAL_CONTACT.covers(Role.FINANCIAL_CONTACT)).isTrue(); + assertThat(Role.CONTRACTUAL_CONTACT.covers(Role.TECHNICAL_CONTACT)).isTrue(); + assertThat(Role.TECHNICAL_CONTACT.covers(Role.ANY_CUSTOMER_USER)).isTrue(); + + assertThat(Role.ACTUAL_CUSTOMER_USER.covers((Role.ANY_CUSTOMER_USER))).isTrue(); + assertThat(Role.ANY_CUSTOMER_USER.covers((Role.ANYBODY))).isTrue(); + } + + @Test + public void financialContactShouldNotCoverAnyCustomersUsersRoleRequirement() { + assertThat(Role.FINANCIAL_CONTACT.covers(Role.ACTUAL_CUSTOMER_USER)).isFalse(); + } + + @Test + public void isAllowedToInit() { + } + + @Test + public void isAllowedToUpdate() { + } + + @Test + public void isAllowedToRead() { + } +}