Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1489#discussion_r60743595
--- Diff: server/src/org/apache/cloudstack/acl/RoleManagerImpl.java ---
@@ -0,0 +1,273 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.acl;
+
+import com.cloud.event.ActionEvent;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.PermissionDeniedException;
+import com.cloud.user.Account;
+import com.cloud.user.dao.AccountDao;
+import com.cloud.utils.PropertiesUtil;
+import com.cloud.utils.component.ManagerBase;
+import com.cloud.utils.component.PluggableService;
+import com.cloud.utils.db.Transaction;
+import com.cloud.utils.db.TransactionCallback;
+import com.cloud.utils.db.TransactionStatus;
+import com.google.common.base.Strings;
+import org.apache.cloudstack.acl.dao.RoleDao;
+import org.apache.cloudstack.acl.dao.RolePermissionsDao;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.admin.acl.CreateRoleCmd;
+import org.apache.cloudstack.api.command.admin.acl.CreateRolePermissionCmd;
+import org.apache.cloudstack.api.command.admin.acl.DeleteRoleCmd;
+import org.apache.cloudstack.api.command.admin.acl.DeleteRolePermissionCmd;
+import org.apache.cloudstack.api.command.admin.acl.ListRolePermissionsCmd;
+import org.apache.cloudstack.api.command.admin.acl.ListRolesCmd;
+import org.apache.cloudstack.api.command.admin.acl.UpdateRoleCmd;
+import org.apache.cloudstack.api.command.admin.acl.UpdateRolePermissionCmd;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+
+import javax.ejb.Local;
+import javax.inject.Inject;
+import java.io.File;
+import java.util.ArrayList;
+import java.util.List;
+
+@Local(value = {RoleService.class})
+public class RoleManagerImpl extends ManagerBase implements RoleService,
Configurable, PluggableService {
+ @Inject
+ private AccountDao accountDao;
+ @Inject
+ private RoleDao roleDao;
+ @Inject
+ private RolePermissionsDao rolePermissionsDao;
+
+ private void checkCallerAccess() {
+ if (!isEnabled()) {
+ throw new PermissionDeniedException("Dynamic api checker is
not enabled, aborting role operation");
+ }
+ Account caller = CallContext.current().getCallingAccount();
+ if (caller == null || caller.getRoleId() == null) {
+ throw new PermissionDeniedException("Restricted API called by
an invalid user account");
+ }
+ Role callerRole = findRole(caller.getRoleId());
+ if (callerRole == null || callerRole.getRoleType() !=
RoleType.Admin) {
+ throw new PermissionDeniedException("Restricted API called by
an user account of non-Admin role type");
+ }
+ }
+
+ @Override
+ public boolean isEnabled() {
+ File apiCmdFile =
PropertiesUtil.findConfigFile(PropertiesUtil.getDefaultApiCommandsFileName());
+ return RoleService.EnableDynamicApiChecker.value() && (apiCmdFile
== null || !apiCmdFile.exists());
+ }
+
+ @Override
+ public Role findRole(final Long id) {
+ if (id == null || id < 1L) {
+ return null;
+ }
+ return roleDao.findById(id);
+ }
+
+ @Override
+ public RolePermission findRolePermission(final Long id) {
+ if (id == null) {
+ return null;
+ }
+ return rolePermissionsDao.findById(id);
+ }
+
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_ROLE_CREATE,
eventDescription = "creating Role")
+ public Role createRole(final String name, final RoleType roleType,
final String description) {
+ checkCallerAccess();
+ if (roleType == null || roleType == RoleType.Unknown) {
+ throw new ServerApiException(ApiErrorCode.PARAM_ERROR,
"Invalid role type provided");
+ }
+ return Transaction.execute(new TransactionCallback<RoleVO>() {
+ @Override
+ public RoleVO doInTransaction(TransactionStatus status) {
+ return roleDao.persist(new RoleVO(name, roleType,
description));
+ }
+ });
+ }
+
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_ROLE_UPDATE,
eventDescription = "updating Role")
+ public boolean updateRole(final Role role, final String name, final
RoleType roleType, final String description) {
+ checkCallerAccess();
+ if (role == null) {
+ return false;
+ }
+ if (roleType != null && roleType == RoleType.Unknown) {
+ throw new ServerApiException(ApiErrorCode.PARAM_ERROR,
"Unknown is not a valid role type");
+ }
+ RoleVO roleVO = (RoleVO) role;
+ if (!Strings.isNullOrEmpty(name)) {
+ roleVO.setName(name);
+ }
+ if (roleType != null) {
+ if (role.getId() <= RoleType.User.getId()) {
+ throw new PermissionDeniedException("The role type of
default roles cannot be changed");
+ }
+ List<? extends Account> accounts =
accountDao.findAccountsByRole(role.getId());
+ if (accounts == null || accounts.isEmpty()) {
+ roleVO.setRoleType(roleType);
+ } else {
+ throw new PermissionDeniedException("Found accounts that
have role in use, won't allow to change role type");
+ }
+ }
+ if (!Strings.isNullOrEmpty(description)) {
+ roleVO.setDescription(description);
+ }
+ return roleDao.update(role.getId(), roleVO);
+ }
+
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_ROLE_DELETE,
eventDescription = "deleting Role")
+ public boolean deleteRole(final Role role) {
+ checkCallerAccess();
+ if (role == null) {
+ return false;
+ }
+ if (role.getId() <= RoleType.User.getId()) {
+ throw new PermissionDeniedException("Default roles cannot be
deleted");
+ }
+ List<? extends Account> accounts =
accountDao.findAccountsByRole(role.getId());
+ if (accounts == null || accounts.size() == 0) {
+ return Transaction.execute(new TransactionCallback<Boolean>() {
+ @Override
+ public Boolean doInTransaction(TransactionStatus status) {
+ List<? extends RolePermission> rolePermissions =
rolePermissionsDao.findAllByRoleId(role.getId());
+ if (rolePermissions != null &&
!rolePermissions.isEmpty()) {
+ for (RolePermission rolePermission :
rolePermissions) {
+
rolePermissionsDao.remove(rolePermission.getId());
+ }
+ }
+ return roleDao.remove(role.getId());
+ }
+ });
+ }
+ throw new PermissionDeniedException("Found accounts that have role
in use, won't allow to delete role");
+ }
+
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_ROLE_PERMISSION_CREATE,
eventDescription = "creating Role Permission")
+ public RolePermission createRolePermission(final Role role, final Rule
rule, final RolePermission.Permission permission, final String description) {
+ checkCallerAccess();
+ return Transaction.execute(new
TransactionCallback<RolePermissionVO>() {
+ @Override
+ public RolePermissionVO doInTransaction(TransactionStatus
status) {
+ return rolePermissionsDao.persist(new
RolePermissionVO(role.getId(), rule.toString(), permission, description));
+ }
+ });
+ }
+
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_ROLE_PERMISSION_UPDATE,
eventDescription = "updating Role Permission")
+ public boolean updateRolePermission(final RolePermission
rolePermission, final Rule rule, final RolePermission.Permission permission,
final String description) {
+ checkCallerAccess();
+ if (rolePermission == null) {
+ return false;
+ }
+ RolePermissionVO rolePermissionVO = (RolePermissionVO)
rolePermission;
+ if (rule != null) {
+ rolePermissionVO.setRule(rule.toString());
+ }
+ if (permission != null) {
+ rolePermissionVO.setPermission(permission);
+ }
+ if (!Strings.isNullOrEmpty(description)) {
+ rolePermissionVO.setDescription(description);
+ }
+ return rolePermissionsDao.update(rolePermission.getId(),
rolePermissionVO);
+ }
+
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_ROLE_PERMISSION_DELETE,
eventDescription = "deleting Role Permission")
+ public boolean deleteRolePermission(final RolePermission
rolePermission) {
+ checkCallerAccess();
+ return rolePermission != null &&
rolePermissionsDao.remove(rolePermission.getId());
+ }
+
+ private List<Role> toRoleList(final List<? extends Role> roles) {
+ if (roles != null) {
+ return new ArrayList<>(roles);
+ }
+ return new ArrayList<>();
--- End diff --
Minor nit: ``Collection.emptyList()`` avoids unnecessary object creation in
the empty case.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
