This is an automated email from the ASF dual-hosted git repository.
dzamo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/drill.git
The following commit(s) were added to refs/heads/master by this push:
new bfbf322 DRILL-8168: Do not duplicate attempts to impersonate a user
in the REST API (#2495)
bfbf322 is described below
commit bfbf32217d987b51d5f4b694486ba24b715f5239
Author: James Turton <[email protected]>
AuthorDate: Wed Mar 16 12:12:05 2022 +0200
DRILL-8168: Do not duplicate attempts to impersonate a user in the REST API
(#2495)
---
.../java/org/apache/drill/exec/ExecConstants.java | 2 +-
.../drill/exec/server/rest/BaseQueryRunner.java | 55 +++++++++++++---------
.../drill/exec/server/rest/DrillRestServer.java | 8 ++++
3 files changed, 42 insertions(+), 23 deletions(-)
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
index 312a9c4..1ca51bc 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
@@ -274,7 +274,7 @@ public final class ExecConstants {
public static final String HTTP_WEB_CLIENT_RESULTSET_AUTOLIMIT_CHECKED =
"drill.exec.http.web.client.resultset.autolimit.checked";
public static final String HTTP_WEB_CLIENT_RESULTSET_AUTOLIMIT_ROWS =
"drill.exec.http.web.client.resultset.autolimit.rows";
public static final String HTTP_WEB_CLIENT_RESULTSET_ROWS_PER_PAGE_VALUES =
"drill.exec.http.web.client.resultset.rowsPerPageValues";
- //Control Heap usage runaway
+ @Deprecated // TODO: Remove any logic based on this option now that REST
query results stream.
public static final String HTTP_MEMORY_HEAP_FAILURE_THRESHOLD =
"drill.exec.http.memory.heap.failure.threshold";
//Customize filters in options
public static final String HTTP_WEB_OPTIONS_FILTERS =
"drill.exec.http.web.options.filters";
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/BaseQueryRunner.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/BaseQueryRunner.java
index 842cb8e..17caa14 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/BaseQueryRunner.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/BaseQueryRunner.java
@@ -57,28 +57,39 @@ public abstract class BaseQueryRunner {
}
protected void applyUserName(String userName) {
- if (!Strings.isNullOrEmpty(userName)) {
- DrillConfig config = workManager.getContext().getConfig();
- 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());
- }
+ if (Strings.isNullOrEmpty(userName)) {
+ return;
+ }
+
+ DrillConfig config = workManager.getContext().getConfig();
+ if (!config.getBoolean(ExecConstants.IMPERSONATION_ENABLED)) {
+ throw UserException.permissionError()
+ .message("User impersonation is not enabled")
+ .build(logger);
+ }
+
+ String proxyUserName =
webUserConnection.getSession().getCredentials().getUserName();
+ if (proxyUserName.equals(userName)) {
+ // Either the proxy user is impersonating itself, which is a no-op, or
+ // the userName on the UserSession has already been modified to be the
+ // impersonated user by an earlier request belonging to the same session.
+ return;
+ }
+
+ InboundImpersonationManager inboundImpersonationManager = new
InboundImpersonationManager();
+ boolean isAdmin =
!config.getBoolean(ExecConstants.USER_AUTHENTICATION_ENABLED) ||
+ ImpersonationUtil.hasAdminPrivileges(
+ proxyUserName,
+ 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());
}
}
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java
index 30dcb2d..1639d58 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java
@@ -25,6 +25,7 @@ import io.swagger.v3.oas.annotations.OpenAPIDefinition;
import io.swagger.v3.oas.annotations.info.Contact;
import io.swagger.v3.oas.annotations.info.Info;
import io.swagger.v3.oas.annotations.info.License;
+
import org.apache.drill.shaded.guava.com.google.common.base.Strings;
import freemarker.cache.ClassTemplateLoader;
import freemarker.cache.FileTemplateLoader;
@@ -321,7 +322,14 @@ public class DrillRestServer extends ResourceConfig {
* @param config drill config
* @param request client request
* @return session user principal
+ *
+ * @deprecated a userName property has since been added to POST
/query.json.
+ * and the web UI now never sets a User-Name header. The restriction to
+ * unauthenticated Drill is also not enough for general impersonation.
+ * Choose one way to request impersonation over HTTP, this or the other.
+ * @link{org.apache.drill.exec.server.rest.QueryResources#submitQuery}
*/
+ @Deprecated
private Principal createSessionUserPrincipal(DrillConfig config,
HttpServletRequest request) {
if (WebServer.isOnlyImpersonationEnabled(config)) {
final String userName = request.getHeader("User-Name");
