[jira] [Comment Edited] (SLING-6053) SlingAuthenticator identifies wrong sibling node with AuthenticationInfo
[
https://issues.apache.org/jira/browse/SLING-6053?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16004498#comment-16004498
]
Konrad Windszus edited comment on SLING-6053 at 5/10/17 11:22 AM:
--
But {{isNodeRequiresAuthHandler("/resource1.test.html", "/resource1")}} returns
true, which is not intended if there is a resource named "/resource1.test".
was (Author: kwin):
But {{isNodeRequiresAuthHandler("/resource1.test.html", "/resource1")}} returns
true
> 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
>Affects Versions: Auth Core 1.3.18
>Reporter: Miklos Csere
>Assignee: Antonio Sanso
>Priority: Blocker
> Fix For: Auth Core 1.3.26
>
> Attachments: SLING-6053-patch.txt
>
>
> 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.15#6346)
[jira] [Comment Edited] (SLING-6053) SlingAuthenticator identifies wrong sibling node with AuthenticationInfo
[
https://issues.apache.org/jira/browse/SLING-6053?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15885587#comment-15885587
]
Konrad Windszus edited comment on SLING-6053 at 2/27/17 11:12 AM:
--
[~asanso] The patch is only a heuristic and does not work for all cases. Just
imagine the following use case
{{resource1}} requires authentication
{{resource1.test2}} does not require authentication
In that case the latter would also be covered by your logic in
{{isNodeRequiresAuthHandler}} which returns {{true}} but in fact it should not.
I am not sure, whether that behavior is better or worse than before. (Better
because it will for most of the cases work as expected, worse, because it is
even harder to document the behavior for resource names containing "." itself).
The problem is that with just having the request URL String you cannot tell,
what is a selector and what belongs to the resource's name.
Also the import to {{import javax.crypto.spec.OAEPParameterSpec}} seems wrong
in the patch.
was (Author: kwin):
[~asanso] The patch is only a heuristic and does not work for all cases. Just
imagine the following use case
{{resource1}} requires authentication
{{resource1.test2}} does not require authentication
In that case the latter would also be covered by your logic in
{{isNodeRequiresAuthHandler}} which returns {{true}} but in fact it should not.
I am not sure, whether that behavior is better or worse than before. (Better
because it will for most of the cases work as expected, worse, because it is
even harder to document the behavior for resource names containing "." itself).
The problem is that with just having the request URL String you cannot tell,
what is a selector and what belongs to the resource's name.
> 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
>Affects Versions: Auth Core 1.3.18
>Reporter: Miklos Csere
>Assignee: Antonio Sanso
>Priority: Blocker
> Attachments: SLING-6053-patch.txt
>
>
> 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.15#6346)
[jira] [Comment Edited] (SLING-6053) SlingAuthenticator identifies wrong sibling node with AuthenticationInfo
[
https://issues.apache.org/jira/browse/SLING-6053?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15885587#comment-15885587
]
Konrad Windszus edited comment on SLING-6053 at 2/27/17 11:09 AM:
--
[~asanso] The patch is only a heuristic and does not work for all cases. Just
imagine the following use case
{{resource1}} requires authentication
{{resource1.test2}} does not require authentication
In that case the latter would also be covered by your logic in
{{isNodeRequiresAuthHandler}} which returns {{true}} but in fact it should not.
I am not sure, whether that behavior is better or worse than before. (Better
because it will for most of the cases work as expected, worse, because it is
even harder to document the behavior for resource names containing "." itself).
The problem is that with just having the request URL String you cannot tell,
what is a selector and what belongs to the resource's name.
was (Author: kwin):
[~asanso] The patch is only a heuristic and does not work for all cases. Just
imagine the following use case
{{resource1}} requires authentication
{{resource1.test2}} does not require authentication
In that case the latter would also be covered by your logic in
{{isNodeRequiresAuthHandler}} but in fact it should not.
I am not sure, whether that behavior is better or worse then before. (Better
because it will for most of the cases work as expected, worse, because it is
even harder to document the behaviour for resource names containing "." itself).
The problem is that with just having the request URL String you cannot tell,
what is a selector and what belongs to the resource's name.
> 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
>Affects Versions: Auth Core 1.3.18
>Reporter: Miklos Csere
>Assignee: Antonio Sanso
>Priority: Blocker
> Attachments: SLING-6053-patch.txt
>
>
> 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.15#6346)
[jira] [Comment Edited] (SLING-6053) SlingAuthenticator identifies wrong sibling node with AuthenticationInfo
[
https://issues.apache.org/jira/browse/SLING-6053?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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
authRequiredCache = new
PathBasedHolderCache();
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
authRequiredCache = new
PathBasedHolderCache();
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 authProtectedPathpath 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"));
[jira] [Comment Edited] (SLING-6053) SlingAuthenticator identifies wrong sibling node with AuthenticationInfo
[
https://issues.apache.org/jira/browse/SLING-6053?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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 an auth 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
authRequiredCache = new
PathBasedHolderCache();
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
authRequiredCache = new
PathBasedHolderCache();
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 authProtectedPathpath 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"));
