[GitHub] drill pull request #983: DRILL-5819: Default value of security.admin.user_gr...
Github user asfgit closed the pull request at: https://github.com/apache/drill/pull/983 ---
[GitHub] drill pull request #983: DRILL-5819: Default value of security.admin.user_gr...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/983#discussion_r144149536
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/test/ClientFixture.java ---
@@ -128,6 +128,21 @@ public void alterSystem(String key, Object value) {
}
/**
+ * Reset a system option
+ * @param key
+ */
+
+ public void resetSystem(String key) {
--- End diff --
These are added in PR #970, in [this
file](https://github.com/apache/drill/pull/970/files#diff-e2de7c62cedd56fbabbf3cff0f0663a2).
But, this PR is likely to be committed before this one, so I'll deal with
the merge conflict later.
---
[GitHub] drill pull request #983: DRILL-5819: Default value of security.admin.user_gr...
Github user bitblender commented on a diff in the pull request:
https://github.com/apache/drill/pull/983#discussion_r144143076
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/server/TestOptionsAuthEnabled.java
---
@@ -115,4 +124,59 @@ private void setOptHelper() throws Exception {
test(String.format("ALTER SYSTEM SET `%s` = %d;",
ExecConstants.SLICE_TARGET, ExecConstants.SLICE_TARGET_DEFAULT));
}
}
+
+ @Test
+ public void testAdminUserOptions() throws Exception {
+FixtureBuilder builder = ClusterFixture.builder();
+
+try (ClusterFixture cluster = builder.build();
+ ClientFixture client = cluster.clientFixture()) {
+ OptionManager optionManager =
cluster.drillbit().getContext().getOptionManager();
+
+ // Admin Users Tests
+ // config file should have the 'fake' default admin user and it
should be returned
+ // by the option manager if the option has not been set by the user
+ String configAdminUser =
optionManager.getOption(ExecConstants.ADMIN_USERS_VALIDATOR);;
+ assertEquals(configAdminUser,
ExecConstants.ADMIN_USERS_VALIDATOR.DEFAULT_ADMIN_USERS);
+
+ // Option accessor should never return the 'fake' default from the
config
+ String adminUser1 =
ExecConstants.ADMIN_USERS_VALIDATOR.getAdminUsers(optionManager);
+ assertNotEquals(adminUser1,
ExecConstants.ADMIN_USERS_VALIDATOR.DEFAULT_ADMIN_USERS);
+
+ // Change TEST_ADMIN_USER if necessary
+ String TEST_ADMIN_USER = "ronswanson";
+ if (adminUser1.equals(TEST_ADMIN_USER)) {
+TEST_ADMIN_USER += "thefirst";
+ }
+ // Check if the admin option accessor honors a user-supplied values
+ String sql = String.format("ALTER SYSTEM SET `%s`='%s'",
ExecConstants.ADMIN_USERS_KEY, TEST_ADMIN_USER);
+ client.queryBuilder().sql(sql).run();
+ String adminUser2 =
ExecConstants.ADMIN_USERS_VALIDATOR.getAdminUsers(optionManager);
+ assertEquals(adminUser2, TEST_ADMIN_USER);
+
+ // Admin User Groups Tests
+
+ // config file should have the 'fake' default admin user and it
should be returned
+ // by the option manager if the option has not been set by the user
+ String configAdminUserGroups =
optionManager.getOption(ExecConstants.ADMIN_USER_GROUPS_VALIDATOR);
+ assertEquals(configAdminUserGroups,
ExecConstants.ADMIN_USER_GROUPS_VALIDATOR.DEFAULT_ADMIN_USER_GROUPS);
+
+ // Option accessor should never return the 'fake' default from the
config
+ String adminUserGroups1 =
ExecConstants.ADMIN_USER_GROUPS_VALIDATOR.getAdminUserGroups(optionManager);
+ assertNotEquals(adminUserGroups1,
ExecConstants.ADMIN_USER_GROUPS_VALIDATOR.DEFAULT_ADMIN_USER_GROUPS);
+
+ // Change TEST_ADMIN_USER_GROUPS if necessary
+ String TEST_ADMIN_USER_GROUPS = "yakshavers";
+ if (adminUserGroups1.equals(TEST_ADMIN_USER_GROUPS)) {
+TEST_ADMIN_USER_GROUPS += ",wormracers";
+ }
+ // Check if the admin option accessor honors a user-supplied values
+ sql = String.format("ALTER SYSTEM SET `%s`='%s'",
ExecConstants.ADMIN_USER_GROUPS_KEY, TEST_ADMIN_USER_GROUPS);
+ client.queryBuilder().sql(sql).run();
+ String adminUserGroups2 =
ExecConstants.ADMIN_USER_GROUPS_VALIDATOR.getAdminUserGroups(optionManager);
+ assertEquals(adminUserGroups2, TEST_ADMIN_USER_GROUPS);
--- End diff --
I am pushing changes which add these tests.
---
[GitHub] drill pull request #983: DRILL-5819: Default value of security.admin.user_gr...
Github user bitblender commented on a diff in the pull request:
https://github.com/apache/drill/pull/983#discussion_r144143072
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
---
@@ -75,6 +80,29 @@ public ClusterInfo getClusterInfoJSON() {
final DrillConfig config = dbContext.getConfig();
final boolean userEncryptionEnabled =
config.getBoolean(ExecConstants.USER_ENCRYPTION_SASL_ENABLED);
final boolean bitEncryptionEnabled =
config.getBoolean(ExecConstants.BIT_ENCRYPTION_SASL_ENABLED);
+// If the user is logged in and is admin user then show the admin user
info
+// For all other cases the user info need-not or should-not be
displayed
+OptionManager optionManager = work.getContext().getOptionManager();
+final boolean isUserLoggedIn = AuthDynamicFeature.isUserLoggedIn(sc);
+String adminUsers = isUserLoggedIn ?
+
ExecConstants.ADMIN_USERS_VALIDATOR.getAdminUsers(optionManager) : null;
+String adminUserGroups = isUserLoggedIn ?
+
ExecConstants.ADMIN_USER_GROUPS_VALIDATOR.getAdminUserGroups(optionManager) :
null;
+
+// separate groups by comma + space
+if (adminUsers != null) {
+ String[] groups = adminUsers.split(",");
+ adminUsers = DrillStringUtils.join(groups, ", ");
+}
+
+// separate groups by comma + space
+if (adminUserGroups != null) {
+ String[] groups = adminUserGroups.split(",");
+ adminUserGroups = DrillStringUtils.join(groups, ", ");
--- End diff --
I pushing changes that handle ill-formatted user input
---
[GitHub] drill pull request #983: DRILL-5819: Default value of security.admin.user_gr...
Github user bitblender commented on a diff in the pull request:
https://github.com/apache/drill/pull/983#discussion_r144143097
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/server/TestOptionsAuthEnabled.java
---
@@ -115,4 +124,59 @@ private void setOptHelper() throws Exception {
test(String.format("ALTER SYSTEM SET `%s` = %d;",
ExecConstants.SLICE_TARGET, ExecConstants.SLICE_TARGET_DEFAULT));
}
}
+
+ @Test
+ public void testAdminUserOptions() throws Exception {
+FixtureBuilder builder = ClusterFixture.builder();
+
+try (ClusterFixture cluster = builder.build();
+ ClientFixture client = cluster.clientFixture()) {
+ OptionManager optionManager =
cluster.drillbit().getContext().getOptionManager();
+
+ // Admin Users Tests
+ // config file should have the 'fake' default admin user and it
should be returned
+ // by the option manager if the option has not been set by the user
+ String configAdminUser =
optionManager.getOption(ExecConstants.ADMIN_USERS_VALIDATOR);;
+ assertEquals(configAdminUser,
ExecConstants.ADMIN_USERS_VALIDATOR.DEFAULT_ADMIN_USERS);
+
+ // Option accessor should never return the 'fake' default from the
config
+ String adminUser1 =
ExecConstants.ADMIN_USERS_VALIDATOR.getAdminUsers(optionManager);
+ assertNotEquals(adminUser1,
ExecConstants.ADMIN_USERS_VALIDATOR.DEFAULT_ADMIN_USERS);
+
+ // Change TEST_ADMIN_USER if necessary
+ String TEST_ADMIN_USER = "ronswanson";
+ if (adminUser1.equals(TEST_ADMIN_USER)) {
+TEST_ADMIN_USER += "thefirst";
+ }
+ // Check if the admin option accessor honors a user-supplied values
+ String sql = String.format("ALTER SYSTEM SET `%s`='%s'",
ExecConstants.ADMIN_USERS_KEY, TEST_ADMIN_USER);
+ client.queryBuilder().sql(sql).run();
--- End diff --
Done.
---
[GitHub] drill pull request #983: DRILL-5819: Default value of security.admin.user_gr...
Github user bitblender commented on a diff in the pull request:
https://github.com/apache/drill/pull/983#discussion_r144143113
--- Diff:
common/src/main/java/org/apache/drill/common/util/DrillStringUtils.java ---
@@ -1,203 +1,258 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.drill.common.util;
-
-import io.netty.buffer.ByteBuf;
-
-import org.apache.commons.lang3.StringEscapeUtils;
-import org.apache.commons.lang3.StringUtils;
-
-public class DrillStringUtils {
-
- /**
- * Converts the long number into more human readable string.
- */
- public static String readable(long bytes) {
-int unit = 1024;
-long absBytes = Math.abs(bytes);
-if (absBytes < unit) {
- return bytes + " B";
-}
-int exp = (int) (Math.log(absBytes) / Math.log(unit));
-char pre = ("KMGTPE").charAt(exp-1);
-return String.format("%s%.1f %ciB", (bytes == absBytes ? "" : "-"),
absBytes / Math.pow(unit, exp), pre);
- }
-
-
- /**
- * Unescapes any Java literals found in the {@code String}.
- * For example, it will turn a sequence of {@code '\'} and
- * {@code 'n'} into a newline character, unless the {@code '\'}
- * is preceded by another {@code '\'}.
- *
- * @param input the {@code String} to unescape, may be null
- * @return a new unescaped {@code String}, {@code null} if null string
input
- */
- public static final String unescapeJava(String input) {
-return StringEscapeUtils.unescapeJava(input);
- }
-
- /**
- * Escapes the characters in a {@code String} according to Java string
literal
- * rules.
- *
- * Deals correctly with quotes and control-chars (tab, backslash, cr, ff,
- * etc.) so, for example, a tab becomes the characters {@code '\\'} and
- * {@code 't'}.
- *
- * Example:
- *
- * input string: He didn't say, "Stop!"
- * output string: He didn't say, \"Stop!\"
- *
- *
- * @param input String to escape values in, may be null
- * @return String with escaped values, {@code null} if null string input
- */
- public static final String escapeJava(String input) {
-return StringEscapeUtils.escapeJava(input);
- }
-
- public static final String escapeNewLines(String input) {
-if (input == null) {
- return null;
-}
-StringBuilder result = new StringBuilder();
-boolean sawNewline = false;
-for (int i = 0; i < input.length(); i++) {
- char curChar = input.charAt(i);
- if (curChar == '\r' || curChar == '\n') {
-if (sawNewline) {
- continue;
-}
-sawNewline = true;
-result.append("\\n");
- } else {
-sawNewline = false;
-result.append(curChar);
- }
-}
-return result.toString();
- }
-
- /**
- * Copied form commons-lang 2.x code as common-lang 3.x has this API
removed.
- *
(http://commons.apache.org/proper/commons-lang/article3_0.html#StringEscapeUtils.escapeSql)
- * @param str
- * @return
- */
- public static String escapeSql(String str) {
-return (str == null) ? null : StringUtils.replace(str, "'", "''");
- }
-
- /**
- * Return a printable representation of a byte buffer, escaping the
non-printable
- * bytes as '\\xNN' where NN is the hexadecimal representation of such
bytes.
- *
- * This function does not modify the {@code readerIndex} and {@code
writerIndex}
- * of the byte buffer.
- */
- public static String toBinaryString(ByteBuf buf, int strStart, int
strEnd) {
-StringBuilder result = new StringBuilder();
-for (int i = strStart; i < strEnd ; ++i) {
- appendByte(result, buf.getByte(i));
-}
-return result.toString();
- }
-
- /**
- * Return a printable representation of a byte array,
[GitHub] drill pull request #983: DRILL-5819: Default value of security.admin.user_gr...
Github user bitblender commented on a diff in the pull request:
https://github.com/apache/drill/pull/983#discussion_r144143087
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/server/TestOptionsAuthEnabled.java
---
@@ -115,4 +124,59 @@ private void setOptHelper() throws Exception {
test(String.format("ALTER SYSTEM SET `%s` = %d;",
ExecConstants.SLICE_TARGET, ExecConstants.SLICE_TARGET_DEFAULT));
}
}
+
+ @Test
+ public void testAdminUserOptions() throws Exception {
+FixtureBuilder builder = ClusterFixture.builder();
+
+try (ClusterFixture cluster = builder.build();
+ ClientFixture client = cluster.clientFixture()) {
+ OptionManager optionManager =
cluster.drillbit().getContext().getOptionManager();
+
+ // Admin Users Tests
+ // config file should have the 'fake' default admin user and it
should be returned
+ // by the option manager if the option has not been set by the user
+ String configAdminUser =
optionManager.getOption(ExecConstants.ADMIN_USERS_VALIDATOR);;
+ assertEquals(configAdminUser,
ExecConstants.ADMIN_USERS_VALIDATOR.DEFAULT_ADMIN_USERS);
+
+ // Option accessor should never return the 'fake' default from the
config
+ String adminUser1 =
ExecConstants.ADMIN_USERS_VALIDATOR.getAdminUsers(optionManager);
+ assertNotEquals(adminUser1,
ExecConstants.ADMIN_USERS_VALIDATOR.DEFAULT_ADMIN_USERS);
+
+ // Change TEST_ADMIN_USER if necessary
+ String TEST_ADMIN_USER = "ronswanson";
+ if (adminUser1.equals(TEST_ADMIN_USER)) {
+TEST_ADMIN_USER += "thefirst";
+ }
+ // Check if the admin option accessor honors a user-supplied values
+ String sql = String.format("ALTER SYSTEM SET `%s`='%s'",
ExecConstants.ADMIN_USERS_KEY, TEST_ADMIN_USER);
+ client.queryBuilder().sql(sql).run();
+ String adminUser2 =
ExecConstants.ADMIN_USERS_VALIDATOR.getAdminUsers(optionManager);
+ assertEquals(adminUser2, TEST_ADMIN_USER);
+
+ // Admin User Groups Tests
+
+ // config file should have the 'fake' default admin user and it
should be returned
+ // by the option manager if the option has not been set by the user
+ String configAdminUserGroups =
optionManager.getOption(ExecConstants.ADMIN_USER_GROUPS_VALIDATOR);
+ assertEquals(configAdminUserGroups,
ExecConstants.ADMIN_USER_GROUPS_VALIDATOR.DEFAULT_ADMIN_USER_GROUPS);
+
+ // Option accessor should never return the 'fake' default from the
config
+ String adminUserGroups1 =
ExecConstants.ADMIN_USER_GROUPS_VALIDATOR.getAdminUserGroups(optionManager);
+ assertNotEquals(adminUserGroups1,
ExecConstants.ADMIN_USER_GROUPS_VALIDATOR.DEFAULT_ADMIN_USER_GROUPS);
+
+ // Change TEST_ADMIN_USER_GROUPS if necessary
+ String TEST_ADMIN_USER_GROUPS = "yakshavers";
+ if (adminUserGroups1.equals(TEST_ADMIN_USER_GROUPS)) {
+TEST_ADMIN_USER_GROUPS += ",wormracers";
--- End diff --
Done. It was a constant before I changed the code to handle the corner
case.
---
[GitHub] drill pull request #983: DRILL-5819: Default value of security.admin.user_gr...
Github user bitblender commented on a diff in the pull request:
https://github.com/apache/drill/pull/983#discussion_r144143103
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/server/TestOptionsAuthEnabled.java
---
@@ -115,4 +124,59 @@ private void setOptHelper() throws Exception {
test(String.format("ALTER SYSTEM SET `%s` = %d;",
ExecConstants.SLICE_TARGET, ExecConstants.SLICE_TARGET_DEFAULT));
}
}
+
+ @Test
+ public void testAdminUserOptions() throws Exception {
+FixtureBuilder builder = ClusterFixture.builder();
+
+try (ClusterFixture cluster = builder.build();
+ ClientFixture client = cluster.clientFixture()) {
+ OptionManager optionManager =
cluster.drillbit().getContext().getOptionManager();
+
+ // Admin Users Tests
+ // config file should have the 'fake' default admin user and it
should be returned
+ // by the option manager if the option has not been set by the user
+ String configAdminUser =
optionManager.getOption(ExecConstants.ADMIN_USERS_VALIDATOR);;
--- End diff --
Done
---
