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
Closed
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 @@ -19,8 +19,6 @@ public interface CacheableRepositoryHelperCE {

Mono<String> getDefaultOrganizationId();

Mono<String> getInstanceAdminPermissionGroupId();

Mono<Organization> fetchDefaultOrganization(String organizationId);

Mono<Void> evictCachedOrganization(String organizationId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
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;
Expand All @@ -32,8 +31,6 @@
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;

@Slf4j
Expand Down Expand Up @@ -62,33 +59,28 @@ public Mono<Set<String>> getPermissionGroupsOfUser(User user) {
return Mono.error(new AppsmithException(AppsmithError.SESSION_BAD_STATE));
}

Mono<Query> createQueryMono = getInstanceAdminPermissionGroupId().map(instanceAdminPermissionGroupId -> {
BridgeQuery<PermissionGroup> assignedToUserIdsCriteria =
Bridge.equal(PermissionGroup.Fields.assignedToUserIds, user.getId());
BridgeQuery<PermissionGroup> assignedToUserIdsCriteria =
Bridge.equal(PermissionGroup.Fields.assignedToUserIds, user.getId());

BridgeQuery<PermissionGroup> notDeletedCriteria = notDeleted();
BridgeQuery<PermissionGroup> notDeletedCriteria = notDeleted();

// The roles should be either workspace default roles, user management role, or instance admin role
BridgeQuery<PermissionGroup> 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 organization admin role
BridgeQuery<PermissionGroup> ceSupportedRolesCriteria = Bridge.or(
Bridge.equal(PermissionGroup.Fields.defaultDomainType, Workspace.class.getSimpleName()),
Bridge.equal(PermissionGroup.Fields.defaultDomainType, User.class.getSimpleName()),
Bridge.equal(PermissionGroup.Fields.defaultDomainType, Organization.class.getSimpleName()));

BridgeQuery<PermissionGroup> andCriteria =
Bridge.and(assignedToUserIdsCriteria, notDeletedCriteria, ceSupportedRolesCriteria);
BridgeQuery<PermissionGroup> andCriteria =
Bridge.and(assignedToUserIdsCriteria, notDeletedCriteria, ceSupportedRolesCriteria);

Comment on lines +62 to 75
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Ensure domain types align with the new admin concept
The updated criteria logic looks streamlined and removes references to instance-based roles, focusing on organization-based roles. It's essential to verify that no leftover references to the old "instance admin" or "tenant admin" roles remain within the codebase or mismatch the newly introduced organization-centric approach.

Run the following script to confirm if any vestiges of "instanceAdminPermissionGroupId", "TENANT_ADMIN", or "instance admin" remain:


🏁 Script executed:

#!/bin/bash
# Description: Locate references to old instance/tenant admin roles in the codebase
rg -A 3 -i "instanceAdminPermissionGroupId|TENANT_ADMIN|instanceadmin" 

Length of output: 50719


Action Required: Align All Legacy Admin References with the New Organization-Centric Approach

The updated criteria in CacheableRepositoryHelperCEImpl.java is correctly streamlined for organization-based roles. However, the search results show that legacy instance/tenant admin references (e.g., "instanceAdminPermissionGroupId", TENANT_ADMIN, and "instance admin") still appear in several parts of the codebase. Please review and determine if these references:

  • Are intentionally retained (for migration, backward compatibility, or transitional logic), or
  • Require further refactoring (in controllers, migration scripts, helper classes, and tests) to fully conform to the new admin concept.

Files to check:

  • app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/InMemoryCacheableRepositoryHelper.java
  • Migration files (e.g., Migration066_RenameInstanceAdminRoleToOrgAdmin.java, Migration10000_UpdateSuperUser.java)
  • Relevant controllers and service files (e.g., InstanceAdminController, OrganizationController)
  • Test files under the src/test/java paths related to user and permission group management

Please update or remove outdated references where appropriate to avoid potential inconsistencies with the organization-centric role logic.

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);
Query query = new Query();
query.addCriteria(andCriteria);

return query;
});
// Since we are only interested in the permission group ids, we can project only the id field.
query.fields().include(PermissionGroup.Fields.id);

return createQueryMono
.map(query -> mongoOperations.find(query, PermissionGroup.class))
.flatMapMany(obj -> obj)
return mongoOperations
.find(query, PermissionGroup.class)
.map(permissionGroup -> permissionGroup.getId())
.collect(Collectors.toSet());
}
Expand Down Expand Up @@ -152,25 +144,6 @@ public Mono<String> getDefaultOrganizationId() {
});
}

@Override
public Mono<String> getInstanceAdminPermissionGroupId() {
String instanceAdminPermissionGroupId = inMemoryCacheableRepositoryHelper.getInstanceAdminPermissionGroupId();
if (instanceAdminPermissionGroupId != null && !instanceAdminPermissionGroupId.isEmpty()) {
return Mono.just(instanceAdminPermissionGroupId);
}

BridgeQuery<Config> 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.
Expand Down
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
Loading