Add SELECT privilege check for MySQL when register or alter storage u…

…nit (#32185) * Add SELECT privilege check when register or alter storage unit * Update error message for select privilege
apache · Jul 19, 2024 · 372efb8 · 372efb8
1 parent 6337b1b
commit 372efb8
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 12 deletions.
diff --git a/...shardingsphere/infra/database/core/exception/CheckDatabaseEnvironmentFailedException.java b/...shardingsphere/infra/database/core/exception/CheckDatabaseEnvironmentFailedException.java
@@ -30,6 +30,6 @@ public final class CheckDatabaseEnvironmentFailedException extends MetaDataSQLEx
     private static final long serialVersionUID = 3913140870320566898L;
 
     public CheckDatabaseEnvironmentFailedException(final SQLException cause) {
-        super(XOpenSQLState.CONNECTION_EXCEPTION, 5, "Check database environment failed.", cause);
+        super(XOpenSQLState.CONNECTION_EXCEPTION, 5, "Check database environment failed", cause);
     }
 }
diff --git a/...pache/shardingsphere/infra/database/core/exception/MissingRequiredPrivilegeException.java b/...pache/shardingsphere/infra/database/core/exception/MissingRequiredPrivilegeException.java
@@ -30,6 +30,6 @@ public final class MissingRequiredPrivilegeException extends MetaDataSQLExceptio
     private static final long serialVersionUID = 3755362278200749857L;
 
     public MissingRequiredPrivilegeException(final Collection<String> privileges) {
-        super(XOpenSQLState.PRIVILEGE_NOT_GRANTED, 6, "Missing required privilege(s) `%s`.", privileges);
+        super(XOpenSQLState.PRIVILEGE_NOT_GRANTED, 6, "Missing required privilege(s) `%s`", privileges);
     }
 }
diff --git a/...org/apache/shardingsphere/infra/database/core/exception/MissingRequiredUserException.java b/...org/apache/shardingsphere/infra/database/core/exception/MissingRequiredUserException.java
@@ -28,6 +28,6 @@ public final class MissingRequiredUserException extends MetaDataSQLException {
     private static final long serialVersionUID = -656859547059598488L;
 
     public MissingRequiredUserException(final String username) {
-        super(XOpenSQLState.NOT_FOUND, 7, "User '%s' does exist.", username);
+        super(XOpenSQLState.NOT_FOUND, 7, "User '%s' does exist", username);
     }
 }
diff --git a/...g/apache/shardingsphere/infra/database/mysql/checker/MySQLDatabaseEnvironmentChecker.java b/...g/apache/shardingsphere/infra/database/mysql/checker/MySQLDatabaseEnvironmentChecker.java
@@ -53,18 +53,15 @@ public final class MySQLDatabaseEnvironmentChecker implements DialectDatabaseEnv
 
     private static final String[][] XA_REQUIRED_PRIVILEGES = {{"ALL PRIVILEGES", "ON *.*"}, {"XA_RECOVER_ADMIN", "ON *.*"}};
 
