From 3be086710ebd47ba0785a8b95d09c439a6ce870d Mon Sep 17 00:00:00 2001 From: Nirmal Sarswat <25587962+vivonk@users.noreply.github.com> Date: Thu, 5 Oct 2023 10:02:38 +0530 Subject: [PATCH] feat: PAC Service code split for configuration (#27821) ## Description PAC configuration service related CE changes #### Type of change - Chore (housekeeping or task changes that don't impact user perception) #### How Has This Been Tested? - [x] JUnit ## Checklist: #### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --- .../services/PACConfigurationService.java | 5 +++ .../services/PACConfigurationServiceImpl.java | 10 +++++ .../server/services/UserServiceImpl.java | 6 ++- .../ce/PACConfigurationServiceCE.java | 10 +++++ .../ce/PACConfigurationServiceCEImpl.java | 24 +++++++++++ .../server/services/ce/UserServiceCEImpl.java | 20 ++++----- .../PACConfigurationServiceCECompatible.java | 10 +++++ ...CConfigurationServiceCECompatibleImpl.java | 10 +++++ .../UserServiceCECompatibleImpl.java | 7 +++- .../ce/PACConfigurationServiceCETest.java | 42 +++++++++++++++++++ 10 files changed, 127 insertions(+), 17 deletions(-) create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/services/PACConfigurationService.java create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/services/PACConfigurationServiceImpl.java create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/PACConfigurationServiceCE.java create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/PACConfigurationServiceCEImpl.java create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce_compatible/PACConfigurationServiceCECompatible.java create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce_compatible/PACConfigurationServiceCECompatibleImpl.java create mode 100644 app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/PACConfigurationServiceCETest.java diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/PACConfigurationService.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/PACConfigurationService.java new file mode 100644 index 0000000000..1c7ce4bdf5 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/PACConfigurationService.java @@ -0,0 +1,5 @@ +package com.appsmith.server.services; + +import com.appsmith.server.services.ce_compatible.PACConfigurationServiceCECompatible; + +public interface PACConfigurationService extends PACConfigurationServiceCECompatible {} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/PACConfigurationServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/PACConfigurationServiceImpl.java new file mode 100644 index 0000000000..fcaf9edcf0 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/PACConfigurationServiceImpl.java @@ -0,0 +1,10 @@ +package com.appsmith.server.services; + +import com.appsmith.server.services.ce_compatible.PACConfigurationServiceCECompatibleImpl; +import lombok.extern.slf4j.Slf4j; +import org.springframework.stereotype.Service; + +@Service +@Slf4j +public class PACConfigurationServiceImpl extends PACConfigurationServiceCECompatibleImpl + implements PACConfigurationService {} 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 81ca44347f..9f53632fe7 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 @@ -49,7 +49,8 @@ public class UserServiceImpl extends UserServiceCECompatibleImpl implements User UserUtils userUtils, EmailVerificationTokenRepository emailVerificationTokenRepository, EmailService emailService, - RateLimitService rateLimitService) { + RateLimitService rateLimitService, + PACConfigurationService pacConfigurationService) { super( scheduler, validator, @@ -74,6 +75,7 @@ public class UserServiceImpl extends UserServiceCECompatibleImpl implements User userUtils, emailVerificationTokenRepository, emailService, - rateLimitService); + rateLimitService, + pacConfigurationService); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/PACConfigurationServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/PACConfigurationServiceCE.java new file mode 100644 index 0000000000..371ce06cea --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/PACConfigurationServiceCE.java @@ -0,0 +1,10 @@ +package com.appsmith.server.services.ce; + +import com.appsmith.server.domains.User; +import com.appsmith.server.dtos.UserProfileDTO; +import reactor.core.publisher.Mono; + +public interface PACConfigurationServiceCE { + Mono setRolesAndGroups( + UserProfileDTO profile, User user, boolean showUsersAndGroups, boolean isCloudHosting); +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/PACConfigurationServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/PACConfigurationServiceCEImpl.java new file mode 100644 index 0000000000..debabcf11e --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/PACConfigurationServiceCEImpl.java @@ -0,0 +1,24 @@ +package com.appsmith.server.services.ce; + +import com.appsmith.server.domains.User; +import com.appsmith.server.dtos.UserProfileDTO; +import org.springframework.stereotype.Service; +import reactor.core.publisher.Mono; + +import java.util.List; + +import static com.appsmith.server.constants.ce.AccessControlConstantsCE.UPGRADE_TO_BUSINESS_EDITION_TO_ACCESS_ROLES_AND_GROUPS_FOR_CONDITIONAL_BUSINESS_LOGIC; + +@Service +public class PACConfigurationServiceCEImpl implements PACConfigurationServiceCE { + @Override + public Mono setRolesAndGroups( + UserProfileDTO profile, User user, boolean showUsersAndGroups, boolean isCloudHosting) { + profile.setRoles( + List.of(UPGRADE_TO_BUSINESS_EDITION_TO_ACCESS_ROLES_AND_GROUPS_FOR_CONDITIONAL_BUSINESS_LOGIC)); + profile.setGroups( + List.of(UPGRADE_TO_BUSINESS_EDITION_TO_ACCESS_ROLES_AND_GROUPS_FOR_CONDITIONAL_BUSINESS_LOGIC)); + + return Mono.just(profile); + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java index d37e028e06..f412879c08 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java @@ -37,6 +37,7 @@ import com.appsmith.server.repositories.UserRepository; import com.appsmith.server.services.AnalyticsService; import com.appsmith.server.services.BaseService; import com.appsmith.server.services.EmailService; +import com.appsmith.server.services.PACConfigurationService; import com.appsmith.server.services.PermissionGroupService; import com.appsmith.server.services.SessionUserService; import com.appsmith.server.services.TenantService; @@ -88,7 +89,6 @@ import java.util.UUID; import java.util.regex.Pattern; import static com.appsmith.server.acl.AclPermission.MANAGE_USERS; -import static com.appsmith.server.constants.AccessControlConstants.UPGRADE_TO_BUSINESS_EDITION_TO_ACCESS_ROLES_AND_GROUPS_FOR_CONDITIONAL_BUSINESS_LOGIC; import static com.appsmith.server.helpers.RedirectHelper.DEFAULT_REDIRECT_URL; import static com.appsmith.server.helpers.ValidationUtils.LOGIN_PASSWORD_MAX_LENGTH; import static com.appsmith.server.helpers.ValidationUtils.LOGIN_PASSWORD_MIN_LENGTH; @@ -116,6 +116,7 @@ public class UserServiceCEImpl extends BaseService private final UserUtils userUtils; private final EmailService emailService; private final RateLimitService rateLimitService; + private final PACConfigurationService pacConfigurationService; private static final WebFilterChain EMPTY_WEB_FILTER_CHAIN = serverWebExchange -> Mono.empty(); private static final String FORGOT_PASSWORD_CLIENT_URL_FORMAT = "%s/user/resetPassword?token=%s"; @@ -154,7 +155,8 @@ public class UserServiceCEImpl extends BaseService UserUtils userUtils, EmailVerificationTokenRepository emailVerificationTokenRepository, EmailService emailService, - RateLimitService rateLimitService) { + RateLimitService rateLimitService, + PACConfigurationService pacConfigurationService) { super(scheduler, validator, mongoConverter, reactiveMongoTemplate, repository, analyticsService); this.workspaceService = workspaceService; @@ -172,6 +174,7 @@ public class UserServiceCEImpl extends BaseService this.rateLimitService = rateLimitService; this.emailVerificationTokenRepository = emailVerificationTokenRepository; this.emailService = emailService; + this.pacConfigurationService = pacConfigurationService; } @Override @@ -758,20 +761,11 @@ public class UserServiceCEImpl extends BaseService commonConfig.isCloudHosting() ? true : userData.isIntercomConsentGiven()); profile.setSuperUser(isSuperUser); profile.setConfigurable(!StringUtils.isEmpty(commonConfig.getEnvFilePath())); - return setRolesAndGroups(profile, userFromDb, true, commonConfig.isCloudHosting()); + return pacConfigurationService.setRolesAndGroups( + profile, userFromDb, true, commonConfig.isCloudHosting()); }); } - protected Mono setRolesAndGroups( - UserProfileDTO profile, User user, boolean showRolesAndGroups, boolean isCloudHosting) { - profile.setRoles( - List.of(UPGRADE_TO_BUSINESS_EDITION_TO_ACCESS_ROLES_AND_GROUPS_FOR_CONDITIONAL_BUSINESS_LOGIC)); - profile.setGroups( - List.of(UPGRADE_TO_BUSINESS_EDITION_TO_ACCESS_ROLES_AND_GROUPS_FOR_CONDITIONAL_BUSINESS_LOGIC)); - - return Mono.just(profile); - } - private EmailTokenDTO parseValueFromEncryptedToken(String encryptedToken) { String decryptString = encryptionService.decryptString(encryptedToken); List nameValuePairs = URLEncodedUtils.parse(decryptString, StandardCharsets.UTF_8); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce_compatible/PACConfigurationServiceCECompatible.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce_compatible/PACConfigurationServiceCECompatible.java new file mode 100644 index 0000000000..e71f31baf6 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce_compatible/PACConfigurationServiceCECompatible.java @@ -0,0 +1,10 @@ +package com.appsmith.server.services.ce_compatible; + +import com.appsmith.server.services.ce.PACConfigurationServiceCE; + +/** + * PACConfigurationService - Controls the configurations for PAC + *
+ * - PAC : programmatic access control + */ +public interface PACConfigurationServiceCECompatible extends PACConfigurationServiceCE {} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce_compatible/PACConfigurationServiceCECompatibleImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce_compatible/PACConfigurationServiceCECompatibleImpl.java new file mode 100644 index 0000000000..4fd6076df5 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce_compatible/PACConfigurationServiceCECompatibleImpl.java @@ -0,0 +1,10 @@ +package com.appsmith.server.services.ce_compatible; + +import com.appsmith.server.services.ce.PACConfigurationServiceCEImpl; +import lombok.extern.slf4j.Slf4j; +import org.springframework.stereotype.Service; + +@Service +@Slf4j +public class PACConfigurationServiceCECompatibleImpl extends PACConfigurationServiceCEImpl + implements PACConfigurationServiceCECompatible {} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce_compatible/UserServiceCECompatibleImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce_compatible/UserServiceCECompatibleImpl.java index d27345aa6b..79c8d40398 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce_compatible/UserServiceCECompatibleImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce_compatible/UserServiceCECompatibleImpl.java @@ -12,6 +12,7 @@ import com.appsmith.server.repositories.PasswordResetTokenRepository; import com.appsmith.server.repositories.UserRepository; import com.appsmith.server.services.AnalyticsService; import com.appsmith.server.services.EmailService; +import com.appsmith.server.services.PACConfigurationService; import com.appsmith.server.services.PermissionGroupService; import com.appsmith.server.services.SessionUserService; import com.appsmith.server.services.TenantService; @@ -53,7 +54,8 @@ public class UserServiceCECompatibleImpl extends UserServiceCEImpl implements Us UserUtils userUtils, EmailVerificationTokenRepository emailVerificationTokenRepository, EmailService emailService, - RateLimitService rateLimitService) { + RateLimitService rateLimitService, + PACConfigurationService pacConfigurationService) { super( scheduler, validator, @@ -78,6 +80,7 @@ public class UserServiceCECompatibleImpl extends UserServiceCEImpl implements Us userUtils, emailVerificationTokenRepository, emailService, - rateLimitService); + rateLimitService, + pacConfigurationService); } } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/PACConfigurationServiceCETest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/PACConfigurationServiceCETest.java new file mode 100644 index 0000000000..9365e186ef --- /dev/null +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/PACConfigurationServiceCETest.java @@ -0,0 +1,42 @@ +package com.appsmith.server.services.ce; + +import com.appsmith.server.dtos.UserProfileDTO; +import com.appsmith.server.services.PACConfigurationService; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; + +import java.util.List; + +import static com.appsmith.server.constants.ce.AccessControlConstantsCE.UPGRADE_TO_BUSINESS_EDITION_TO_ACCESS_ROLES_AND_GROUPS_FOR_CONDITIONAL_BUSINESS_LOGIC; +import static org.assertj.core.api.Assertions.assertThat; + +@ExtendWith(SpringExtension.class) +@SpringBootTest +class PACConfigurationServiceCETest { + @Autowired + PACConfigurationService pacConfigurationService; + + @Test + public void test_setRolesAndGroups_featureFlagDisabled() { + UserProfileDTO userProfileDTO = new UserProfileDTO(); + Mono userProfileDTOMono = + pacConfigurationService.setRolesAndGroups(userProfileDTO, null, false, false); + StepVerifier.create(userProfileDTOMono) + .assertNext(userProfileDTO1 -> { + assertThat(userProfileDTO1.getRoles()) + .isEqualTo( + List.of( + UPGRADE_TO_BUSINESS_EDITION_TO_ACCESS_ROLES_AND_GROUPS_FOR_CONDITIONAL_BUSINESS_LOGIC)); + assertThat(userProfileDTO1.getGroups()) + .isEqualTo( + List.of( + UPGRADE_TO_BUSINESS_EDITION_TO_ACCESS_ROLES_AND_GROUPS_FOR_CONDITIONAL_BUSINESS_LOGIC)); + }) + .verifyComplete(); + } +}