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()); }