lmccay commented on code in PR #681:
URL: https://github.com/apache/knox/pull/681#discussion_r1035199113
##########
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();
Review Comment:
I think we have audit log details for mapped principals. If so, I think we
need to add impersonation details as well.
##########
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);
Review Comment:
I could use some walkthrough of the logic here. If it is not more clear in
the full context of the file then it may help to describe here in comments or
make the code more obvious. I do think that I have it but would like to make
sure.
It seems that we are setting the createdBy and userName for the token
metadata when there is indeed a stateService in play. If impersonation/doAs is
enabled then we use the primaryPrincipal from the Subject to reflect the
createdBy. If it isn't then we use the realUserName from the request
attributes. I DO NOT KNOW HOW THIS IS SET OR WHATUSER IT REPRESENTS. If
realUser sounds like hadoop auth module and UGI terms. Does this assume that
the HadoopAuthProvider and/or the HadoopGroupProvider is being used? Given that
the doAs support is being added to a base identity assertion provider class, is
this a safe assumption? If so, is that because we have added a dependency on
hadoop for all identity assertion providers somewhere along the line? Anyway,
back to the logic flow... In the case where subject is impersonating, we take
the impersonatedPrincipalName as the userName. However, in the case that there
is no impersonating, the userName is not set? Is this defaulted to the same as
th
e createdBy somwhere else?
##########
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:
nit; Wouldn't principalName = doAsUser; be better here than returning a
separate variable and potentially having to accommodate this later if there is
code needed before the final return?
##########
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:
The above comment does point to an assumption about the hadoopauth provider.
Since we are adding this capability to the common identity assertion
capabilities this is no longer true and it is not clear that the hadoop auth or
hadoop groups providers are being used. I think that we need to make the above
two conditional blocks more uniform and decoupled from hadoop
terms/dependencies. In otherwords, by the time we get here we should only be
relying on Subject Principals.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
