diff --git a/backend/src/main/java/org/hawkular/accounts/backend/boundary/PermissionChecker.java b/backend/src/main/java/org/hawkular/accounts/backend/boundary/PermissionChecker.java index a2fe597..7971a48 100644 --- a/backend/src/main/java/org/hawkular/accounts/backend/boundary/PermissionChecker.java +++ b/backend/src/main/java/org/hawkular/accounts/backend/boundary/PermissionChecker.java @@ -84,8 +84,8 @@ public boolean hasAccessTo(HawkularUser currentUser, Owner owner) { * @param principal the {@link KeycloakPrincipal} representing the current user * @param owner the owner of the resource * @return true if the user is the owner or if the user belongs to an organization that owns the resource - * @see PermissionChecker#hasAccessTo( - *org.hawkular.accounts.backend.entity.User, org.hawkular.accounts.backend.entity.Owner) + * @see PermissionChecker#hasAccessTo(org.hawkular.accounts.backend.entity.HawkularUser, + * org.hawkular.accounts.backend.entity.Owner) */ public boolean hasAccessTo(KeycloakPrincipal principal, Owner owner) { // Here's a bit of explanation: judging by the name of this class, we wouldn't expect any record to be created. @@ -133,6 +133,7 @@ public boolean isMemberOf(Owner member, Organization organization) { // if the member is part of a child organization, then it's a member of this one // example: jdoe is member of emca, emca is member of acme, therefore, jdoe is member of acme + // TODO: is this appropriate? should jdoe really have access to data from acme? if not, just remove this part for (Owner organizationMember : organization.getMembers()) { if (organizationMember instanceof Organization) { return isMemberOf(member, (Organization) organizationMember); diff --git a/backend/src/test/java/org/hawkular/accounts/backend/boundary/PermissionCheckerTest.java b/backend/src/test/java/org/hawkular/accounts/backend/boundary/PermissionCheckerTest.java index 850d7ea..9df11cd 100644 --- a/backend/src/test/java/org/hawkular/accounts/backend/boundary/PermissionCheckerTest.java +++ b/backend/src/test/java/org/hawkular/accounts/backend/boundary/PermissionCheckerTest.java @@ -31,12 +31,20 @@ */ public class PermissionCheckerTest extends BaseEntityManagerEnabledTest { @Test - public void userHasAccessToOwnResources() { - HawkularUser owner = new HawkularUser(UUID.randomUUID().toString()); - Resource resource = new Resource(UUID.randomUUID().toString(), owner); + public void userHasAccessToItself() { + HawkularUser jsmith = new HawkularUser(UUID.randomUUID().toString()); + PermissionChecker checker = new PermissionChecker(); + assertTrue("User has access no access to another user's resource", checker.hasAccessTo(jsmith, jsmith)); + } + + @Test + public void userDontHaveAccessToAnotherUsersResource() { + HawkularUser jsmith = new HawkularUser(UUID.randomUUID().toString()); + HawkularUser jdoe = new HawkularUser(UUID.randomUUID().toString()); + Resource resource = new Resource(UUID.randomUUID().toString(), jsmith); PermissionChecker checker = new PermissionChecker(); - assertTrue("User has access to own resource", checker.hasAccessTo(owner, resource)); + assertTrue("User has access no access to another user's resource", !checker.hasAccessTo(jdoe, resource)); } @Test @@ -78,39 +86,44 @@ public void ownerBelongsToInnerOrganization() { } @Test - public void memberBelongsToInnerOrganization() { + public void siblingsDontBelongToEachOther() { // case here: - // acme owns emca + // acme owns marketing + // acme owns finance // jdoe is the owner of acme - // jsmith is a member of acme - // therefore, jsmith is a member of emca + // finance DOES NOT BELONGS to marketing HawkularUser jdoe = new HawkularUser(UUID.randomUUID().toString()); - HawkularUser jsmith = new HawkularUser(UUID.randomUUID().toString()); Organization acme = new Organization(UUID.randomUUID().toString(), jdoe); - Organization emca = new Organization(UUID.randomUUID().toString(), acme); - acme.addMember(jsmith); + Organization finance = new Organization(UUID.randomUUID().toString(), acme); + Organization marketing = new Organization(UUID.randomUUID().toString(), acme); PermissionChecker checker = new PermissionChecker(); - assertTrue("Owner of parent organization should be a member of it", checker.hasAccessTo(jsmith, emca)); + assertTrue("Siblings are not a member of each other", !checker.isMemberOf(marketing, finance)); + assertTrue("Siblings are not a member of each other", !checker.isMemberOf(finance, marketing)); } @Test - public void siblingsDontBelongToEachOther() { + public void memberOfInnerOrganizationHasAccessToParent() { // case here: - // acme owns marketing - // acme owns finance - // jdoe is the owner of acme - // finance DOES NOT BELONGS to marketing + // jdoe is the owner of acme and owner of marketing + // marketing is a member of acme + // jsmith is a member of marketing + // jsmith can access marketing and acme (should this be changed?) HawkularUser jdoe = new HawkularUser(UUID.randomUUID().toString()); + HawkularUser jsmith = new HawkularUser(UUID.randomUUID().toString()); + Organization acme = new Organization(UUID.randomUUID().toString(), jdoe); - Organization finance = new Organization(UUID.randomUUID().toString(), acme); - Organization marketing = new Organization(UUID.randomUUID().toString(), acme); + Organization marketing = new Organization(UUID.randomUUID().toString(), jdoe); + + acme.addMember(marketing); + marketing.addMember(jsmith); PermissionChecker checker = new PermissionChecker(); - assertTrue("Siblings are not a member of each other", !checker.isMemberOf(marketing, finance)); - assertTrue("Siblings are not a member of each other", !checker.isMemberOf(finance, marketing)); + assertTrue("Member should have access to organization", checker.hasAccessTo(jsmith, marketing)); + assertTrue("Member of child organization should have access to parent organization", + checker.hasAccessTo(jsmith, acme)); } @Test diff --git a/backend/src/test/java/org/hawkular/accounts/backend/boundary/ResourceServiceTest.java b/backend/src/test/java/org/hawkular/accounts/backend/boundary/ResourceServiceTest.java index 86745e2..b62b6c7 100644 --- a/backend/src/test/java/org/hawkular/accounts/backend/boundary/ResourceServiceTest.java +++ b/backend/src/test/java/org/hawkular/accounts/backend/boundary/ResourceServiceTest.java @@ -21,6 +21,7 @@ import org.hawkular.accounts.backend.entity.Resource; import org.junit.Before; import org.junit.Test; +import org.keycloak.KeycloakPrincipal; import java.util.UUID; @@ -58,7 +59,7 @@ public void existingResourceIsRetrieved() { } @Test - public void nonExistingResourceIsCreated() { + public void nonExistingResourceIsCreatedWithUser() { entityManager.getTransaction().begin(); Owner user = new HawkularUser(UUID.randomUUID().toString()); entityManager.persist(user); @@ -72,4 +73,22 @@ public void nonExistingResourceIsCreated() { assertNotNull(resourceService.getById(resource.getId())); entityManager.getTransaction().commit(); } + + @Test + public void nonExistingResourceIsCreatedWithPrincipal() { + entityManager.getTransaction().begin(); + String userId = UUID.randomUUID().toString(); + KeycloakPrincipal principal = new KeycloakPrincipal(userId, null); + Owner user = new HawkularUser(userId); + entityManager.persist(user); + entityManager.getTransaction().commit(); + + entityManager.getTransaction().begin(); + Resource resource = resourceService.getOrCreateById(UUID.randomUUID().toString(), principal); + entityManager.getTransaction().commit(); + + entityManager.getTransaction().begin(); + assertNotNull(resourceService.getById(resource.getId())); + entityManager.getTransaction().commit(); + } } diff --git a/backend/src/test/java/org/hawkular/accounts/backend/entity/BasicPersistenceUnitTest.java b/backend/src/test/java/org/hawkular/accounts/backend/entity/BasicPersistenceUnitTest.java index 96a9990..4feb101 100644 --- a/backend/src/test/java/org/hawkular/accounts/backend/entity/BasicPersistenceUnitTest.java +++ b/backend/src/test/java/org/hawkular/accounts/backend/entity/BasicPersistenceUnitTest.java @@ -24,7 +24,10 @@ import javax.persistence.EntityManagerFactory; import javax.persistence.Persistence; +import java.time.ZonedDateTime; + import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; /** * @author Juraci Paixão Kröhling @@ -62,8 +65,34 @@ public void createdAtIsConsistent() { assertEquals("Timezone should be kept", fromDatabase.getCreatedAt(), baseEntityTest.getCreatedAt()); } + @Test + public void updatedAtIsRefreshed() { + BaseEntityTest baseEntityTest = new BaseEntityTest(); + + entityManager.getTransaction().begin(); + entityManager.persist(baseEntityTest); + entityManager.getTransaction().commit(); + ZonedDateTime updatedAt = baseEntityTest.getUpdatedAt(); + + entityManager.getTransaction().begin(); + baseEntityTest.setName("different name"); + entityManager.persist(baseEntityTest); + entityManager.getTransaction().commit(); + + assertNotEquals("Updated at should have been changed", updatedAt, baseEntityTest.getUpdatedAt()); + } + @Entity public class BaseEntityTest extends BaseEntity { + private String name; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } } }