Skip to content

Various improvements for the webconsole plugin #78

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Various improvements for the webconsole plugin #78

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a42d3fa
cleanup: use diamond operator where possible
rombert Aug 5, 2022
ec10105
SLING-11511 - Allow checking the map/resolve result for an empty path
rombert Aug 5, 2022
11f26ad
SLING-11513 - Allow impersonating an user when checking the result of…
rombert Aug 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions bnd.bnd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
Import-Package:\
javax.jcr;resolution:=optional,\
org.apache.sling.commons.metrics;resolution:=optional,\
!org.apache.sling.auth.*,\
!org.apache.sling.jcr.*,\
*

Provide-Capability:\
Expand Down
14 changes: 14 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,20 @@
<version>1.3.4</version>
<scope>provided</scope>
</dependency>
<!-- for constant values only -->
<dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.auth.core</artifactId>
<version>1.0.0</version>
<scope>provided</scope>
</dependency>
<!-- for constant values only -->
<dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.jcr.resource</artifactId>
<version>3.0.0</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
Expand Down
71 changes: 60 additions & 11 deletions ...java/org/apache/sling/resourceresolver/impl/console/ResourceResolverWebConsolePlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,15 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Dictionary;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Hashtable;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;

import javax.jcr.Session;
import javax.servlet.Servlet;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
Expand All @@ -40,12 +43,16 @@
import javax.servlet.http.HttpServletResponse;

