cleaning code

Author: bernardodemarco

api/src/main/java/org/apache/cloudstack/api/command/admin/user/DeleteUserKeysCmd.java

@@ -31,7 +31,6 @@
 @APICommand(name = "deleteUserKeys", description = "Deletes a keypair from a user", responseObject = SuccessResponse.class,
         since = "4.23.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
 public class DeleteUserKeysCmd extends BaseAsyncCmd {
-
     @ACL
     @Parameter(name = ApiConstants.KEYPAIR_ID, type = CommandType.UUID, entityType = ApiKeyPairResponse.class, required = true, description = "ID of the keypair to be deleted.")
     private Long id;

api/src/main/java/org/apache/cloudstack/api/command/admin/user/ListUserKeyRulesCmd.java

@@ -36,16 +36,14 @@
 import java.util.List;
 
 @APICommand(name = "listUserKeyRules",
-            description = "This command allows the user to query the rules defined for a API access keypair.",
+            description = "Lists the rules defined for a API key pair.",
             responseObject = BaseRolePermissionResponse.class,
             requestHasSensitiveInfo = false,
             responseHasSensitiveInfo = false,
             since = "4.23.0")
-
 public class ListUserKeyRulesCmd extends BaseListDomainResourcesCmd {
-
     @ACL
-    @Parameter(name=ApiConstants.KEYPAIR_ID, type = CommandType.UUID,  entityType = ApiKeyPairResponse.class, description = "ID of the keypair.", required = true)
+    @Parameter(name = ApiConstants.KEYPAIR_ID, type = CommandType.UUID,  entityType = ApiKeyPairResponse.class, description = "ID of the key pair.", required = true)
     private Long id;
 
     public Long getId() {

api/src/main/java/org/apache/cloudstack/api/command/admin/user/ListUserKeysCmd.java

@@ -38,19 +38,17 @@
         responseHasSensitiveInfo = true,
         authorized = {RoleType.User, RoleType.Admin, RoleType.DomainAdmin, RoleType.ResourceAdmin},
         since = "4.23.0")
-
 public class ListUserKeysCmd extends BaseListDomainResourcesCmd {
-
     @ACL
     @Parameter(name = ApiConstants.USER_ID, type = CommandType.UUID, entityType = UserResponse.class, description = "ID of the user that owns the keys.")
     private Long userId;
 
     @ACL
-    @Parameter(name = ApiConstants.KEYPAIR_ID, type = CommandType.UUID, entityType = ApiKeyPairResponse.class, description = "ID of the keypair.")
+    @Parameter(name = ApiConstants.KEYPAIR_ID, type = CommandType.UUID, entityType = ApiKeyPairResponse.class, description = "ID of the key pair.")
     private Long keyPairId;
 
     @ACL
-    @Parameter(name = ApiConstants.API_KEY_FILTER, type = CommandType.STRING, description = "API Key of the keypair.")
+    @Parameter(name = ApiConstants.API_KEY_FILTER, type = CommandType.STRING, description = "API key of the key pair.")
     private String apiKeyFilter;
 
     @Parameter(name = ApiConstants.SHOW_PERMISSIONS, type = CommandType.BOOLEAN, description = "Whether API Key rules should be returned.")
@@ -67,6 +65,7 @@ public Long getKeyId() {
     public String getApiKeyFilter() {
         return apiKeyFilter;
     }
+
     public Boolean getShowPermissions() {
         return showPermissions;
     }

api/src/main/java/org/apache/cloudstack/api/command/admin/user/RegisterUserKeysCmd.java

@@ -17,6 +17,7 @@
 package org.apache.cloudstack.api.command.admin.user;
 
 import com.cloud.event.EventTypes;
+import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.user.Account;
 import com.cloud.user.User;
 import org.apache.cloudstack.acl.Rule;
@@ -41,29 +42,31 @@
 
 @APICommand(name = "registerUserKeys",
             responseObject = ApiKeyPairResponse.class,
-            description = "This command allows a user to register for the developer API, returning a secret key and an API key. This request is made through the integration API port, so it is a privileged command and must be made on behalf of a user. It is up to the implementer just how the username and password are entered, and then how that translates to an integration API request. Both secret key and API key should be returned to the user",
+            description = "Registers an API key pair (API and secret keys) for a user.",
             requestHasSensitiveInfo = false, responseHasSensitiveInfo = true)
 public class RegisterUserKeysCmd extends BaseAsyncCmd {
-
-    @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = UserResponse.class, required = true, description = "User ID.")
+    @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = UserResponse.class, required = true, description = "ID of the user.")
     private Long id;
 
-    @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, description = "API keypair name.")
+    @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, description = "API key pair name.")
     private String name;
 
-    @Parameter(name = ApiConstants.DESCRIPTION, type = CommandType.STRING, description = "API keypair description.")
+    @Parameter(name = ApiConstants.DESCRIPTION, type = CommandType.STRING, description = "API key pair description.")
     private String description;
 
-    @Parameter(name = ApiConstants.START_DATE, type = CommandType.DATE, description = "Start date of the API keypair. " +
+    @Parameter(name = ApiConstants.START_DATE, type = CommandType.DATE, description = "Start date of the API key pair. " +
             ApiConstants.PARAMETER_DESCRIPTION_START_DATE_POSSIBLE_FORMATS)
     private Date startDate;
 
-    @Parameter(name = ApiConstants.END_DATE, type = CommandType.DATE, description = "Expiration date of the API keypair. " +
+    @Parameter(name = ApiConstants.END_DATE, type = CommandType.DATE, description = "Expiration date of the API key pair. " +
             ApiConstants.PARAMETER_DESCRIPTION_END_DATE_POSSIBLE_FORMATS)
     private Date endDate;
 
-    @Parameter(name = ApiConstants.RULES, type = CommandType.MAP, description = "Rules param list, lower indexed rules take precedence over higher. If no rules are informed, " +
-            "defaults to allowing all account permissions. Example input: rules[0].rule=* rules[0].permission=allow")
+    @Parameter(name = ApiConstants.RULES, type = CommandType.MAP, description = "The rules of the API key pair. If no rules are informed, " +
+            "defaults to allowing all account permissions. Otherwise, only the explicitly informed permissions for the key pair will be " +
+            "considered. Lower indexed rules take precedence over higher. Thus, in the following example: " +
+            "\"rules[0].rule=deleteUserKeys rules[0].permission=deny rules[1].rule=*UserKeys* rules[1].permission=allow\", all rules matching " +
+            "the expression \"*UserKeys*\" will be allowed, except for \"deleteUserKeys\".")
     private Map rules;
 
     public void setUserId(Long userId) {
@@ -126,8 +129,8 @@ public List<Map<String, Object>> getRules() {
 
             String permission = detail.get(ApiConstants.PERMISSION);
             if (StringUtils.isEmpty(permission)) {
-                throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Rule %s has no permission associated with it," +
-                        " please specify if it is either allow or deny.", rule));
+                throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Rule [%s] has no permission associated with it," +
+                        " please specify if it is either [allow] or [deny].", rule));
             }
             ruleDetails.put(ApiConstants.PERMISSION, roleService.getRolePermission(permission));
 

api/src/main/java/org/apache/cloudstack/api/command/admin/user/UpdateUserCmd.java

@@ -46,7 +46,7 @@ public class UpdateUserCmd extends BaseCmd {
     //////////////// API parameters /////////////////////
     /////////////////////////////////////////////////////
 
-    @Parameter(name = ApiConstants.USER_API_KEY, type = CommandType.STRING, description = "The API key for the user. Must be specified with usersecretkey")
+    @Parameter(name = ApiConstants.USER_API_KEY, type = CommandType.STRING, description = "Updates the latest API key of the user. Must be specified with usersecretkey")
     private String userApiKey;
 
     @Parameter(name = ApiConstants.EMAIL, type = CommandType.STRING, description = "Email")
@@ -70,7 +70,7 @@ public class UpdateUserCmd extends BaseCmd {
     @Parameter(name = ApiConstants.CURRENT_PASSWORD, type = CommandType.STRING, description = "Current password that was being used by the user. You must inform the current password when updating the password.", acceptedOnAdminPort = false)
     private String currentPassword;
 
-    @Parameter(name = ApiConstants.USER_SECRET_KEY, type = CommandType.STRING, description = "The secret key for the user. Must be specified with userapikey.")
+    @Parameter(name = ApiConstants.USER_SECRET_KEY, type = CommandType.STRING, description = "Updates the latest secret key of the user. Must be specified with userapikey.")
     private String userSecretKey;
 
     @Parameter(name = ApiConstants.API_KEY_ACCESS, type = CommandType.STRING, description = "Determines if Api key access for this user is enabled, disabled or inherits the value from its parent, the owning account", since = "4.20.1.0", authorized = {RoleType.Admin})

engine/schema/src/main/java/org/apache/cloudstack/acl/dao/ApiKeyPairDaoImpl.java

@@ -81,7 +81,7 @@ public Pair<List<ApiKeyPairVO>, Integer> listByUserIdsPaginated(List<Long> userI
 
         Pair<List<ApiKeyPairVO>, Integer> apiKeyPairVOList = searchAndCount(sc, searchFilter);
         if (CollectionUtils.isEmpty(apiKeyPairVOList.first())) {
-            return new Pair(List.of(), 0);
+            return new Pair<>(List.of(), 0);
         }
         return apiKeyPairVOList;
     }

server/src/main/java/com/cloud/api/query/QueryManagerImpl.java

@@ -861,7 +861,6 @@ public List<Long> searchForAccessibleUsers() {
             try {
                 accountMgr.checkCallerRoleTypeAllowedForUserOrAccountOperations(accountVO, userVO);
             } catch (PermissionDeniedException exception) {
-                logger.debug(exception.getMessage());
                 return false;
             }
             return allowedRolesId.contains(userAccount.getAccountRoleId());

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

@@ -3170,7 +3170,6 @@ public Pair<Boolean, Map<String, String>> getKeys(GetUserKeysCmd cmd) {
         verifyCallerPrivilegeForUserOrAccountOperations(user);
 
         String accessingApiKey = getAccessingApiKey(cmd);
-
         ApiKeyPair keyPair;
         if (accessingApiKey != null) {
             ApiKeyPair accessingKeyPair = apiKeyPairService.findByApiKey(accessingApiKey);
@@ -3241,7 +3240,7 @@ private Integer fetchMultipleKeyPairs(List<ApiKeyPairResponse> responses, ListUs
             users = cmd.listAll() && isAdmin(callerUser.getAccountId()) ? queryService.searchForAccessibleUsers() : List.of(callerUser.getId());
         }
 
-        Pair<List<ApiKeyPairVO>,  Integer> keyPairs = apiKeyPairDao.listByUserIdsPaginated(users, cmd);
+        Pair<List<ApiKeyPairVO>, Integer> keyPairs = apiKeyPairDao.listByUserIdsPaginated(users, cmd);
         keyPairs.first().stream()
                 .filter(keyPair -> isAccessingKeypairSuperset(keyPair, cmd))
                 .forEach(keyPair -> {
@@ -3273,7 +3272,7 @@ private void validateKeyPairIsNotNull(ApiKeyPair keyPair) {
 
     private void validateAccessingKeyPairPermissionsIsSupersetOfAccessedKeyPair(ApiKeyPair keyPair, BaseCmd cmd) {
         if (!isAccessingKeypairSuperset(keyPair, cmd)) {
-            logger.info("Accessing API keypair has less permissions than accessed API keypair.");
+            logger.info("Accessing API key pair [{}] has less permissions than accessed API key pair.", keyPair.getId());
             throw new PermissionDeniedException("Could not complete request.");
         }
     }
@@ -3345,7 +3344,7 @@ public void validateCallingUserHasAccessToDesiredUser(Long userId) {
     public void deleteApiKey(ApiKeyPair keyPair) {
         User user = _userDao.findByIdIncludingRemoved(keyPair.getUserId());
         if (user == null) {
-            throw new InvalidParameterValueException("User associated to the key does not exist.");
+            throw new InvalidParameterValueException("User associated with the API key pair does not exist.");
         }
 
         if ((BaremetalUtils.BAREMETAL_SYSTEM_ACCOUNT_NAME.equals(user.getUsername()) || user.getId() == User.UID_SYSTEM)
@@ -3501,15 +3500,14 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
         return keys;
     }
 
-    /***
-     * Validates if the Keypair has at least one rule, then gets all account role permissions and calls a method that
-     * validates if the user permissions are a superset of permissions of the Keypair that is being created
-     * @param account is the user's account, from which the default permissions are pulled.
-     * @param newApiKeyPair is the new keypair being created
-     * @param rules are the rules passed to the API which are being validated, if no rules were passed, defaults to all
-     *              account permissions
-     * @throws InvalidParameterValueException if the user's permissions are not a superset of the Keypair, or there are
-     * no rules associated with the Keypair
+    /**
+     * Persists the API key pair and its corresponding permissions. Verifies whether
+     * the key pair being created is a superset of its owner's permissions.
+     * @param account Account owner of the key pair.
+     * @param newApiKeyPair The key pair object to be persisted.
+     * @param rules The set of rules of the key pair.
+     * @param cmd The API's command.
+     * @return The persisted key pair object.
      */
     @DB
     private ApiKeyPairVO validateAndPersistKeyPairAndPermissions(Account account, ApiKeyPairVO newApiKeyPair,
@@ -3528,8 +3526,8 @@ private ApiKeyPairVO validateAndPersistKeyPairAndPermissions(Account account, Ap
         }
 
         if (!isApiKeySupersetOfPermission(allPermissions, permissions)) {
-            throw new InvalidParameterValueException(String.format("The keypair being created has a bigger set of permissions than the account [%s] that owns it. This is " +
-                    "not allowed.", account.getUuid()));
+            throw new InvalidParameterValueException(String.format("The key pair being created has a bigger set of permissions than the account [%s] " +
+                    "that owns it. This is not allowed.", account.getUuid()));
         }
 
         ApiKeyPairVO savedApiKeyPair = apiKeyPairDao.persist(newApiKeyPair);
@@ -3541,18 +3539,15 @@ private ApiKeyPairVO validateAndPersistKeyPairAndPermissions(Account account, Ap
         return savedApiKeyPair;
     }
 
-    /**
-     * Gets all API keypair permissions for the given apiKey
-     */
     @Override
     public List<RolePermissionEntity> getAllKeypairPermissions(String apiKey) {
         if (apiKey == null) {
-            throw new InvalidParameterValueException("API key not present in URL, cannot fetch API key rules.");
+            throw new InvalidParameterValueException("API key not present in the request's URL and, thus, unable to fetch API key rules.");
         }
         ApiKeyPair apiKeyPair = keyPairManager.findByApiKey(apiKey);
         Account account = _accountDao.findById(apiKeyPair.getAccountId());
-        List<ApiKeyPairPermission> allApiKeyRolePermissions = keyPairManager.findAllPermissionsByKeyPairId(apiKeyPair.getId(), account.getRoleId());
-        return new ArrayList<>(allApiKeyRolePermissions);
+        List<ApiKeyPairPermission> keyPairPermissions = keyPairManager.findAllPermissionsByKeyPairId(apiKeyPair.getId(), account.getRoleId());
+        return new ArrayList<>(keyPairPermissions);
     }
 
     private String createUserApiKey(long userId, ApiKeyPairVO newApiKeyPair) {