Fix accessible typo and execute keypair migration through SQL only

Author: Nicole Schmidt

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

@@ -144,8 +144,8 @@ public List<Map<String, Object>> getRules() {
     @Override
     public long getEntityOwnerId() {
         User user = _entityMgr.findById(User.class, getUserId());
-        List<Long> accessableUsers = _queryService.searchForAccessableUsers();
-        if (user != null && accessableUsers.stream().anyMatch(u -> u == user.getId())) {
+        List<Long> accessibleUsers = _queryService.searchForAccessibleUsers();
+        if (user != null && accessibleUsers.stream().anyMatch(u -> u == user.getId())) {
             return user.getAccountId();
         }
         return Account.ACCOUNT_ID_SYSTEM;

api/src/main/java/org/apache/cloudstack/query/QueryService.java

@@ -135,7 +135,7 @@ public interface QueryService {
 
     ListResponse<UserResponse> searchForUsers(Long domainId, boolean recursive) throws PermissionDeniedException;
 
-    List<Long> searchForAccessableUsers();
+    List<Long> searchForAccessibleUsers();
 
     ListResponse<EventResponse> searchForEvents(ListEventsCmd cmd);
 

engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42010to42100.java

@@ -19,12 +19,8 @@
 import com.cloud.upgrade.SystemVmTemplateRegistration;
 import com.cloud.utils.db.TransactionLegacy;
 import com.cloud.utils.exception.CloudRuntimeException;
-import java.sql.Date;
 import java.sql.PreparedStatement;
-import java.sql.ResultSet;
 import java.sql.SQLException;
-import java.time.LocalDate;
-import java.util.UUID;
 import java.io.InputStream;
 import java.sql.Connection;
 import java.util.List;
@@ -60,52 +56,9 @@ public InputStream[] getPrepareScripts() {
         return new InputStream[] {script};
     }
 
-    protected void performKeyPairMigration(Connection conn) throws SQLException {
-        try {
-            logger.debug("Performing keypair migration from user table to api_keypair table.");
-            PreparedStatement pstmt = conn.prepareStatement("SELECT u.id, u.api_key, u.secret_key, a.domain_id, u.id FROM `cloud`.`user` AS u JOIN `cloud`.`account` AS a " +
-                    "ON u.account_id = a.id WHERE u.api_key IS NOT NULL AND u.secret_key IS NOT NULL");
-            ResultSet resultSet = pstmt.executeQuery();
-
-            while (resultSet.next()) {
-                long id = resultSet.getLong(1);
-                String apiKey = resultSet.getString(2);
-                String secretKey = resultSet.getString(3);
-                Long domainId = resultSet.getLong(4);
-                Long accountId = resultSet.getLong(5);
-                Date timestamp = Date.valueOf(LocalDate.now());
-
-                PreparedStatement preparedStatement = conn.prepareStatement("INSERT IGNORE INTO `cloud`.`api_keypair` (uuid, user_id, domain_id, account_id, api_key, secret_key, created, name) VALUES (?, ?, ?, ?, ?, ?, ?, ?)");
-                String uuid = UUID.randomUUID().toString();
-                preparedStatement.setString(1, uuid);
-                preparedStatement.setLong(2, id);
-                preparedStatement.setLong(3, domainId);
-                preparedStatement.setLong(4, accountId);
-
-                preparedStatement.setString(5, apiKey);
-                preparedStatement.setString(6, secretKey);
-                preparedStatement.setDate(7, timestamp);
-                preparedStatement.setString(8, uuid);
-
-                preparedStatement.executeUpdate();
-            }
-            pstmt = conn.prepareStatement("ALTER TABLE `cloud`.`user` DROP COLUMN api_key, DROP COLUMN secret_key;");
-            pstmt.executeUpdate();
-            logger.info("Successfully performed keypair migration.");
-        } catch (SQLException ex) {
-            logger.info("Unexpected exception in user keypair migration", ex);
-            throw ex;
-        }
-    }
-
     @Override
     public void performDataMigration(Connection conn) {
         migrateConfigurationScopeToBitmask(conn);
-        try {
-            performKeyPairMigration(conn);
-        } catch (SQLException e) {
-            throw new RuntimeException(e);
-        }
     }
 
     @Override

engine/schema/src/main/resources/META-INF/db/schema-42010to42100.sql

@@ -71,3 +71,12 @@ CREATE TABLE IF NOT EXISTS `cloud`.`api_keypair_permissions` (
     PRIMARY KEY (`id`),
     CONSTRAINT `fk_keypair_permissions__api_keypair_id` FOREIGN KEY(`api_keypair_id`) REFERENCES `cloud`.`api_keypair`(`id`)
     );
+
+INSERT INTO `cloud`.`api_keypair` (uuid, user_id, domain_id, account_id, api_key, secret_key, created, name)
+SELECT  uuid(), user.id, account.domain_id, account.id, user.api_key, user.secret_key, now(), 'Active key pair'
+FROM    `cloud`.`user` AS user
+JOIN    `cloud`.`account` AS account ON user.account_id = account.id
+WHERE   user.api_key IS NOT NULL
+  AND     user.secret_key IS NOT NULL;
+
+ALTER TABLE `cloud`.`user` DROP COLUMN IF EXISTS api_key, DROP COLUMN IF EXISTS secret_key;

engine/schema/src/test/java/com/cloud/upgrade/dao/Upgrade42010to42100Test.java

@@ -47,7 +47,6 @@ public void testPerformDataMigration() throws SQLException {
             when(dbUpgradeUtils.getTableColumnType(conn, "configuration", "scope")).thenReturn("varchar(255)");
 
             try (MockedStatic<TransactionLegacy> ignored2 = Mockito.mockStatic(TransactionLegacy.class)) {
-                Mockito.doNothing().when(upgrade).performKeyPairMigration(conn);
                 TransactionLegacy txn = Mockito.mock(TransactionLegacy.class);
                 when(TransactionLegacy.currentTxn()).thenReturn(txn);
                 PreparedStatement pstmt = Mockito.mock(PreparedStatement.class);

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

@@ -857,7 +857,7 @@ private Pair<List<UserAccountJoinVO>, Integer> getUserListInternal(Account calle
     }
 
     @Override
-    public List<Long> searchForAccessableUsers() {
+    public List<Long> searchForAccessibleUsers() {
         List<Long> permittedAccounts = new ArrayList<>();
         Account callingAccount = CallContext.current().getCallingAccount();
         Filter searchFilter = new Filter(UserAccountJoinVO.class, "id", true);

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

@@ -3016,7 +3016,7 @@ private Integer fetchMultipleKeypairs(List<ApiKeyPairResponse> responses, ListUs
             users = List.of(cmd.getUserId());
         } else {
             User callerUser = CallContext.current().getCallingUser();
-            users = cmd.listAll() && isAdmin(callerUser.getAccountId()) ? queryService.searchForAccessableUsers() : List.of(callerUser.getId());
+            users = cmd.listAll() && isAdmin(callerUser.getAccountId()) ? queryService.searchForAccessibleUsers() : List.of(callerUser.getId());
         }
 
         Pair<List<ApiKeyPairVO>,  Integer> keyPairs = apiKeyPairDao.listByUserIdsPaginated(users, cmd);
@@ -3119,7 +3119,7 @@ public void validateCallingUserHasAccessToDesiredUser(Long userId) {
         if (!isAdmin(callerUser.getAccountId()) && callerUser.getId() != userId) {
             throw new PermissionDeniedException("Only admins can operate on API keys owned by other users");
         }
-        List<Long> accessibleUsers = queryService.searchForAccessableUsers();
+        List<Long> accessibleUsers = queryService.searchForAccessibleUsers();
         User desiredUser = _userDao.getUser(userId);
         if (accessibleUsers.stream().noneMatch(u -> Objects.equals(u, userId))) {
             throw new PermissionDeniedException(String.format("Could not perform operation because calling user has less permissions " +

server/src/main/java/org/apache/cloudstack/acl/ApiKeyPairManagerImpl.java

@@ -73,9 +73,9 @@ public ApiKeyPair findById(Long id) {
 
     @Override
     public void validateCallingUserHasAccessToDesiredUser(Long userId) {
-        List<Long> accessableUsers = queryService.searchForAccessableUsers();
+        List<Long> accessibleUsers = queryService.searchForAccessibleUsers();
         User desiredUser = userDao.getUser(userId);
-        if (accessableUsers.stream().noneMatch(u -> Objects.equals(u, userId))) {
+        if (accessibleUsers.stream().noneMatch(u -> Objects.equals(u, userId))) {
             throw new InvalidParameterValueException(String.format("Could not perform operation because calling user has less permissions " +
                     "than the informed user [%s].", desiredUser.getId()));
         }