This is an automated email from the ASF dual-hosted git repository. volodymyr pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/drill.git
commit 7aa698336d963dbb7f03ae7554012cf769bfda83 Author: Dobes Vandermeer <[email protected]> AuthorDate: Tue Feb 25 11:57:34 2020 -0800 DRILL-7203: Accept impersonation userName as form field & fix back button for query page Requiring the impersonation username to be sent as an HTTP header made it difficult to implement the Web UI submission. This changes it so that the userName can be sent as a form field instead, allowing the Web UI to do a "normal" form submission, which fixes the back button, avoids the use of the deprecated "document.open" API call, and may avoid other glitches in the future. closes #1994 --- .../org/apache/drill/exec/server/Drillbit.java | 6 ++ .../drill/exec/server/rest/QueryResources.java | 6 +- .../drill/exec/server/rest/QueryWrapper.java | 45 ++++++++++++-- .../drill/exec/server/rest/UserNameFilter.java | 61 ------------------- .../apache/drill/exec/server/rest/WebServer.java | 6 -- .../src/main/resources/rest/query/query.ftl | 13 ++-- .../resources/rest/static/js/querySubmission.js | 23 +------- .../drill/exec/server/rest/RestServerTest.java | 69 ++++++++++++++++++++++ .../drill/exec/server/rest/TestQueryWrapper.java | 60 +++++++++++++++++++ .../server/rest/TestQueryWrapperImpersonation.java | 53 +++++++++++++++++ 10 files changed, 240 insertions(+), 102 deletions(-) diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java index 180b2e0..3bf20e0 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java @@ -207,6 +207,12 @@ public class Drillbit implements AutoCloseable { return webServer.getPort(); } + @VisibleForTesting + public WorkManager getManager() { return manager; } + + @VisibleForTesting + public WebServer getWebServer() { return webServer; } + public void run() throws Exception { final Stopwatch w = Stopwatch.createStarted(); logger.debug("Startup begun."); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryResources.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryResources.java index 75d6653..bf07fae 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryResources.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryResources.java @@ -42,7 +42,6 @@ import javax.ws.rs.Path; import javax.ws.rs.Produces; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.SecurityContext; - import java.util.Collections; import java.util.List; import java.util.Map; @@ -96,10 +95,11 @@ public class QueryResources { @Produces(MediaType.TEXT_HTML) public Viewable submitQuery(@FormParam("query") String query, @FormParam("queryType") String queryType, - @FormParam("autoLimit") String autoLimit) throws Exception { + @FormParam("autoLimit") String autoLimit, + @FormParam("userName") String userName) throws Exception { try { final String trimmedQueryString = CharMatcher.is(';').trimTrailingFrom(query.trim()); - final QueryResult result = submitQueryJSON(new QueryWrapper(trimmedQueryString, queryType, autoLimit)); + final QueryResult result = submitQueryJSON(new QueryWrapper(trimmedQueryString, queryType, autoLimit, userName)); List<Integer> rowsPerPageValues = work.getContext().getConfig().getIntList(ExecConstants.HTTP_WEB_CLIENT_RESULTSET_ROWS_PER_PAGE_VALUES); Collections.sort(rowsPerPageValues); final String rowsPerPageValuesAsStr = Joiner.on(",").join(rowsPerPageValues); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryWrapper.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryWrapper.java index 5a4ce80..3b99491 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryWrapper.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryWrapper.java @@ -19,20 +19,24 @@ package org.apache.drill.exec.server.rest; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; - +import org.apache.drill.common.config.DrillConfig; import org.apache.drill.common.exceptions.UserException; import org.apache.drill.common.exceptions.UserRemoteException; import org.apache.drill.exec.ExecConstants; +import org.apache.drill.exec.proto.UserBitShared; import org.apache.drill.exec.proto.UserBitShared.QueryId; import org.apache.drill.exec.proto.UserBitShared.QueryResult.QueryState; import org.apache.drill.exec.proto.UserBitShared.QueryType; +import org.apache.drill.exec.proto.UserProtos.QueryResultsMode; import org.apache.drill.exec.proto.UserProtos.RunQuery; import org.apache.drill.exec.proto.helper.QueryIdHelper; -import org.apache.drill.exec.proto.UserProtos.QueryResultsMode; +import org.apache.drill.exec.rpc.user.InboundImpersonationManager; +import org.apache.drill.exec.server.options.SessionOptionManager; +import org.apache.drill.exec.util.ImpersonationUtil; import org.apache.drill.exec.work.WorkManager; +import org.apache.parquet.Strings; import javax.xml.bind.annotation.XmlRootElement; - import java.lang.management.ManagementFactory; import java.lang.management.MemoryMXBean; import java.util.Collection; @@ -48,13 +52,20 @@ public class QueryWrapper { private final String queryType; private final int autoLimitRowCount; + private final String userName; + private static MemoryMXBean memMXBean = ManagementFactory.getMemoryMXBean(); @JsonCreator - public QueryWrapper(@JsonProperty("query") String query, @JsonProperty("queryType") String queryType, @JsonProperty("autoLimit") String autoLimit) { + public QueryWrapper( + @JsonProperty("query") String query, + @JsonProperty("queryType") String queryType, + @JsonProperty("autoLimit") String autoLimit, + @JsonProperty("userName") String userName) { this.query = query; this.queryType = queryType.toUpperCase(); this.autoLimitRowCount = autoLimit != null && autoLimit.matches("[0-9]+") ? Integer.valueOf(autoLimit) : 0; + this.userName = userName; } public String getQuery() { @@ -76,6 +87,8 @@ public class QueryWrapper { .setAutolimitRowcount(autoLimitRowCount) .build(); + applyUserName(workManager, webUserConnection); + int defaultMaxRows = webUserConnection.getSession().getOptions().getOption(ExecConstants.QUERY_MAX_ROWS).num_val.intValue(); int maxRows; if (autoLimitRowCount > 0 && defaultMaxRows > 0) { @@ -131,6 +144,30 @@ public class QueryWrapper { return new QueryResult(queryId, webUserConnection, webUserConnection.results); } + private void applyUserName(WorkManager workManager, WebUserConnection webUserConnection) { + SessionOptionManager options = webUserConnection.getSession().getOptions(); + DrillConfig config = workManager.getContext().getConfig(); + if (!Strings.isNullOrEmpty(userName)) { + if (!config.getBoolean(ExecConstants.IMPERSONATION_ENABLED)) { + throw UserException.permissionError().message("User impersonation is not enabled").build(logger); + } + InboundImpersonationManager inboundImpersonationManager = new InboundImpersonationManager(); + boolean isAdmin = !config.getBoolean(ExecConstants.USER_AUTHENTICATION_ENABLED) || + ImpersonationUtil.hasAdminPrivileges(webUserConnection.getSession().getCredentials().getUserName(), + ExecConstants.ADMIN_USERS_VALIDATOR.getAdminUsers(options), + ExecConstants.ADMIN_USER_GROUPS_VALIDATOR.getAdminUserGroups(options)); + if (isAdmin) { + // Admin user can impersonate any user they want to (when authentication is disabled, all users are admin) + webUserConnection.getSession().replaceUserCredentials( + inboundImpersonationManager, + UserBitShared.UserCredentials.newBuilder().setUserName(userName).build()); + } else { + // Check configured impersonation rules to see if this user is allowed to impersonate the given user + inboundImpersonationManager.replaceUserOnSession(userName, webUserConnection.getSession()); + } + } + } + //Detect possible excess heap private float getHeapUsage() { return (float) memMXBean.getHeapMemoryUsage().getUsed() / memMXBean.getHeapMemoryUsage().getMax(); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/UserNameFilter.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/UserNameFilter.java deleted file mode 100644 index 90736a1..0000000 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/UserNameFilter.java +++ /dev/null @@ -1,61 +0,0 @@ -/* - * 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.exec.server.rest; - -import org.apache.drill.shaded.guava.com.google.common.base.Strings; - -import javax.servlet.Filter; -import javax.servlet.FilterChain; -import javax.servlet.FilterConfig; -import javax.servlet.ServletException; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import javax.ws.rs.HttpMethod; -import java.io.IOException; - -/** - * Responsible for filtering out POST requests that do not contain valid <b>User-Name</b> header. - */ -public class UserNameFilter implements Filter { - - @Override - public void init(FilterConfig filterConfig) throws ServletException { - - } - - @Override - public void doFilter(ServletRequest req, ServletResponse resp, FilterChain chain) throws IOException, ServletException { - HttpServletRequest request = (HttpServletRequest) req; - HttpServletResponse response = (HttpServletResponse) resp; - - if (HttpMethod.POST.equalsIgnoreCase(request.getMethod()) && Strings.isNullOrEmpty(request.getHeader("User-Name"))) { - response.reset(); - response.sendError(HttpServletResponse.SC_PRECONDITION_FAILED, "User-Name header is not set"); - return; - } - - chain.doFilter(request, response); - } - - @Override - public void destroy() { - - } -} \ No newline at end of file diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java index 90fd629..b648de5 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java @@ -248,12 +248,6 @@ public class WebServer implements AutoCloseable { servletContextHandler.addFilter(CsrfTokenValidateFilter.class, path, EnumSet.of(DispatcherType.REQUEST)); } - if (isOnlyImpersonationEnabled(workManager.getContext().getConfig())) { - for (String path : new String[]{"/query", "/query.json"}) { - servletContextHandler.addFilter(UserNameFilter.class, path, EnumSet.of(DispatcherType.REQUEST)); - } - } - if (config.getBoolean(ExecConstants.HTTP_CORS_ENABLED)) { FilterHolder holder = new FilterHolder(CrossOriginFilter.class); holder.setInitParameter(CrossOriginFilter.ALLOWED_ORIGINS_PARAM, diff --git a/exec/java-exec/src/main/resources/rest/query/query.ftl b/exec/java-exec/src/main/resources/rest/query/query.ftl index a20cf3e..520a93b 100644 --- a/exec/java-exec/src/main/resources/rest/query/query.ftl +++ b/exec/java-exec/src/main/resources/rest/query/query.ftl @@ -43,15 +43,14 @@ <#include "*/runningQuery.ftl"> - <#if model.isOnlyImpersonationEnabled()> - <div class="form-group"> - <label for="userName">User Name</label> - <input type="text" size="30" name="userName" id="userName" placeholder="User Name"> - </div> - </#if> - <form role="form" id="queryForm" action="/query" method="POST"> + <#if model.isOnlyImpersonationEnabled()> <div class="form-group"> + <label for="userName">User Name</label> + <input type="text" size="30" name="userName" id="userName" placeholder="User Name"> + </div> + </#if> + <div class="form-group"> <label for="queryType">Query Type</label> <div class="radio"> <label> diff --git a/exec/java-exec/src/main/resources/rest/static/js/querySubmission.js b/exec/java-exec/src/main/resources/rest/static/js/querySubmission.js index 25c85dd..99d50ab 100644 --- a/exec/java-exec/src/main/resources/rest/static/js/querySubmission.js +++ b/exec/java-exec/src/main/resources/rest/static/js/querySubmission.js @@ -83,25 +83,6 @@ function doSubmitQueryWithAutoLimit() { //Submit Query function submitQuery() { popupAndWait(); - //Submit query - $.ajax({ - type: "POST", - beforeSend: function (request) { - if (typeof userName !== 'undefined' && userName !== null && userName.length > 0) { - request.setRequestHeader("User-Name", userName); - } - }, - url: "/query", - data: $("#queryForm").serializeArray(), - success: function (response) { - closePopup(); - var newDoc = document.open("text/html", "replace"); - newDoc.write(response); - newDoc.close(); - }, - error: function (request, textStatus, errorThrown) { - closePopup(); - alert(errorThrown); - } - }); + $("#queryForm").submit(); + $(window).bind("pageshow", function(event) { closePopup();}); } diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/server/rest/RestServerTest.java b/exec/java-exec/src/test/java/org/apache/drill/exec/server/rest/RestServerTest.java new file mode 100644 index 0000000..7136a7d --- /dev/null +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/server/rest/RestServerTest.java @@ -0,0 +1,69 @@ +/* + * 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.exec.server.rest; + +import io.netty.channel.DefaultChannelPromise; +import io.netty.channel.local.LocalAddress; +import org.apache.drill.exec.proto.UserBitShared; +import org.apache.drill.exec.proto.helper.QueryIdHelper; +import org.apache.drill.exec.rpc.user.UserSession; +import org.apache.drill.exec.server.options.SystemOptionManager; +import org.apache.drill.exec.server.rest.auth.DrillUserPrincipal; +import org.apache.drill.exec.work.WorkManager; +import org.apache.drill.exec.work.foreman.Foreman; +import org.apache.drill.test.ClusterTest; + +public class RestServerTest extends ClusterTest { + protected QueryWrapper.QueryResult runQuery(String sql) throws Exception { + return runQuery(new QueryWrapper(sql, "SQL", null, null)); + } + + protected QueryWrapper.QueryResult runQueryWithUsername(String sql, String userName) throws Exception { + return runQuery(new QueryWrapper(sql, "SQL", null, userName)); + } + + protected QueryWrapper.QueryResult runQuery(QueryWrapper q) throws Exception { + SystemOptionManager systemOptions = cluster.drillbit().getContext().getOptionManager(); + DrillUserPrincipal principal = new DrillUserPrincipal.AnonDrillUserPrincipal(); + WebSessionResources webSessionResources = new WebSessionResources( + cluster.drillbit().getContext().getAllocator(), + new LocalAddress("test"), + UserSession.Builder.newBuilder() + .withOptionManager(systemOptions) + .withCredentials(UserBitShared.UserCredentials.newBuilder().setUserName(principal.getName()).build()) + .build(), + new DefaultChannelPromise(null)); + WebUserConnection connection = new WebUserConnection.AnonWebUserConnection(webSessionResources); + return q.run(cluster.drillbit().getManager(), connection); + } + + + protected UserBitShared.QueryProfile getQueryProfile(QueryWrapper.QueryResult result) { + String queryId = result.getQueryId(); + WorkManager workManager = cluster.drillbit().getManager(); + Foreman f = workManager.getBee().getForemanForQueryId(QueryIdHelper.getQueryIdFromString(queryId)); + if (f != null) { + UserBitShared.QueryProfile queryProfile = f.getQueryManager().getQueryProfile(); + if (queryProfile != null) { + return queryProfile; + } + } + return workManager.getContext().getProfileStoreContext().getCompletedProfileStore().get(queryId); + } + +} diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/server/rest/TestQueryWrapper.java b/exec/java-exec/src/test/java/org/apache/drill/exec/server/rest/TestQueryWrapper.java new file mode 100644 index 0000000..ed8ac2d --- /dev/null +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/server/rest/TestQueryWrapper.java @@ -0,0 +1,60 @@ +/* + * 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.exec.server.rest; + +import org.apache.drill.common.exceptions.UserException; +import org.apache.drill.exec.ExecConstants; +import org.apache.drill.test.ClusterFixture; +import org.junit.BeforeClass; +import org.junit.Test; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; + +public class TestQueryWrapper extends RestServerTest { + + @BeforeClass + public static void setupServer() throws Exception { + startCluster(ClusterFixture.bareBuilder(dirTestWatcher) + .clusterSize(1) + .configProperty(ExecConstants.ALLOW_LOOPBACK_ADDRESS_BINDING, true)); + } + + @Test + public void testShowSchemas() throws Exception { + QueryWrapper.QueryResult result = runQuery("SHOW SCHEMAS"); + assertEquals("COMPLETED", result.queryState); + assertNotEquals(0, result.rows.size()); + assertEquals(1, result.columns.size()); + assertEquals(result.columns.iterator().next(), "SCHEMA_NAME"); + } + + @Test + public void testImpersonationDisabled() throws Exception { + try { + QueryWrapper q = new QueryWrapper("SHOW SCHEMAS", "SQL", null, "alfred"); + runQuery(q); + fail("Should have thrown exception"); + } catch (UserException e) { + assertThat(e.getMessage(), containsString("User impersonation is not enabled")); + } + } +} diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/server/rest/TestQueryWrapperImpersonation.java b/exec/java-exec/src/test/java/org/apache/drill/exec/server/rest/TestQueryWrapperImpersonation.java new file mode 100644 index 0000000..fef8f1a --- /dev/null +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/server/rest/TestQueryWrapperImpersonation.java @@ -0,0 +1,53 @@ +/* + * 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.exec.server.rest; + +import org.apache.drill.exec.ExecConstants; +import org.apache.drill.exec.proto.UserBitShared; +import org.apache.drill.test.ClusterFixture; +import org.junit.BeforeClass; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +public final class TestQueryWrapperImpersonation extends RestServerTest { + @BeforeClass + public static void setupServer() throws Exception { + startCluster(ClusterFixture.bareBuilder(dirTestWatcher) + .configProperty(ExecConstants.ALLOW_LOOPBACK_ADDRESS_BINDING, true) + .configProperty(ExecConstants.IMPERSONATION_ENABLED, true)); + } + + @Test + public void testImpersonation() throws Exception { + QueryWrapper.QueryResult result = runQueryWithUsername("SELECT CATALOG_NAME, SCHEMA_NAME FROM information_schema.SCHEMATA", "alfred"); + UserBitShared.QueryProfile queryProfile = getQueryProfile(result); + assertNotNull(queryProfile); + assertEquals("alfred", queryProfile.getUser()); + } + + @Test + public void testImpersonationEnabledButUserNameNotProvided() throws Exception { + QueryWrapper.QueryResult result = runQueryWithUsername("SELECT CATALOG_NAME, SCHEMA_NAME FROM information_schema.SCHEMATA", null); + UserBitShared.QueryProfile queryProfile = getQueryProfile(result); + assertNotNull(queryProfile); + assertEquals("anonymous", queryProfile.getUser()); + } + +}
