Skip to content

When creating a new account (via domain admin) it is possible to select “root admin” as the role for the new user #2606

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.

Sign up for GitHub

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

Merged
DaanHoogland merged 5 commits into from
May 2, 2018
Merged

When creating a new account (via domain admin) it is possible to select “root admin” as the role for the new user #2606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3a34e2d
create account with domain admin showing 'root admin' role
rafaelweingartner Apr 25, 2018
64693d3
License and formatting
rafaelweingartner Apr 25, 2018
34b2c2a
Break long sentence into multiple lines
rafaelweingartner Apr 26, 2018
27374a3
Fix wording of method 'getCurrentAccount'
rafaelweingartner Apr 26, 2018
e25884f
fix typo in variable name
rafaelweingartner Apr 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 43 additions & 17 deletions api/src/main/java/org/apache/cloudstack/acl/RoleService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,38 +17,64 @@

package org.apache.cloudstack.acl;

import java.util.List;

import org.apache.cloudstack.acl.RolePermission.Permission;
import org.apache.cloudstack.framework.config.ConfigKey;

import java.util.List;

public interface RoleService {

ConfigKey<Boolean> EnableDynamicApiChecker = new ConfigKey<>("Advanced", Boolean.class, "dynamic.apichecker.enabled", "false",
"If set to true, this enables the dynamic role-based api access checker and disables the default static role-based api access checker.",
true);
"If set to true, this enables the dynamic role-based api access checker and disables the default static role-based api access checker.", true);

boolean isEnabled();
Role findRole(final Long id);
Role createRole(final String name, final RoleType roleType, final String description);
Role updateRole(final Role role, final String name, final RoleType roleType, final String description);
boolean deleteRole(final Role role);

RolePermission findRolePermission(final Long id);
RolePermission findRolePermissionByUuid(final String uuid);
/**
* Searches for a role with the given ID. If the ID is null or less than zero, this method will return null.
* This method will also return null if no role is found with the provided ID.
* Moreover, we will check if the requested role is of 'Admin' type; roles with 'Admin' type should only be visible to 'root admins'.
* Therefore, if a non-'root admin' user tries to search for an 'Admin' role, this method will return null.
*/
Role findRole(Long id);

Role createRole(String name, RoleType roleType, String description);

Role updateRole(Role role, String name, RoleType roleType, String description);

boolean deleteRole(Role role);

RolePermission findRolePermission(Long id);

RolePermission findRolePermissionByUuid(String uuid);

RolePermission createRolePermission(Role role, Rule rule, Permission permission, String description);

RolePermission createRolePermission(final Role role, final Rule rule, final Permission permission, final String description);
/**
* updateRolePermission updates the order/position of an role permission
* @param role The role whose permissions needs to be re-ordered
* @param newOrder The new list of ordered role permissions
*/
boolean updateRolePermission(final Role role, final List<RolePermission> newOrder);
boolean updateRolePermission(final Role role, final RolePermission rolePermission, final Permission permission);
boolean deleteRolePermission(final RolePermission rolePermission);
boolean updateRolePermission(Role role, List<RolePermission> newOrder);

boolean updateRolePermission(Role role, RolePermission rolePermission, Permission permission);

boolean deleteRolePermission(RolePermission rolePermission);

/**
* List all roles configured in the database. Roles that have the type {@link RoleType#Admin} will not be shown for users that are not 'root admin'.
*/
List<Role> listRoles();
List<Role> findRolesByName(final String name);
List<Role> findRolesByType(final RoleType roleType);
List<RolePermission> findAllPermissionsBy(final Long roleId);

/**
* Find all roles that have the giving {@link String} as part of their name.
* If the user calling the method is not a 'root admin', roles of type {@link RoleType#Admin} wil lbe removed of the returned list.
*/
List<Role> findRolesByName(String name);

/**
* Find all roles by {@link RoleType}. If the role type is {@link RoleType#Admin}, the calling account must be a root admin, otherwise we return an empty list.
*/
List<Role> findRolesByType(RoleType roleType);

List<RolePermission> findAllPermissionsBy(Long roleId);
}
32 changes: 13 additions & 19 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/acl/ListRolesCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,31 +17,25 @@

package org.apache.cloudstack.api.command.admin.acl;

import com.cloud.exception.ConcurrentOperationException;
import com.cloud.exception.InsufficientCapacityException;
import com.cloud.exception.NetworkRuleConflictException;
import com.cloud.exception.ResourceAllocationException;
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.user.Account;
import com.google.common.base.Strings;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import org.apache.cloudstack.acl.Role;
import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.api.APICommand;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.api.BaseCmd;
import org.apache.cloudstack.api.Parameter;
import org.apache.cloudstack.api.ServerApiException;
import org.apache.cloudstack.api.response.ListResponse;
import org.apache.cloudstack.api.response.RoleResponse;
import org.apache.commons.lang3.StringUtils;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import com.cloud.user.Account;
import com.google.common.base.Strings;

