[jira] [Work logged] (KNOX-2839) Refactor impersonation from KnoxToken service

2022-12-12 Thread ASF GitHub Bot (Jira)


 [ 
https://issues.apache.org/jira/browse/KNOX-2839?focusedWorklogId=832712=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-832712
 ]

ASF GitHub Bot logged work on KNOX-2839:


Author: ASF GitHub Bot
Created on: 12/Dec/22 11:08
Start Date: 12/Dec/22 11:08
Worklog Time Spent: 10m 
  Work Description: smolnar82 commented on code in PR #681:
URL: https://github.com/apache/knox/pull/681#discussion_r1045689517


##
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AccessTokenFederationFilter.java:
##
@@ -176,6 +172,6 @@ private Subject createSubjectFromToken(JWTToken token) {
 // To modify the Principals Set, the caller must have 
AuthPermission("modifyPrincipals").
 // To modify the public credential Set, the caller must have 
AuthPermission("modifyPublicCredentials").
 // To modify the private credential Set, the caller must have 
AuthPermission("modifyPrivateCredentials").
-return new javax.security.auth.Subject(true, principals, emptySet, 
emptySet);
+return new javax.security.auth.Subject(true, principals, 
Collections.emptySet(), Collections.emptySet());

Review Comment:
   This is a valid question. The answer is that it's even better than the 
implementation before due to the very same reason you described: once this 
subject is created, the principals cannot be modified. In lower layers, like in 
identity assertion, if someone wants to proceed with different principals, a 
new Subject has to be created and call `Subject.doAs`.





Issue Time Tracking
---

Worklog Id: (was: 832712)
Time Spent: 3h 10m  (was: 3h)

> 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: 3h 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)


[GitHub] [knox] smolnar82 commented on a diff in pull request #681: KNOX-2839 - Identity assertion provider handles Hadoop ProxyUser auth using the 'doAs' query parameter

2022-12-12 Thread GitBox


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


##
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AccessTokenFederationFilter.java:
##
@@ -176,6 +172,6 @@ private Subject createSubjectFromToken(JWTToken token) {
 // To modify the Principals Set, the caller must have 
AuthPermission("modifyPrincipals").
 // To modify the public credential Set, the caller must have 
AuthPermission("modifyPublicCredentials").
 // To modify the private credential Set, the caller must have 
AuthPermission("modifyPrivateCredentials").
-return new javax.security.auth.Subject(true, principals, emptySet, 
emptySet);
+return new javax.security.auth.Subject(true, principals, 
Collections.emptySet(), Collections.emptySet());

Review Comment:
   This is a valid question. The answer is that it's even better than the 
implementation before due to the very same reason you described: once this 
subject is created, the principals cannot be modified. In lower layers, like in 
identity assertion, if someone wants to proceed with different principals, a 
new Subject has to be created and call `Subject.doAs`.



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



[jira] [Work logged] (KNOX-2839) Refactor impersonation from KnoxToken service

2022-12-12 Thread ASF GitHub Bot (Jira)


 [ 
https://issues.apache.org/jira/browse/KNOX-2839?focusedWorklogId=832711=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-832711
 ]

ASF GitHub Bot logged work on KNOX-2839:


Author: ASF GitHub Bot
Created on: 12/Dec/22 11:01
Start Date: 12/Dec/22 11:01
Worklog Time Spent: 10m 
  Work Description: zeroflag commented on code in PR #681:
URL: https://github.com/apache/knox/pull/681#discussion_r1045683325


##
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AccessTokenFederationFilter.java:
##
@@ -176,6 +172,6 @@ private Subject createSubjectFromToken(JWTToken token) {
 // To modify the Principals Set, the caller must have 
AuthPermission("modifyPrincipals").
 // To modify the public credential Set, the caller must have 
AuthPermission("modifyPublicCredentials").
 // To modify the private credential Set, the caller must have 
AuthPermission("modifyPrivateCredentials").
-return new javax.security.auth.Subject(true, principals, emptySet, 
emptySet);
+return new javax.security.auth.Subject(true, principals, 
Collections.emptySet(), Collections.emptySet());

Review Comment:
   LGTM with one note. 
   I'm not sure if it's a real problem, but since we're using 
`Collections.emptySet()` here, this means that adding a new principal after 
this point (e.g.: `subject.getPrincipals().add()`) to the subject might fail 
because the `Collection.emptySet()` is unmodifiable. Unlike the `new 
HashSet<>();`.





Issue Time Tracking
---

Worklog Id: (was: 832711)
Time Spent: 3h  (was: 2h 50m)

> 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: 3h
>  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)


[GitHub] [knox] zeroflag commented on a diff in pull request #681: KNOX-2839 - Identity assertion provider handles Hadoop ProxyUser auth using the 'doAs' query parameter

2022-12-12 Thread GitBox


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


##
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AccessTokenFederationFilter.java:
##
@@ -176,6 +172,6 @@ private Subject createSubjectFromToken(JWTToken token) {
 // To modify the Principals Set, the caller must have 
AuthPermission("modifyPrincipals").
 // To modify the public credential Set, the caller must have 
AuthPermission("modifyPublicCredentials").
 // To modify the private credential Set, the caller must have 
AuthPermission("modifyPrivateCredentials").
-return new javax.security.auth.Subject(true, principals, emptySet, 
emptySet);
+return new javax.security.auth.Subject(true, principals, 
Collections.emptySet(), Collections.emptySet());

Review Comment:
   LGTM with one note. 
   I'm not sure if it's a real problem, but since we're using 
`Collections.emptySet()` here, this means that adding a new principal after 
this point (e.g.: `subject.getPrincipals().add()`) to the subject might fail 
because the `Collection.emptySet()` is unmodifiable. Unlike the `new 
HashSet<>();`.



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



[jira] [Work logged] (KNOX-2839) Refactor impersonation from KnoxToken service

2022-12-12 Thread ASF GitHub Bot (Jira)


 [ 
https://issues.apache.org/jira/browse/KNOX-2839?focusedWorklogId=832708=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-832708
 ]

ASF GitHub Bot logged work on KNOX-2839:


Author: ASF GitHub Bot
Created on: 12/Dec/22 10:47
Start Date: 12/Dec/22 10:47
Worklog Time Spent: 10m 
  Work Description: smolnar82 commented on code in PR #681:
URL: https://github.com/apache/knox/pull/681#discussion_r1045668894


##
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:
   Done





Issue Time Tracking
---

Worklog Id: (was: 832708)
Time Spent: 2h 50m  (was: 2h 40m)

> 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: 2h 50m
>  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)


[GitHub] [knox] smolnar82 commented on a diff in pull request #681: KNOX-2839 - Identity assertion provider handles Hadoop ProxyUser auth using the 'doAs' query parameter

2022-12-12 Thread GitBox


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


##
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:
   Done



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