From 605ad47c516ae65693a054a10355d4a479599078 Mon Sep 17 00:00:00 2001 From: Trisha Anand Date: Wed, 5 Mar 2025 11:58:10 +0530 Subject: [PATCH] feat: Introducing Organization Administrator Role (#39549) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description > [!TIP] > _Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team)._ > > _Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR._ Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /test sanity ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: 60c77b1803fb9bf870a62e4475bb59ad2a471fb6 > Cypress dashboard. > Tags: `@tag.Sanity` > Spec: >
Tue, 04 Mar 2025 12:58:58 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **New Features** - Introduced an organization-based admin role and enhanced permission management for organization administrators. - Added functionality to assign and remove instance administrator privileges, streamlining role transitions. - **Refactor** - Updated user role terminology and logic across services to replace legacy tenant and super user assignments with the new organization-focused model. - Revised migration and repository processes to support organization-specific permission groups. - **Tests** - Improved test coverage to verify accurate assignment and removal of the updated administrative roles. --- .../com/appsmith/server/acl/AppsmithRole.java | 3 +- .../server/constants/ce/FieldNameCE.java | 1 + .../InMemoryCacheableRepositoryHelper.java | 12 + .../appsmith/server/helpers/UserUtils.java | 20 +- .../server/helpers/ce/UserUtilsCE.java | 256 +++++++++--------- .../server/migrations/DatabaseChangelog2.java | 4 +- ...066_RenameInstanceAdminRoleToOrgAdmin.java | 121 +++++++++ .../db/ce/Migration10000_UpdateSuperUser.java | 20 +- .../ce/CacheableRepositoryHelperCE.java | 4 +- .../ce/CacheableRepositoryHelperCEImpl.java | 90 +++--- .../ce/CustomOrganizationRepositoryCE.java | 4 + .../CustomOrganizationRepositoryCEImpl.java | 12 + .../server/services/ce/UserServiceCEImpl.java | 4 +- .../server/solutions/ce/EnvManagerCEImpl.java | 4 +- .../server/solutions/ce/UserSignupCEImpl.java | 2 +- .../server/helpers/UserUtilsTest.java | 127 +++++++++ .../repositories/CacheableRepositoryTest.java | 2 +- .../services/PermissionGroupServiceTest.java | 2 +- .../services/UserWorkspaceServiceTest.java | 2 +- .../ce/OrganizationServiceCETest.java | 2 +- 20 files changed, 501 insertions(+), 191 deletions(-) create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration066_RenameInstanceAdminRoleToOrgAdmin.java create mode 100644 app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/UserUtilsTest.java diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/acl/AppsmithRole.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/acl/AppsmithRole.java index 85f0a91176..3378a7c12a 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/acl/AppsmithRole.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/acl/AppsmithRole.java @@ -25,6 +25,7 @@ import static com.appsmith.server.constants.FieldName.VIEWER; import static com.appsmith.server.constants.FieldName.WORKSPACE_ADMINISTRATOR_DESCRIPTION; import static com.appsmith.server.constants.FieldName.WORKSPACE_DEVELOPER_DESCRIPTION; import static com.appsmith.server.constants.FieldName.WORKSPACE_VIEWER_DESCRIPTION; +import static com.appsmith.server.constants.ce.FieldNameCE.ORGANIZATION_ADMINISTRATOR_ROLE; @Getter public enum AppsmithRole { @@ -62,7 +63,7 @@ public enum AppsmithRole { WORKSPACE_READ_APPLICATIONS, WORKSPACE_INVITE_USERS, WORKSPACE_EXECUTE_DATASOURCES)), - TENANT_ADMIN("", "", Set.of(MANAGE_ORGANIZATION)), + ORGANIZATION_ADMIN(ORGANIZATION_ADMINISTRATOR_ROLE, "", Set.of(MANAGE_ORGANIZATION)), ; private Set permissions; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ce/FieldNameCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ce/FieldNameCE.java index 2b53895370..6ce5fb87a3 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ce/FieldNameCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ce/FieldNameCE.java @@ -207,4 +207,5 @@ public class FieldNameCE { public static final String BODY = "body"; public static final String ORGANIZATION_ID = "organizationId"; public static final String NONE = "none"; + public static final String ORGANIZATION_ADMINISTRATOR_ROLE = "Organization Administrator Role"; } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/InMemoryCacheableRepositoryHelper.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/InMemoryCacheableRepositoryHelper.java index b409e60b34..0284a3c462 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/InMemoryCacheableRepositoryHelper.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/InMemoryCacheableRepositoryHelper.java @@ -2,6 +2,8 @@ package com.appsmith.server.helpers; import org.springframework.stereotype.Component; +import java.util.HashMap; +import java.util.Map; import java.util.Set; @Component @@ -12,6 +14,8 @@ public class InMemoryCacheableRepositoryHelper { private String instanceAdminPermissionGroupId = null; + private Map inMemoryOrganizationIdOrganizationPermissionGroupIdMap = new HashMap<>(); + public Set getAnonymousUserPermissionGroupIds() { return anonymousUserPermissionGroupIds; } @@ -35,4 +39,12 @@ public class InMemoryCacheableRepositoryHelper { public String getInstanceAdminPermissionGroupId() { return instanceAdminPermissionGroupId; } + + public String getOrganizationAdminPermissionGroupId(String organizationId) { + return this.inMemoryOrganizationIdOrganizationPermissionGroupIdMap.get(organizationId); + } + + public void setOrganizationAdminPermissionGroupId(String organizationId, String permissionGroupId) { + this.inMemoryOrganizationIdOrganizationPermissionGroupIdMap.put(organizationId, permissionGroupId); + } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/UserUtils.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/UserUtils.java index 7bbbfd6ee8..8dbd479026 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/UserUtils.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/UserUtils.java @@ -1,27 +1,35 @@ package com.appsmith.server.helpers; +import com.appsmith.server.configurations.CommonConfig; import com.appsmith.server.helpers.ce.UserUtilsCE; import com.appsmith.server.repositories.CacheableRepositoryHelper; import com.appsmith.server.repositories.ConfigRepository; +import com.appsmith.server.repositories.OrganizationRepository; import com.appsmith.server.repositories.PermissionGroupRepository; -import com.appsmith.server.solutions.PermissionGroupPermission; +import com.appsmith.server.services.SessionUserService; import io.micrometer.observation.ObservationRegistry; import org.springframework.stereotype.Component; @Component public class UserUtils extends UserUtilsCE { + public UserUtils( ConfigRepository configRepository, PermissionGroupRepository permissionGroupRepository, CacheableRepositoryHelper cacheableRepositoryHelper, - PermissionGroupPermission permissionGroupPermission, - ObservationRegistry observationRegistry) { - + ObservationRegistry observationRegistry, + CommonConfig commonConfig, + InMemoryCacheableRepositoryHelper inMemoryCacheableRepositoryHelper, + OrganizationRepository organizationRepository, + SessionUserService sessionUserService) { super( configRepository, permissionGroupRepository, cacheableRepositoryHelper, - permissionGroupPermission, - observationRegistry); + observationRegistry, + commonConfig, + inMemoryCacheableRepositoryHelper, + organizationRepository, + sessionUserService); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/UserUtilsCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/UserUtilsCE.java index b927e4f6d7..ad1a28c30c 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/UserUtilsCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/UserUtilsCE.java @@ -1,39 +1,33 @@ package com.appsmith.server.helpers.ce; -import com.appsmith.external.models.Policy; import com.appsmith.server.acl.AclPermission; -import com.appsmith.server.constants.FieldName; -import com.appsmith.server.domains.Config; +import com.appsmith.server.configurations.CommonConfig; +import com.appsmith.server.domains.Organization; import com.appsmith.server.domains.PermissionGroup; import com.appsmith.server.domains.User; -import com.appsmith.server.dtos.Permission; +import com.appsmith.server.helpers.InMemoryCacheableRepositoryHelper; import com.appsmith.server.helpers.ce.bridge.Bridge; import com.appsmith.server.helpers.ce.bridge.BridgeUpdate; import com.appsmith.server.repositories.CacheableRepositoryHelper; import com.appsmith.server.repositories.ConfigRepository; +import com.appsmith.server.repositories.OrganizationRepository; import com.appsmith.server.repositories.PermissionGroupRepository; -import com.appsmith.server.solutions.PermissionGroupPermission; +import com.appsmith.server.services.SessionUserService; import io.micrometer.observation.ObservationRegistry; import net.minidev.json.JSONObject; import reactor.core.observability.micrometer.Micrometer; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; -import reactor.util.function.Tuple2; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Set; import java.util.stream.Collectors; import static com.appsmith.external.constants.spans.UserSpan.CHECK_SUPER_USER_SPAN; -import static com.appsmith.server.acl.AclPermission.ASSIGN_PERMISSION_GROUPS; -import static com.appsmith.server.acl.AclPermission.MANAGE_INSTANCE_CONFIGURATION; -import static com.appsmith.server.acl.AclPermission.READ_INSTANCE_CONFIGURATION; -import static com.appsmith.server.acl.AclPermission.READ_PERMISSION_GROUP_MEMBERS; -import static com.appsmith.server.acl.AclPermission.UNASSIGN_PERMISSION_GROUPS; import static com.appsmith.server.constants.FieldName.DEFAULT_PERMISSION_GROUP; import static com.appsmith.server.constants.FieldName.INSTANCE_CONFIG; +import static org.springframework.util.StringUtils.hasLength; public class UserUtilsCE { @@ -41,71 +35,91 @@ public class UserUtilsCE { private final PermissionGroupRepository permissionGroupRepository; - private final PermissionGroupPermission permissionGroupPermission; private final ObservationRegistry observationRegistry; + private final CacheableRepositoryHelper cacheableRepositoryHelper; + private final CommonConfig commonConfig; + private final InMemoryCacheableRepositoryHelper inMemoryCacheableRepositoryHelper; + private final OrganizationRepository organizationRepository; + private final SessionUserService sessionUserService; public UserUtilsCE( ConfigRepository configRepository, PermissionGroupRepository permissionGroupRepository, CacheableRepositoryHelper cacheableRepositoryHelper, - PermissionGroupPermission permissionGroupPermission, - ObservationRegistry observationRegistry) { + ObservationRegistry observationRegistry, + CommonConfig commonConfig, + InMemoryCacheableRepositoryHelper inMemoryCacheableRepositoryHelper, + OrganizationRepository organizationRepository, + SessionUserService sessionUserService) { this.configRepository = configRepository; this.permissionGroupRepository = permissionGroupRepository; - this.permissionGroupPermission = permissionGroupPermission; this.observationRegistry = observationRegistry; + this.cacheableRepositoryHelper = cacheableRepositoryHelper; + this.commonConfig = commonConfig; + this.inMemoryCacheableRepositoryHelper = inMemoryCacheableRepositoryHelper; + this.organizationRepository = organizationRepository; + this.sessionUserService = sessionUserService; } public Mono isSuperUser(User user) { - return configRepository - .findByNameAsUser(INSTANCE_CONFIG, user, AclPermission.MANAGE_INSTANCE_CONFIGURATION) - .map(config -> Boolean.TRUE) + + return organizationRepository + .findByIdAsUser(user, user.getOrganizationId(), AclPermission.MANAGE_ORGANIZATION) + .map(organization -> Boolean.TRUE) .switchIfEmpty(Mono.just(Boolean.FALSE)) .name(CHECK_SUPER_USER_SPAN) .tap(Micrometer.observation(observationRegistry)); } public Mono isCurrentUserSuperUser() { - return configRepository - .findByName(INSTANCE_CONFIG, AclPermission.MANAGE_INSTANCE_CONFIGURATION) - .map(config -> Boolean.TRUE) - .switchIfEmpty(Mono.just(Boolean.FALSE)); + return sessionUserService.getCurrentUser().flatMap(this::isSuperUser); } - public Mono makeSuperUser(List users) { - return getSuperAdminPermissionGroup() - .flatMap(permissionGroup -> { + public Mono makeInstanceAdministrator(List users) { + + // TODO : Replace cloud hosting check with a feature flag check for multi tenancy + boolean cloudHosting = commonConfig.isCloudHosting(); + Mono organizationAdminPgMono = Mono.just(new PermissionGroup()); + + if (!cloudHosting) { + organizationAdminPgMono = getDefaultOrganizationAdminPermissionGroup(); + } + + return Mono.zip(getInstanceAdminPermissionGroup(), organizationAdminPgMono) + .flatMap(tuple -> { + PermissionGroup instanceAdminPg = tuple.getT1(); + PermissionGroup organizationAdminPg = tuple.getT2(); + Set assignedToUserIds = new HashSet<>(); - if (permissionGroup.getAssignedToUserIds() != null) { - assignedToUserIds.addAll(permissionGroup.getAssignedToUserIds()); + if (instanceAdminPg.getAssignedToUserIds() != null) { + assignedToUserIds.addAll(instanceAdminPg.getAssignedToUserIds()); } assignedToUserIds.addAll(users.stream().map(User::getId).collect(Collectors.toList())); BridgeUpdate updateObj = Bridge.update(); String path = PermissionGroup.Fields.assignedToUserIds; updateObj.set(path, assignedToUserIds); - // Make Super User is called before the first administrator is created. - return permissionGroupRepository.updateById(permissionGroup.getId(), updateObj); - }) - .then(Mono.just(users)) - .flatMapMany(Flux::fromIterable) - .flatMap(user -> permissionGroupRepository.evictAllPermissionGroupCachesForUser( - user.getEmail(), user.getOrganizationId())) - .then(Mono.just(Boolean.TRUE)); - } + // Make Instance Admin is called before the first administrator is created. + Mono updateInstanceAdminPgAssignmentMono = + permissionGroupRepository.updateById(instanceAdminPg.getId(), updateObj); - public Mono removeSuperUser(List users) { - return getSuperAdminPermissionGroup() - .flatMap(permissionGroup -> { - if (permissionGroup.getAssignedToUserIds() == null) { - permissionGroup.setAssignedToUserIds(new HashSet<>()); + if (!hasLength(organizationAdminPg.getId())) { + return updateInstanceAdminPgAssignmentMono; } - permissionGroup - .getAssignedToUserIds() - .removeAll(users.stream().map(User::getId).collect(Collectors.toList())); - return permissionGroupRepository.updateById( - permissionGroup.getId(), permissionGroup, permissionGroupPermission.getAssignPermission()); + + // Also assign the users to the organization admin permission group + Set organizationAdminAssignedToUserIds = new HashSet<>(); + if (organizationAdminPg.getAssignedToUserIds() != null) { + organizationAdminAssignedToUserIds.addAll(organizationAdminPg.getAssignedToUserIds()); + } + organizationAdminAssignedToUserIds.addAll( + users.stream().map(User::getId).collect(Collectors.toList())); + BridgeUpdate updateObj2 = Bridge.update(); + String path2 = PermissionGroup.Fields.assignedToUserIds; + updateObj2.set(path2, organizationAdminAssignedToUserIds); + return updateInstanceAdminPgAssignmentMono.then( + permissionGroupRepository.updateById(organizationAdminPg.getId(), updateObj2)); }) .then(Mono.just(users)) .flatMapMany(Flux::fromIterable) @@ -114,102 +128,88 @@ public class UserUtilsCE { .then(Mono.just(Boolean.TRUE)); } - protected Mono createInstanceConfigForSuperUser() { + public Mono removeInstanceAdmin(List users) { - Mono> savedConfigAndPermissionGroupMono = - createConfigAndPermissionGroupForSuperAdmin(); + // TODO : Replace cloud hosting check with a feature flag check for multi tenancy + boolean cloudHosting = commonConfig.isCloudHosting(); + Mono organizationAdminPgMono = Mono.just(new PermissionGroup()); - // return the saved instance config - return savedConfigAndPermissionGroupMono.map(Tuple2::getT2); - } + if (!cloudHosting) { + organizationAdminPgMono = getDefaultOrganizationAdminPermissionGroup(); + } - protected Mono> createConfigAndPermissionGroupForSuperAdmin() { - return Mono.zip(createInstanceAdminConfigObject(), createInstanceAdminPermissionGroupWithoutPermissions()) + return Mono.zip(getInstanceAdminPermissionGroup(), organizationAdminPgMono) .flatMap(tuple -> { - Config savedInstanceConfig = tuple.getT1(); - PermissionGroup savedPermissionGroup = tuple.getT2(); + PermissionGroup instanceAdminPg = tuple.getT1(); + PermissionGroup organizationAdminPg = tuple.getT2(); - // Update the instance config with the permission group id - savedInstanceConfig.setConfig( - new JSONObject(Map.of(DEFAULT_PERMISSION_GROUP, savedPermissionGroup.getId()))); + if (instanceAdminPg.getAssignedToUserIds() == null) { + instanceAdminPg.setAssignedToUserIds(new HashSet<>()); + } + Set assignedToUserIds = new HashSet<>(instanceAdminPg.getAssignedToUserIds()); + assignedToUserIds.removeAll(users.stream().map(User::getId).collect(Collectors.toList())); - Policy editConfigPolicy = Policy.builder() - .permission(MANAGE_INSTANCE_CONFIGURATION.getValue()) - .permissionGroups(Set.of(savedPermissionGroup.getId())) - .build(); - Policy readConfigPolicy = Policy.builder() - .permission(READ_INSTANCE_CONFIGURATION.getValue()) - .permissionGroups(Set.of(savedPermissionGroup.getId())) - .build(); + BridgeUpdate updateObj = Bridge.update(); + String path = PermissionGroup.Fields.assignedToUserIds; - savedInstanceConfig.setPolicies(Set.of(editConfigPolicy, readConfigPolicy)); + updateObj.set(path, assignedToUserIds); + // Make Instance Admin is called before the first administrator is created. + Mono updateInstanceAdminPgAssignmentMono = + permissionGroupRepository.updateById(instanceAdminPg.getId(), updateObj); - // Add config permissions to permission group - Set configPermissions = - Set.of(new Permission(savedInstanceConfig.getId(), MANAGE_INSTANCE_CONFIGURATION)); + if (!hasLength(organizationAdminPg.getId())) { + return updateInstanceAdminPgAssignmentMono; + } - return Mono.zip( - addPermissionsToPermissionGroup(savedPermissionGroup, configPermissions), - configRepository.save(savedInstanceConfig)); - }); + // Also unassign the users from the organization admin permission group + Set organizationAdminAssignedToUserIds = new HashSet<>(); + if (organizationAdminPg.getAssignedToUserIds() != null) { + organizationAdminAssignedToUserIds.addAll(organizationAdminPg.getAssignedToUserIds()); + } + organizationAdminAssignedToUserIds.removeAll( + users.stream().map(User::getId).collect(Collectors.toList())); + BridgeUpdate updateObj2 = Bridge.update(); + String path2 = PermissionGroup.Fields.assignedToUserIds; + updateObj2.set(path2, organizationAdminAssignedToUserIds); + return updateInstanceAdminPgAssignmentMono.then( + permissionGroupRepository.updateById(organizationAdminPg.getId(), updateObj2)); + }) + .then(Mono.just(users)) + .flatMapMany(Flux::fromIterable) + .flatMap(user -> permissionGroupRepository.evictAllPermissionGroupCachesForUser( + user.getEmail(), user.getOrganizationId())) + .then(Mono.just(Boolean.TRUE)); } - private Mono createInstanceAdminConfigObject() { - Config instanceAdminConfiguration = new Config(); - instanceAdminConfiguration.setName(FieldName.INSTANCE_CONFIG); + public Mono getInstanceAdminPermissionGroup() { - return configRepository.save(instanceAdminConfiguration); - } + String instanceAdminPermissionGroupId = inMemoryCacheableRepositoryHelper.getInstanceAdminPermissionGroupId(); + if (hasLength(instanceAdminPermissionGroupId)) { + return permissionGroupRepository.findById(instanceAdminPermissionGroupId); + } - private Mono createInstanceAdminPermissionGroupWithoutPermissions() { - PermissionGroup instanceAdminPermissionGroup = new PermissionGroup(); - instanceAdminPermissionGroup.setName(FieldName.INSTANCE_ADMIN_ROLE); - - return permissionGroupRepository.save(instanceAdminPermissionGroup).flatMap(savedPermissionGroup -> { - Set permissions = Set.of( - new Permission(savedPermissionGroup.getId(), READ_PERMISSION_GROUP_MEMBERS), - new Permission(savedPermissionGroup.getId(), ASSIGN_PERMISSION_GROUPS), - new Permission(savedPermissionGroup.getId(), UNASSIGN_PERMISSION_GROUPS)); - savedPermissionGroup.setPermissions(permissions); - - Policy readPermissionGroupPolicy = Policy.builder() - .permission(READ_PERMISSION_GROUP_MEMBERS.getValue()) - .permissionGroups(Set.of(savedPermissionGroup.getId())) - .build(); - - Policy assignPermissionGroupPolicy = Policy.builder() - .permission(ASSIGN_PERMISSION_GROUPS.getValue()) - .permissionGroups(Set.of(savedPermissionGroup.getId())) - .build(); - - Policy unassignPermissionGroupPolicy = Policy.builder() - .permission(UNASSIGN_PERMISSION_GROUPS.getValue()) - .permissionGroups(Set.of(savedPermissionGroup.getId())) - .build(); - - savedPermissionGroup.setPolicies( - Set.of(readPermissionGroupPolicy, assignPermissionGroupPolicy, unassignPermissionGroupPolicy)); - - return permissionGroupRepository.save(savedPermissionGroup); + return configRepository.findByName(INSTANCE_CONFIG).flatMap(instanceConfig -> { + JSONObject config = instanceConfig.getConfig(); + String defaultPermissionGroup = (String) config.getOrDefault(DEFAULT_PERMISSION_GROUP, ""); + return permissionGroupRepository + .findById(defaultPermissionGroup) + .doOnSuccess(permissionGroup -> inMemoryCacheableRepositoryHelper.setInstanceAdminPermissionGroupId( + permissionGroup.getId())); }); } - protected Mono addPermissionsToPermissionGroup( - PermissionGroup permissionGroup, Set permissions) { - Set existingPermissions = new HashSet<>(permissionGroup.getPermissions()); - existingPermissions.addAll(permissions); - permissionGroup.setPermissions(existingPermissions); - return permissionGroupRepository.save(permissionGroup); - } - - public Mono getSuperAdminPermissionGroup() { - return configRepository - .findByName(INSTANCE_CONFIG) - .switchIfEmpty(Mono.defer(() -> createInstanceConfigForSuperUser())) - .flatMap(instanceConfig -> { - JSONObject config = instanceConfig.getConfig(); - String defaultPermissionGroup = (String) config.getOrDefault(DEFAULT_PERMISSION_GROUP, ""); - return permissionGroupRepository.findById(defaultPermissionGroup); - }); + public Mono getDefaultOrganizationAdminPermissionGroup() { + return cacheableRepositoryHelper.getDefaultOrganizationId().flatMap(orgId -> { + String permissionGroupId = inMemoryCacheableRepositoryHelper.getOrganizationAdminPermissionGroupId(orgId); + if (hasLength(permissionGroupId)) { + return permissionGroupRepository.findById(permissionGroupId); + } + return permissionGroupRepository + .findByDefaultDomainIdAndDefaultDomainType(orgId, Organization.class.getSimpleName()) + .next() + .doOnSuccess( + permissionGroup -> inMemoryCacheableRepositoryHelper.setOrganizationAdminPermissionGroupId( + orgId, permissionGroup.getId())); + }); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog2.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog2.java index bfd58c438d..fa91ce241b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog2.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog2.java @@ -60,7 +60,7 @@ import static com.appsmith.server.acl.AclPermission.MANAGE_INSTANCE_ENV; import static com.appsmith.server.acl.AclPermission.READ_INSTANCE_CONFIGURATION; import static com.appsmith.server.acl.AclPermission.READ_PERMISSION_GROUP_MEMBERS; import static com.appsmith.server.acl.AclPermission.READ_THEMES; -import static com.appsmith.server.acl.AppsmithRole.TENANT_ADMIN; +import static com.appsmith.server.acl.AppsmithRole.ORGANIZATION_ADMIN; import static com.appsmith.server.constants.FieldName.DEFAULT_PERMISSION_GROUP; import static com.appsmith.server.constants.FieldName.PERMISSION_GROUP_ID; import static com.appsmith.server.migrations.DatabaseChangelog1.dropIndexIfExists; @@ -550,7 +550,7 @@ public class DatabaseChangelog2 { policySolution.addPoliciesToExistingObject(readPermissionGroupPolicyMap, instanceAdminPGBeforeChanges); // Now add admin permissions to the tenant - Set tenantPermissions = TENANT_ADMIN.getPermissions().stream() + Set tenantPermissions = ORGANIZATION_ADMIN.getPermissions().stream() .map(permission -> new Permission(defaultTenant.getId(), permission)) .collect(Collectors.toSet()); HashSet permissions = new HashSet<>(instanceAdminPG.getPermissions()); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration066_RenameInstanceAdminRoleToOrgAdmin.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration066_RenameInstanceAdminRoleToOrgAdmin.java new file mode 100644 index 0000000000..2826872191 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration066_RenameInstanceAdminRoleToOrgAdmin.java @@ -0,0 +1,121 @@ +package com.appsmith.server.migrations.db.ce; + +import com.appsmith.external.models.Policy; +import com.appsmith.server.constants.FieldName; +import com.appsmith.server.domains.Config; +import com.appsmith.server.domains.Organization; +import com.appsmith.server.domains.PermissionGroup; +import io.mongock.api.annotations.ChangeUnit; +import io.mongock.api.annotations.Execution; +import io.mongock.api.annotations.RollbackExecution; +import lombok.extern.slf4j.Slf4j; +import net.minidev.json.JSONObject; +import org.springframework.data.mongodb.core.MongoTemplate; +import org.springframework.data.mongodb.core.query.Criteria; +import org.springframework.data.mongodb.core.query.Query; +import org.springframework.data.mongodb.core.query.Update; + +import java.util.Set; + +import static com.appsmith.external.models.BaseDomain.policySetToMap; + +@Slf4j +@ChangeUnit(id = "rename-instance-admin-role-to-org-admin-role", order = "066") +public class Migration066_RenameInstanceAdminRoleToOrgAdmin { + + private final MongoTemplate mongoTemplate; + + public Migration066_RenameInstanceAdminRoleToOrgAdmin(MongoTemplate mongoTemplate) { + this.mongoTemplate = mongoTemplate; + } + + @RollbackExecution + public void rollbackExecution() {} + + @Execution + public void execute() { + // 1. Find instanceConfig + Config instanceConfig = + mongoTemplate.findOne(Query.query(Criteria.where("name").is("instanceConfig")), Config.class); + + if (instanceConfig == null || instanceConfig.getConfig() == null) { + log.error("Instance config not found"); + return; + } + + // 2. Find default organization - add null check + Organization defaultOrganization = mongoTemplate.findOne(new Query(), Organization.class); + if (defaultOrganization == null) { + log.error("Default organization not found"); + return; + } + + renameInstanceAdminPermissionGroupToOrganizationAdmin(instanceConfig, defaultOrganization); + createInstanceAdminRole(instanceConfig, defaultOrganization); + } + + private void createInstanceAdminRole(Config instanceConfig, Organization defaultOrganization) { + + // Create instance management permission group + PermissionGroup instanceManagerPermissionGroup = new PermissionGroup(); + instanceManagerPermissionGroup.setName(FieldName.INSTANCE_ADMIN_ROLE); + + // No permissions given to instance admin role. This would be a hidden permission group. + + // Fetch the organization admin role + PermissionGroup organizationAdminRole = mongoTemplate.findOne( + Query.query(Criteria.where("defaultDomainId").is(defaultOrganization.getId())), PermissionGroup.class); + + // Assign the permission group to all the users and user groups who already are assigned to the org admin role + instanceManagerPermissionGroup.setAssignedToUserIds(organizationAdminRole.getAssignedToUserIds()); + instanceManagerPermissionGroup.setAssignedToGroupIds(organizationAdminRole.getAssignedToGroupIds()); + + // Save the permission group + PermissionGroup savedInstanceAdminRole = mongoTemplate.save(instanceManagerPermissionGroup); + + // Update the config document + JSONObject config = instanceConfig.getConfig(); + config.put("defaultPermissionGroup", savedInstanceAdminRole.getId()); + instanceConfig.setConfig(config); + + Set policies = instanceConfig.getPolicies(); + policies.stream().forEach(policy -> { + policy.getPermissionGroups().remove(organizationAdminRole.getId()); + policy.getPermissionGroups().add(savedInstanceAdminRole.getId()); + }); + instanceConfig.setPolicies(policies); + instanceConfig.setPolicyMap(policySetToMap(policies)); + mongoTemplate.save(instanceConfig); + } + + private void renameInstanceAdminPermissionGroupToOrganizationAdmin( + Config instanceConfig, Organization defaultOrganization) { + try { + + String instanceAdminRoleId = + instanceConfig.getConfig().get("defaultPermissionGroup").toString(); + + // 3. Update permission group with all fields in a single update + Update update = Update.update("name", "Organization Administrator Role") + .set("defaultDomainId", defaultOrganization.getId()) + .set("defaultDomainType", "Organization"); // Use string directly instead of class name + + long modifiedCount = mongoTemplate + .updateFirst( + Query.query(Criteria.where("_id").is(instanceAdminRoleId)), update, PermissionGroup.class) + .getModifiedCount(); + + if (modifiedCount > 0) { + log.info( + "Successfully renamed instance admin role to organization admin role for group: {}", + instanceAdminRoleId); + } else { + log.warn("No permission group was updated for id: {}", instanceAdminRoleId); + } + + } catch (Exception e) { + log.error("Error while renaming instance admin role", e); + throw e; + } + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration10000_UpdateSuperUser.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration10000_UpdateSuperUser.java index 03cd7e1117..91bd2e9055 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration10000_UpdateSuperUser.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration10000_UpdateSuperUser.java @@ -70,17 +70,24 @@ public class Migration10000_UpdateSuperUser { String instanceAdminPermissionGroupId = (String) instanceAdminConfiguration.getConfig().get(DEFAULT_PERMISSION_GROUP); - Query permissionGroupQuery = new Query(); - permissionGroupQuery + Query instanceAdminPgQuery = new Query(); + instanceAdminPgQuery .addCriteria(where(PermissionGroup.Fields.id).is(instanceAdminPermissionGroupId)) .fields() .include(PermissionGroup.Fields.assignedToUserIds); - PermissionGroup instanceAdminPG = mongoTemplate.findOne(permissionGroupQuery, PermissionGroup.class); + PermissionGroup instanceAdminPG = mongoTemplate.findOne(instanceAdminPgQuery, PermissionGroup.class); Query organizationQuery = new Query(); organizationQuery.addCriteria(where(Organization.Fields.slug).is("default")); Organization organization = mongoTemplate.findOne(organizationQuery, Organization.class); + // Find the default organization admin permission group + Query orgAdminPermissionGroupQuery = new Query(); + orgAdminPermissionGroupQuery.addCriteria( + where(PermissionGroup.Fields.defaultDomainType).is(Organization.class.getSimpleName())); + orgAdminPermissionGroupQuery.addCriteria( + where(PermissionGroup.Fields.defaultDomainId).is(organization.getId())); + Set userIds = adminEmails.stream() .map(email -> email.trim()) .map(String::toLowerCase) @@ -103,8 +110,13 @@ public class Migration10000_UpdateSuperUser { Set updatedUserIds = findSymmetricDiff(oldSuperUsers, userIds); evictPermissionCacheForUsers(updatedUserIds, mongoTemplate, cacheableRepositoryHelper); + // Assign the users to the instance admin pg Update update = new Update().set(PermissionGroup.Fields.assignedToUserIds, userIds); - mongoTemplate.updateFirst(permissionGroupQuery, update, PermissionGroup.class); + mongoTemplate.updateFirst(instanceAdminPgQuery, update, PermissionGroup.class); + + // Assign the users to the default organization admin pg + Update orgAdminUpdate = new Update().set(PermissionGroup.Fields.assignedToUserIds, userIds); + mongoTemplate.updateFirst(orgAdminPermissionGroupQuery, orgAdminUpdate, PermissionGroup.class); // Assign all super users to the default role updateSuperUserMigrationHelper.assignAllSuperUsersToDefaultRole( diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CacheableRepositoryHelperCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CacheableRepositoryHelperCE.java index 7c6e7e3e00..c7421d1107 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CacheableRepositoryHelperCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CacheableRepositoryHelperCE.java @@ -11,6 +11,8 @@ public interface CacheableRepositoryHelperCE { Mono> getPermissionGroupsOfUser(User user); + Mono getOrganizationAdminPermissionGroupId(String organizationId); + Mono> preFillAnonymousUserPermissionGroupIdsCache(); Mono> getPermissionGroupsOfAnonymousUser(); @@ -19,8 +21,6 @@ public interface CacheableRepositoryHelperCE { Mono getDefaultOrganizationId(); - Mono getInstanceAdminPermissionGroupId(); - Mono fetchDefaultOrganization(String organizationId); Mono evictCachedOrganization(String organizationId); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CacheableRepositoryHelperCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CacheableRepositoryHelperCEImpl.java index d0d67118af..6a8f1cfe07 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CacheableRepositoryHelperCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CacheableRepositoryHelperCEImpl.java @@ -17,7 +17,6 @@ import com.appsmith.server.helpers.ce.bridge.BridgeQuery; import io.micrometer.observation.ObservationRegistry; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; -import net.minidev.json.JSONObject; import org.springframework.data.mongodb.core.ReactiveMongoOperations; import org.springframework.data.mongodb.core.query.Query; import org.springframework.stereotype.Component; @@ -32,9 +31,8 @@ import java.util.stream.Collectors; import static com.appsmith.external.constants.spans.OrganizationSpan.FETCH_ORGANIZATION_FROM_DB_SPAN; import static com.appsmith.server.constants.FieldName.PERMISSION_GROUP_ID; import static com.appsmith.server.constants.ce.FieldNameCE.ANONYMOUS_USER; -import static com.appsmith.server.constants.ce.FieldNameCE.DEFAULT_PERMISSION_GROUP; -import static com.appsmith.server.constants.ce.FieldNameCE.INSTANCE_CONFIG; import static com.appsmith.server.repositories.ce.BaseAppsmithRepositoryCEImpl.notDeleted; +import static org.springframework.util.StringUtils.hasLength; @Slf4j @Component @@ -58,33 +56,35 @@ public class CacheableRepositoryHelperCEImpl implements CacheableRepositoryHelpe if (user.getEmail() == null || user.getEmail().isEmpty() || user.getId() == null - || user.getId().isEmpty()) { + || user.getId().isEmpty() + || user.getOrganizationId() == null) { return Mono.error(new AppsmithException(AppsmithError.SESSION_BAD_STATE)); } - Mono createQueryMono = getInstanceAdminPermissionGroupId().map(instanceAdminPermissionGroupId -> { - BridgeQuery assignedToUserIdsCriteria = - Bridge.equal(PermissionGroup.Fields.assignedToUserIds, user.getId()); + Mono createQueryMono = getOrganizationAdminPermissionGroupId(user.getOrganizationId()) + .map(organizationAdminPermissionGroupId -> { + BridgeQuery assignedToUserIdsCriteria = + Bridge.equal(PermissionGroup.Fields.assignedToUserIds, user.getId()); - BridgeQuery notDeletedCriteria = notDeleted(); + BridgeQuery notDeletedCriteria = notDeleted(); - // The roles should be either workspace default roles, user management role, or instance admin role - BridgeQuery ceSupportedRolesCriteria = Bridge.or( - Bridge.equal(PermissionGroup.Fields.defaultDomainType, Workspace.class.getSimpleName()), - Bridge.equal(PermissionGroup.Fields.defaultDomainType, User.class.getSimpleName()), - Bridge.equal(PermissionGroup.Fields.id, instanceAdminPermissionGroupId)); + // The roles should be either workspace default roles, user management role, or instance admin role + BridgeQuery ceSupportedRolesCriteria = Bridge.or( + Bridge.equal(PermissionGroup.Fields.defaultDomainType, Workspace.class.getSimpleName()), + Bridge.equal(PermissionGroup.Fields.defaultDomainType, User.class.getSimpleName()), + Bridge.equal(PermissionGroup.Fields.id, organizationAdminPermissionGroupId)); - BridgeQuery andCriteria = - Bridge.and(assignedToUserIdsCriteria, notDeletedCriteria, ceSupportedRolesCriteria); + BridgeQuery andCriteria = + Bridge.and(assignedToUserIdsCriteria, notDeletedCriteria, ceSupportedRolesCriteria); - Query query = new Query(); - query.addCriteria(andCriteria); + Query query = new Query(); + query.addCriteria(andCriteria); - // Since we are only interested in the permission group ids, we can project only the id field. - query.fields().include(PermissionGroup.Fields.id); + // Since we are only interested in the permission group ids, we can project only the id field. + query.fields().include(PermissionGroup.Fields.id); - return query; - }); + return query; + }); return createQueryMono .map(query -> mongoOperations.find(query, PermissionGroup.class)) @@ -93,6 +93,35 @@ public class CacheableRepositoryHelperCEImpl implements CacheableRepositoryHelpe .collect(Collectors.toSet()); } + @Override + public Mono getOrganizationAdminPermissionGroupId(String organizationId) { + + String organizationAdminPermissionGroupId = + inMemoryCacheableRepositoryHelper.getOrganizationAdminPermissionGroupId(organizationId); + if (hasLength(organizationAdminPermissionGroupId)) { + return Mono.just(organizationAdminPermissionGroupId); + } + + // Find the permission group id of the organization admin + BridgeQuery andCriteria = Bridge.and( + Bridge.equal(PermissionGroup.Fields.defaultDomainType, Organization.class.getSimpleName()), + Bridge.equal(PermissionGroup.Fields.defaultDomainId, organizationId)); + + Query query = new Query(); + query.addCriteria(andCriteria); + + // Since we are only interested in the permission group ids, we can project only the id field. + query.fields().include(PermissionGroup.Fields.id); + + return mongoOperations + .find(query, PermissionGroup.class) + .map(permissionGroup -> permissionGroup.getId()) + .next() + .doOnSuccess( + permissionGroupId -> inMemoryCacheableRepositoryHelper.setOrganizationAdminPermissionGroupId( + organizationId, permissionGroupId)); + } + @Override public Mono> preFillAnonymousUserPermissionGroupIdsCache() { Set roleIdsForAnonymousUser = inMemoryCacheableRepositoryHelper.getAnonymousUserPermissionGroupIds(); @@ -152,25 +181,6 @@ public class CacheableRepositoryHelperCEImpl implements CacheableRepositoryHelpe }); } - @Override - public Mono getInstanceAdminPermissionGroupId() { - String instanceAdminPermissionGroupId = inMemoryCacheableRepositoryHelper.getInstanceAdminPermissionGroupId(); - if (instanceAdminPermissionGroupId != null && !instanceAdminPermissionGroupId.isEmpty()) { - return Mono.just(instanceAdminPermissionGroupId); - } - - BridgeQuery configName = Bridge.equal(Config.Fields.name, INSTANCE_CONFIG); - - return mongoOperations - .findOne(new Query().addCriteria(configName), Config.class) - .map(instanceConfig -> { - JSONObject config = instanceConfig.getConfig(); - return (String) config.getOrDefault(DEFAULT_PERMISSION_GROUP, ""); - }) - .doOnSuccess(permissionGroupId -> - inMemoryCacheableRepositoryHelper.setInstanceAdminPermissionGroupId(permissionGroupId)); - } - /** * Returns the default organization from the cache if present. * If not present in cache, then it fetches the default organization from the database and adds to redis. diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomOrganizationRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomOrganizationRepositoryCE.java index a6d4c75630..84cbef2f1b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomOrganizationRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomOrganizationRepositoryCE.java @@ -1,9 +1,13 @@ package com.appsmith.server.repositories.ce; +import com.appsmith.server.acl.AclPermission; import com.appsmith.server.domains.Organization; +import com.appsmith.server.domains.User; import com.appsmith.server.repositories.AppsmithRepository; import reactor.core.publisher.Mono; public interface CustomOrganizationRepositoryCE extends AppsmithRepository { Mono disableRestartForAllTenants(); + + Mono findByIdAsUser(User user, String id, AclPermission permission); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomOrganizationRepositoryCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomOrganizationRepositoryCEImpl.java index 8d57c4a0b8..0902a5d544 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomOrganizationRepositoryCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomOrganizationRepositoryCEImpl.java @@ -1,6 +1,9 @@ package com.appsmith.server.repositories.ce; +import com.appsmith.external.models.BaseDomain; +import com.appsmith.server.acl.AclPermission; import com.appsmith.server.domains.Organization; +import com.appsmith.server.domains.User; import com.appsmith.server.helpers.ce.bridge.Bridge; import com.appsmith.server.repositories.BaseAppsmithRepositoryImpl; import lombok.extern.slf4j.Slf4j; @@ -19,4 +22,13 @@ public class CustomOrganizationRepositoryCEImpl extends BaseAppsmithRepositoryIm .criteria(Bridge.isTrue(organizationConfiguration_isRestartRequired)) .updateAll(Bridge.update().set(organizationConfiguration_isRestartRequired, false)); } + + @Override + public Mono findByIdAsUser(User user, String id, AclPermission permission) { + return getAllPermissionGroupsForUser(user).flatMap(permissionGroups -> queryBuilder() + .criteria(Bridge.equal(BaseDomain.Fields.id, id)) + .permission(permission) + .permissionGroups(permissionGroups) + .one()); + } } 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 c6ced67e84..20aa2af91d 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 @@ -417,7 +417,9 @@ public class UserServiceCEImpl extends BaseService .flatMap(this::addUserPoliciesAndSaveToRepo) .flatMap(crudUser -> { if (isAdminUser) { - return userUtils.makeSuperUser(List.of(crudUser)).then(Mono.just(crudUser)); + return userUtils + .makeInstanceAdministrator(List.of(crudUser)) + .then(Mono.just(crudUser)); } return Mono.just(crudUser); }) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java index 72528b88a9..7c47496a64 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java @@ -611,7 +611,7 @@ public class EnvManagerCEImpl implements EnvManagerCE { Mono removedUsersMono = Flux.fromIterable(removedUsers) .flatMap(userService::findByEmail) .collectList() - .flatMap(userUtils::removeSuperUser); + .flatMap(userUtils::removeInstanceAdmin); Flux> usersFlux = Flux.fromIterable(newUsers) .flatMap(email -> userService @@ -644,7 +644,7 @@ public class EnvManagerCEImpl implements EnvManagerCE { .map(results -> results.stream().allMatch(result -> result)); Mono existingUsersMono = existingUsersWhichAreNotAlreadySuperUsersMono.flatMap(users -> userUtils - .makeSuperUser(users) + .makeInstanceAdministrator(users) .flatMap( success -> Flux.fromIterable(users) .flatMap(user -> sessionUserService diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/UserSignupCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/UserSignupCEImpl.java index 80cc7ae682..c4b35c172a 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/UserSignupCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/UserSignupCEImpl.java @@ -290,7 +290,7 @@ public class UserSignupCEImpl implements UserSignupCE { }) .flatMap(user -> { Mono makeSuperUserMono = userUtils - .makeSuperUser(List.of(user)) + .makeInstanceAdministrator(List.of(user)) .elapsed() .map(pair -> { log.debug( diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/UserUtilsTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/UserUtilsTest.java new file mode 100644 index 0000000000..237bd137c1 --- /dev/null +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/UserUtilsTest.java @@ -0,0 +1,127 @@ +package com.appsmith.server.helpers; + +import com.appsmith.server.domains.PermissionGroup; +import com.appsmith.server.domains.User; +import com.appsmith.server.helpers.ce.UserUtilsCE; +import com.appsmith.server.repositories.PermissionGroupRepository; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; + +import java.util.Arrays; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +@SpringBootTest +class UserUtilsTest { + + @Autowired + private UserUtilsCE userUtils; + + @Autowired + private PermissionGroupRepository permissionGroupRepository; + + private User user1; + private User user2; + + @BeforeEach + void setUp() { + // Create test users + user1 = new User(); + user1.setId("user1"); + user1.setEmail("user1@test.com"); + user1.setOrganizationId("org1"); + + user2 = new User(); + user2.setId("user2"); + user2.setEmail("user2@test.com"); + user2.setOrganizationId("org1"); + } + + @Test + void makeInstanceAdministrator_WhenUsersProvided_AssignsPermissionsSuccessfully() { + List users = Arrays.asList(user1, user2); + + StepVerifier.create(userUtils + .makeInstanceAdministrator(users) + .then(Mono.zip( + userUtils.getInstanceAdminPermissionGroup(), + userUtils.getDefaultOrganizationAdminPermissionGroup()))) + .assertNext(tuple -> { + PermissionGroup instanceAdminPG = tuple.getT1(); + PermissionGroup organizationAdminPG = tuple.getT2(); + + // Verify instance admin assignments + assertThat(instanceAdminPG.getAssignedToUserIds()) + .contains(user1.getId(), user2.getId()) + .as("Users should be assigned to instance admin group"); + + // Verify organization admin assignments + assertThat(organizationAdminPG.getAssignedToUserIds()) + .contains(user1.getId(), user2.getId()) + .as("Users should be assigned to organization admin group"); + }) + .verifyComplete(); + } + + @Test + void removeInstanceAdmin_WhenUsersProvided_RemovesPermissionsSuccessfully() { + List users = Arrays.asList(user1, user2); + + // First add the users as admins + StepVerifier.create(userUtils + .makeInstanceAdministrator(users) + .then(userUtils.removeInstanceAdmin(users)) + .then(Mono.zip( + userUtils.getInstanceAdminPermissionGroup(), + userUtils.getDefaultOrganizationAdminPermissionGroup()))) + .assertNext(tuple -> { + PermissionGroup instanceAdminPG = tuple.getT1(); + PermissionGroup organizationAdminPG = tuple.getT2(); + + // Verify instance admin unassignments + assertThat(instanceAdminPG.getAssignedToUserIds()) + .doesNotContain(user1.getId(), user2.getId()) + .as("Users should be removed from instance admin group"); + + // Verify organization admin unassignments + assertThat(organizationAdminPG.getAssignedToUserIds()) + .doesNotContain(user1.getId(), user2.getId()) + .as("Users should be removed from organization admin group"); + }) + .verifyComplete(); + } + + @Test + void makeInstanceAdministrator_WhenUserAlreadyAdmin_MaintainsPermissionsSuccessfully() { + List users = List.of(user1); + + // Add user as admin twice + StepVerifier.create(userUtils + .makeInstanceAdministrator(users) + .then(userUtils.makeInstanceAdministrator(users)) + .then(Mono.zip( + userUtils.getInstanceAdminPermissionGroup(), + userUtils.getDefaultOrganizationAdminPermissionGroup()))) + .assertNext(tuple -> { + PermissionGroup instanceAdminPG = tuple.getT1(); + PermissionGroup organizationAdminPG = tuple.getT2(); + + // Verify user is still assigned exactly once + assertThat(instanceAdminPG.getAssignedToUserIds()) + .contains(user1.getId()) + .hasSize(1) + .as("User should be assigned exactly once to instance admin group"); + + assertThat(organizationAdminPG.getAssignedToUserIds()) + .contains(user1.getId()) + .hasSize(1) + .as("User should be assigned exactly once to organization admin group"); + }) + .verifyComplete(); + } +} diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/CacheableRepositoryTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/CacheableRepositoryTest.java index 7ab84155ee..968ee3caeb 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/CacheableRepositoryTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/CacheableRepositoryTest.java @@ -40,7 +40,7 @@ public class CacheableRepositoryTest { @BeforeEach() public void setup() { User api_user = userRepository.findByEmail("api_user").block(); - userUtils.makeSuperUser(List.of(api_user)).block(); + userUtils.makeInstanceAdministrator(List.of(api_user)).block(); } @Test diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PermissionGroupServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PermissionGroupServiceTest.java index caed8d7fbe..50519bed07 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PermissionGroupServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PermissionGroupServiceTest.java @@ -96,7 +96,7 @@ public class PermissionGroupServiceTest { // Make api_user instance admin before running the test userRepository .findByEmail("api_user") - .flatMap(user -> userUtils.makeSuperUser(List.of(user))) + .flatMap(user -> userUtils.makeInstanceAdministrator(List.of(user))) .block(); PermissionGroup testPermissionGroup = new PermissionGroup(); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserWorkspaceServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserWorkspaceServiceTest.java index de83b25729..afdc484d8d 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserWorkspaceServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserWorkspaceServiceTest.java @@ -328,7 +328,7 @@ public class UserWorkspaceServiceTest { // Ensure neither of the users are super users User api_user = userRepository.findByEmail("api_user").block(); User test_user = userRepository.findByEmail("usertest@usertest.com").block(); - userUtils.removeSuperUser(List.of(api_user, test_user)).block(); + userUtils.removeInstanceAdmin(List.of(api_user, test_user)).block(); Workspace workspace = new Workspace(); workspace.setName("Test org"); Workspace createdWorkspace = workspaceService.create(workspace).block(); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/OrganizationServiceCETest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/OrganizationServiceCETest.java index a9b1dd8c82..8f81ff4639 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/OrganizationServiceCETest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/OrganizationServiceCETest.java @@ -101,7 +101,7 @@ class OrganizationServiceCETest { // Todo change this to organization admin once we introduce multitenancy userRepository .findByEmail("api_user") - .flatMap(user -> userUtils.makeSuperUser(List.of(user))) + .flatMap(user -> userUtils.makeInstanceAdministrator(List.of(user))) .block(); doReturn(Mono.empty()).when(cacheManager).get(anyString(), anyString()); }