Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Introduce Organization Administrator concept along with Instance Administrator #39381

Closed
wants to merge 6 commits into from
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
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 {
Expand Down Expand Up @@ -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<AclPermission> permissions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
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;
Expand All @@ -10,18 +11,19 @@

@Component
public class UserUtils extends UserUtilsCE {

public UserUtils(
ConfigRepository configRepository,
PermissionGroupRepository permissionGroupRepository,
CacheableRepositoryHelper cacheableRepositoryHelper,
PermissionGroupPermission permissionGroupPermission,
ObservationRegistry observationRegistry) {

ObservationRegistry observationRegistry,
CommonConfig commonConfig) {
super(
configRepository,
permissionGroupRepository,
cacheableRepositoryHelper,
permissionGroupPermission,
observationRegistry);
observationRegistry,
commonConfig);
}
}

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
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;
Expand Down Expand Up @@ -550,7 +550,7 @@ public void addTenantAdminPermissionsToInstanceAdmin(
policySolution.addPoliciesToExistingObject(readPermissionGroupPolicyMap, instanceAdminPGBeforeChanges);

// Now add admin permissions to the tenant
Set<Permission> tenantPermissions = TENANT_ADMIN.getPermissions().stream()
Set<Permission> tenantPermissions = ORGANIZATION_ADMIN.getPermissions().stream()
.map(permission -> new Permission(defaultTenant.getId(), permission))
.collect(Collectors.toSet());
HashSet<Permission> permissions = new HashSet<>(instanceAdminPG.getPermissions());
Expand Down
Original file line number Diff line number Diff line change
@@ -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<Policy> 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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,9 @@ public Mono<User> userCreate(User user, boolean isAdminUser) {
.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);
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ private Mono<Boolean> updateAdminUserPolicies(String oldAdminEmailsCsv, String o
Mono<Boolean> removedUsersMono = Flux.fromIterable(removedUsers)
.flatMap(userService::findByEmail)
.collectList()
.flatMap(userUtils::removeSuperUser);
.flatMap(userUtils::removeInstanceAdmin);

Flux<Tuple2<User, Boolean>> usersFlux = Flux.fromIterable(newUsers)
.flatMap(email -> userService
Expand Down Expand Up @@ -644,7 +644,7 @@ private Mono<Boolean> updateAdminUserPolicies(String oldAdminEmailsCsv, String o
.map(results -> results.stream().allMatch(result -> result));

Mono<Boolean> existingUsersMono = existingUsersWhichAreNotAlreadySuperUsersMono.flatMap(users -> userUtils
.makeSuperUser(users)
.makeInstanceAdministrator(users)
.flatMap(
success -> Flux.fromIterable(users)
.flatMap(user -> sessionUserService
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ public Mono<User> signupAndLoginSuper(
})
.flatMap(user -> {
Mono<Boolean> makeSuperUserMono = userUtils
.makeSuperUser(List.of(user))
.makeInstanceAdministrator(List.of(user))
.elapsed()
.map(pair -> {
log.debug(
Expand Down
Original file line number Diff line number Diff line change
@@ -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("[email protected]");
user1.setOrganizationId("org1");

user2 = new User();
user2.setId("user2");
user2.setEmail("[email protected]");
user2.setOrganizationId("org1");
}

@Test
void makeInstanceAdministrator_WhenUsersProvided_AssignsPermissionsSuccessfully() {
List<User> users = Arrays.asList(user1, user2);

StepVerifier.create(userUtils
.makeInstanceAdministrator(users)
.then(Mono.zip(
userUtils.getInstanceAdminPermissionGroup(),
userUtils.getOrganizationAdminPermissionGroup())))
.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<User> 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.getOrganizationAdminPermissionGroup())))
.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<User> users = List.of(user1);

// Add user as admin twice
StepVerifier.create(userUtils
.makeInstanceAdministrator(users)
.then(userUtils.makeInstanceAdministrator(users))
.then(Mono.zip(
userUtils.getInstanceAdminPermissionGroup(),
userUtils.getOrganizationAdminPermissionGroup())))
.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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public void invalid_leaveRole() {
// 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public void setup() throws IOException {
// 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());
}
Expand Down
Loading