[
https://issues.apache.org/jira/browse/SLING-3524?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16459515#comment-16459515
]
Csaba Varga commented on SLING-3524:
------------------------------------
{quote}I think the "needsCloning/needsSudo/explicitSessionUsed" might need some
explanation in comments, it's not straightforward to follow.
{quote}
Fair point. I've added some comments now. explicitSessionUsed is already
commented in the Javadoc of handleImpersonation(). Does it need some
clarification?
{quote}Are all possible combinations covered correctly?
{quote}
I think so, but I've gone through them this morning to double check. I believe
we have three independent variables to consider, giving eight cases in total:
* Is it a normal login or is a session given in the auth info? (For this
variable, I believe we can treat service logins and administrative logins as
normal logins.)
* Does the login request impersonation or not?
* Is the login caused by a clone() call?
This is what happens in the eight cases:
||Scenario||No clone||Clone||
|Normal login, no sudo|logoutSession: true
no impersonation call
doLogoutSession: true|logoutSession: true
no impersonation call
doLogoutSession: true|
|Normal login with sudo|logoutSession: true
original session impersonated then closed, USER_IMPERSONATOR set
doLogoutSession: true|logoutSession: true
original session impersonated then closed, USER_IMPERSONATOR set
doLogoutSession: true|
|Session login, no sudo|logoutSession: false
session used as-is
doLogoutSession: false|logoutSession: false
session self-impersonated
doLogoutSession: true|
|Session login with sudo|logoutSession: false
session impersonated, USER_IMPERSONATOR set
doLogoutSession: true|logoutSession: false
session impersonated, USER_IMPERSONATOR set
doLogoutSession: true|
The only case when cloning behavior is different from normal behavior is when
you pass a session but you don't want to impersonate. If you don't pass a
session, cloning will just log you in again with your credentials, just like
before the patch. If you pass a session and you request impersonation,
session.impersonate() was already called before this patch, and will keep being
called after it.
Am I missing something? If these are all the factors we need to worry about, do
you think it's worthwhile to build a parameterized unit test that covers all
possible combinations?
> ResourceResolver.clone(null) should not share the same JCR session
> ------------------------------------------------------------------
>
> Key: SLING-3524
> URL: https://issues.apache.org/jira/browse/SLING-3524
> Project: Sling
> Issue Type: Improvement
> Components: JCR, ResourceResolver
> Affects Versions: Resource Resolver 1.0.6
> Reporter: Alexander Klimetschek
> Priority: Major
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> {{ResourceResolver.clone()}} will reuse the same JCR session in case it was
> created by passing an existing session using
> {{JcrResourceConstants.AUTHENTICATION_INFO_SESSION}}. If you need a clone of
> the resource resolver to pass into a new, separate thread, and use
> {{ResourceResolver.clone(null)}}, you will actually share the session, but
> this is not obvious. The problem is that a JCR session cannot be shared
> across threads.
> The javadocs of clone() say "the same credential data is used as was used to
> create this instance".
> There are a few problems with this:
> - seeing the session object itself as "credential data" is unintuitive
> - in my code, I have no idea what the original credential data was, so I
> don't know what kind of credential data it was to make the right decision
> - since sharing a JCR session is to be avoided at all times, the resource
> resolver should prevent one from this
> A solution would be if a plain {{ResourceResolver.clone(null)}} would return
> a session that impersonated itself, abstracting this from the resource
> resolver user. Additionally, it might be worth looking that clone always
> returns a new session, unless specifically stated.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
