You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iotdb.apache.org by ro...@apache.org on 2022/03/10 09:05:05 UTC
[iotdb] branch rel/0.12 updated: [To rel/0.12][ISSUE-4399] When non-root user returns a null value, return no permission error message (#5168)
This is an automated email from the ASF dual-hosted git repository.
rong pushed a commit to branch rel/0.12
in repository https://gitbox.apache.org/repos/asf/iotdb.git
The following commit(s) were added to refs/heads/rel/0.12 by this push:
new 8648fe7 [To rel/0.12][ISSUE-4399] When non-root user returns a null value, return no permission error message (#5168)
8648fe7 is described below
commit 8648fe72124dc0a780a10d9da16a8fa2a0fbfaca
Author: Yifu Zhou <ef...@outlook.com>
AuthorDate: Thu Mar 10 16:36:52 2022 +0800
[To rel/0.12][ISSUE-4399] When non-root user returns a null value, return no permission error message (#5168)
---
.../iotdb/db/qp/logical/crud/SFWOperator.java | 10 +++
.../apache/iotdb/db/qp/physical/PhysicalPlan.java | 4 ++
.../iotdb/db/qp/physical/crud/QueryPlan.java | 10 +++
.../iotdb/db/qp/strategy/PhysicalGenerator.java | 1 +
.../org/apache/iotdb/db/service/TSServiceImpl.java | 23 +++++--
.../db/integration/auth/IoTDBAuthorizationIT.java | 73 ++++++++++++++++++++++
6 files changed, 116 insertions(+), 5 deletions(-)
diff --git a/server/src/main/java/org/apache/iotdb/db/qp/logical/crud/SFWOperator.java b/server/src/main/java/org/apache/iotdb/db/qp/logical/crud/SFWOperator.java
index 5eb3820..8dc5faa 100644
--- a/server/src/main/java/org/apache/iotdb/db/qp/logical/crud/SFWOperator.java
+++ b/server/src/main/java/org/apache/iotdb/db/qp/logical/crud/SFWOperator.java
@@ -84,6 +84,16 @@ public abstract class SFWOperator extends RootOperator {
return selectOperator != null ? selectOperator.getSuffixPaths() : null;
}
+ /**
+ * get information from FromOperator in case if getSelectedPaths are null and need to do path
+ * verification.
+ *
+ * @return - a list of seriesPath
+ */
+ public List<PartialPath> getFromPaths() {
+ return fromOperator != null ? fromOperator.getPrefixPaths() : null;
+ }
+
public boolean hasAggregation() {
return hasAggregation;
}
diff --git a/server/src/main/java/org/apache/iotdb/db/qp/physical/PhysicalPlan.java b/server/src/main/java/org/apache/iotdb/db/qp/physical/PhysicalPlan.java
index ca586b5..d037450 100644
--- a/server/src/main/java/org/apache/iotdb/db/qp/physical/PhysicalPlan.java
+++ b/server/src/main/java/org/apache/iotdb/db/qp/physical/PhysicalPlan.java
@@ -110,6 +110,10 @@ public abstract class PhysicalPlan {
public abstract List<PartialPath> getPaths();
+ public List<PartialPath> getFromPaths() {
+ return Collections.emptyList();
+ }
+
public void setPaths(List<PartialPath> paths) {}
public boolean isQuery() {
diff --git a/server/src/main/java/org/apache/iotdb/db/qp/physical/crud/QueryPlan.java b/server/src/main/java/org/apache/iotdb/db/qp/physical/crud/QueryPlan.java
index a602c16..b5b0f76 100644
--- a/server/src/main/java/org/apache/iotdb/db/qp/physical/crud/QueryPlan.java
+++ b/server/src/main/java/org/apache/iotdb/db/qp/physical/crud/QueryPlan.java
@@ -32,6 +32,7 @@ import java.util.Map;
public abstract class QueryPlan extends PhysicalPlan {
protected List<PartialPath> paths = null;
+ protected List<PartialPath> fromPaths = null;
protected List<TSDataType> dataTypes = null;
private boolean alignByTime = true; // for disable align sql
@@ -69,6 +70,15 @@ public abstract class QueryPlan extends PhysicalPlan {
this.paths = paths;
}
+ @Override
+ public List<PartialPath> getFromPaths() {
+ return fromPaths;
+ }
+
+ public void setFromPaths(List<PartialPath> fromPaths) {
+ this.fromPaths = fromPaths;
+ }
+
public List<TSDataType> getDataTypes() {
return dataTypes;
}
diff --git a/server/src/main/java/org/apache/iotdb/db/qp/strategy/PhysicalGenerator.java b/server/src/main/java/org/apache/iotdb/db/qp/strategy/PhysicalGenerator.java
index 194e04b..832bbc0 100644
--- a/server/src/main/java/org/apache/iotdb/db/qp/strategy/PhysicalGenerator.java
+++ b/server/src/main/java/org/apache/iotdb/db/qp/strategy/PhysicalGenerator.java
@@ -562,6 +562,7 @@ public class PhysicalGenerator {
queryPlan = getAlignQueryPlan(queryOperator, fetchSize, queryPlan);
} else {
queryPlan.setPaths(queryOperator.getSelectedPaths());
+ queryPlan.setFromPaths(queryOperator.getFromPaths());
// Last query result set will not be affected by alignment
if (queryPlan instanceof LastQueryPlan && !queryOperator.isAlignByTime()) {
throw new QueryProcessException("Disable align cannot be applied to LAST query.");
diff --git a/server/src/main/java/org/apache/iotdb/db/service/TSServiceImpl.java b/server/src/main/java/org/apache/iotdb/db/service/TSServiceImpl.java
index 79af8f1..92bc3c5 100644
--- a/server/src/main/java/org/apache/iotdb/db/service/TSServiceImpl.java
+++ b/server/src/main/java/org/apache/iotdb/db/service/TSServiceImpl.java
@@ -131,6 +131,7 @@ import org.apache.iotdb.tsfile.read.common.Path;
import org.apache.iotdb.tsfile.read.query.dataset.QueryDataSet;
import org.antlr.v4.runtime.misc.ParseCancellationException;
+import org.apache.commons.collections4.CollectionUtils;
import org.apache.thrift.TException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -875,11 +876,23 @@ public class TSServiceImpl implements TSIService.Iface {
List<String> columnsTypes = new ArrayList<>();
// check permissions
- if (!checkAuthorization(physicalPlan.getAuthPaths(), physicalPlan, username)) {
- return RpcUtils.getTSExecuteStatementResp(
- RpcUtils.getStatus(
- TSStatusCode.NO_PERMISSION_ERROR,
- "No permissions for this operation " + physicalPlan.getOperatorType()));
+ if ((physicalPlan.getAuthPaths().isEmpty()
+ && !CollectionUtils.isEmpty(physicalPlan.getFromPaths()))
+ || !physicalPlan.getAuthPaths().isEmpty()) {
+ List<PartialPath> checkPaths =
+ physicalPlan.getAuthPaths().isEmpty()
+ ? physicalPlan.getFromPaths()
+ : physicalPlan.getAuthPaths();
+ if (!checkAuthorization(checkPaths, physicalPlan, username)) {
+ OperatorType operatorType = physicalPlan.getOperatorType();
+ if (operatorType == OperatorType.UDTF) {
+ operatorType = OperatorType.QUERY;
+ }
+ return RpcUtils.getTSExecuteStatementResp(
+ RpcUtils.getStatus(
+ TSStatusCode.NO_PERMISSION_ERROR,
+ "No permissions for this operation " + operatorType));
+ }
}
TSExecuteStatementResp resp = RpcUtils.getTSExecuteStatementResp(TSStatusCode.SUCCESS_STATUS);
diff --git a/server/src/test/java/org/apache/iotdb/db/integration/auth/IoTDBAuthorizationIT.java b/server/src/test/java/org/apache/iotdb/db/integration/auth/IoTDBAuthorizationIT.java
index 812d266..f0f8829 100644
--- a/server/src/test/java/org/apache/iotdb/db/integration/auth/IoTDBAuthorizationIT.java
+++ b/server/src/test/java/org/apache/iotdb/db/integration/auth/IoTDBAuthorizationIT.java
@@ -39,6 +39,7 @@ import java.util.List;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
/**
* Notice that, all test begins with "IoTDB" is integration test. All test which will start the
@@ -1105,4 +1106,76 @@ public class IoTDBAuthorizationIT {
assertTrue(resultSet.next());
}
}
+
+ /** ISSUE-4399 */
+ @Test
+ public void testUDTFPermissionReturnMessage() throws ClassNotFoundException, SQLException {
+ Class.forName(Config.JDBC_DRIVER_NAME);
+ try (Connection adminConnection =
+ DriverManager.getConnection(
+ Config.IOTDB_URL_PREFIX + "127.0.0.1:6667/", "root", "root");
+ Statement adminStatement = adminConnection.createStatement()) {
+ adminStatement.execute("CREATE USER only_read_sg1 'only_read_sg1'");
+ adminStatement.execute("CREATE ROLE application_role");
+ adminStatement.execute(
+ "GRANT ROLE application_role PRIVILEGES 'READ_TIMESERIES' ON root.sg1");
+ adminStatement.execute("GRANT application_role TO only_read_sg1");
+
+ adminStatement.execute("SET STORAGE GROUP TO root.sg1;");
+ adminStatement.execute(
+ "CREATE TIMESERIES root.sg1.d1.s1 WITH DATATYPE=INT32, ENCODING=PLAIN;");
+ adminStatement.execute(
+ "CREATE TIMESERIES root.sg1.d1.s2 WITH DATATYPE=INT32, ENCODING=PLAIN;");
+ adminStatement.execute("INSERT INTO root.sg1.d1(timestamp, s1, s2) VALUES (0, -1, 1);");
+
+ adminStatement.execute("SET STORAGE GROUP TO root.sg2;");
+ adminStatement.execute(
+ "CREATE TIMESERIES root.sg2.d1.s1 WITH DATATYPE=INT32, ENCODING=PLAIN;");
+ adminStatement.execute(
+ "CREATE TIMESERIES root.sg2.d1.s2 WITH DATATYPE=INT32, ENCODING=PLAIN;");
+ adminStatement.execute("INSERT INTO root.sg2.d1(timestamp, s1, s2) VALUES (0, -1, 1);");
+
+ adminStatement.execute("INSERT INTO root.test(time, s1) VALUES(1, 1)");
+ }
+
+ try (Connection userConnection =
+ DriverManager.getConnection(
+ Config.IOTDB_URL_PREFIX + "127.0.0.1:6667/", "only_read_sg1", "only_read_sg1");
+ Statement userStatement = userConnection.createStatement()) {
+ // Case 1:
+ ResultSet resultSet = userStatement.executeQuery("select sin(s1) from root.sg1.d1;");
+ ResultSetMetaData resultSetMetaData = resultSet.getMetaData();
+ assertEquals(2, resultSetMetaData.getColumnCount());
+
+ // Case 2:
+ resultSet = userStatement.executeQuery("select sin(s8) from root.sg1.d1;");
+ resultSetMetaData = resultSet.getMetaData();
+ assertEquals(1, resultSetMetaData.getColumnCount());
+ while (resultSet.next()) {
+ fail();
+ }
+
+ // Case 3:
+ resultSet = userStatement.executeQuery("select s8 from root.sg1.d1;");
+ resultSetMetaData = resultSet.getMetaData();
+ assertEquals(1, resultSetMetaData.getColumnCount());
+ while (resultSet.next()) {
+ fail();
+ }
+
+ // Case 4:
+ try {
+ userStatement.executeQuery("select sin(s1) from root.sg2.d1;");
+ } catch (SQLException e) {
+ assertEquals("602: No permissions for this operation QUERY", e.getMessage());
+ }
+
+ // Case 5:
+ try {
+ userStatement.executeQuery("select sin(s8) from root.sg2.d1;");
+ } catch (SQLException e) {
+ assertEquals("602: No permissions for this operation QUERY", e.getMessage());
+ }
+ }
+ }
}