Re: [PR] SLING-12320 - Add support for retrieving a service resource resolver with impersonation without requiring extra configuration [sling-org-apache-sling-jcr-resource]
anchela commented on PR #43: URL: https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/43#issuecomment-2117930435 @raducotescu , as discussed... i am fine now but making the comment a bit clearer and maybe also explaining why the extra attribute on the credentials is set might make this easier to read. specially since SlingRepository.impersonateFromService which is called does not require the attribute on the credentials -- 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
Re: [PR] SLING-12320 - Add support for retrieving a service resource resolver with impersonation without requiring extra configuration [sling-org-apache-sling-jcr-resource]
anchela commented on code in PR #43:
URL:
https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/43#discussion_r1605249171
##
src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java:
##
@@ -111,7 +111,20 @@ private static Bundle extractCallingBundle(@NotNull
Map authenti
} else {
final Object subService =
authenticationInfo.get(ResourceResolverFactory.SUBSERVICE);
final String subServiceName = subService instanceof
String ? (String) subService : null;
-session = repo.loginService(subServiceName, null);
+// let's shortcut the impersonation for service users,
if impersonation was requested
+String sudoUser = getSudoUser(authenticationInfo);
+if (sudoUser != null) {
+SimpleCredentials creds = new
SimpleCredentials(sudoUser, new char[0]);
+
creds.setAttribute(ResourceResolver.USER_IMPERSONATOR, subServiceName == null ?
Review Comment:
@raducotescu , as discussed in private ok with leaving it in since
apparently the resource-resolver api allows to retrieve the impersonator and
does so via a session attribute.
althought that feels a bit awkward to me to use a jcr-session attribute for
that which by coincidence gets populated from session attributes *autsch*
--
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
Re: [PR] SLING-12320 - Add support for retrieving a service resource resolver with impersonation without requiring extra configuration [sling-org-apache-sling-jcr-resource]
anchela commented on code in PR #43:
URL:
https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/43#discussion_r1605245518
##
src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java:
##
@@ -111,7 +111,20 @@ private static Bundle extractCallingBundle(@NotNull
Map authenti
} else {
final Object subService =
authenticationInfo.get(ResourceResolverFactory.SUBSERVICE);
final String subServiceName = subService instanceof
String ? (String) subService : null;
-session = repo.loginService(subServiceName, null);
+// let's shortcut the impersonation for service users,
if impersonation was requested
Review Comment:
👍
--
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
Re: [PR] SLING-12320 - Add support for retrieving a service resource resolver with impersonation without requiring extra configuration [sling-org-apache-sling-jcr-resource]
raducotescu commented on code in PR #43:
URL:
https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/43#discussion_r1605242759
##
src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java:
##
@@ -111,7 +111,20 @@ private static Bundle extractCallingBundle(@NotNull
Map authenti
} else {
final Object subService =
authenticationInfo.get(ResourceResolverFactory.SUBSERVICE);
final String subServiceName = subService instanceof
String ? (String) subService : null;
-session = repo.loginService(subServiceName, null);
+// let's shortcut the impersonation for service users,
if impersonation was requested
Review Comment:
It is a shortcut, since impersonation would have been handled in
https://github.com/apache/sling-org-apache-sling-jcr-resource/blob/f82678ae650a67538d261bcdc5d1e254039e9a37/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java#L150,
through calling
https://github.com/apache/sling-org-apache-sling-jcr-resource/blob/f82678ae650a67538d261bcdc5d1e254039e9a37/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java#L189.
I think I should rephrase the comment to "[...] impersonation for services
[...]", since that was my intention.
--
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
Re: [PR] SLING-12320 - Add support for retrieving a service resource resolver with impersonation without requiring extra configuration [sling-org-apache-sling-jcr-resource]
raducotescu commented on code in PR #43:
URL:
https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/43#discussion_r1605239318
##
src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java:
##
@@ -111,7 +111,20 @@ private static Bundle extractCallingBundle(@NotNull
Map authenti
} else {
final Object subService =
authenticationInfo.get(ResourceResolverFactory.SUBSERVICE);
final String subServiceName = subService instanceof
String ? (String) subService : null;
-session = repo.loginService(subServiceName, null);
+// let's shortcut the impersonation for service users,
if impersonation was requested
+String sudoUser = getSudoUser(authenticationInfo);
+if (sudoUser != null) {
+SimpleCredentials creds = new
SimpleCredentials(sudoUser, new char[0]);
+
creds.setAttribute(ResourceResolver.USER_IMPERSONATOR, subServiceName == null ?
Review Comment:
That attribute seems to be retrieved via
`org.apache.sling.jcr.resource.internal.helper.jcr.JcrResourceProvider#getAttribute`
for `org.apache.sling.api.resource.ResourceResolver#getAttribute` calls.
--
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
Re: [PR] SLING-12320 - Add support for retrieving a service resource resolver with impersonation without requiring extra configuration [sling-org-apache-sling-jcr-resource]
anchela commented on code in PR #43:
URL:
https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/43#discussion_r1605209974
##
src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java:
##
@@ -111,7 +111,20 @@ private static Bundle extractCallingBundle(@NotNull
Map authenti
} else {
final Object subService =
authenticationInfo.get(ResourceResolverFactory.SUBSERVICE);
final String subServiceName = subService instanceof
String ? (String) subService : null;
-session = repo.loginService(subServiceName, null);
+// let's shortcut the impersonation for service users,
if impersonation was requested
+String sudoUser = getSudoUser(authenticationInfo);
+if (sudoUser != null) {
+SimpleCredentials creds = new
SimpleCredentials(sudoUser, new char[0]);
+
creds.setAttribute(ResourceResolver.USER_IMPERSONATOR, subServiceName == null ?
Review Comment:
why is that attribute needed?
SlingRepository.impersonateFromService just takes a credential object and
the method itself already implies that an impersonation is requested to be
performed.
the extra attribute is not needed afaik and the implementation of
SlingRepository.impersonateFromService that is executed in AEM and the default
in sling
(https://github.com/apache/sling-org-apache-sling-jcr-base/blob/master/src/main/java/org/apache/sling/jcr/base/AbstractSlingRepository2.java#L391-L427)
don't respect it. but maybe that explains my confusion with the comment (see
above)
##
src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java:
##
@@ -111,7 +111,20 @@ private static Bundle extractCallingBundle(@NotNull
Map authenti
} else {
final Object subService =
authenticationInfo.get(ResourceResolverFactory.SUBSERVICE);
final String subServiceName = subService instanceof
String ? (String) subService : null;
-session = repo.loginService(subServiceName, null);
+// let's shortcut the impersonation for service users,
if impersonation was requested
Review Comment:
the comment confuses me... what you are handling here is getServiceResolver
request that in addition comes with a hint that an impersonation should be
performed i.e. calling impersonateFromService instead of just loginService.
the first part 'impersonation for service users is the one that confuses
me impersonateFromService means that a service should perform an
impersonation of any user. from the osgi service (bundle/subservice) a given
specified user should be impersonated. it's not that the service user is in
charge here... it could be but that's an impl detail.
--
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
Re: [PR] SLING-12320 - Add support for retrieving a service resource resolver with impersonation without requiring extra configuration [sling-org-apache-sling-jcr-resource]
sonarcloud[bot] commented on PR #43: URL: https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/43#issuecomment-2117858021 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=43) **Quality Gate passed** Issues  [2 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=43&resolved=false&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=43&resolutions=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=43&resolved=false&sinceLeakPeriod=true)  [91.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=43&metric=new_coverage&view=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=43&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-resource&pullRequest=43) -- 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
[PR] SLING-12320 - Add support for retrieving a service resource resolver with impersonation without requiring extra configuration [sling-org-apache-sling-jcr-resource]
raducotescu opened a new pull request, #43: URL: https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/43 * for service users that want to impersonate, simply call SlingRepository#impersonateFromService -- 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
[jira] [Created] (SLING-12320) Add support for retrieving a service resource resolver with impersonation without requiring extra configuration
Radu Cotescu created SLING-12320: Summary: Add support for retrieving a service resource resolver with impersonation without requiring extra configuration Key: SLING-12320 URL: https://issues.apache.org/jira/browse/SLING-12320 Project: Sling Issue Type: Improvement Components: JCR Reporter: Radu Cotescu Fix For: JCR Resource 3.3.2 SLING-4888 simplified retrieving impersonated sessions from service users, however the JCR Resource Provider seems to still use the approach before this improvement. -- This message was sent by Atlassian Jira (v8.20.10#820010)
Re: [PR] SLING-11716 ability to cache the results of a caconfig lookup [sling-org-apache-sling-caconfig-impl]
stefanseifert commented on PR #9: URL: https://github.com/apache/sling-org-apache-sling-caconfig-impl/pull/9#issuecomment-2116999103 lgtm in general, some remarks: * making it configurable with default switched off is not really helpful - either we consider this caching safe then we can enable it by default - or if not, what can we do to make it safe * when may this caching be harmful? i see two use cases ** you have a long-running resource resolver e.g. in an OSGi service. this is an anti-pattern, but sometimes required e.g. if you are listing for resource change events. ** you are actually writing caconfig in your current request and reading it later (most time this is a rare use case) * maybe instead of having an OSGi configuration we should add a switch to the caconfig API where you can disable the caching if you are using caconfig in one of this use cases * more cosmetic: we should adapt the pattern to build cache keys to use separator chars which are not allowed in JCR paths and not "--" - to be on the safe side. -- 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
