diff --git a/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/CliSecurityIT.java b/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/CliSecurityIT.java index 76b66ebb8ee..9c4107b38f0 100644 --- a/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/CliSecurityIT.java +++ b/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/CliSecurityIT.java @@ -9,6 +9,7 @@ import org.elasticsearch.common.CheckedConsumer; import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.xpack.qa.sql.cli.RemoteCli; import org.elasticsearch.xpack.qa.sql.cli.RemoteCli.SecurityConfig; + import java.net.URISyntaxException; import java.nio.file.Files; import java.nio.file.Path; @@ -128,10 +129,15 @@ public class CliSecurityIT extends SqlSecurityTestCase { @Override public void expectShowTables(List tables, String user) throws Exception { try (RemoteCli cli = new RemoteCli(elasticsearchAddress(), true, userSecurity(user))) { - assertThat(cli.command("SHOW TABLES"), containsString("table")); - assertEquals("---------------", cli.readLine()); + String tablesOutput = cli.command("SHOW TABLES"); + assertThat(tablesOutput, containsString("name")); + assertThat(tablesOutput, containsString("type")); + assertEquals("---------------+---------------", cli.readLine()); for (String table : tables) { - assertThat(cli.readLine(), containsString(table)); + String line = cli.readLine(); + if (!line.startsWith(".security")) { + assertThat(line, containsString(table)); + } } assertEquals("", cli.readLine()); } diff --git a/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/JdbcSecurityIT.java b/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/JdbcSecurityIT.java index 074ea8795f7..3c91099f360 100644 --- a/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/JdbcSecurityIT.java +++ b/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/JdbcSecurityIT.java @@ -10,6 +10,7 @@ import org.elasticsearch.common.CheckedConsumer; import org.elasticsearch.common.CheckedFunction; import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.xpack.qa.sql.jdbc.LocalH2; + import java.net.URISyntaxException; import java.nio.file.Files; import java.nio.file.Path; @@ -18,6 +19,7 @@ import java.sql.DriverManager; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Properties; @@ -179,25 +181,22 @@ public class JdbcSecurityIT extends SqlSecurityTestCase { @Override public void expectShowTables(List tables, String user) throws Exception { - try (Connection h2 = LocalH2.anonymousDb(); - Connection es = es(userProperties(user))) { - // h2 doesn't spit out the same columns we do so we emulate - h2.createStatement().executeUpdate("CREATE TABLE mock (table VARCHAR)"); - StringBuilder insert = new StringBuilder(); - insert.append("INSERT INTO mock (table) VALUES "); - boolean first = true; - for (String table : tables) { - if (first) { - first = false; - } else { - insert.append(", "); - } - insert.append("('").append(table).append("')"); - } - h2.createStatement().executeUpdate(insert.toString()); + try (Connection es = es(userProperties(user))) { + ResultSet actual = es.createStatement().executeQuery("SHOW TABLES"); - ResultSet expected = h2.createStatement().executeQuery("SELECT * FROM mock ORDER BY table"); - assertResultSets(expected, es.createStatement().executeQuery("SHOW TABLES")); + // depending on whether security is enabled and a test is run in isolation or suite + // .security or .security6 index can appear + // to filter these out, the result set is flatten to a list + List actualList = new ArrayList<>(); + + while (actual.next()) { + String name = actual.getString("name"); + if (!name.startsWith(".security")) { + actualList.add(name); + } + } + + assertEquals(tables, actualList); } } diff --git a/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/RestSqlSecurityIT.java b/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/RestSqlSecurityIT.java index 4fbee4e242a..4788f90a50f 100644 --- a/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/RestSqlSecurityIT.java +++ b/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/RestSqlSecurityIT.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.Map; import java.util.TreeMap; +import java.util.stream.Collectors; import static java.util.Collections.singletonMap; import static org.elasticsearch.xpack.qa.sql.rest.RestSqlTestCase.columnInfo; import static java.util.Collections.singletonList; @@ -106,14 +107,27 @@ public class RestSqlSecurityIT extends SqlSecurityTestCase { @Override public void expectShowTables(List tables, String user) throws Exception { String mode = randomMode(); + List columns = new ArrayList<>(); + columns.add(columnInfo(mode, "name", "keyword", JDBCType.VARCHAR, 0)); + columns.add(columnInfo(mode, "type", "keyword", JDBCType.VARCHAR, 0)); Map expected = new HashMap<>(); - expected.put("columns", singletonList(columnInfo(mode, "table", "keyword", JDBCType.VARCHAR, 0))); + expected.put("columns", columns); List> rows = new ArrayList<>(); for (String table : tables) { - rows.add(singletonList(table)); + List fields = new ArrayList<>(); + fields.add(table); + fields.add("INDEX"); + rows.add(fields); } expected.put("rows", rows); - assertResponse(expected, runSql(user, mode, "SHOW TABLES")); + + Map actual = runSql(user, mode, "SHOW TABLES"); + List> rowsNoSecurity = ((List>) actual.get("rows")) + .stream() + .filter(ls -> ls.get(0).startsWith(".security") == false) + .collect(Collectors.toList()); + actual.put("rows", rowsNoSecurity); + assertResponse(expected, actual); } @Override diff --git a/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/SqlSecurityTestCase.java b/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/SqlSecurityTestCase.java index 9d60f7642c5..87bf0e2a713 100644 --- a/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/SqlSecurityTestCase.java +++ b/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/SqlSecurityTestCase.java @@ -334,9 +334,8 @@ public abstract class SqlSecurityTestCase extends ESRestTestCase { .assertLogs(); } - @AwaitsFix(bugUrl = "https://github.com/elastic/x-pack-elasticsearch/issues/3423") public void testShowTablesWorksAsAdmin() throws Exception { - actions.expectShowTables(Arrays.asList(".security-6", "bort", "test"), null); + actions.expectShowTables(Arrays.asList("bort", "test"), null); new AuditLogAsserter() .expectSqlCompositeAction("test_admin", "bort", "test") .assertLogs(); diff --git a/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcResultSet.java b/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcResultSet.java index 317ff80f79c..5cf09ac3ee3 100644 --- a/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcResultSet.java +++ b/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcResultSet.java @@ -59,12 +59,12 @@ class JdbcResultSet implements ResultSet, JdbcWrapper { this.statement = statement; this.cursor = cursor; // statement can be null so we have to extract the timeZone from the non-nullable cfg - // TODO: should we consider the locale as well? + // TODO: should we consider the locale as well? this.defaultCalendar = Calendar.getInstance(cfg.timeZone(), Locale.ROOT); List columns = cursor.columns(); for (int i = 0; i < columns.size(); i++) { - nameToIndex.put(columns.get(i).name, Integer.valueOf(i)); + nameToIndex.put(columns.get(i).name, Integer.valueOf(i + 1)); } }