From f1e1e6959ac9e7d01706f6189a2af1e7811fdff3 Mon Sep 17 00:00:00 2001 From: Trisha Anand Date: Fri, 29 May 2020 05:28:30 +0000 Subject: [PATCH] Bug : New users aren't able to read their own user object and hence homepage doesnt load. Fix : Added lateral permissions for user on create. --- .../services/ApplicationServiceImpl.java | 2 +- .../server/services/UserServiceImpl.java | 19 ++++--------------- .../server/services/UserServiceTest.java | 12 +++++++++++- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationServiceImpl.java index 6dd9d593f4..e1a306fc79 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationServiceImpl.java @@ -215,7 +215,7 @@ public class ApplicationServiceImpl extends BaseService applicationCollection = applicationsCollectionByOrgId.get(orgId); - List applicationList = null; + List applicationList = new ArrayList<>(); if (applicationCollection!=null && !applicationCollection.isEmpty()) { applicationList = applicationCollection.stream().collect(Collectors.toList()); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java index 07d1261f54..778330ccfd 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java @@ -41,11 +41,11 @@ import java.io.IOException; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.UUID; -import java.util.stream.Collectors; import static com.appsmith.server.acl.AclPermission.MANAGE_APPLICATIONS; import static com.appsmith.server.acl.AclPermission.MANAGE_USERS; @@ -507,23 +507,12 @@ public class UserServiceImpl extends BaseService i } private Set crudUserPolicy(User user) { - Policy manageUserPolicy = Policy.builder() - .permission(MANAGE_USERS.getValue()) - .users(Set.of(user.getUsername())).build(); - Policy manageUserOrgPolicy = Policy.builder() - .permission(USER_MANAGE_ORGANIZATIONS.getValue()) - .users(Set.of(user.getUsername())).build(); + Set aclPermissions = Set.of(MANAGE_USERS, USER_MANAGE_ORGANIZATIONS); - user.getPolicies().addAll(Set.of(manageUserPolicy, manageUserOrgPolicy)); + Map userPolicies = policyUtils.generatePolicyFromPermission(aclPermissions, user); - Set policySet = user.getPolicies().stream() - .filter(policy -> - policy.getPermission().equals(MANAGE_USERS.getValue()) || - policy.getPermission().equals(USER_MANAGE_ORGANIZATIONS.getValue()) - ).collect(Collectors.toSet()); - - return policyGenerator.getAllChildPolicies(user, policySet, User.class); + return new HashSet<>(userPolicies.values()); } /** diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java index 8bee0a8fe7..c2fefc00e5 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java @@ -29,7 +29,9 @@ import java.util.Set; import static com.appsmith.server.acl.AclPermission.MANAGE_APPLICATIONS; import static com.appsmith.server.acl.AclPermission.MANAGE_USERS; import static com.appsmith.server.acl.AclPermission.READ_APPLICATIONS; +import static com.appsmith.server.acl.AclPermission.READ_USERS; import static com.appsmith.server.acl.AclPermission.USER_MANAGE_ORGANIZATIONS; +import static com.appsmith.server.acl.AclPermission.USER_READ_ORGANIZATIONS; import static org.assertj.core.api.Assertions.assertThat; @Slf4j @@ -143,6 +145,14 @@ public class UserServiceTest { .permission(USER_MANAGE_ORGANIZATIONS.getValue()) .users(Set.of(newUser.getUsername())).build(); + Policy readUserPolicy = Policy.builder() + .permission(READ_USERS.getValue()) + .users(Set.of(newUser.getUsername())).build(); + + Policy readUserOrgPolicy = Policy.builder() + .permission(USER_READ_ORGANIZATIONS.getValue()) + .users(Set.of(newUser.getUsername())).build(); + Mono userMono = userService.create(newUser); StepVerifier.create(userMono) @@ -153,7 +163,7 @@ public class UserServiceTest { assertThat(user.getEmail()).isEqualTo("new-user-email@email.com"); assertThat(user.getName()).isEqualTo("new-user-email@email.com"); assertThat(user.getPolicies()).isNotEmpty(); - assertThat(user.getPolicies()).containsAll(Set.of(manageUserPolicy, manageUserOrgPolicy)); + assertThat(user.getPolicies()).containsAll(Set.of(manageUserPolicy, manageUserOrgPolicy, readUserPolicy, readUserOrgPolicy)); }) .verifyComplete(); }