[
https://issues.apache.org/jira/browse/SLING-6053?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15487165#comment-15487165
]
Miklos Csere edited comment on SLING-6053 at 9/13/16 1:14 PM:
--------------------------------------------------------------
>From what I understand the path is checked using String.startsWith because
>also child nodes should be protected.
This does not take into consideration the case when there is a sibling node
with a path that starts the same.
Eg: /content/page && /content/page1
I have created the following tests in SlingAuthenticatorTest class:
http://pastebin.com/8fzseKLY
test_siblingNodeShouldNotHaveAuthenticationInfo --> Fails due to reported issue
test_childNodeShouldHaveAuthenticationInfo --> Test is ok for above
presented case.
{code:java}
/**
* Test is KO, not working.
*
* Issue can be reproduced with the following steps:
*
* Create node "/page"
* Create sibling node "/page1"
* Define a protection handler for node: "/page"
*
* Expected: "/page" has AuthenticationInfo
* "/page1" does not have AuthenticationInfo (has anonymous)
*
* Actual: "/page" & "page1" are both having AuthenticationInfo
*
* Reason: SlingAuthenticator.java line 726: if
(path.startsWith(holder.path))
* Warning: The same check is used in 4 more places in code with similar
behaviour.
*
* @throws Throwable
*/
public void test_siblingNodeShouldNotHaveAuthenticationInfo() throws
Throwable {
final String AUTH_TYPE = "AUTH_TYPE_TEST";
final String PROTECTED_PATH = "/test";
final String REQUEST_NOT_PROTECTED_PATH = "/test2";
SlingAuthenticator slingAuthenticator = new SlingAuthenticator();
PathBasedHolderCache<AbstractAuthenticationHandlerHolder>
authRequiredCache = new
PathBasedHolderCache<AbstractAuthenticationHandlerHolder>();
authRequiredCache.addHolder(buildAuthHolderForAuthTypeAndPath(AUTH_TYPE,
PROTECTED_PATH));
PrivateAccessor.setField(slingAuthenticator, "authHandlerCache",
authRequiredCache);
final HttpServletRequest request =
context.mock(HttpServletRequest.class);
buildExpectationsForRequestPathAndAuthPath(request,
REQUEST_NOT_PROTECTED_PATH, PROTECTED_PATH);
AuthenticationInfo authInfo = (AuthenticationInfo)
PrivateAccessor.invoke(slingAuthenticator, "getAuthenticationInfo",
new Class[]{HttpServletRequest.class,
HttpServletResponse.class}, new Object[]{request,
context.mock(HttpServletResponse.class)});
/**
* The AUTH TYPE defined aboved should not be used for the path /test2.
*/
assertFalse(AUTH_TYPE.equals(authInfo.getAuthType()));
}
/**
* Test is OK for child node;
* @throws Throwable
*/
public void test_childNodeShouldHaveAuthenticationInfo() throws Throwable {
final String AUTH_TYPE = "AUTH_TYPE_TEST";
final String PROTECTED_PATH = "/test";
final String REQUEST_CHILD_NODE = "/test/childnodetest";
SlingAuthenticator slingAuthenticator = new SlingAuthenticator();
PathBasedHolderCache<AbstractAuthenticationHandlerHolder>
authRequiredCache = new
PathBasedHolderCache<AbstractAuthenticationHandlerHolder>();
authRequiredCache.addHolder(buildAuthHolderForAuthTypeAndPath(AUTH_TYPE,
PROTECTED_PATH));
PrivateAccessor.setField(slingAuthenticator, "authHandlerCache",
authRequiredCache);
final HttpServletRequest request =
context.mock(HttpServletRequest.class);
buildExpectationsForRequestPathAndAuthPath(request, REQUEST_CHILD_NODE,
PROTECTED_PATH);
AuthenticationInfo authInfo = (AuthenticationInfo)
PrivateAccessor.invoke(slingAuthenticator, "getAuthenticationInfo",
new Class[]{HttpServletRequest.class,
HttpServletResponse.class}, new Object[]{request,
context.mock(HttpServletResponse.class)});
/**
* The AUTH TYPE defined aboved should be used for the path /test and
his children: eg /test/childnode.
*/
assertTrue(AUTH_TYPE.equals(authInfo.getAuthType()));
}
/**
* Mocks the request to accept method calls on path;
*
* @param request http request
* @param requestPath path in the http request
* @param authProtectedPath path protected by the auth handler
*/
private void buildExpectationsForRequestPathAndAuthPath(final
HttpServletRequest request,
final String
requestPath,
final String
authProtectedPath) {
{
context.checking(new Expectations() {
{
allowing(request).getServletPath();
will(returnValue(requestPath));
allowing(request).getPathInfo();
will(returnValue(null));
allowing(request).getServerName();
will(returnValue("localhost"));
allowing(request).getServerPort();
will(returnValue(80));
allowing(request).getScheme();
will(returnValue("http"));
allowing(request).getAttribute("path");
will(returnValue(requestPath));
allowing(request).setAttribute("path", requestPath);
allowing(request).setAttribute("path", authProtectedPath);
}
});
}
}
/**
* Builds an auth handler for a specific path;
* @param authType name of the auth for this path
* @param authProtectedPath path protected by the auth handler
* @return
*/
private AbstractAuthenticationHandlerHolder
buildAuthHolderForAuthTypeAndPath(final String authType, final String
authProtectedPath) {
return new AbstractAuthenticationHandlerHolder(authProtectedPath, null)
{
@Override
protected AuthenticationFeedbackHandler getFeedbackHandler() {
return null;
}
@Override
protected AuthenticationInfo
doExtractCredentials(HttpServletRequest request, HttpServletResponse response) {
return new AuthenticationInfo(authType);
}
@Override
protected boolean doRequestCredentials(HttpServletRequest request,
HttpServletResponse response) throws IOException {
return false;
}
@Override
protected void doDropCredentials(HttpServletRequest request,
HttpServletResponse response) throws IOException {
}
};
}
{code}
was (Author: mkbrv):
>From what I understand the path is checked using String.startsWith because
>also child nodes should be protected.
This does not take into consideration the case when there is a sibling node
with a path that starts the same.
Eg: /content/page && /content/page1
I have created the following tests in SlingAuthenticatorTest class:
http://pastebin.com/8fzseKLY
test_siblingNodeShouldNotHaveAuthenticationInfo --> Fails due to reported issue
test_childNodeShouldHaveAuthenticationInfo --> Test is ok for above
presented case.
{quote}
/**
* Test is KO, not working.
*
* Issue can be reproduced with the following steps:
*
* Create node "/page"
* Create sibling node "/page1"
* Define a protection handler for node: "/page"
*
* Expected: "/page" has AuthenticationInfo
* "/page1" does not have AuthenticationInfo (has anonymous)
*
* Actual: "/page" & "page1" are both having AuthenticationInfo
*
* Reason: SlingAuthenticator.java line 726: if
(path.startsWith(holder.path))
* Warning: The same check is used in 4 more places in code with similar
behaviour.
*
* @throws Throwable
*/
public void test_siblingNodeShouldNotHaveAuthenticationInfo() throws
Throwable {
final String AUTH_TYPE = "AUTH_TYPE_TEST";
final String PROTECTED_PATH = "/test";
final String REQUEST_NOT_PROTECTED_PATH = "/test2";
SlingAuthenticator slingAuthenticator = new SlingAuthenticator();
PathBasedHolderCache<AbstractAuthenticationHandlerHolder>
authRequiredCache = new
PathBasedHolderCache<AbstractAuthenticationHandlerHolder>();
authRequiredCache.addHolder(buildAuthHolderForAuthTypeAndPath(AUTH_TYPE,
PROTECTED_PATH));
PrivateAccessor.setField(slingAuthenticator, "authHandlerCache",
authRequiredCache);
final HttpServletRequest request =
context.mock(HttpServletRequest.class);
buildExpectationsForRequestPathAndAuthPath(request,
REQUEST_NOT_PROTECTED_PATH, PROTECTED_PATH);
AuthenticationInfo authInfo = (AuthenticationInfo)
PrivateAccessor.invoke(slingAuthenticator, "getAuthenticationInfo",
new Class[]{HttpServletRequest.class,
HttpServletResponse.class}, new Object[]{request,
context.mock(HttpServletResponse.class)});
/**
* The AUTH TYPE defined aboved should not be used for the path /test2.
*/
assertFalse(AUTH_TYPE.equals(authInfo.getAuthType()));
}
/**
* Test is OK for child node;
* @throws Throwable
*/
public void test_childNodeShouldHaveAuthenticationInfo() throws Throwable {
final String AUTH_TYPE = "AUTH_TYPE_TEST";
final String PROTECTED_PATH = "/test";
final String REQUEST_CHILD_NODE = "/test/childnodetest";
SlingAuthenticator slingAuthenticator = new SlingAuthenticator();
PathBasedHolderCache<AbstractAuthenticationHandlerHolder>
authRequiredCache = new
PathBasedHolderCache<AbstractAuthenticationHandlerHolder>();
authRequiredCache.addHolder(buildAuthHolderForAuthTypeAndPath(AUTH_TYPE,
PROTECTED_PATH));
PrivateAccessor.setField(slingAuthenticator, "authHandlerCache",
authRequiredCache);
final HttpServletRequest request =
context.mock(HttpServletRequest.class);
buildExpectationsForRequestPathAndAuthPath(request, REQUEST_CHILD_NODE,
PROTECTED_PATH);
AuthenticationInfo authInfo = (AuthenticationInfo)
PrivateAccessor.invoke(slingAuthenticator, "getAuthenticationInfo",
new Class[]{HttpServletRequest.class,
HttpServletResponse.class}, new Object[]{request,
context.mock(HttpServletResponse.class)});
/**
* The AUTH TYPE defined aboved should be used for the path /test and
his children: eg /test/childnode.
*/
assertTrue(AUTH_TYPE.equals(authInfo.getAuthType()));
}
/**
* Mocks the request to accept method calls on path;
*
* @param request http request
* @param requestPath path in the http request
* @param authProtectedPath path protected by the auth handler
*/
private void buildExpectationsForRequestPathAndAuthPath(final
HttpServletRequest request,
final String
requestPath,
final String
authProtectedPath) {
{
context.checking(new Expectations() {
{
allowing(request).getServletPath();
will(returnValue(requestPath));
allowing(request).getPathInfo();
will(returnValue(null));
allowing(request).getServerName();
will(returnValue("localhost"));
allowing(request).getServerPort();
will(returnValue(80));
allowing(request).getScheme();
will(returnValue("http"));
allowing(request).getAttribute("path");
will(returnValue(requestPath));
allowing(request).setAttribute("path", requestPath);
allowing(request).setAttribute("path", authProtectedPath);
}
});
}
}
/**
* Builds an auth handler for a specific path;
* @param authType name of the auth for this path
* @param authProtectedPath path protected by the auth handler
* @return
*/
private AbstractAuthenticationHandlerHolder
buildAuthHolderForAuthTypeAndPath(final String authType, final String
authProtectedPath) {
return new AbstractAuthenticationHandlerHolder(authProtectedPath, null)
{
@Override
protected AuthenticationFeedbackHandler getFeedbackHandler() {
return null;
}
@Override
protected AuthenticationInfo
doExtractCredentials(HttpServletRequest request, HttpServletResponse response) {
return new AuthenticationInfo(authType);
}
@Override
protected boolean doRequestCredentials(HttpServletRequest request,
HttpServletResponse response) throws IOException {
return false;
}
@Override
protected void doDropCredentials(HttpServletRequest request,
HttpServletResponse response) throws IOException {
}
};
}
{quote}
> SlingAuthenticator identifies wrong sibling node with AuthenticationInfo
> ------------------------------------------------------------------------
>
> Key: SLING-6053
> URL: https://issues.apache.org/jira/browse/SLING-6053
> Project: Sling
> Issue Type: Bug
> Components: Authentication
> Reporter: Miklos Csere
>
> Issue can be reproduced with the following steps:
> Create node "/page"
> Create sibling node "/page1"
> Define a protection handler for node: "/page"
> Expected:
> "/page" has AuthenticationInfo
> "/page1" does not have AuthenticationInfo (has anonymous)
>
> Actual: "/page" & "page1" are both having AuthenticationInfo
>
> Reason: SlingAuthenticator.java line 726: if (path.startsWith(holder.path))
> Warning: The same check is used in 4 more places in code with similar
> behaviour.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
