Skip to content

Commit e5848ac

Browse files
weizhouapacheweizhouapache
committed
server: optimize account creation by pre-loading the role permissions
1 parent 1a251c8 commit e5848ac

File tree

6 files changed

+81
-17
lines changed

6 files changed

+81
-17
lines changed

‎api/src/main/java/org/apache/cloudstack/acl/APIChecker.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.cloud.exception.PermissionDeniedException;
2020
import com.cloud.user.Account;
2121
import com.cloud.user.User;
22+
import com.cloud.utils.Pair;
2223
import com.cloud.utils.component.Adapter;
2324

2425
import java.util.List;
@@ -43,4 +44,7 @@ public interface APIChecker extends Adapter {
4344
*/
4445
List<String> getApisAllowedToUser(Role role, User user, List<String> apiNames) throws PermissionDeniedException;
4546
boolean isEnabled();
47+
48+
Pair<Role, List<RolePermission>> getRolePermissions(long roleId);
49+
boolean checkAccess(Account account, String commandName, Role accountRole, List<RolePermission> allPermissions);
4650
}

‎plugins/acl/dynamic-role-based/src/main/java/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,8 @@ protected Account getAccountFromId(long accountId) {
107107
return accountService.getAccount(accountId);
108108
}
109109

110-
protected Pair<Role, List<RolePermission>> getRolePermissions(long roleId) {
110+
@Override
111+
public Pair<Role, List<RolePermission>> getRolePermissions(long roleId) {
111112
final Role accountRole = roleService.findRole(roleId);
112113
if (accountRole == null || accountRole.getId() < 1L) {
113114
return new Pair<>(null, null);
@@ -149,7 +150,7 @@ public boolean checkAccess(User user, String commandName) throws PermissionDenie
149150
throw new PermissionDeniedException(String.format("Account role for user id [%s] cannot be found.", user.getUuid()));
150151
}
151152
if (accountRole.getRoleType() == RoleType.Admin && accountRole.getId() == RoleType.Admin.getId()) {
152-
logger.info("Account for user id {} is Root Admin or Domain Admin, all APIs are allowed.", user.getUuid());
153+
logger.info("Account for user id {} is Root Admin, all APIs are allowed.", user.getUuid());
153154
return true;
154155
}
155156
List<RolePermission> allPermissions = roleAndPermissions.second();
@@ -180,6 +181,25 @@ public boolean checkAccess(Account account, String commandName) {
180181
throw new UnavailableCommandException(String.format("The API [%s] does not exist or is not available for the account %s.", commandName, account));
181182
}
182183

184+
@Override
185+
public boolean checkAccess(Account account, String commandName, Role accountRole, List<RolePermission> allPermissions) {
186+
if (accountRole == null) {
187+
throw new PermissionDeniedException(String.format("The account [%s] has role null or unknown.", account));
188+
}
189+
190+
if (accountRole.getRoleType() == RoleType.Admin && accountRole.getId() == RoleType.Admin.getId()) {
191+
if (logger.isTraceEnabled()) {
192+
logger.trace(String.format("Account [%s] is Root Admin, all APIs are allowed.", account));
193+
}
194+
return true;
195+
}
196+
197+
if (checkApiPermissionByRole(accountRole, commandName, allPermissions)) {
198+
return true;
199+
}
200+
throw new UnavailableCommandException(String.format("The API [%s] does not exist or is not available for the account %s.", commandName, account));
201+
}
202+
183203
/**
184204
* Only one strategy should be used between StaticRoleBasedAPIAccessChecker and DynamicRoleBasedAPIAccessChecker
185205
* Default behavior is to use the Dynamic version. The StaticRoleBasedAPIAccessChecker is the legacy version.

‎plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@
2121

2222
import javax.inject.Inject;
2323
import javax.naming.ConfigurationException;
24-
import org.apache.cloudstack.acl.RolePermissionEntity.Permission;
2524

25+
import org.apache.cloudstack.acl.RolePermissionEntity.Permission;
2626
import org.apache.cloudstack.context.CallContext;
2727

2828
import com.cloud.exception.PermissionDeniedException;
@@ -33,6 +33,7 @@
3333
import com.cloud.user.Account;
3434
import com.cloud.user.AccountService;
3535
import com.cloud.user.User;
36+
import com.cloud.utils.Pair;
3637
import com.cloud.utils.component.AdapterBase;
3738
import com.cloud.utils.component.PluggableService;
3839

@@ -195,4 +196,14 @@ public List<PluggableService> getServices() {
195196
public void setServices(List<PluggableService> services) {
196197
this.services = services;
197198
}
199+
200+
@Override
201+
public Pair<Role, List<RolePermission>> getRolePermissions(long roleId) {
202+
return null;
203+
}
204+
205+
@Override
206+
public boolean checkAccess(Account account, String commandName, Role accountRole, List<RolePermission> allPermissions) {
207+
return false;
208+
}
198209
}

‎plugins/acl/static-role-based/src/main/java/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import com.cloud.user.Account;
3434
import com.cloud.user.AccountService;
3535
import com.cloud.user.User;
36+
import com.cloud.utils.Pair;
3637
import com.cloud.utils.PropertiesUtil;
3738
import com.cloud.utils.component.AdapterBase;
3839
import com.cloud.utils.component.PluggableService;
@@ -200,4 +201,13 @@ public void setCommandPropertyFiles(Set<String> commandPropertyFiles) {
200201
this.commandPropertyFiles = commandPropertyFiles;
201202
}
202203

204+
@Override
205+
public Pair<Role, List<RolePermission>> getRolePermissions(long roleId) {
206+
return null;
207+
}
208+
209+
@Override
210+
public boolean checkAccess(Account account, String commandName, Role accountRole, List<RolePermission> allPermissions) {
211+
return false;
212+
}
203213
}

‎plugins/api/rate-limit/src/main/java/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import net.sf.ehcache.CacheManager;
2828

2929
import org.apache.cloudstack.acl.Role;
30+
import org.apache.cloudstack.acl.RolePermission;
3031
import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
3132
import org.springframework.stereotype.Component;
3233

@@ -42,6 +43,7 @@
4243
import com.cloud.user.Account;
4344
import com.cloud.user.AccountService;
4445
import com.cloud.user.User;
46+
import com.cloud.utils.Pair;
4547
import com.cloud.utils.component.AdapterBase;
4648

4749
@Component
@@ -256,4 +258,14 @@ public void setEnabled(boolean enabled) {
256258
this.enabled = enabled;
257259

258260
}
261+
262+
@Override
263+
public Pair<Role, List<RolePermission>> getRolePermissions(long roleId) {
264+
return null;
265+
}
266+
267+
@Override
268+
public boolean checkAccess(Account account, String commandName, Role accountRole, List<RolePermission> allPermissions) {
269+
return false;
270+
}
259271
}

‎server/src/main/java/com/cloud/user/AccountManagerImpl.java

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import org.apache.cloudstack.acl.InfrastructureEntity;
4848
import org.apache.cloudstack.acl.QuerySelector;
4949
import org.apache.cloudstack.acl.Role;
50+
import org.apache.cloudstack.acl.RolePermission;
5051
import org.apache.cloudstack.acl.RoleService;
5152
import org.apache.cloudstack.acl.RoleType;
5253
import org.apache.cloudstack.acl.SecurityChecker;
@@ -1431,29 +1432,35 @@ protected void checkRoleEscalation(Account caller, Account requested) {
14311432
requested.getUuid(),
14321433
requested.getRoleId()));
14331434
}
1435+
if (caller.getRoleId().equals(requested.getRoleId())) {
1436+
return;
1437+
}
14341438
List<APIChecker> apiCheckers = getEnabledApiCheckers();
1439+
for (APIChecker apiChecker : apiCheckers) {
1440+
checkApiAccess(apiChecker, caller, requested);
1441+
}
1442+
}
1443+
1444+
private void checkApiAccess(APIChecker apiChecker, Account caller, Account requested) throws PermissionDeniedException {
1445+
Pair<Role, List<RolePermission>> roleAndPermissionsForCaller = apiChecker.getRolePermissions(caller.getRoleId());
1446+
Pair<Role, List<RolePermission>> roleAndPermissionsForRequested = apiChecker.getRolePermissions(requested.getRoleId());
14351447
for (String command : apiNameList) {
14361448
try {
1437-
checkApiAccess(apiCheckers, requested, command);
1438-
} catch (PermissionDeniedException pde) {
1439-
if (logger.isTraceEnabled()) {
1440-
logger.trace(String.format(
1441-
"Checking for permission to \"%s\" is irrelevant as it is not requested for %s [%s]",
1442-
command,
1443-
requested.getAccountName(),
1444-
requested.getUuid()
1445-
)
1446-
);
1449+
if (roleAndPermissionsForRequested == null) {
1450+
apiChecker.checkAccess(caller, command);
1451+
} else {
1452+
apiChecker.checkAccess(caller, command, roleAndPermissionsForRequested.first(), roleAndPermissionsForRequested.second());
14471453
}
1454+
} catch (PermissionDeniedException pde) {
14481455
continue;
14491456
}
14501457
// so requested can, now make sure caller can as well
14511458
try {
1452-
if (logger.isTraceEnabled()) {
1453-
logger.trace(String.format("permission to \"%s\" is requested",
1454-
command));
1459+
if (roleAndPermissionsForCaller == null) {
1460+
apiChecker.checkAccess(caller, command);
1461+
} else {
1462+
apiChecker.checkAccess(caller, command, roleAndPermissionsForCaller.first(), roleAndPermissionsForCaller.second());
14551463
}
1456-
checkApiAccess(apiCheckers, caller, command);
14571464
} catch (PermissionDeniedException pde) {
14581465
String msg = String.format("User of Account %s and domain %s can not create an account with access to more privileges they have themself.",
14591466
caller, _domainMgr.getDomain(caller.getDomainId()));

0 commit comments

Comments
 (0)