@APICommand(name = ListRolesCmd.APINAME, description = "Lists dynamic roles in CloudStack", responseObject = RoleResponse.class,
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
since = "4.9.0",
authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin})
@APICommand(name = ListRolesCmd.APINAME, description = "Lists dynamic roles in CloudStack", responseObject = RoleResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.9.0", authorized = {
RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin})
public class ListRolesCmd extends BaseCmd {
public static final String APINAME = "listRoles";

Expand Down Expand Up @@ -112,13 +106,13 @@ private void setupResponse(final List<Role> roles) {
}

@Override
public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException {
final List<Role> roles;
public void execute() {
List<Role> roles;
if (getId() != null && getId() > 0L) {
roles = Collections.singletonList(roleService.findRole(getId()));
} else if (!Strings.isNullOrEmpty(getName())) {
} else if (StringUtils.isNotBlank(getName())) {
roles = roleService.findRolesByName(getName());
} else if (getRoleType() != null){
} else if (getRoleType() != null) {
roles = roleService.findRolesByType(getRoleType());
} else {
roles = roleService.listRoles();
Expand Down
83 changes: 71 additions & 12 deletions server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;

import javax.inject.Inject;
Expand All @@ -37,11 +38,15 @@
import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.framework.config.ConfigKey;
import org.apache.cloudstack.framework.config.Configurable;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.log4j.Logger;

import com.cloud.event.ActionEvent;
import com.cloud.event.EventTypes;
import com.cloud.exception.PermissionDeniedException;
import com.cloud.user.Account;
import com.cloud.user.AccountManager;
import com.cloud.user.dao.AccountDao;
import com.cloud.utils.ListUtils;
import com.cloud.utils.component.ManagerBase;
Expand All @@ -52,18 +57,23 @@
import com.google.common.base.Strings;

public class RoleManagerImpl extends ManagerBase implements RoleService, Configurable, PluggableService {

private Logger logger = Logger.getLogger(getClass());

@Inject
private AccountDao accountDao;
@Inject
private RoleDao roleDao;
@Inject
private RolePermissionsDao rolePermissionsDao;
@Inject
private AccountManager accountManager;

private void checkCallerAccess() {
if (!isEnabled()) {
throw new PermissionDeniedException("Dynamic api checker is not enabled, aborting role operation");
}
Account caller = CallContext.current().getCallingAccount();
Account caller = getCurrentAccount();
if (caller == null || caller.getRoleId() == null) {
throw new PermissionDeniedException("Restricted API called by an invalid user account");
}
Expand All @@ -79,11 +89,30 @@ public boolean isEnabled() {
}

@Override
public Role findRole(final Long id) {
public Role findRole(Long id) {
if (id == null || id < 1L) {
logger.trace(String.format("Role ID is invalid [%s]", id));
return null;
}
RoleVO role = roleDao.findById(id);
if (role == null) {
logger.trace(String.format("Role not found [id=%s]", id));
return null;
}
return roleDao.findById(id);
Account account = getCurrentAccount();
if (!accountManager.isRootAdmin(account.getId()) && RoleType.Admin == role.getRoleType()) {
logger.debug(String.format("Role [id=%s, name=%s] is of 'Admin' type and is only visible to 'Root admins'.", id, role.getName()));
return null;
}
return role;
}

/**
* Simple call to {@link CallContext#current()} to retrieve the current calling account.
* This method facilitates unit testing, it avoids mocking static methods.
*/
protected Account getCurrentAccount() {
return CallContext.current().getCallingAccount();
}

@Override
Expand Down Expand Up @@ -125,7 +154,7 @@ public Role updateRole(final Role role, final String name, final RoleType roleTy
if (roleType != null && roleType == RoleType.Unknown) {
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Unknown is not a valid role type");
}
RoleVO roleVO = (RoleVO) role;
RoleVO roleVO = (RoleVO)role;
if (!Strings.isNullOrEmpty(name)) {
roleVO.setName(name);
}
Expand Down Expand Up @@ -214,26 +243,56 @@ public boolean deleteRolePermission(final RolePermission rolePermission) {
}

@Override
public List<Role> findRolesByName(final String name) {
public List<Role> findRolesByName(String name) {
List<? extends Role> roles = null;
if (!Strings.isNullOrEmpty(name)) {
if (StringUtils.isNotBlank(name)) {
roles = roleDao.findAllByName(name);
}
removeRootAdminRolesIfNeeded(roles);
return ListUtils.toListOfInterface(roles);
}

/**
* Removes roles of the given list that have the type '{@link RoleType#Admin}' if the user calling the method is not a 'root admin'.
* The actual removal is executed via {@link #removeRootAdminRoles(List)}. Therefore, if the method is called by a 'root admin', we do nothing here.
*/
protected void removeRootAdminRolesIfNeeded(List<? extends Role> roles) {
Account account = getCurrentAccount();
if (!accountManager.isRootAdmin(account.getId())) {
removeRootAdminRoles(roles);
}
}

/**
* Remove all roles that have the {@link RoleType#Admin}.
*/
protected void removeRootAdminRoles(List<? extends Role> roles) {
if (CollectionUtils.isEmpty(roles)) {
return;
}
Iterator<? extends Role> rolesIterator = roles.iterator();
while (rolesIterator.hasNext()) {
Role role = rolesIterator.next();
if (RoleType.Admin == role.getRoleType()) {
rolesIterator.remove();
}
}

}

@Override
public List<Role> findRolesByType(final RoleType roleType) {
List<? extends Role> roles = null;
if (roleType != null) {
roles = roleDao.findAllByRoleType(roleType);
public List<Role> findRolesByType(RoleType roleType) {
if (roleType == null || RoleType.Admin == roleType && !accountManager.isRootAdmin(getCurrentAccount().getId())) {
return Collections.emptyList();
}
List<? extends Role> roles = roleDao.findAllByRoleType(roleType);
return ListUtils.toListOfInterface(roles);
}

@Override
public List<Role> listRoles() {
List<? extends Role> roles = roleDao.listAll();
removeRootAdminRolesIfNeeded(roles);
return ListUtils.toListOfInterface(roles);
}

Expand All @@ -253,7 +312,7 @@ public String getConfigComponentName() {

@Override
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey<?>[]{RoleService.EnableDynamicApiChecker};
return new ConfigKey<?>[] {RoleService.EnableDynamicApiChecker};
}

@Override
Expand All @@ -269,4 +328,4 @@ public List<Class<?>> getCommands() {
cmdList.add(DeleteRolePermissionCmd.class);
return cmdList;
}
}
}
Loading