[GitHub] [knox] pzampino commented on a diff in pull request #681: KNOX-2839 - Identity assertion provider handles Hadoop ProxyUser auth using the 'doAs' query parameter
pzampino commented on code in PR #681:
URL: https://github.com/apache/knox/pull/681#discussion_r1052256852
##
gateway-provider-security-hadoopauth/src/main/java/org/apache/knox/gateway/hadoopauth/filter/HadoopAuthFilter.java:
##
@@ -200,12 +202,12 @@ protected void doFilter(FilterChain filterChain,
HttpServletRequest request, Htt
HttpServletRequest proxyRequest = null;
final String remoteUser = request.getRemoteUser();
if (!ignoreDoAs(remoteUser)) {
- final String doAsUser = request.getParameter(QUERY_PARAMETER_DOAS);
+ final String doAsUser =
request.getParameter(AuthFilterUtils.QUERY_PARAMETER_DOAS);
if (doAsUser != null && !doAsUser.equals(remoteUser)) {
LOG.hadoopAuthDoAsUser(doAsUser, remoteUser, request.getRemoteAddr());
if (request.getUserPrincipal() != null) {
try {
-proxyRequest = AuthFilterUtils.getProxyRequest(request, doAsUser,
topologyName, HadoopAuthDeploymentContributor.NAME);
+proxyRequest = AuthFilterUtils.getProxyRequest(request,
request.getUserPrincipal().getName(), doAsUser, topologyName,
HadoopAuthDeploymentContributor.NAME);
Review Comment:
@moresandeep, That's making an assumption that the principal name is ALWAYS
gotten from the request.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@knox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [knox] pzampino commented on a diff in pull request #681: KNOX-2839 - Identity assertion provider handles Hadoop ProxyUser auth using the 'doAs' query parameter
pzampino commented on code in PR #681:
URL: https://github.com/apache/knox/pull/681#discussion_r1051151340
##
gateway-provider-identity-assertion-common/pom.xml:
##
@@ -97,7 +97,14 @@
org.jboss.shrinkwrap
shrinkwrap-api
-
+
+org.apache.hadoop
+hadoop-common
+
+
+com.google.guava
+guava
Review Comment:
This is needed by hadoop-common?
##
gateway-provider-security-authz-acls/src/main/java/org/apache/knox/gateway/filter/AclsAuthorizationFilter.java:
##
@@ -128,19 +124,19 @@ protected boolean
enforceAclAuthorizationPolicy(ServletRequest request,
boolean groupAccess = false;
boolean ipAddrAccess;
-Subject subject = Subject.getSubject(AccessController.getContext());
-Principal primaryPrincipal =
(Principal)subject.getPrincipals(PrimaryPrincipal.class).toArray()[0];
-log.primaryPrincipal(primaryPrincipal.getName());
-Object[] impersonations =
subject.getPrincipals(ImpersonatedPrincipal.class).toArray();
-if (impersonations.length > 0) {
- log.impersonatedPrincipal(((Principal)impersonations[0]).getName());
- userAccess = checkUserAcls((Principal)impersonations[0]);
+final Subject subject = SubjectUtils.getCurrentSubject();
Review Comment:
Could this be simplified to
`
effectivePrincipalName = SubjectUtils.getCurrentEffectivePrincipalName();
log.effectivePrincipal(effectivePrincipalName);
userAccess = checkUserAcls(effectivePrincipalName);
`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@knox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [knox] pzampino commented on a diff in pull request #681: KNOX-2839 - Identity assertion provider handles Hadoop ProxyUser auth using the 'doAs' query parameter
pzampino commented on code in PR #681:
URL: https://github.com/apache/knox/pull/681#discussion_r1035343473
##
gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java:
##
@@ -720,19 +705,21 @@ private Response getAuthenticationToken() {
String createdBy = null;
// checking the doAs user only makes sense if tokens are managed (this is
where we store the userName information)
// and if impersonation is enabled
-if (impersonationEnabled && tokenStateService != null) {
- final String doAsUser = request.getParameter(QUERY_PARAMETER_DOAS);
- if (doAsUser != null && !doAsUser.equals(userName)) {
-try {
- //this call will authorize the doAs request
- AuthFilterUtils.authorizeImpersonationRequest(request, doAsUser,
getTopologyName(), TokenServiceDeploymentContributor.ROLE);
- createdBy = userName;
- userName = doAsUser;
- log.tokenImpersonationSuccess(createdBy, doAsUser);
-} catch (AuthorizationException e) {
- log.tokenImpersonationFailed(e);
- return Response.status(Response.Status.FORBIDDEN).entity("{ \"" +
e.getMessage() + "\" }").build();
+if (tokenStateService != null) {
+ final String realUserName = (String)
request.getAttribute(AuthFilterUtils.REAL_USER_NAME_ATTRIBUTE);
+ final Subject subject = SubjectUtils.getCurrentSubject();
+ if (subject != null && SubjectUtils.isImpersonating(subject)) {
+String primaryPrincipalName =
SubjectUtils.getPrimaryPrincipalName(subject);
+String impersonatedPrincipalName =
SubjectUtils.getImpersonatedPrincipalName(subject);
+if (!primaryPrincipalName.equals(impersonatedPrincipalName)) {
+ createdBy = primaryPrincipalName;
+ userName = impersonatedPrincipalName;
+ log.tokenImpersonationSuccess(createdBy, userName);
}
+ } else if (StringUtils.isNotBlank(realUserName) &&
!realUserName.equals(userName)) {
+// real user name is set by HadoopAuth filter for impersonated
requests (part of 'doAs' processing)
+createdBy = realUserName;
Review Comment:
I agree that using the Subject Principals as the vehicle for communicating
this information would be ideal.
##
gateway-provider-identity-assertion-common/src/main/java/org/apache/knox/gateway/identityasserter/common/filter/CommonIdentityAssertionFilter.java:
##
@@ -187,21 +219,46 @@ public void doFilter(ServletRequest request,
ServletResponse response, FilterCha
}
String principalName = getPrincipalName(subject);
+String mappedPrincipalName = null;
+try {
+ mappedPrincipalName = handleProxyUserImpersonation(request,
principalName);
+} catch(AuthorizationException e) {
+ LOG.hadoopAuthProxyUserFailed(e);
+ HttpExceptionUtils.createServletExceptionResponse((HttpServletResponse)
response, HttpServletResponse.SC_FORBIDDEN, e);
+ return;
+}
-String mappedPrincipalName = mapUserPrincipalBase(principalName);
+// mapping principal name using user principal mapping (if configured)
+mappedPrincipalName = mapUserPrincipalBase(mappedPrincipalName);
mappedPrincipalName = mapUserPrincipal(mappedPrincipalName);
+
String[] mappedGroups = mapGroupPrincipalsBase(mappedPrincipalName,
subject);
String[] groups = mapGroupPrincipals(mappedPrincipalName, subject);
String[] virtualGroups = virtualGroupMapper.mapGroups(mappedPrincipalName,
combine(subject, groups), request).toArray(new String[0]);
groups = combineGroupMappings(mappedGroups, groups);
groups = combineGroupMappings(virtualGroups, groups);
-HttpServletRequestWrapper wrapper = wrapHttpServletRequest(
-request, mappedPrincipalName);
+HttpServletRequestWrapper wrapper = wrapHttpServletRequest(request,
mappedPrincipalName);
+
continueChainAsPrincipal(wrapper, response, chain, mappedPrincipalName,
unique(groups));
}
+ private String handleProxyUserImpersonation(ServletRequest request, String
principalName) throws AuthorizationException {
+if (impersonationEnabled) {
+ final String doAsUser =
request.getParameter(AuthFilterUtils.QUERY_PARAMETER_DOAS);
+ if (doAsUser != null && !doAsUser.equals(principalName)) {
+LOG.hadoopAuthDoAsUser(doAsUser, principalName,
request.getRemoteAddr());
+if (principalName != null) {
+ AuthFilterUtils.authorizeImpersonationRequest((HttpServletRequest)
request, principalName, doAsUser, topologyName, ROLE);
+ LOG.hadoopAuthProxyUserSuccess();
+ return doAsUser;
Review Comment:
+1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@knox.apache.org
For queries about this service, please contact Infrastructure at:
[GitHub] [knox] pzampino commented on a diff in pull request #681: KNOX-2839 - Identity assertion provider handles Hadoop ProxyUser auth using the 'doAs' query parameter
pzampino commented on code in PR #681:
URL: https://github.com/apache/knox/pull/681#discussion_r1034795153
##
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 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 loadVirtualGroups(FilterConfig
filterConfig) {
+ private void initProxyUserConfiguration(FilterConfig filterConfig,
List initParameterNames) {
+final String impersonationEnabledValue =
filterConfig.getInitParameter(IMPERSONATION_ENABLED_PARAM);
+this.impersonationEnabled = impersonationEnabledValue == null ?
Boolean.FALSE : Boolean.parseBoolean(impersonationEnabledValue);
Review Comment:
It's a very minor thing, but as you've pointed out, it's not standard
convention in the Knox code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@knox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [knox] pzampino commented on a diff in pull request #681: KNOX-2839 - Identity assertion provider handles Hadoop ProxyUser auth using the 'doAs' query parameter
pzampino commented on code in PR #681:
URL: https://github.com/apache/knox/pull/681#discussion_r1034240821
##
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 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 loadVirtualGroups(FilterConfig
filterConfig) {
+ private void initProxyUserConfiguration(FilterConfig filterConfig,
List initParameterNames) {
+final String impersonationEnabledValue =
filterConfig.getInitParameter(IMPERSONATION_ENABLED_PARAM);
+this.impersonationEnabled = impersonationEnabledValue == null ?
Boolean.FALSE : Boolean.parseBoolean(impersonationEnabledValue);
Review Comment:
Why the explicit "this" used with the impersonationEnabled reference in
this method?
##
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:
This almost implies that we know the values are the same. Perhaps, something
like this is better?: "Ignoring the proxyuser configuration in favor of the
HadoopAuth provider's configuration."
##
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:
Is it intentional that
[GitHub] [knox] pzampino commented on a diff in pull request #681: KNOX-2839 - Identity assertion provider handles Hadoop ProxyUser auth using the 'doAs' query parameter
pzampino commented on code in PR #681:
URL: https://github.com/apache/knox/pull/681#discussion_r1033944407
##
gateway-provider-identity-assertion-common/src/main/java/org/apache/knox/gateway/identityasserter/common/filter/CommonIdentityAssertionFilter.java:
##
@@ -187,21 +216,46 @@ public void doFilter(ServletRequest request,
ServletResponse response, FilterCha
}
String principalName = getPrincipalName(subject);
+String mappedPrincipalName = null;
+try {
+ mappedPrincipalName = handleProxyUserImpersonation(request,
principalName);
Review Comment:
I think the principal mapping is orthogonal to the requests that are
received by Knox. In the case without impersonation, the authenticated user
will be mapped to whatever is configured. Similarly, with impersonation, the
authenticated user (e.g., user1) is requesting to act on behalf of another user
(e.g., user2), who is then mapped to user3 just as in the non-impersonation
scenario. The client never has control over the mappings, impersonation or not;
That's a Knox admin decision.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@knox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [knox] pzampino commented on a diff in pull request #681: KNOX-2839 - Identity assertion provider handles Hadoop ProxyUser auth using the 'doAs' query parameter
pzampino commented on code in PR #681:
URL: https://github.com/apache/knox/pull/681#discussion_r1033944407
##
gateway-provider-identity-assertion-common/src/main/java/org/apache/knox/gateway/identityasserter/common/filter/CommonIdentityAssertionFilter.java:
##
@@ -187,21 +216,46 @@ public void doFilter(ServletRequest request,
ServletResponse response, FilterCha
}
String principalName = getPrincipalName(subject);
+String mappedPrincipalName = null;
+try {
+ mappedPrincipalName = handleProxyUserImpersonation(request,
principalName);
Review Comment:
I think the principal mapping is orthogonal to the requests that are
received by Knox. In the case without impersonation, the authenticated user
will be mapped to whatever is configured. Similarly, with impersonation, the
authenticated user (e.g., user1) is requesting to act on behalf of another user
(e.g., user2), who is then mapped to user3 just as in the non-impersonation
scenario.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@knox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [knox] pzampino commented on a diff in pull request #681: KNOX-2839 - Identity assertion provider handles Hadoop ProxyUser auth using the 'doAs' query parameter
pzampino commented on code in PR #681:
URL: https://github.com/apache/knox/pull/681#discussion_r1033944407
##
gateway-provider-identity-assertion-common/src/main/java/org/apache/knox/gateway/identityasserter/common/filter/CommonIdentityAssertionFilter.java:
##
@@ -187,21 +216,46 @@ public void doFilter(ServletRequest request,
ServletResponse response, FilterCha
}
String principalName = getPrincipalName(subject);
+String mappedPrincipalName = null;
+try {
+ mappedPrincipalName = handleProxyUserImpersonation(request,
principalName);
Review Comment:
I think the principal mapping is orthogonal to the requests that are
received by Knox. Just as in the case without impersonation, the authenticated
user will be mapped to whatever is configured. Similarly, with impersonation,
the authenticated user (e.g., user1) is requesting to act on behalf of another
user (e.g., user2), who is then mapped to user3 just as in the
non-impersonation scenario.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@knox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
