Various improvements for the webconsole plugin #78

rombert · 2022-08-05T11:31:22Z

No description provided.

- allow the path to be empty, but not null - return a more informative message if no mappings are found

… a map/resolve call - extend the form to optionally accept a user to impersonate - use the resolver of the current user as a basis for impersonation - make sure that we don't accidentally generate imports from the auth.core and jcr.resource bundles, they are only used for the constant values, which are inlined.

cziegeler

lgtm, thanks

sonarqubecloud · 2022-08-06T15:06:22Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

2.1% Coverage
0.0% Duplication

kwin

LGTM overall, just some minor remarks to better handle some edge cases.

kwin · 2022-08-08T06:52:12Z

...in/java/org/apache/sling/resourceresolver/impl/console/ResourceResolverWebConsolePlugin.java

-        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='");


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

kwin · 2022-08-08T06:53:23Z

...in/java/org/apache/sling/resourceresolver/impl/console/ResourceResolverWebConsolePlugin.java

+
+        // 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);


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

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

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

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.

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

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?

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.

kwin · 2022-08-08T06:54:00Z

...in/java/org/apache/sling/resourceresolver/impl/console/ResourceResolverWebConsolePlugin.java

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


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

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

There are two issues to discuss here

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?

The adaptTo call can return null nonetheless and I will take it into acount

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

cziegeler · 2022-08-08T13:53:38Z

...in/java/org/apache/sling/resourceresolver/impl/console/ResourceResolverWebConsolePlugin.java

+        // 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");


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

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

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

sonarqubecloud · 2023-04-19T15:45:32Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

2.1% Coverage
0.0% Duplication

sonarqubecloud · 2023-10-05T15:46:06Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

2.1% Coverage
0.0% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

sonarqubecloud · 2024-02-03T19:46:33Z

Quality Gate failed

Failed conditions

2.1% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

sonarqubecloud · 2024-09-23T19:46:16Z

Quality Gate failed

Failed conditions
2.1% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

sonarqubecloud · 2024-10-29T19:46:32Z

Quality Gate failed

Failed conditions
2.1% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

sonarqubecloud · 2025-01-18T19:47:04Z

Quality Gate failed

Failed conditions
2.1% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

rombert added 3 commits August 5, 2022 11:34

cleanup: use diamond operator where possible

a42d3fa

SLING-11511 - Allow checking the map/resolve result for an empty path

ec10105

- allow the path to be empty, but not null - return a more informative message if no mappings are found

rombert requested review from kwin and cziegeler August 5, 2022 11:31

cziegeler approved these changes Aug 5, 2022

View reviewed changes

kwin requested changes Aug 8, 2022

View reviewed changes

cziegeler reviewed Aug 8, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Various improvements for the webconsole plugin #78

Various improvements for the webconsole plugin #78

rombert commented Aug 5, 2022

cziegeler left a comment

sonarqubecloud bot commented Aug 6, 2022

kwin left a comment

kwin Aug 8, 2022

rombert Aug 8, 2022

kwin Aug 8, 2022

rombert Aug 8, 2022

kwin Aug 8, 2022 •

edited

Loading

rombert Aug 9, 2022

kwin Aug 9, 2022 •

edited

Loading

rombert Aug 9, 2022 •

edited

Loading

kwin Aug 9, 2022 •

edited

Loading

kwin Aug 8, 2022

cziegeler Aug 8, 2022

rombert Aug 8, 2022

cziegeler Aug 8, 2022

cziegeler Aug 8, 2022 •

edited

Loading

rombert Aug 9, 2022

cziegeler Mar 13, 2023

sonarqubecloud bot commented Apr 19, 2023

sonarqubecloud bot commented Oct 5, 2023

sonarqubecloud bot commented Feb 3, 2024

sonarqubecloud bot commented Sep 23, 2024

sonarqubecloud bot commented Oct 29, 2024

sonarqubecloud bot commented Jan 18, 2025

Various improvements for the webconsole plugin #78

Are you sure you want to change the base?

Various improvements for the webconsole plugin #78

Conversation

rombert commented Aug 5, 2022

cziegeler left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Aug 6, 2022

kwin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwin Aug 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwin Aug 9, 2022 • edited Loading

Choose a reason for hiding this comment

rombert Aug 9, 2022 • edited Loading

Choose a reason for hiding this comment

kwin Aug 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cziegeler Aug 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Apr 19, 2023

sonarqubecloud bot commented Oct 5, 2023

sonarqubecloud bot commented Feb 3, 2024

Quality Gate failed

sonarqubecloud bot commented Sep 23, 2024

Quality Gate failed

sonarqubecloud bot commented Oct 29, 2024

Quality Gate failed

sonarqubecloud bot commented Jan 18, 2025

Quality Gate failed

kwin Aug 8, 2022 •

edited

Loading

kwin Aug 9, 2022 •

edited

Loading

rombert Aug 9, 2022 •

edited

Loading

kwin Aug 9, 2022 •

edited

Loading

cziegeler Aug 8, 2022 •

edited

Loading