[GitHub] [sling-org-apache-sling-resourceresolver] rombert commented on a diff in pull request #78: Various improvements for the webconsole plugin

2022-08-09 Thread GitBox


rombert commented on code in PR #78:
URL: 
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/78#discussion_r941399874


##
src/main/java/org/apache/sling/resourceresolver/impl/console/ResourceResolverWebConsolePlugin.java:
##
@@ -255,19 +278,42 @@ protected void doPost(HttpServletRequest request,
 // finally redirect
 final String path = request.getContextPath() + request.getServletPath()
 + request.getPathInfo();
-final String redirectTo;
+String redirectTo;
 if (msg == null) {
 redirectTo = path;
 } else {
 redirectTo = path + '?' + PAR_MSG + '=' + encodeParam(msg) + '&'
 + PAR_TEST + '=' + encodeParam(test);
+if ( user != null && user.length() > 0 ) {
+redirectTo += '&' + PAR_USER + '=' + encodeParam(user);
+}
 }
 response.sendRedirect(redirectTo);
 }
 
+private ResourceResolver 
getImpersonatedResourceResolver(HttpServletRequest request, final String user)
+throws LoginException {
+
+// resolver is set by the auth.core bundle in case of successful 
authentication, so it should
+// always be there
+Object resolverAttribute = 
request.getAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_RESOLVER);
+if ( !(resolverAttribute instanceof ResourceResolver) ) {
+throw new IllegalArgumentException("No " + 
ResourceResolver.class.getSimpleName() + " found in request, unable to proceed 
with impersonation");

Review Comment:
   @kwin suggested that we use an admin resolver instead (and include the 
bundle in the allow list ). If we would stop looking up the ResourceResolver in 
the request attribute, would it solve this issue?
   
   
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/78#discussion_r941353233



-- 
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...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [sling-org-apache-sling-resourceresolver] rombert commented on a diff in pull request #78: Various improvements for the webconsole plugin

2022-08-09 Thread GitBox


rombert commented on code in PR #78:
URL: 
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/78#discussion_r941330770


##
src/main/java/org/apache/sling/resourceresolver/impl/console/ResourceResolverWebConsolePlugin.java:
##
@@ -255,19 +278,42 @@ protected void doPost(HttpServletRequest request,
 // finally redirect
 final String path = request.getContextPath() + request.getServletPath()
 + request.getPathInfo();
-final String redirectTo;
+String redirectTo;
 if (msg == null) {
 redirectTo = path;
 } else {
 redirectTo = path + '?' + PAR_MSG + '=' + encodeParam(msg) + '&'
 + PAR_TEST + '=' + encodeParam(test);
+if ( user != null && user.length() > 0 ) {
+redirectTo += '&' + PAR_USER + '=' + encodeParam(user);
+}
 }
 response.sendRedirect(redirectTo);
 }
 
+private ResourceResolver 
getImpersonatedResourceResolver(HttpServletRequest request, final String user)
+throws LoginException {
+
+// resolver is set by the auth.core bundle in case of successful 
authentication, so it should
+// always be there
+Object resolverAttribute = 
request.getAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_RESOLVER);

Review Comment:
   Looking at the Oak implementation, I see that impersonation works if either:
   - the impersonator is an admin
   - the impersonator is included in the `rep:impersonators` property of the 
impersonated user
   
   
https://github.com/apache/jackrabbit-oak/blob/a90566744551246535f65c2aefc5a44fd5275490/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/ImpersonationImpl.java#L125-L146
   
   I am not sure if either of these is possible or desireable for a service 
user. Do you see another way?



##
src/main/java/org/apache/sling/resourceresolver/impl/console/ResourceResolverWebConsolePlugin.java:
##
@@ -255,19 +278,42 @@ protected void doPost(HttpServletRequest request,
 // finally redirect
 final String path = request.getContextPath() + request.getServletPath()
 + request.getPathInfo();
-final String redirectTo;
+String redirectTo;
 if (msg == null) {
 redirectTo = path;
 } else {
 redirectTo = path + '?' + PAR_MSG + '=' + encodeParam(msg) + '&'
 + PAR_TEST + '=' + encodeParam(test);
+if ( user != null && user.length() > 0 ) {
+redirectTo += '&' + PAR_USER + '=' + encodeParam(user);
+}
 }
 response.sendRedirect(redirectTo);
 }
 
+private ResourceResolver 
getImpersonatedResourceResolver(HttpServletRequest request, final String user)
+throws LoginException {
+
+// resolver is set by the auth.core bundle in case of successful 
authentication, so it should
+// always be there
+Object resolverAttribute = 
request.getAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_RESOLVER);

Review Comment:
   Looking at the Oak implementation, I see that impersonation works if either:
   - the impersonator is an admin
   - the impersonator is included in the `rep:impersonators` property of the 
impersonated user
   
   
https://github.com/apache/jackrabbit-oak/blob/a90566744551246535f65c2aefc5a44fd5275490/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/ImpersonationImpl.java#L125-L146
   
   I am not sure if either of these is possible or desirable for a service 
user. Do you see another way?



-- 
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...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [sling-org-apache-sling-resourceresolver] rombert commented on a diff in pull request #78: Various improvements for the webconsole plugin

2022-08-09 Thread GitBox


rombert commented on code in PR #78:
URL: 
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/78#discussion_r941063449


##
src/main/java/org/apache/sling/resourceresolver/impl/console/ResourceResolverWebConsolePlugin.java:
##
@@ -255,19 +278,42 @@ protected void doPost(HttpServletRequest request,
 // finally redirect
 final String path = request.getContextPath() + request.getServletPath()
 + request.getPathInfo();
-final String redirectTo;
+String redirectTo;
 if (msg == null) {
 redirectTo = path;
 } else {
 redirectTo = path + '?' + PAR_MSG + '=' + encodeParam(msg) + '&'
 + PAR_TEST + '=' + encodeParam(test);
+if ( user != null && user.length() > 0 ) {
+redirectTo += '&' + PAR_USER + '=' + encodeParam(user);
+}
 }
 response.sendRedirect(redirectTo);
 }
 
+private ResourceResolver 
getImpersonatedResourceResolver(HttpServletRequest request, final String user)
+throws LoginException {
+
+// resolver is set by the auth.core bundle in case of successful 
authentication, so it should
+// always be there
+Object resolverAttribute = 
request.getAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_RESOLVER);

Review Comment:
   If I try to set up impersonation based on the existing resolver ( 
https://github.com/apache/sling-org-apache-sling-resourceresolver/blob/11f26ad706a350269e27ca42a2cbcf22a4724ce1/src/main/java/org/apache/sling/resourceresolver/impl/console/ResourceResolverWebConsolePlugin.java#L246
 ) I get back 
   
   > Test Failure: org.apache.sling.api.resource.LoginException: Impersonation 
not allowed.



-- 
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...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [sling-org-apache-sling-resourceresolver] rombert commented on a diff in pull request #78: Various improvements for the webconsole plugin

2022-08-08 Thread GitBox


rombert commented on code in PR #78:
URL: 
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/78#discussion_r940259595


##
src/main/java/org/apache/sling/resourceresolver/impl/console/ResourceResolverWebConsolePlugin.java:
##
@@ -255,19 +278,42 @@ protected void doPost(HttpServletRequest request,
 // finally redirect
 final String path = request.getContextPath() + request.getServletPath()
 + request.getPathInfo();
-final String redirectTo;
+String redirectTo;
 if (msg == null) {
 redirectTo = path;
 } else {
 redirectTo = path + '?' + PAR_MSG + '=' + encodeParam(msg) + '&'
 + PAR_TEST + '=' + encodeParam(test);
+if ( user != null && user.length() > 0 ) {
+redirectTo += '&' + PAR_USER + '=' + encodeParam(user);
+}
 }
 response.sendRedirect(redirectTo);
 }
 
+private ResourceResolver 
getImpersonatedResourceResolver(HttpServletRequest request, final String user)
+throws LoginException {
+
+// resolver is set by the auth.core bundle in case of successful 
authentication, so it should
+// always be there
+Object resolverAttribute = 
request.getAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_RESOLVER);

Review Comment:
   Do you mean that we should try to create an additional mapping for the 
console using a service user that has impersonation rights?



-- 
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...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [sling-org-apache-sling-resourceresolver] rombert commented on a diff in pull request #78: Various improvements for the webconsole plugin

2022-08-08 Thread GitBox


rombert commented on code in PR #78:
URL: 
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/78#discussion_r940255957


##
src/main/java/org/apache/sling/resourceresolver/impl/console/ResourceResolverWebConsolePlugin.java:
##
@@ -255,19 +278,42 @@ protected void doPost(HttpServletRequest request,
 // finally redirect
 final String path = request.getContextPath() + request.getServletPath()
 + request.getPathInfo();
-final String redirectTo;
+String redirectTo;
 if (msg == null) {
 redirectTo = path;
 } else {
 redirectTo = path + '?' + PAR_MSG + '=' + encodeParam(msg) + '&'
 + PAR_TEST + '=' + encodeParam(test);
+if ( user != null && user.length() > 0 ) {
+redirectTo += '&' + PAR_USER + '=' + encodeParam(user);
+}
 }
 response.sendRedirect(redirectTo);
 }
 
+private ResourceResolver 
getImpersonatedResourceResolver(HttpServletRequest request, final String user)
+throws LoginException {
+
+// resolver is set by the auth.core bundle in case of successful 
authentication, so it should
+// always be there
+Object resolverAttribute = 
request.getAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_RESOLVER);
+if ( !(resolverAttribute instanceof ResourceResolver) ) {
+throw new IllegalArgumentException("No " + 
ResourceResolver.class.getSimpleName() + " found in request, unable to proceed 
with impersonation");
+}
+
+@SuppressWarnings("resource") // not a leak, we don't own this resolver
+ResourceResolver currentResolver = (ResourceResolver) 
resolverAttribute;
+
+Map authenticationInfo = new HashMap<>();
+authenticationInfo.put(ResourceResolverFactory.USER_IMPERSONATION, 
user);
+
authenticationInfo.put(JcrResourceConstants.AUTHENTICATION_INFO_SESSION, 
currentResolver.adaptTo(Session.class));

Review Comment:
   There are two issues to discuss here
   
   1. The javax.jcr import is already optional - 
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/78/files#diff-6d1033f3cc4642b36cd395cf203fdc2d963f51e46cb1949508765a9756743c45R2
 . I _think_ that a `NoClassDefFoundError` will be thrown from the method, then 
caught at 
https://github.com/apache/sling-org-apache-sling-resourceresolver/blob/11f26ad706a350269e27ca42a2cbcf22a4724ce1/src/main/java/org/apache/sling/resourceresolver/impl/console/ResourceResolverWebConsolePlugin.java#L265
 . This should not affect the path where impersonation is not used. Do we need 
to handle this in some other way, e.g. using the bundle class loader to 
dynamically load the class definition?
   2. The `adaptTo` call can return null nonetheless and I will take it into 
acount



##
src/main/java/org/apache/sling/resourceresolver/impl/console/ResourceResolverWebConsolePlugin.java:
##
@@ -160,14 +169,22 @@ protected void doGet(final HttpServletRequest request,
 + "clearly marked, and the others listed for 
completeness.");
 
 pw.println("");
-pw.println("Test");
 pw.print("");
-pw.print("");
+pw.println("");
+pw.print("Test ");
 pw.print("");
+pw.println("' class='input' size='20'>");
+pw.print("User (optional)");
+