import org.apache.sling.api.request.ResponseUtil;
import org.apache.sling.api.resource.LoginException;
import org.apache.sling.api.resource.ResourceResolver;
import org.apache.sling.api.resource.ResourceResolverFactory;
import org.apache.sling.api.resource.mapping.ResourceMapper;
import org.apache.sling.api.resource.runtime.RuntimeService;
import org.apache.sling.api.resource.runtime.dto.ResourceProviderDTO;
import org.apache.sling.api.resource.runtime.dto.ResourceProviderFailureDTO;
import org.apache.sling.api.resource.runtime.dto.RuntimeDTO;
import org.apache.sling.auth.core.AuthenticationSupport;
import org.apache.sling.jcr.resource.api.JcrResourceConstants;
import org.apache.sling.resourceresolver.impl.CommonResourceResolverFactoryImpl;
import org.apache.sling.resourceresolver.impl.helper.URI;
import org.apache.sling.resourceresolver.impl.helper.URIException;
Expand All @@ -63,11 +70,12 @@ public class ResourceResolverWebConsolePlugin extends HttpServlet {
private static final long serialVersionUID = 0;

private static final String ATTR_TEST = "plugin.test";

private static final String ATTR_SUBMIT = "plugin.submit";
private static final String ATTR_USER = "plugin.user";

private static final String PAR_MSG = "msg";
private static final String PAR_TEST = "test";
private static final String PAR_USER = "user";

private final transient CommonResourceResolverFactoryImpl resolverFactory;

Expand All @@ -84,7 +92,7 @@ public ResourceResolverWebConsolePlugin(final BundleContext context,
this.runtimeService = runtimeService;
this.bundleContext = context;

Dictionary<String, Object> props = new Hashtable<String, Object>();
Dictionary<String, Object> props = new Hashtable<>();
props.put(Constants.SERVICE_DESCRIPTION,
"Apache Sling Resource Resolver Web Console Plugin");
props.put(Constants.SERVICE_VENDOR, "The Apache Software Foundation");
Expand Down Expand Up @@ -116,6 +124,7 @@ protected void doGet(final HttpServletRequest request,
} else {
test = null;
}
final String user = request.getParameter(PAR_USER);

final PrintWriter pw = response.getWriter();

Expand Down Expand Up @@ -160,14 +169,22 @@ protected void doGet(final HttpServletRequest request,
+ "clearly marked, and the others listed for completeness.");

pw.println("<tr class='content'>");
pw.println("<td class='content'>Test</td>");
pw.print("<td class='content' colspan='2'>");
pw.print("<form method='post'>");
pw.println("<form method='post'>");
pw.print("Test ");
pw.print("<input type='text' name='" + ATTR_TEST + "' value='");
if (test != null) {
pw.print(ResponseUtil.escapeXml(test));
}
pw.println("' class='input' size='50'>");
pw.println("' class='input' size='20'>");
pw.print("User (optional)");
pw.print("<input type='text' name='" + ATTR_USER + "' value='");
Copy link
Member

@kwin kwin Aug 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a hint that this does only work/has an effect on resources resolved by the JCR resource provider.

Copy link
Contributor Author

@rombert rombert Aug 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.

if ( user != null ) {
pw.print(ResponseUtil.escapeXml(user));
}
pw.println("' class='input' size='20'>");
pw.println("</td>");
pw.print("<td class='content'>");
pw.println("&nbsp;&nbsp;<input type='submit' name='" + ATTR_SUBMIT
+ "' value='Resolve' class='submit'>");
pw.println("&nbsp;&nbsp;<input type='submit' name='" + ATTR_SUBMIT
Expand Down Expand Up @@ -213,15 +230,21 @@ protected void doPost(HttpServletRequest request,
HttpServletResponse response) throws ServletException, IOException {

final String test = request.getParameter(ATTR_TEST);
final String user = request.getParameter(ATTR_USER);
String msg = null;
if (test != null && test.length() > 0) {
if (test != null) {

ResourceResolver resolver = null;
try {
// prepare the request for the resource resolver
HttpServletRequest helper = new ResolverRequest(request, test);

resolver = resolverFactory.getServiceResourceResolver(this.resolverFactory.getServiceUserAuthenticationInfo("console"));
// impersonate if asked
if ( user != null && user.length() > 0 ) {
resolver = getImpersonatedResourceResolver(request, user);
} else {
resolver = resolverFactory.getServiceResourceResolver(this.resolverFactory.getServiceUserAuthenticationInfo("console"));
}

// map or resolve as instructed
Object result;
Expand Down Expand Up @@ -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);
Copy link
Member

@kwin kwin Aug 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this rather be based on service resource resolver as well to make it work with all web console security providers?

Copy link
Contributor Author

@rombert rombert Aug 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@kwin kwin Aug 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just try to get a resourceresolver based on the service one and the user is to impersonate. The one returned as request attribute might not have the according rights

Copy link
Contributor Author

@rombert rombert Aug 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I try to set up impersonation based on the existing resolver (

sling-org-apache-sling-resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/console/ResourceResolverWebConsolePlugin.java

Line 246 in 11f26ad

resolver = resolverFactory.getServiceResourceResolver(this.resolverFactory.getServiceUserAuthenticationInfo("console"));
) I get back

Test Failure: org.apache.sling.api.resource.LoginException: Impersonation not allowed.

Copy link
Member

@kwin kwin Aug 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to adjust privileges of the underlying technical user: https://jackrabbit.apache.org/oak/docs/security/authentication/default.html#impersonation

Copy link
Contributor Author

@rombert rombert Aug 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

@kwin kwin Aug 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the user needs to be "admin", just being member of the administrators group is IMHO not enough. I don't think that there is an option yet for a user to enable him to impersonate as anyone else. Might be a good extension though for Oak.
The same limitation applies to the user doing the webconsole request (in case the Sling Webconsole Security provider is used), so in fact this option does only work for admin with all other users.
Therefore I would suggest to use a new administrative resource resolver with impersonation and whitelist the usage accordingly.

if ( !(resolverAttribute instanceof ResourceResolver) ) {
throw new IllegalArgumentException("No " + ResourceResolver.class.getSimpleName() + " found in request, unable to proceed with impersonation");
Copy link
Contributor

@cziegeler cziegeler Aug 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a common case if the web console is not using the Sling authentication provider that the request attribute is not set. In that case, it currently displays:
Test Failure: java.lang.IllegalArgumentException: No ResourceResolver found in request, unable to proceed with impersonation
I think it would be good if just the message without the exception type would be displayed

Copy link
Contributor Author

@rombert rombert Aug 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

#78 (comment)

Copy link
Contributor

@cziegeler cziegeler Mar 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really a fan of adding another case for using an admin resource resolver, especially not in the web console

}

@SuppressWarnings("resource") // not a leak, we don't own this resolver
ResourceResolver currentResolver = (ResourceResolver) resolverAttribute;

Map<String, Object> authenticationInfo = new HashMap<>();
authenticationInfo.put(ResourceResolverFactory.USER_IMPERSONATION, user);
authenticationInfo.put(JcrResourceConstants.AUTHENTICATION_INFO_SESSION, currentResolver.adaptTo(Session.class));
Copy link
Member

@kwin kwin Aug 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adaptTo may return null in case JCR resource provider is not used, a null check is necessary here.

Copy link
Contributor

@cziegeler cziegeler Aug 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import of JCR packages is optional, I assume the above code does not work if the JCR bundle is not installed?
We should keep that dependency as optional and not require JCR api to be installed

Copy link
Contributor Author

@rombert rombert Aug 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

    sling-org-apache-sling-resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/console/ResourceResolverWebConsolePlugin.java

    Line 265 in 11f26ad

    } catch (final Throwable t) {
    . 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

Copy link
Contributor

@cziegeler cziegeler Aug 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, I didn't read the full code. You are right, that this will work. And just to be sure I gave it a test run in a sling application that does not have the jcr api and except for impersonation, everything works


return resolverFactory.getResourceResolver(authenticationInfo);
}

private static String mappingsToString(Collection<String> allMappings) {
if ( allMappings.size() == 0 )
return ""; // should not happen
return "(no mappings)"; // should not happen
if ( allMappings.size() == 1)
return allMappings.iterator().next();

Expand Down Expand Up @@ -326,7 +372,7 @@ private void dumpMapHtml(PrintWriter pw, String title, String description,
pw.println("<th class='content'>Redirect</th>");
pw.println("</tr>");

final Set<String> usedPatterns = new HashSet<String>();
final Set<String> usedPatterns = new HashSet<>();

for (final MapEntry entry : list) {
final String pattern = entry.getPattern();
Expand Down Expand Up @@ -630,7 +676,10 @@ public int getServerPort() {
@Override
public String getPathInfo() {
try {
return uri.getPath();
String path = uri.getPath();
if ( path == null )
path = "";
return path;
} catch (URIException ue) {
return "";
}
Expand Down