lmccay commented on code in PR #681:
URL: https://github.com/apache/knox/pull/681#discussion_r1036271138


##########
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:
   Subject.doAs should be used everywhere. Let me check the HadoopAuthProvider 
code. This is a fundamental aspect of the Knox provider and service separation 
though. By the time the request gets to a service the security context 
(Subject) needs to have everything that is needed. All provider specific 
details are normalized into Knox standard Principals. If the Knox standard 
needs to change to accommodate things then it needs to be done in there. I only 
point this out because I don't think this has been clear enough which I think 
is how we got here.



-- 
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

Reply via email to