-    private static final Map<PrivilegeCheckType, String[][]> REQUIRED_PRIVILEGES = new EnumMap<>(PrivilegeCheckType.class);
-
     private static final Map<PrivilegeCheckType, Collection<String>> REQUIRED_PRIVILEGES_FOR_MESSAGE = new EnumMap<>(PrivilegeCheckType.class);
 
     private static final Map<String, String> REQUIRED_VARIABLES = new HashMap<>(3, 1F);
 
     private static final String SHOW_VARIABLES_SQL;
 
     static {
-        REQUIRED_PRIVILEGES.put(PrivilegeCheckType.PIPELINE, PIPELINE_REQUIRED_PRIVILEGES);
         REQUIRED_PRIVILEGES_FOR_MESSAGE.put(PrivilegeCheckType.PIPELINE, Arrays.asList("REPLICATION SLAVE", "REPLICATION CLIENT"));
-        REQUIRED_PRIVILEGES.put(PrivilegeCheckType.XA, XA_REQUIRED_PRIVILEGES);
+        REQUIRED_PRIVILEGES_FOR_MESSAGE.put(PrivilegeCheckType.SELECT, Collections.singleton("SELECT ON DATABASE"));
         REQUIRED_PRIVILEGES_FOR_MESSAGE.put(PrivilegeCheckType.XA, Collections.singleton("XA_RECOVER_ADMIN"));
         REQUIRED_VARIABLES.put("LOG_BIN", "ON");
         REQUIRED_VARIABLES.put("BINLOG_FORMAT", "ROW");
@@ -91,7 +88,7 @@ private void checkPrivilege(final Connection connection, final PrivilegeCheckTyp
                 ResultSet resultSet = preparedStatement.executeQuery()) {
             while (resultSet.next()) {
                 String privilege = resultSet.getString(1).toUpperCase();
-                if (matchPrivileges(privilege, REQUIRED_PRIVILEGES.get(privilegeCheckType))) {
+                if (matchPrivileges(privilege, getRequiredPrivileges(connection, privilegeCheckType))) {
                     return;
                 }
             }
@@ -101,8 +98,25 @@ private void checkPrivilege(final Connection connection, final PrivilegeCheckTyp
         throw new MissingRequiredPrivilegeException(REQUIRED_PRIVILEGES_FOR_MESSAGE.get(privilegeCheckType));
     }
 
-    private boolean matchPrivileges(final String privilege, final String[][] requiredPrivileges) {
-        return Arrays.stream(requiredPrivileges).anyMatch(each -> Arrays.stream(each).allMatch(privilege::contains));
+    private String[][] getRequiredPrivileges(final Connection connection, final PrivilegeCheckType privilegeCheckType) throws SQLException {
+        switch (privilegeCheckType) {
+            case PIPELINE:
+                return PIPELINE_REQUIRED_PRIVILEGES;
+            case SELECT:
+                return getSelectRequiredPrivilege(connection);
+            case XA:
+                return XA_REQUIRED_PRIVILEGES;
+            default:
+                return new String[0][0];
+        }
+    }
+
+    private String[][] getSelectRequiredPrivilege(final Connection connection) throws SQLException {
+        return new String[][]{{"ALL PRIVILEGES", "ON *.*"}, {"SELECT", "ON *.*"}, {"SELECT", String.format("ON `%s`.*", connection.getCatalog()).toUpperCase()}};
+    }
+
+    private boolean matchPrivileges(final String grantedPrivileges, final String[][] requiredPrivileges) {
+        return Arrays.stream(requiredPrivileges).anyMatch(each -> Arrays.stream(each).allMatch(grantedPrivileges::contains));
     }
 
     @Override

diff --git a/...apache/shardingsphere/distsql/handler/executor/rdl/resource/AlterStorageUnitExecutor.java b/...apache/shardingsphere/distsql/handler/executor/rdl/resource/AlterStorageUnitExecutor.java
@@ -65,7 +65,7 @@ public final class AlterStorageUnitExecutor implements DistSQLUpdateExecutor<Alt
     public void executeUpdate(final AlterStorageUnitStatement sqlStatement, final ContextManager contextManager) {
         checkBefore(sqlStatement);
         Map<String, DataSourcePoolProperties> propsMap = DataSourceSegmentsConverter.convert(database.getProtocolType(), sqlStatement.getStorageUnits());
-        validateHandler.validate(propsMap, sqlStatement.getExpectedPrivileges().stream().map(each -> PrivilegeCheckType.valueOf(each.toUpperCase())).collect(Collectors.toSet()));
+        validateHandler.validate(propsMap, getExpectedPrivileges(sqlStatement));
         try {
             MetaDataContexts originalMetaDataContexts = contextManager.getMetaDataContexts();
             contextManager.getPersistServiceFacade().getMetaDataManagerPersistService().alterStorageUnits(database.getName(), propsMap);
@@ -116,6 +116,14 @@ private boolean isSameDatabase(final DataSourceSegment segment, final StorageUni
         return Objects.equals(hostName, connectionProps.getHostname()) && Objects.equals(port, String.valueOf(connectionProps.getPort())) && Objects.equals(database, connectionProps.getCatalog());
     }
 
+    private Collection<PrivilegeCheckType> getExpectedPrivileges(final AlterStorageUnitStatement sqlStatement) {
+        Collection<PrivilegeCheckType> result = sqlStatement.getExpectedPrivileges().stream().map(each -> PrivilegeCheckType.valueOf(each.toUpperCase())).collect(Collectors.toSet());
+        if (result.isEmpty()) {
+            result.add(PrivilegeCheckType.SELECT);
+        }
+        return result;
+    }
+
     @Override
     public Class<AlterStorageUnitStatement> getType() {
         return AlterStorageUnitStatement.class;

diff --git a/...che/shardingsphere/distsql/handler/executor/rdl/resource/RegisterStorageUnitExecutor.java b/...che/shardingsphere/distsql/handler/executor/rdl/resource/RegisterStorageUnitExecutor.java
@@ -67,7 +67,7 @@ public void executeUpdate(final RegisterStorageUnitStatement sqlStatement, final
         if (propsMap.isEmpty()) {
             return;
         }
-        validateHandler.validate(propsMap, sqlStatement.getExpectedPrivileges().stream().map(each -> PrivilegeCheckType.valueOf(each.toUpperCase())).collect(Collectors.toSet()));
+        validateHandler.validate(propsMap, getExpectedPrivileges(sqlStatement));
         try {
             MetaDataContexts originalMetaDataContexts = contextManager.getMetaDataContexts();
             contextManager.getPersistServiceFacade().getMetaDataManagerPersistService().registerStorageUnits(database.getName(), propsMap);
@@ -113,6 +113,14 @@ private Collection<String> getLogicalDataSourceNames() {
         return database.getRuleMetaData().getAttributes(DataSourceMapperRuleAttribute.class).stream().flatMap(each -> each.getDataSourceMapper().keySet().stream()).collect(Collectors.toList());
     }
 
+    private Collection<PrivilegeCheckType> getExpectedPrivileges(final RegisterStorageUnitStatement sqlStatement) {
+        Collection<PrivilegeCheckType> result = sqlStatement.getExpectedPrivileges().stream().map(each -> PrivilegeCheckType.valueOf(each.toUpperCase())).collect(Collectors.toSet());
+        if (result.isEmpty()) {
+            result.add(PrivilegeCheckType.SELECT);
+        }
+        return result;
+    }
+
     @Override
     public Class<RegisterStorageUnitStatement> getType() {
         return RegisterStorageUnitStatement.class;