You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by bitblender <gi...@git.apache.org> on 2017/10/10 20:10:13 UTC
[GitHub] drill pull request #983: MD-2769: DRILL-5819: Default value of security.admi...
GitHub user bitblender opened a pull request:
https://github.com/apache/drill/pull/983
MD-2769: DRILL-5819: Default value of security.admin.user_groups and security.admin.users is true
The values for admin user/groups in the config file was incorrectly set to "true".
These options have no meaningful static default values. At runtime, the process user/groups should be used as the default admin user/groups. New accessors have been added for these options and these have to be used instead of the usual option manager accessors. Dummy default strings are used in the config file, which when returned by the usual options manager, indicate that the user has not changed these options.
This change implements the above-mentioned items and also adds a UI section to see the current admin user/groups. This UI section is only shown to authenticated admin users.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/bitblender/drill DRILL-5819
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/983.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #983
----
commit 3ba5401a3f6780dbbffd36ef2a9f67b12089a9ef
Author: karthik <km...@maprtech.com>
Date: 2017-10-04T18:23:04Z
MD-2769: DRILL-5819: Default value of security.admin.user_groups and security.admin.users is true
----
---
[GitHub] drill pull request #983: MD-2769: DRILL-5819: Default value of security.admi...
Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/983#discussion_r143910090
--- 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 --
`client.alterSystem(key, value)`
---
[GitHub] drill pull request #983: DRILL-5819: Default value of security.admin.user_gr...
Posted by paul-rogers <gi...@git.apache.org>.
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: MD-2769: DRILL-5819: Default value of security.admi...
Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/983#discussion_r143910606
--- 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();
--- End diff --
No big deal, but if you set no options, then a shorthand is:
```
cluster = ClusterFixture.standardCluster();
```
---
[GitHub] drill pull request #983: MD-2769: DRILL-5819: Default value of security.admi...
Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/983#discussion_r143902334
--- 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:
- * <pre>
- * input string: He didn't say, "Stop!"
- * output string: He didn't say, \"Stop!\"
- * </pre>
- *
- * @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, escaping the non-printable
- * bytes as '\\xNN' where NN is the hexadecimal representation of such bytes.
- */
- public static String toBinaryString(byte[] buf) {
- return toBinaryString(buf, 0, buf.length);
- }
-
- /**
- * Return a printable representation of a byte array, escaping the non-printable
- * bytes as '\\xNN' where NN is the hexadecimal representation of such bytes.
- */
- public static String toBinaryString(byte[] buf, int strStart, int strEnd) {
- StringBuilder result = new StringBuilder();
- for (int i = strStart; i < strEnd; ++i) {
- appendByte(result, buf[i]);
- }
- return result.toString();
- }
-
- private static void appendByte(StringBuilder result, byte b) {
- int ch = b & 0xFF;
- if ( (ch >= '0' && ch <= '9')
- || (ch >= 'A' && ch <= 'Z')
- || (ch >= 'a' && ch <= 'z')
- || " `~!@#$%^&*()-_=+[]{}|;:'\",.<>/?".indexOf(ch) >= 0 ) {
- result.append((char)ch);
- } else {
- result.append(String.format("\\x%02X", ch));
- }
- }
-
- /**
- * parsing a hex encoded binary string and write to an output buffer.
- *
- * This function does not modify the {@code readerIndex} and {@code writerIndex}
- * of the byte buffer.
- *
- * @return Index in the byte buffer just after the last written byte.
- */
- public static int parseBinaryString(ByteBuf str, int strStart, int strEnd, ByteBuf out) {
- int dstEnd = 0;
- for (int i = strStart; i < strEnd; i++) {
- byte b = str.getByte(i);
- if (b == '\\'
- && strEnd > i+3
- && (str.getByte(i+1) == 'x' || str.getByte(i+1) == 'X')) {
- // ok, take next 2 hex digits.
- byte hd1 = str.getByte(i+2);
- byte hd2 = str.getByte(i+3);
- if (isHexDigit(hd1) && isHexDigit(hd2)) { // [a-fA-F0-9]
- // turn hex ASCII digit -> number
- b = (byte) ((toBinaryFromHex(hd1) << 4) + toBinaryFromHex(hd2));
- i += 3; // skip 3
- }
- }
- out.setByte(dstEnd++, b);
- }
- return dstEnd;
- }
-
- /**
- * Takes a ASCII digit in the range A-F0-9 and returns
- * the corresponding integer/ordinal value.
- * @param ch The hex digit.
- * @return The converted hex value as a byte.
- */
- private static byte toBinaryFromHex(byte ch) {
- if ( ch >= 'A' && ch <= 'F' ) {
- return (byte) ((byte)10 + (byte) (ch - 'A'));
- } else if ( ch >= 'a' && ch <= 'f' ) {
- return (byte) ((byte)10 + (byte) (ch - 'a'));
- }
- return (byte) (ch - '0');
- }
-
- private static boolean isHexDigit(byte c) {
- return (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F') || (c >= '0' && c <= '9');
- }
-
-}
+/**
+ * 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:
+ * <pre>
+ * input string: He didn't say, "Stop!"
+ * output string: He didn't say, \"Stop!\"
+ * </pre>
+ *
+ * @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, escaping the non-printable
+ * bytes as '\\xNN' where NN is the hexadecimal representation of such bytes.
+ */
+ public static String toBinaryString(byte[] buf) {
+ return toBinaryString(buf, 0, buf.length);
+ }
+
+ /**
+ * Return a printable representation of a byte array, escaping the non-printable
+ * bytes as '\\xNN' where NN is the hexadecimal representation of such bytes.
+ */
+ public static String toBinaryString(byte[] buf, int strStart, int strEnd) {
+ StringBuilder result = new StringBuilder();
+ for (int i = strStart; i < strEnd; ++i) {
+ appendByte(result, buf[i]);
+ }
+ return result.toString();
+ }
+
+ private static void appendByte(StringBuilder result, byte b) {
+ int ch = b & 0xFF;
+ if ( (ch >= '0' && ch <= '9')
+ || (ch >= 'A' && ch <= 'Z')
+ || (ch >= 'a' && ch <= 'z')
+ || " `~!@#$%^&*()-_=+[]{}|;:'\",.<>/?".indexOf(ch) >= 0 ) {
+ result.append((char)ch);
+ } else {
+ result.append(String.format("\\x%02X", ch));
+ }
+ }
+
+ /**
+ * parsing a hex encoded binary string and write to an output buffer.
+ *
+ * This function does not modify the {@code readerIndex} and {@code writerIndex}
+ * of the byte buffer.
+ *
+ * @return Index in the byte buffer just after the last written byte.
+ */
+ public static int parseBinaryString(ByteBuf str, int strStart, int strEnd, ByteBuf out) {
+ int dstEnd = 0;
+ for (int i = strStart; i < strEnd; i++) {
+ byte b = str.getByte(i);
+ if (b == '\\'
+ && strEnd > i+3
+ && (str.getByte(i+1) == 'x' || str.getByte(i+1) == 'X')) {
+ // ok, take next 2 hex digits.
+ byte hd1 = str.getByte(i+2);
+ byte hd2 = str.getByte(i+3);
+ if (isHexDigit(hd1) && isHexDigit(hd2)) { // [a-fA-F0-9]
+ // turn hex ASCII digit -> number
+ b = (byte) ((toBinaryFromHex(hd1) << 4) + toBinaryFromHex(hd2));
+ i += 3; // skip 3
+ }
+ }
+ out.setByte(dstEnd++, b);
+ }
+ return dstEnd;
+ }
+
+ /**
+ * Takes a ASCII digit in the range A-F0-9 and returns
+ * the corresponding integer/ordinal value.
+ * @param ch The hex digit.
+ * @return The converted hex value as a byte.
+ */
+ private static byte toBinaryFromHex(byte ch) {
+ if ( ch >= 'A' && ch <= 'F' ) {
+ return (byte) ((byte)10 + (byte) (ch - 'A'));
+ } else if ( ch >= 'a' && ch <= 'f' ) {
+ return (byte) ((byte)10 + (byte) (ch - 'a'));
+ }
+ return (byte) (ch - '0');
+ }
+
+ private static boolean isHexDigit(byte c) {
+ return (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F') || (c >= '0' && c <= '9');
+ }
+
+
+ /**
+ * Concatenates the individual strings into a single string,
+ * where each individual string is separated by the provided separator.
+ * Copied from org.apache.commons.lang3.StringUtils (to avoid
+ * bringing in the entire package)
+ * @param array - array holding the strings to be concatenated
+ * @param separator - separator to be used in the concatenation
+ * @return The separator separated concatenated string
+ */
+
+ public static String join(Object[] array, String separator) {
+ return array == null?null:join(array, separator, 0, array.length);
--- End diff --
Not sure why most of this file shows up as deleted and re-added. Something wrong with the commit?
Guava `Joiner.joinOn()`? Of course, Java 8 has a String method for this, but we're not on Java 8 yet...
---
[GitHub] drill pull request #983: DRILL-5819: Default value of security.admin.user_gr...
Posted by bitblender <gi...@git.apache.org>.
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:
- * <pre>
- * input string: He didn't say, "Stop!"
- * output string: He didn't say, \"Stop!\"
- * </pre>
- *
- * @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, escaping the non-printable
- * bytes as '\\xNN' where NN is the hexadecimal representation of such bytes.
- */
- public static String toBinaryString(byte[] buf) {
- return toBinaryString(buf, 0, buf.length);
- }
-
- /**
- * Return a printable representation of a byte array, escaping the non-printable
- * bytes as '\\xNN' where NN is the hexadecimal representation of such bytes.
- */
- public static String toBinaryString(byte[] buf, int strStart, int strEnd) {
- StringBuilder result = new StringBuilder();
- for (int i = strStart; i < strEnd; ++i) {
- appendByte(result, buf[i]);
- }
- return result.toString();
- }
-
- private static void appendByte(StringBuilder result, byte b) {
- int ch = b & 0xFF;
- if ( (ch >= '0' && ch <= '9')
- || (ch >= 'A' && ch <= 'Z')
- || (ch >= 'a' && ch <= 'z')
- || " `~!@#$%^&*()-_=+[]{}|;:'\",.<>/?".indexOf(ch) >= 0 ) {
- result.append((char)ch);
- } else {
- result.append(String.format("\\x%02X", ch));
- }
- }
-
- /**
- * parsing a hex encoded binary string and write to an output buffer.
- *
- * This function does not modify the {@code readerIndex} and {@code writerIndex}
- * of the byte buffer.
- *
- * @return Index in the byte buffer just after the last written byte.
- */
- public static int parseBinaryString(ByteBuf str, int strStart, int strEnd, ByteBuf out) {
- int dstEnd = 0;
- for (int i = strStart; i < strEnd; i++) {
- byte b = str.getByte(i);
- if (b == '\\'
- && strEnd > i+3
- && (str.getByte(i+1) == 'x' || str.getByte(i+1) == 'X')) {
- // ok, take next 2 hex digits.
- byte hd1 = str.getByte(i+2);
- byte hd2 = str.getByte(i+3);
- if (isHexDigit(hd1) && isHexDigit(hd2)) { // [a-fA-F0-9]
- // turn hex ASCII digit -> number
- b = (byte) ((toBinaryFromHex(hd1) << 4) + toBinaryFromHex(hd2));
- i += 3; // skip 3
- }
- }
- out.setByte(dstEnd++, b);
- }
- return dstEnd;
- }
-
- /**
- * Takes a ASCII digit in the range A-F0-9 and returns
- * the corresponding integer/ordinal value.
- * @param ch The hex digit.
- * @return The converted hex value as a byte.
- */
- private static byte toBinaryFromHex(byte ch) {
- if ( ch >= 'A' && ch <= 'F' ) {
- return (byte) ((byte)10 + (byte) (ch - 'A'));
- } else if ( ch >= 'a' && ch <= 'f' ) {
- return (byte) ((byte)10 + (byte) (ch - 'a'));
- }
- return (byte) (ch - '0');
- }
-
- private static boolean isHexDigit(byte c) {
- return (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F') || (c >= '0' && c <= '9');
- }
-
-}
+/**
+ * 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:
+ * <pre>
+ * input string: He didn't say, "Stop!"
+ * output string: He didn't say, \"Stop!\"
+ * </pre>
+ *
+ * @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, escaping the non-printable
+ * bytes as '\\xNN' where NN is the hexadecimal representation of such bytes.
+ */
+ public static String toBinaryString(byte[] buf) {
+ return toBinaryString(buf, 0, buf.length);
+ }
+
+ /**
+ * Return a printable representation of a byte array, escaping the non-printable
+ * bytes as '\\xNN' where NN is the hexadecimal representation of such bytes.
+ */
+ public static String toBinaryString(byte[] buf, int strStart, int strEnd) {
+ StringBuilder result = new StringBuilder();
+ for (int i = strStart; i < strEnd; ++i) {
+ appendByte(result, buf[i]);
+ }
+ return result.toString();
+ }
+
+ private static void appendByte(StringBuilder result, byte b) {
+ int ch = b & 0xFF;
+ if ( (ch >= '0' && ch <= '9')
+ || (ch >= 'A' && ch <= 'Z')
+ || (ch >= 'a' && ch <= 'z')
+ || " `~!@#$%^&*()-_=+[]{}|;:'\",.<>/?".indexOf(ch) >= 0 ) {
+ result.append((char)ch);
+ } else {
+ result.append(String.format("\\x%02X", ch));
+ }
+ }
+
+ /**
+ * parsing a hex encoded binary string and write to an output buffer.
+ *
+ * This function does not modify the {@code readerIndex} and {@code writerIndex}
+ * of the byte buffer.
+ *
+ * @return Index in the byte buffer just after the last written byte.
+ */
+ public static int parseBinaryString(ByteBuf str, int strStart, int strEnd, ByteBuf out) {
+ int dstEnd = 0;
+ for (int i = strStart; i < strEnd; i++) {
+ byte b = str.getByte(i);
+ if (b == '\\'
+ && strEnd > i+3
+ && (str.getByte(i+1) == 'x' || str.getByte(i+1) == 'X')) {
+ // ok, take next 2 hex digits.
+ byte hd1 = str.getByte(i+2);
+ byte hd2 = str.getByte(i+3);
+ if (isHexDigit(hd1) && isHexDigit(hd2)) { // [a-fA-F0-9]
+ // turn hex ASCII digit -> number
+ b = (byte) ((toBinaryFromHex(hd1) << 4) + toBinaryFromHex(hd2));
+ i += 3; // skip 3
+ }
+ }
+ out.setByte(dstEnd++, b);
+ }
+ return dstEnd;
+ }
+
+ /**
+ * Takes a ASCII digit in the range A-F0-9 and returns
+ * the corresponding integer/ordinal value.
+ * @param ch The hex digit.
+ * @return The converted hex value as a byte.
+ */
+ private static byte toBinaryFromHex(byte ch) {
+ if ( ch >= 'A' && ch <= 'F' ) {
+ return (byte) ((byte)10 + (byte) (ch - 'A'));
+ } else if ( ch >= 'a' && ch <= 'f' ) {
+ return (byte) ((byte)10 + (byte) (ch - 'a'));
+ }
+ return (byte) (ch - '0');
+ }
+
+ private static boolean isHexDigit(byte c) {
+ return (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F') || (c >= '0' && c <= '9');
+ }
+
+
+ /**
+ * Concatenates the individual strings into a single string,
+ * where each individual string is separated by the provided separator.
+ * Copied from org.apache.commons.lang3.StringUtils (to avoid
+ * bringing in the entire package)
+ * @param array - array holding the strings to be concatenated
+ * @param separator - separator to be used in the concatenation
+ * @return The separator separated concatenated string
+ */
+
+ public static String join(Object[] array, String separator) {
+ return array == null?null:join(array, separator, 0, array.length);
--- End diff --
The original file has CRLF line termination. Hence the full diff.
---
[GitHub] drill issue #983: MD-2769: DRILL-5819: Default value of security.admin.user_...
Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on the issue:
https://github.com/apache/drill/pull/983
MD? Please remove that number from the PR title; we need only the Drill tickets here.
---
[GitHub] drill pull request #983: DRILL-5819: Default value of security.admin.user_gr...
Posted by bitblender <gi...@git.apache.org>.
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
---
[GitHub] drill pull request #983: DRILL-5819: Default value of security.admin.user_gr...
Posted by bitblender <gi...@git.apache.org>.
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...
Posted by bitblender <gi...@git.apache.org>.
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: MD-2769: DRILL-5819: Default value of security.admi...
Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/983#discussion_r143910033
--- 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 --
one semicolon should be enough...
---
[GitHub] drill pull request #983: DRILL-5819: Default value of security.admin.user_gr...
Posted by bitblender <gi...@git.apache.org>.
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: MD-2769: DRILL-5819: Default value of security.admi...
Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/983#discussion_r143910336
--- 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 --
What if the user provided the list with spaces: "a, b, c"? Actually, in the above, it is fine; HTML will compress the run of spaces to a single one for display...
But, what about where we do the real check? Do the unit tests cover this case?
---
[GitHub] drill pull request #983: MD-2769: DRILL-5819: Default value of security.admi...
Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/983#discussion_r143910712
--- 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 --
Usually ALL_CAPS is reserved for constants, which means we can't alter them as we do here... I suspect you mean `testAdminUserGroups`...
---
[GitHub] drill pull request #983: MD-2769: DRILL-5819: Default value of security.admi...
Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/983#discussion_r143910797
--- 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 --
Should the tests include actually checking if the user and/or group matches? This will check for problems such as spaces before/after the names, etc.
---
[GitHub] drill pull request #983: DRILL-5819: Default value of security.admin.user_gr...
Posted by asfgit <gi...@git.apache.org>.
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...
Posted by bitblender <gi...@git.apache.org>.
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
---