Caideyipi commented on code in PR #16531:
URL: https://github.com/apache/iotdb/pull/16531#discussion_r2584129336
##########
integration-test/src/test/java/org/apache/iotdb/pipe/it/dual/treemodel/manual/IoTDBPipePermissionIT.java:
##########
@@ -224,4 +233,156 @@ public void testNoPermission() throws Exception {
receiverEnv, "list user", "User,", Collections.singleton("root,"));
}
}
+
+ @Test
+ public void testSourcePermission() {
+ TestUtils.executeNonQuery(senderEnv, "create user `thulab`
'passwD@123456'", null);
+
+ // Shall fail if username is specified without password
+ try (final Connection connection = senderEnv.getConnection();
+ final Statement statement = connection.createStatement()) {
+ statement.execute(
+ String.format(
+ "create pipe a2b"
+ + " with source ("
+ + "'user'='thulab')"
+ + " with sink ("
+ + "'node-urls'='%s')",
+
receiverEnv.getDataNodeWrapperList().get(0).getIpAndPortString()));
+ fail("When the 'user' or 'username' is specified, password must be
specified too.");
+ } catch (final SQLException ignore) {
+ // Expected
+ }
+
+ // Shall fail if password is wrong
+ try (final Connection connection = senderEnv.getConnection();
+ final Statement statement = connection.createStatement()) {
+ statement.execute(
+ String.format(
+ "create pipe a2b"
+ + " with source ("
+ + "'user'='thulab'"
+ + "'password'='hack')"
+ + " with sink ("
+ + "'node-urls'='%s')",
+
receiverEnv.getDataNodeWrapperList().get(0).getIpAndPortString()));
+ fail("Shall fail if password is wrong.");
+ } catch (final SQLException ignore) {
+ // Expected
+ }
+
+ // Use current session, user is root
+ try (final Connection connection = senderEnv.getConnection();
+ final Statement statement = connection.createStatement()) {
+ statement.execute(
+ String.format(
+ "create pipe a2b"
+ + " with source ("
+ + "'inclusion'='all')"
+ + " with sink ("
+ + "'node-urls'='%s')",
+
receiverEnv.getDataNodeWrapperList().get(0).getIpAndPortString()));
+ } catch (final SQLException e) {
+ e.printStackTrace();
+ fail("Create pipe without user shall succeed if use the current
session");
+ }
+
+ // Alter to another user, shall fail because of lack of password
+ try (final Connection connection = senderEnv.getConnection();
+ final Statement statement = connection.createStatement()) {
+ statement.execute("alter pipe a2b modify source ('username'='thulab')");
+ fail("Alter pipe shall fail if only user is specified");
+ } catch (final SQLException ignore) {
+ // Expected
+ }
+
+ // Successfully alter
+ try (final Connection connection = senderEnv.getConnection();
+ final Statement statement = connection.createStatement()) {
+ statement.execute(
+ "alter pipe a2b modify source ('username'='thulab',
'password'='passwD@123456')");
+ } catch (final SQLException e) {
+ e.printStackTrace();
+ fail("Alter pipe shall not fail if user and password are specified");
+ }
+
+ TestUtils.executeNonQuery(senderEnv, "create database root.test");
+
+ // Shall not be transferred
+ TestUtils.assertDataAlwaysOnEnv(
+ receiverEnv, "count databases", "count,", Collections.singleton("0,"));
+
+ // GRANT privileges ON prefixPath (COMMA prefixPath)* TO USER
userName=usernameWithRoot
+ // (grantOpt)?
+ // Grant some privilege
+ TestUtils.executeNonQuery(senderEnv, "grant SYSTEM on root.** to user
thulab");
+
+ TestUtils.executeNonQuery(senderEnv, "create database root.test1");
+
+ // Shall be transferred
+ TestUtils.assertDataEventuallyOnEnv(
+ receiverEnv, "count databases root.tes*", "count,",
Collections.singleton("1,"));
+
+ // Alter pipe, throw exception if no privileges
+ try (final Connection connection = senderEnv.getConnection();
+ final Statement statement = connection.createStatement()) {
+ statement.execute("alter pipe a2b modify source ('skipif'='')");
+ } catch (final SQLException e) {
+ e.printStackTrace();
+ fail(e.getMessage());
+ }
+
+ // Write some data
+ TestUtils.executeNonQueries(
+ senderEnv,
+ Arrays.asList(
+ "create timeSeries root.vehicle.car.temperature DOUBLE",
+ "insert into root.vehicle.car(temperature) values (36.5)"));
+
+ // Exception, block here
+ TableModelUtils.assertCountDataAlwaysOnEnv("test", "test", 0, receiverEnv);
+ TestUtils.assertDataAlwaysOnEnv(
+ receiverEnv, "count timeSeries", "count(timeseries),",
Collections.singleton("0,"));
+
+ // Grant SELECT privilege
+ TestUtils.executeNonQueries(
+ senderEnv, Arrays.asList("grant READ on root.** to user thulab",
"start pipe a2b"));
+
+ // Will finally pass
+ TestUtils.assertDataEventuallyOnEnv(
+ receiverEnv,
+ "select count(*) from root.vehicle.**",
+ "count(root.vehicle.car.temperature),",
+ Collections.singleton("1,"));
+
+ // test showing pipe
+ // Create another pipe, user is root
+ try (final Connection connection = senderEnv.getConnection();
+ final Statement statement = connection.createStatement()) {
+ statement.execute(
+ String.format(
+ "create pipe a2c"
+ + " with source ("
+ + "'inclusion'='all',"
+ + "'capture.tree'='true',"
+ + "'capture.table'='true')"
+ + " with sink ("
+ + "'node-urls'='%s')",
+
receiverEnv.getDataNodeWrapperList().get(0).getIpAndPortString()));
+ } catch (final SQLException e) {
+ e.printStackTrace();
+ fail("Create pipe without user shall succeed if use the current
session");
+ }
+
+ TestUtils.executeNonQuery(senderEnv, "revoke SYSTEM on root.** from user
thulab");
+
+ // A user shall only see its own pipe
+ try (final SyncConfigNodeIServiceClient client =
+ (SyncConfigNodeIServiceClient)
senderEnv.getLeaderConfigNodeConnection()) {
+ Assert.assertEquals(
+ 1, client.showPipe(new
TShowPipeReq().setUserName("thulab")).pipeInfoList.size());
+ } catch (Exception e) {
+ fail(e.getMessage());
+ }
Review Comment:
Yes, but if the config client can be initialized outside of IoTDB, a user
can do everything without permission, because the permissions are checked on
DataNode. So users shall not have the change to directly "set" the request's
username
##########
integration-test/src/test/java/org/apache/iotdb/pipe/it/dual/treemodel/manual/IoTDBPipePermissionIT.java:
##########
@@ -224,4 +233,156 @@ public void testNoPermission() throws Exception {
receiverEnv, "list user", "User,", Collections.singleton("root,"));
}
}
+
+ @Test
+ public void testSourcePermission() {
+ TestUtils.executeNonQuery(senderEnv, "create user `thulab`
'passwD@123456'", null);
+
+ // Shall fail if username is specified without password
+ try (final Connection connection = senderEnv.getConnection();
+ final Statement statement = connection.createStatement()) {
+ statement.execute(
+ String.format(
+ "create pipe a2b"
+ + " with source ("
+ + "'user'='thulab')"
+ + " with sink ("
+ + "'node-urls'='%s')",
+
receiverEnv.getDataNodeWrapperList().get(0).getIpAndPortString()));
+ fail("When the 'user' or 'username' is specified, password must be
specified too.");
+ } catch (final SQLException ignore) {
+ // Expected
+ }
+
+ // Shall fail if password is wrong
+ try (final Connection connection = senderEnv.getConnection();
+ final Statement statement = connection.createStatement()) {
+ statement.execute(
+ String.format(
+ "create pipe a2b"
+ + " with source ("
+ + "'user'='thulab'"
+ + "'password'='hack')"
+ + " with sink ("
+ + "'node-urls'='%s')",
+
receiverEnv.getDataNodeWrapperList().get(0).getIpAndPortString()));
+ fail("Shall fail if password is wrong.");
+ } catch (final SQLException ignore) {
+ // Expected
+ }
+
+ // Use current session, user is root
+ try (final Connection connection = senderEnv.getConnection();
+ final Statement statement = connection.createStatement()) {
+ statement.execute(
+ String.format(
+ "create pipe a2b"
+ + " with source ("
+ + "'inclusion'='all')"
+ + " with sink ("
+ + "'node-urls'='%s')",
+
receiverEnv.getDataNodeWrapperList().get(0).getIpAndPortString()));
+ } catch (final SQLException e) {
+ e.printStackTrace();
+ fail("Create pipe without user shall succeed if use the current
session");
+ }
+
+ // Alter to another user, shall fail because of lack of password
+ try (final Connection connection = senderEnv.getConnection();
+ final Statement statement = connection.createStatement()) {
+ statement.execute("alter pipe a2b modify source ('username'='thulab')");
+ fail("Alter pipe shall fail if only user is specified");
+ } catch (final SQLException ignore) {
+ // Expected
+ }
+
+ // Successfully alter
+ try (final Connection connection = senderEnv.getConnection();
+ final Statement statement = connection.createStatement()) {
+ statement.execute(
+ "alter pipe a2b modify source ('username'='thulab',
'password'='passwD@123456')");
+ } catch (final SQLException e) {
+ e.printStackTrace();
+ fail("Alter pipe shall not fail if user and password are specified");
+ }
+
+ TestUtils.executeNonQuery(senderEnv, "create database root.test");
+
+ // Shall not be transferred
+ TestUtils.assertDataAlwaysOnEnv(
+ receiverEnv, "count databases", "count,", Collections.singleton("0,"));
+
+ // GRANT privileges ON prefixPath (COMMA prefixPath)* TO USER
userName=usernameWithRoot
+ // (grantOpt)?
+ // Grant some privilege
+ TestUtils.executeNonQuery(senderEnv, "grant SYSTEM on root.** to user
thulab");
+
+ TestUtils.executeNonQuery(senderEnv, "create database root.test1");
+
+ // Shall be transferred
+ TestUtils.assertDataEventuallyOnEnv(
+ receiverEnv, "count databases root.tes*", "count,",
Collections.singleton("1,"));
+
+ // Alter pipe, throw exception if no privileges
+ try (final Connection connection = senderEnv.getConnection();
+ final Statement statement = connection.createStatement()) {
+ statement.execute("alter pipe a2b modify source ('skipif'='')");
+ } catch (final SQLException e) {
+ e.printStackTrace();
+ fail(e.getMessage());
+ }
+
+ // Write some data
+ TestUtils.executeNonQueries(
+ senderEnv,
+ Arrays.asList(
+ "create timeSeries root.vehicle.car.temperature DOUBLE",
+ "insert into root.vehicle.car(temperature) values (36.5)"));
+
+ // Exception, block here
+ TableModelUtils.assertCountDataAlwaysOnEnv("test", "test", 0, receiverEnv);
+ TestUtils.assertDataAlwaysOnEnv(
+ receiverEnv, "count timeSeries", "count(timeseries),",
Collections.singleton("0,"));
+
+ // Grant SELECT privilege
+ TestUtils.executeNonQueries(
+ senderEnv, Arrays.asList("grant READ on root.** to user thulab",
"start pipe a2b"));
+
+ // Will finally pass
+ TestUtils.assertDataEventuallyOnEnv(
+ receiverEnv,
+ "select count(*) from root.vehicle.**",
+ "count(root.vehicle.car.temperature),",
+ Collections.singleton("1,"));
+
+ // test showing pipe
+ // Create another pipe, user is root
+ try (final Connection connection = senderEnv.getConnection();
+ final Statement statement = connection.createStatement()) {
+ statement.execute(
+ String.format(
+ "create pipe a2c"
+ + " with source ("
+ + "'inclusion'='all',"
+ + "'capture.tree'='true',"
+ + "'capture.table'='true')"
+ + " with sink ("
+ + "'node-urls'='%s')",
+
receiverEnv.getDataNodeWrapperList().get(0).getIpAndPortString()));
+ } catch (final SQLException e) {
+ e.printStackTrace();
+ fail("Create pipe without user shall succeed if use the current
session");
+ }
+
+ TestUtils.executeNonQuery(senderEnv, "revoke SYSTEM on root.** from user
thulab");
+
+ // A user shall only see its own pipe
+ try (final SyncConfigNodeIServiceClient client =
+ (SyncConfigNodeIServiceClient)
senderEnv.getLeaderConfigNodeConnection()) {
+ Assert.assertEquals(
+ 1, client.showPipe(new
TShowPipeReq().setUserName("thulab")).pipeInfoList.size());
+ } catch (Exception e) {
+ fail(e.getMessage());
+ }
Review Comment:
Yes, but if the config client can be initialized outside of IoTDB, a user
can do everything without permission, because the permissions are checked on
DataNode. So users shall not have the chance to directly "set" the request's
username
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
