[ https://issues.apache.org/jira/browse/SLING-6053?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15487165#comment-15487165 ]
Miklos Csere commented on SLING-6053: ------------------------------------- >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. test_siblingNodeShouldNotHaveAuthenticationInfo --> Fails due to reported issue test_childNodeShouldHaveAuthenticationInfo --> Test is ok for above presented case. /** * 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 { } }; } > SlingAuthenticator identifies wrong 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)