[
https://issues.apache.org/jira/browse/KNOX-2839?focusedWorklogId=829684&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-829684
]
ASF GitHub Bot logged work on KNOX-2839:
----------------------------------------
Author: ASF GitHub Bot
Created on: 29/Nov/22 11:39
Start Date: 29/Nov/22 11:39
Worklog Time Spent: 10m
Work Description: smolnar82 commented on code in PR #681:
URL: https://github.com/apache/knox/pull/681#discussion_r1034635599
##########
gateway-provider-identity-assertion-common/src/main/java/org/apache/knox/gateway/IdentityAsserterMessages.java:
##########
@@ -50,4 +51,16 @@ public interface IdentityAsserterMessages {
@Message( level = MessageLevel.INFO, text = "Using configured impersonation
parameters: {0}")
void impersonationConfig(String config);
+
+ @Message( level = MessageLevel.WARN, text = "Configured proxyuser parameters
are IGNORED because the HadoopAuth filter already has them.")
Review Comment:
Ack; I'll change that.
##########
gateway-provider-identity-assertion-common/src/main/java/org/apache/knox/gateway/identityasserter/common/filter/CommonIdentityAssertionFilter.java:
##########
@@ -84,57 +98,86 @@ public void init(FilterConfig filterConfig) throws
ServletException {
throw new ServletException("Unable to load principal mapping table.",
e);
}
}
- virtualGroupMapper = new
VirtualGroupMapper(loadVirtualGroups(filterConfig));
- String impersonationListFromConfig =
filterConfig.getInitParameter(IMPERSONATION_PARAMS);
- if (impersonationListFromConfig == null ||
impersonationListFromConfig.isEmpty()) {
- impersonationListFromConfig =
filterConfig.getServletContext().getInitParameter(IMPERSONATION_PARAMS);
- }
- initImpersonationParamsList(impersonationListFromConfig);
+
+ //subsequent iterations on filterConfig.getInitParameterNames() would not
work if we simply used that as an enumeration
Review Comment:
I'll add a more detailed explanation in the code too: `getInitParameters()`
returns an enumeration and the first time we iterate thru on its element we can
process the parameter names (`hasMoreElements` returns `true`). The subsequent
calls, however, will not succeed because `getInitParameters()` returns the same
object where the `hasMoreElements` returns `false`. In this class, the first
event is populating virtual group mapping and then we do the same for proxyuser
config.
##########
gateway-provider-identity-assertion-common/src/main/java/org/apache/knox/gateway/identityasserter/common/filter/CommonIdentityAssertionFilter.java:
##########
@@ -84,57 +98,86 @@ public void init(FilterConfig filterConfig) throws
ServletException {
throw new ServletException("Unable to load principal mapping table.",
e);
}
}
- virtualGroupMapper = new
VirtualGroupMapper(loadVirtualGroups(filterConfig));
- String impersonationListFromConfig =
filterConfig.getInitParameter(IMPERSONATION_PARAMS);
- if (impersonationListFromConfig == null ||
impersonationListFromConfig.isEmpty()) {
- impersonationListFromConfig =
filterConfig.getServletContext().getInitParameter(IMPERSONATION_PARAMS);
- }
- initImpersonationParamsList(impersonationListFromConfig);
+
+ //subsequent iterations on filterConfig.getInitParameterNames() would not
work if we simply used that as an enumeration
+ final List<String> initParameterNames =
filterConfig.getInitParameterNames() == null ? Collections.emptyList()
+ : Collections.list(filterConfig.getInitParameterNames());
+
+ virtualGroupMapper = new
VirtualGroupMapper(loadVirtualGroups(filterConfig, initParameterNames));
+
+ initImpersonationParamsList(filterConfig);
+ initProxyUserConfiguration(filterConfig, initParameterNames);
}
- /**
+ /*
* Initialize the impersonation params list.
* This list contains query params that needs to be scrubbed
* from the outgoing request.
- * @param impersonationListFromConfig
- * @return
*/
- private void initImpersonationParamsList(final String
impersonationListFromConfig) {
+ private void initImpersonationParamsList(FilterConfig filterConfig) {
+ String impersonationListFromConfig =
filterConfig.getInitParameter(IMPERSONATION_PARAMS);
+ if (impersonationListFromConfig == null ||
impersonationListFromConfig.isEmpty()) {
+ impersonationListFromConfig =
filterConfig.getServletContext().getInitParameter(IMPERSONATION_PARAMS);
+ }
+
/* Add default impersonation params */
impersonationParamsList.add(DOAS_PRINCIPAL_PARAM);
impersonationParamsList.add(PRINCIPAL_PARAM);
- if(null == impersonationListFromConfig ||
impersonationListFromConfig.isEmpty()) {
- return;
- } else {
+
+ if (impersonationListFromConfig != null &&
!impersonationListFromConfig.isEmpty()) {
/* Add configured impersonation params */
LOG.impersonationConfig(impersonationListFromConfig);
final StringTokenizer t = new
StringTokenizer(impersonationListFromConfig, ",");
- while(t.hasMoreElements()) {
+ while (t.hasMoreElements()) {
final String token = t.nextToken().trim();
- if(!impersonationParamsList.contains(token)) {
+ if (!impersonationParamsList.contains(token)) {
impersonationParamsList.add(token);
}
}
}
}
- private Map<String, AbstractSyntaxTree> loadVirtualGroups(FilterConfig
filterConfig) {
+ private void initProxyUserConfiguration(FilterConfig filterConfig,
List<String> initParameterNames) {
+ final String impersonationEnabledValue =
filterConfig.getInitParameter(IMPERSONATION_ENABLED_PARAM);
+ this.impersonationEnabled = impersonationEnabledValue == null ?
Boolean.FALSE : Boolean.parseBoolean(impersonationEnabledValue);
Review Comment:
Because my IDE put it there :) It really does not matter, but since it's
very rare that we use in Knox's codebase I'll remove that from here.
##########
gateway-provider-identity-assertion-common/src/main/java/org/apache/knox/gateway/IdentityAsserterMessages.java:
##########
@@ -50,4 +51,16 @@ public interface IdentityAsserterMessages {
@Message( level = MessageLevel.INFO, text = "Using configured impersonation
parameters: {0}")
void impersonationConfig(String config);
+
+ @Message( level = MessageLevel.WARN, text = "Configured proxyuser parameters
are IGNORED because the HadoopAuth filter already has them.")
+ void ignoreProxyuserConfig();
+
+ @Message( level = MessageLevel.DEBUG, text = "doAsUser = {0}, RemoteUser =
{1} , RemoteAddress = {2}" )
+ void hadoopAuthDoAsUser(String doAsUser, String remoteUser, String
remoteAddr);
Review Comment:
It's kind of correlating to Hadoop's authentication in a way such as it uses
the Hadoop authorization library for proxyuser authorization.
Issue Time Tracking
-------------------
Worklog Id: (was: 829684)
Time Spent: 1h 10m (was: 1h)
> Refactor impersonation from KnoxToken service
> ---------------------------------------------
>
> Key: KNOX-2839
> URL: https://issues.apache.org/jira/browse/KNOX-2839
> Project: Apache Knox
> Issue Type: Task
> Components: Server
> Reporter: Sandor Molnar
> Assignee: Sandor Molnar
> Priority: Blocker
> Fix For: 2.0.0
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> With KNOX-2714, end-users can create tokens on behalf of other users using
> Hadoop's impersonation mechanism.
> The problem with the current implementation is that the proxyuser
> authorization happens to be on service level, but it should be executed
> sooner.
> As discussed offline with [~lmccay] and [~pzampino] we agreed on the
> following:
> * impersonation support should be done in Knox's identity assertion layer
> and not in the services
> * the proxuyser authorization in HadoopAuth filter should be left as-is.
> When someone configures them in two places (HadoopAuth authentication and in
> identity-assertion), a WARN-level message should indicate that one on the
> identity-assertion level will be ignored.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
