Repository: wicket Updated Branches: refs/heads/sandbox/cryptomapper-request-listener [created] d30a4a59e
WICKET-5326 Wicket doesn't encrypt links and Ajax URLs when CryptoMapper is used Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/c6398901 Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/c6398901 Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/c6398901 Branch: refs/heads/sandbox/cryptomapper-request-listener Commit: c63989018a8fb11a3dbbc68253e74f3fd2117430 Parents: 58fa34b Author: Jesse Long <[email protected]> Authored: Thu Oct 9 17:20:39 2014 +0200 Committer: Jesse Long <[email protected]> Committed: Thu Oct 9 17:20:39 2014 +0200 ---------------------------------------------------------------------- .../core/request/mapper/CryptoMapper.java | 402 ++++++++++++++--- .../core/request/mapper/CryptoMapperTest.java | 449 +++++++++++++++---- .../wicket/settings/ISecuritySettingsTest.java | 46 +- 3 files changed, 734 insertions(+), 163 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/c6398901/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/CryptoMapper.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/CryptoMapper.java b/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/CryptoMapper.java index 58fc6b6..8dde7b1 100755 --- a/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/CryptoMapper.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/CryptoMapper.java @@ -16,6 +16,7 @@ */ package org.apache.wicket.core.request.mapper; +import java.util.Iterator; import java.util.List; import org.apache.wicket.Application; @@ -25,26 +26,49 @@ import org.apache.wicket.request.IRequestMapper; import org.apache.wicket.request.Request; import org.apache.wicket.request.Url; import org.apache.wicket.request.mapper.IRequestMapperDelegate; +import org.apache.wicket.request.mapper.info.PageComponentInfo; import org.apache.wicket.settings.ISecuritySettings; import org.apache.wicket.util.IProvider; import org.apache.wicket.util.crypt.ICrypt; import org.apache.wicket.util.crypt.ICryptFactory; import org.apache.wicket.util.lang.Args; import org.apache.wicket.util.string.Strings; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * Request mapper that encrypts urls generated by another mapper. The original URL (both segments - * and parameters) is encrypted and is represented as URL segment. To be able to handle relative - * URLs for images in .css file the same amount of URL segments that the original URL had are - * appended to the encrypted URL. Each segment has a precise 5 character value, calculated using a - * checksum. This helps in calculating the relative distance from the original URL. When a URL is - * returned by the browser, we iterate through these checksummed placeholder URL segments. If the - * segment matches the expected checksum, then the segment it deemed to be the corresponding segment - * in the encrypted URL. If the segment does not match the expected checksum, then the segment is - * deemed a plain text sibling of the corresponding segment in the encrypted URL, and all subsequent + * <p> + * A request mapper that encrypts URLs generated by another mapper. This mapper encrypts the segments + * and query parameters of URLs starting with {@code /wicket/}, and the just the {@link PageComponentInfo} + * parameter for mounted URLs. + * </p> + * + * <p> + * This mapper can be mounted before or after mounting other pages, but will only encrypt URLs for + * pages mounted before the {@link CryptoMapper}. If required, multiple {@link CryptoMapper}s may be + * installed in an {@link Application}. + * </p> + * + * <p> + * When encrypting URLs in the Wicket namespace (starting with {@code /wicket/}), the entire URL, including + * segments and parameters, is encrypted, with the encrypted form stored in the first segment of the encrypted URL. + * </p> + * + * <p> + * To be able to handle relative URLs, like for image URLs in a CSS file, checksum segments are appended to the + * encrypted URL until the encrypted URL has the same number of segments as the original URL had. + * Each checksum segment has a precise 5 character value, calculated using a checksum. This helps in calculating + * the relative distance from the original URL. When a URL is returned by the browser, we iterate through these + * checksummed placeholder URL segments. If the segment matches the expected checksum, then the segment it deemed + * to be the corresponding segment in the original URL. If the segment does not match the expected checksum, then + * the segment is deemed a plain text sibling of the corresponding segment in the original URL, and all subsequent * segments are considered plain text children of the current segment. + * </p> + * + * <p> + * When encrypting mounted URLs, we look for the {@link PageComponentInfo} parameter, and encrypt only that parameter. + * </p> * * @author igor.vaynberg * @author Jesse Long @@ -54,6 +78,11 @@ public class CryptoMapper implements IRequestMapperDelegate { private static final Logger log = LoggerFactory.getLogger(CryptoMapper.class); + /** + * Name of the parameter which contains encrypted page component info. + */ + private static final String ENCRYPTED_PAGE_COMPONENT_INFO_PARAMETER = "wicket"; + private final IRequestMapper wrappedMapper; private final IProvider<ICrypt> cryptProvider; @@ -88,12 +117,34 @@ public class CryptoMapper implements IRequestMapperDelegate this.cryptProvider = Args.notNull(cryptProvider, "cryptProvider"); } + /** + * {@inheritDoc} + * <p> + * This implementation decrypts the URL and passes the decrypted URL to the wrapped mapper. + * </p> + * @param request + * The request for which to get a compatability score. + * + * @return The compatability score. + */ @Override public int getCompatibilityScore(final Request request) { - return wrappedMapper.getCompatibilityScore(request); + Url decryptedUrl = decryptUrl(request, request.getUrl()); + + if (decryptedUrl == null) + { + return 0; + } + + Request decryptedRequest = request.cloneWithUrl(decryptedUrl); + + return wrappedMapper.getCompatibilityScore(decryptedRequest); } + /** + * {@inheritDoc} + */ @Override public Url mapHandler(final IRequestHandler requestHandler) { @@ -113,6 +164,9 @@ public class CryptoMapper implements IRequestMapperDelegate return encryptUrl(url); } + /** + * {@inheritDoc} + */ @Override public IRequestHandler mapRequest(final Request request) { @@ -120,7 +174,7 @@ public class CryptoMapper implements IRequestMapperDelegate if (url == null) { - return wrappedMapper.mapRequest(request); + return null; } Request decryptedRequest = request.cloneWithUrl(url); @@ -164,18 +218,44 @@ public class CryptoMapper implements IRequestMapperDelegate return wrappedMapper; } + /** + * Encrypts a URL. This method should return a new, encrypted instance of the URL. If the URL starts with {@code /wicket/}, + * the entire URL is encrypted. + * + * @param url + * The URL to encrypt. + * + * @return A new, encrypted version of the URL. + */ protected Url encryptUrl(final Url url) { - if (url.getSegments().isEmpty()) + if (url.getSegments().size() > 0 + && url.getSegments().get(0).equals(Application.get().getMapperContext().getNamespace())) { - return url; + return encryptEntireUrl(url); } + else + { + return encryptRequestListenerParameter(url); + } + } + + /** + * Encrypts an entire URL, segments and query parameters. + * + * @param url + * The URL to encrypt. + * + * @return An encrypted form of the URL. + */ + protected Url encryptEntireUrl(final Url url) + { String encryptedUrlString = getCrypt().encryptUrlSafe(url.toString()); Url encryptedUrl = new Url(url.getCharset()); encryptedUrl.getSegments().add(encryptedUrlString); - int numberOfSegments = url.getSegments().size(); + int numberOfSegments = url.getSegments().size() - 1; HashedSegmentGenerator generator = new HashedSegmentGenerator(encryptedUrlString); for (int segNo = 0; segNo < numberOfSegments; segNo++) { @@ -184,86 +264,274 @@ public class CryptoMapper implements IRequestMapperDelegate return encryptedUrl; } + /** + * Encrypts the {@link PageComponentInfo} query parameter in the URL, if any is found. + * + * @param url + * The URL to encrypt. + * + * @return An encrypted form of the URL. + */ + protected Url encryptRequestListenerParameter(final Url url) + { + Url encryptedUrl = new Url(url); + + for (Iterator<Url.QueryParameter> it = encryptedUrl.getQueryParameters().iterator(); it.hasNext();) + { + Url.QueryParameter qp = it.next(); + + if (Strings.isEmpty(qp.getValue()) == true && Strings.isEmpty(qp.getName()) == false) + { + if (PageComponentInfo.parse(qp.getName()) != null) + { + it.remove(); + String encryptedParameterValue = getCrypt().encryptUrlSafe(qp.getName()); + Url.QueryParameter encryptedParameter + = new Url.QueryParameter(ENCRYPTED_PAGE_COMPONENT_INFO_PARAMETER, encryptedParameterValue); + encryptedUrl.getQueryParameters().add(0, encryptedParameter); + break; + } + } + } + + return encryptedUrl; + } + + /** + * Decrypts a {@link Url}. This method should return {@code null} if the URL is not decryptable, or if the + * URL should have been encrypted but was not. Returning {@code null} results in a 404 error. + * + * @param request + * The {@link Request}. + * @param encryptedUrl + * The encrypted {@link Url}. + * + * @return Returns a decrypted {@link Url}. + */ protected Url decryptUrl(final Request request, final Url encryptedUrl) { - /* - * If the encrypted URL has no segments it is the home page URL, and does not need - * decrypting. - */ - if (encryptedUrl.getSegments().isEmpty()) + Url url = decryptEntireUrl(request, encryptedUrl); + + if (url == null) { - return encryptedUrl; + if (encryptedUrl.getSegments().size() > 0 + && encryptedUrl.getSegments().get(0).equals(Application.get().getMapperContext().getNamespace())) + { + /* + * This URL should have been encrypted, but was not. We should refuse to handle this, except when + * there is more than one CryptoMapper installed, and the request was decrypted by some other + * CryptoMapper. + */ + if (request.getOriginalUrl().getSegments().size() > 0 + && request.getOriginalUrl().getSegments().get(0).equals(Application.get().getMapperContext().getNamespace())) + { + return null; + } + else + { + return encryptedUrl; + } + } } - List<String> encryptedSegments = encryptedUrl.getSegments(); + if (url == null) + { + url = decryptRequestListenerParameter(request, encryptedUrl); + } + + return url; + } + /** + * Decrypts an entire URL, which was previously encrypted by {@link #encryptEntireUrl(org.apache.wicket.request.Url)}. + * This method should return {@code null} if the URL is not decryptable. + * + * @param request + * The request that was made. + * @param encryptedUrl + * The encrypted URL. + * + * @return A decrypted form of the URL, or {@code null} if the URL is not decryptable. + */ + protected Url decryptEntireUrl(final Request request, final Url encryptedUrl) + { Url url = new Url(request.getCharset()); + + List<String> encryptedSegments = encryptedUrl.getSegments(); + + if (encryptedSegments.isEmpty()) + { + return null; + } + + /* + * The first encrypted segment contains an encrypted version of the entire plain text url. + */ + String encryptedUrlString = encryptedSegments.get(0); + if (Strings.isEmpty(encryptedUrlString)) + { + return null; + } + + String decryptedUrl; try { + decryptedUrl = getCrypt().decryptUrlSafe(encryptedUrlString); + } + catch (Exception e) + { + log.error("Error decrypting URL", e); + return null; + } + + if (decryptedUrl == null) + { + return null; + } + + Url originalUrl = Url.parse(decryptedUrl, request.getCharset()); + + int originalNumberOfSegments = originalUrl.getSegments().size(); + int encryptedNumberOfSegments = encryptedUrl.getSegments().size(); + + if (originalNumberOfSegments > 0) + { /* - * The first encrypted segment contains an encrypted version of the entire plain text - * url. + * This should always be true. Home page URLs are the only ones without + * segments, and we dont encrypt those with this method. + * + * We always add the first segment of the URL, because we encrypt a URL like: + * /path/to/something + * to: + * /encrypted_full/hash/hash + * + * Notice the consistent number of segments. If we applied the following relative URL: + * ../../something + * then the resultant URL would be: + * /something + * + * Hence, the mere existence of the first, encrypted version of complete URL, segment + * tells us that the first segment of the original URL is still to be used. */ - String encryptedUrlString = encryptedSegments.get(0); - if (Strings.isEmpty(encryptedUrlString)) + url.getSegments().add(originalUrl.getSegments().get(0)); + } + + HashedSegmentGenerator generator = new HashedSegmentGenerator(encryptedUrlString); + int segNo = 1; + for (; segNo < encryptedNumberOfSegments; segNo++) + { + if (segNo > originalNumberOfSegments) { - return null; + break; } - String decryptedUrl = getCrypt().decryptUrlSafe(encryptedUrlString); - if (decryptedUrl == null) + String next = generator.next(); + String encryptedSegment = encryptedSegments.get(segNo); + if (!next.equals(encryptedSegment)) { - return null; + /* + * This segment received from the browser is not the same as the expected segment generated + * by the HashSegmentGenerator. Hence it, and all subsequent segments are considered plain + * text siblings of the original encrypted url. + */ + break; } - Url originalUrl = Url.parse(decryptedUrl, request.getCharset()); - int originalNumberOfSegments = originalUrl.getSegments().size(); - int encryptedNumberOfSegments = encryptedUrl.getSegments().size(); + /* + * This segments matches the expected checksum, so we add the corresponding segment from the + * original URL. + */ + url.getSegments().add(originalUrl.getSegments().get(segNo)); + } + /* + * Add all remaining segments from the encrypted url as plain text segments. + */ + for (; segNo < encryptedNumberOfSegments; segNo++) + { + // modified or additional segment + url.getSegments().add(encryptedUrl.getSegments().get(segNo)); + } - HashedSegmentGenerator generator = new HashedSegmentGenerator(encryptedUrlString); - int segNo = 1; - for (; segNo < encryptedNumberOfSegments; segNo++) - { - if (segNo > originalNumberOfSegments) - { - break; - } + url.getQueryParameters().addAll(originalUrl.getQueryParameters()); + // WICKET-4923 additional parameters + url.getQueryParameters().addAll(encryptedUrl.getQueryParameters()); + + return url; + } + + /** + * Decrypts a URL which may contain an encrypted {@link PageComponentInfo} query parameter. + * + * @param request + * The request that was made. + * @param encryptedUrl + * The (potentially) encrypted URL. + * + * @return A decrypted form of the URL. + */ + protected Url decryptRequestListenerParameter(final Request request, Url encryptedUrl) + { + Url url = new Url(encryptedUrl); - String next = generator.next(); - String encryptedSegment = encryptedSegments.get(segNo); - if (!next.equals(encryptedSegment)) + url.getQueryParameters().clear(); + + for (Url.QueryParameter qp : encryptedUrl.getQueryParameters()) + { + if (Strings.isEmpty(qp.getValue()) && Strings.isEmpty(qp.getName()) == false) + { + if (PageComponentInfo.parse(qp.getName()) != null) { /* - * This segment received from the browser is not the same as the expected - * segment generated by the HashSegmentGenerator. Hence it, and all subsequent - * segments are considered plain text siblings of the original encrypted url. + * Plain text request listener parameter found. This should have been encrypted, so we + * refuse to map the request unless the original URL did not include this parameter, which + * case there are likely to be multiple cryptomappers installed. */ - break; + if (request.getOriginalUrl().getQueryParameter(qp.getName()) == null) + { + url.getQueryParameters().add(qp); + } + else + { + return null; + } } + } + else if (ENCRYPTED_PAGE_COMPONENT_INFO_PARAMETER.equals(qp.getName())) + { + String encryptedValue = qp.getValue(); - /* - * This segments matches the expected checksum, so we add the corresponding segment - * from the original URL. - */ - url.getSegments().add(originalUrl.getSegments().get(segNo - 1)); + if (Strings.isEmpty(encryptedValue)) + { + url.getQueryParameters().add(qp); + } + else + { + String decryptedValue = null; + + try + { + decryptedValue = getCrypt().decryptUrlSafe(encryptedValue); + } + catch (Exception e) + { + log.error("Error decrypting encrypted request listener query parameter", e); + } + + if (Strings.isEmpty(decryptedValue)) + { + url.getQueryParameters().add(qp); + } + else + { + Url.QueryParameter decryptedParamter = new Url.QueryParameter(decryptedValue, ""); + url.getQueryParameters().add(0, decryptedParamter); + } + } } - /* - * Add all remaining segments from the encrypted url as plain text segments. - */ - for (; segNo < encryptedNumberOfSegments; segNo++) + else { - // modified or additional segment - url.getSegments().add(encryptedUrl.getSegments().get(segNo)); + url.getQueryParameters().add(qp); } - - url.getQueryParameters().addAll(originalUrl.getQueryParameters()); - // WICKET-4923 additional parameters - url.getQueryParameters().addAll(encryptedUrl.getQueryParameters()); - } - catch (Exception e) - { - log.error("Error decrypting URL", e); - url = null; } return url; http://git-wip-us.apache.org/repos/asf/wicket/blob/c6398901/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/CryptoMapperTest.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/CryptoMapperTest.java b/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/CryptoMapperTest.java index e4aa6e5..8a6181c 100644 --- a/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/CryptoMapperTest.java +++ b/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/CryptoMapperTest.java @@ -17,24 +17,30 @@ package org.apache.wicket.core.request.mapper; import org.apache.wicket.MockPage; +import org.apache.wicket.core.request.handler.BookmarkableListenerInterfaceRequestHandler; import org.apache.wicket.core.request.handler.ListenerInterfaceRequestHandler; import org.apache.wicket.core.request.handler.PageAndComponentProvider; import org.apache.wicket.core.request.handler.PageProvider; import org.apache.wicket.core.request.handler.RenderPageRequestHandler; import org.apache.wicket.core.request.handler.RequestSettingRequestHandler; +import org.apache.wicket.markup.IMarkupFragment; +import org.apache.wicket.markup.Markup; +import org.apache.wicket.markup.html.WebPage; import org.apache.wicket.markup.html.link.ILinkListener; import org.apache.wicket.protocol.http.WebApplication; import org.apache.wicket.request.IRequestHandler; +import org.apache.wicket.request.IRequestHandlerDelegate; import org.apache.wicket.request.Request; import org.apache.wicket.request.Url; import org.apache.wicket.request.Url.StringMode; import org.apache.wicket.request.component.IRequestableComponent; import org.apache.wicket.request.handler.resource.ResourceReferenceRequestHandler; +import org.apache.wicket.request.mapper.info.PageComponentInfo; import org.apache.wicket.request.mapper.parameter.PageParameters; import org.apache.wicket.request.resource.PackageResourceReference; import org.apache.wicket.request.resource.UrlResourceReference; import org.apache.wicket.util.string.StringValue; -import org.apache.wicket.util.tester.DummyHomePage; +import org.apache.wicket.util.string.Strings; import org.apache.wicket.util.tester.WicketTester; import org.junit.After; import org.junit.Before; @@ -45,15 +51,11 @@ import org.junit.Test; */ public class CryptoMapperTest extends AbstractMapperTest { - /** - * the encrypted version of {@link #EXPECTED_URL} - */ - private static final String ENCRYPTED_URL = "SnPh82L4Kl4/SnPe4/4Sn8e/nPh75/h8211"; - - /** - * The url to encrypt - */ - private static final Url EXPECTED_URL = Url.parse("a/b/c/d"); + private static final String PLAIN_BOOKMARKABLE_URL = "wicket/bookmarkable/" + Page2.class.getName(); + private static final String ENCRYPTED_BOOKMARKABLE_URL = "L7ExSNbPC4sb6TPJDblCAopL53TWmZP5y7BQEaJSJAC05HXod5M5U7gT2yNT0lK5L6L09ZAOoZkGyUhseyPrC4S5tqUUrV6zipc4_Ni877EmwR8AyCyA-A/L7E59/5y7f2"; + private static final String PLAIN_PAGE_INSTANCE_URL = "wicket/page?5"; + private static final String ENCRYPTED_PAGE_INSTANCE_URL = "fyBfZ9p6trOhokHCzsQS6Q/fyBce"; + private static final String MOUNTED_URL = "path/to/mounted/page"; private CryptoMapper mapper; @@ -61,17 +63,16 @@ public class CryptoMapperTest extends AbstractMapperTest /** * Creates the {@link CryptoMapper} - * + * * @throws Exception */ @Override @Before public void before() throws Exception { - tester = new WicketTester(); WebApplication webApplication = tester.getApplication(); - webApplication.mountPage(EXPECTED_URL.toString(), DummyHomePage.class); + webApplication.mountPage(MOUNTED_URL, Page1.class); mapper = new CryptoMapper(webApplication.getRootRequestMapper(), webApplication); } @@ -85,51 +86,309 @@ public class CryptoMapperTest extends AbstractMapperTest } /** - * Tests that {@link CryptoMapper} wraps the original request mapper and encrypts the url - * produced by it + * Tests that the home page is requestable. */ @Test - public void encrypt() + public void homePage() { - Url url = mapper.mapHandler(new RenderPageRequestHandler(new PageProvider( - DummyHomePage.class, new PageParameters()))); - assertEquals(ENCRYPTED_URL, url.toString()); + IRequestHandler requestHandler = mapper.mapRequest(getRequest(Url.parse(""))); + assertNotNull("Unable to map request for home page", requestHandler); + requestHandler = unwrapRequestHandlerDelegate(requestHandler); + assertTrue(requestHandler instanceof RenderPageRequestHandler); + RenderPageRequestHandler handler = (RenderPageRequestHandler) requestHandler; + assertSame(tester.getApplication().getHomePage(), handler.getPageClass()); } /** - * Tests that {@link CryptoMapper} decrypts the passed url and pass it to the original request - * mapper which resolves the page from the application mounts + * Verifies that the home page can be reached with non-encrypted query parameters. + * https://issues.apache.org/jira/browse/WICKET-4345 + * + * Also, test that the URL for the home page with non-encrypted parameters is not encrypted, to avoid unnecessary redirects. */ @Test - public void decrypt() + public void homePageWithNonEncryptedQueryParameters() { - Request request = getRequest(Url.parse(ENCRYPTED_URL)); + String expectedEncrypted = "?namedKey1=namedValue1"; + PageParameters expectedParameters = new PageParameters(); + expectedParameters.add("namedKey1", "namedValue1"); + + RenderPageRequestHandler renderPageRequestHandler = new RenderPageRequestHandler( + new PageProvider(tester.getApplication().getHomePage(), expectedParameters)); + + Url url = mapper.mapHandler(renderPageRequestHandler); + assertEquals(expectedEncrypted, url.toString()); + + Request request = getRequest(url); IRequestHandler requestHandler = mapper.mapRequest(request); - assertTrue(requestHandler instanceof RequestSettingRequestHandler); - requestHandler = ((RequestSettingRequestHandler)requestHandler).getDelegateHandler(); + assertNotNull(requestHandler); + + requestHandler = unwrapRequestHandlerDelegate(requestHandler); assertTrue(requestHandler instanceof RenderPageRequestHandler); + RenderPageRequestHandler handler = (RenderPageRequestHandler) requestHandler; + assertEquals(tester.getApplication().getHomePage(), handler.getPageClass()); + StringValue queryParam = handler.getPageParameters().get("namedKey1"); + assertEquals("namedValue1", queryParam.toOptionalString()); + } - RenderPageRequestHandler handler = (RenderPageRequestHandler)requestHandler; - assertEquals(DummyHomePage.class, handler.getPageClass()); + /** + * Tests that we do not allow unencrypted URLs to IRequestListeners on the home page, like: ?0-0.ILinkListener-link + */ + @Test + public void homePageForceEncryptionOfRequestListener() + { + PageAndComponentProvider provider = new PageAndComponentProvider(tester.getApplication().getHomePage(), "some:link"); + IRequestHandler requestHandler = new BookmarkableListenerInterfaceRequestHandler(provider, ILinkListener.INTERFACE); + Url plainUrl = mapper.getDelegateMapper().mapHandler(requestHandler); + assertTrue("Plain URL for home page has segments: " + plainUrl.toString(), plainUrl.getSegments().isEmpty()); + assertNull(mapper.mapRequest(getRequest(plainUrl))); } /** - * Verifies that the home page can be reached with non-encrypted query parameters. - * https://issues.apache.org/jira/browse/WICKET-4345 + * Tests that URLs for bookmarkable pages are encrypted. + */ + @Test + public void bookmarkablePageEncrypt() + { + IRequestHandler renderPage2BookmarkableHandler = new RenderPageRequestHandler(new PageProvider( + Page2.class, new PageParameters())); + + Url plainTextUrl = mapper.getDelegateMapper().mapHandler(renderPage2BookmarkableHandler); + + assertEquals(PLAIN_BOOKMARKABLE_URL, plainTextUrl.toString()); + + Url encryptedUrl = mapper.mapHandler(renderPage2BookmarkableHandler); + assertEquals(ENCRYPTED_BOOKMARKABLE_URL, encryptedUrl.toString()); + } + + /** + * Tests that encrypted URLs for bookmarkable pages are decrypted and passed to the wrapped mapper. */ @Test - public void decryptHomePageWithNonEncryptedQueryParameters() + public void bookmarkablePageDecrypt() { - Request request = getRequest(Url.parse("?named1=value1")); + Request request = getRequest(Url.parse(ENCRYPTED_BOOKMARKABLE_URL)); IRequestHandler requestHandler = mapper.mapRequest(request); - assertTrue(requestHandler instanceof RequestSettingRequestHandler); - requestHandler = ((RequestSettingRequestHandler)requestHandler).getDelegateHandler(); + + assertNotNull(requestHandler); + requestHandler = unwrapRequestHandlerDelegate(requestHandler); + assertTrue(requestHandler instanceof RenderPageRequestHandler); - RenderPageRequestHandler handler = (RenderPageRequestHandler)requestHandler; - assertEquals(tester.getApplication().getHomePage(), handler.getPageClass()); - StringValue queryParam = handler.getPageParameters().get("named1"); - assertEquals("value1", queryParam.toOptionalString()); + RenderPageRequestHandler handler = (RenderPageRequestHandler) requestHandler; + assertEquals(Page2.class, handler.getPageClass()); + } + + /** + * Tests that encrypted URLs for bookmarkable pages are decrypted and passed to the wrapped mapper when there is more than + * one cryptomapper installed. + */ + @Test + public void bookmarkablePageDecryptMultipleCryptoMapper() + { + Request request = getRequest(Url.parse(ENCRYPTED_BOOKMARKABLE_URL)); + + IRequestHandler requestHandler = new CryptoMapper(mapper, tester.getApplication()) + .mapRequest(request); + + assertNotNull(requestHandler); + requestHandler = unwrapRequestHandlerDelegate(requestHandler); + + assertTrue(requestHandler instanceof RenderPageRequestHandler); + + RenderPageRequestHandler handler = (RenderPageRequestHandler) requestHandler; + assertEquals(Page2.class, handler.getPageClass()); + } + + /** + * Tests that plain text URLs to bookmarkable pages are not mapped. + */ + @Test + public void bookmarkablePageForceEncryption() + { + IRequestHandler requestHandler = mapper.mapRequest(getRequest(Url.parse(PLAIN_BOOKMARKABLE_URL))); + assertNull(requestHandler); + } + + /** + * Tests that we do not allow unencrypted URLs to IRequestListeners on bookmarkable pages, like: + * wicket/bookmarkable/my.package.page?0-0.ILinkListener-link + */ + @Test + public void bookmarkablePageForceEncryptionOfRequestListener() + { + PageAndComponentProvider provider = new PageAndComponentProvider(Page2.class, "some:link"); + IRequestHandler requestHandler = new BookmarkableListenerInterfaceRequestHandler(provider, ILinkListener.INTERFACE); + Url plainUrl = mapper.getDelegateMapper().mapHandler(requestHandler); + assertTrue("Plain text request listener URL for bookmarkable page does not start with: " + + PLAIN_BOOKMARKABLE_URL + ": " + plainUrl.toString(), + plainUrl.toString().startsWith(PLAIN_BOOKMARKABLE_URL)); + assertNull(mapper.mapRequest(getRequest(plainUrl))); + } + + /** + * Tests that URLs for page instances are encrypted (/wicket/page?5) + */ + @Test + public void pageInstanceEncrypt() + { + MockPage page = new MockPage(5); + IRequestHandler requestHandler = new RenderPageRequestHandler(new PageProvider(page)); + + assertEquals(PLAIN_PAGE_INSTANCE_URL, mapper.getDelegateMapper().mapHandler(requestHandler).toString()); + assertEquals(ENCRYPTED_PAGE_INSTANCE_URL, mapper.mapHandler(requestHandler).toString()); + } + + /** + * Make sure that encrypted page instance URLs are decrypted and the correct handler resolved. + */ + @Test + public void pageInstanceDecrypt() + { + IRequestHandler requestHandler = mapper.mapRequest(getRequest(Url.parse(ENCRYPTED_PAGE_INSTANCE_URL))); + + assertNotNull(requestHandler); + requestHandler = unwrapRequestHandlerDelegate(requestHandler); + + assertTrue(requestHandler instanceof RenderPageRequestHandler); + RenderPageRequestHandler handler = (RenderPageRequestHandler) requestHandler; + assertEquals(5, handler.getPageId().intValue()); + } + + /** + * Make sure that encrypted page instance URLs are decrypted and the correct handler resolved. + */ + @Test + public void pageInstanceDecryptMultipleCryptoMapper() + { + IRequestHandler requestHandler = new CryptoMapper(mapper, tester.getApplication()) + .mapRequest(getRequest(Url.parse(ENCRYPTED_PAGE_INSTANCE_URL))); + + assertNotNull(requestHandler); + requestHandler = unwrapRequestHandlerDelegate(requestHandler); + + assertTrue(requestHandler instanceof RenderPageRequestHandler); + RenderPageRequestHandler handler = (RenderPageRequestHandler) requestHandler; + assertEquals(5, handler.getPageId().intValue()); + } + + /** + * Tests that plain text requests to a page instance URL are not mapped. + */ + @Test + public void pageInstanceForceEncryption() + { + assertNull(mapper.mapRequest(getRequest(Url.parse(PLAIN_PAGE_INSTANCE_URL)))); + } + + /** + * Tests that mounted pages are still accessible through their mounted URL. + */ + @Test + public void mountedPage() + { + IRequestHandler requestHandler = new RenderPageRequestHandler(new PageProvider(Page1.class)); + + assertEquals(MOUNTED_URL, mapper.mapHandler(requestHandler).toString()); + + requestHandler = mapper.mapRequest(getRequest(Url.parse(MOUNTED_URL))); + + assertNotNull(requestHandler); + + requestHandler = unwrapRequestHandlerDelegate(requestHandler); + + assertTrue(requestHandler instanceof RenderPageRequestHandler); + + assertEquals(Page1.class, ((RenderPageRequestHandler) requestHandler).getPageClass()); + } + + /** + * Tests that PageComponentInfo parameters are encrypted on Mounted pages + */ + @Test + public void mountedPageRequestListenerParameter() + { + final String componentPath = "some:path:to:link"; + + PageAndComponentProvider provider = new PageAndComponentProvider(Page1.class, componentPath); + IRequestHandler requestHandler = new ListenerInterfaceRequestHandler(provider, ILinkListener.INTERFACE); + + Url plainUrl = mapper.getDelegateMapper().mapHandler(requestHandler); + assertTrue(plainUrl.toString().startsWith(MOUNTED_URL)); + + /* + * Do not allow unencrypted request listener urls to mounted pages. + */ + assertNull(mapper.mapRequest(getRequest(plainUrl))); + + /* + * Test encryption of request listener parameter. + */ + Url encryptedUrl = mapper.mapHandler(requestHandler); + + assertEquals(Url.parse(MOUNTED_URL).getSegments(), encryptedUrl.getSegments()); + assertTrue(encryptedUrl.getQueryParameters().size() > 0); + + for (Url.QueryParameter qp : encryptedUrl.getQueryParameters()) + { + if (Strings.isEmpty(qp.getValue())) + { + PageComponentInfo pci = PageComponentInfo.parse(qp.getName()); + assertNull("PageComponentInfo query parameter not encrypted", pci); + } + } + + requestHandler = mapper.mapRequest(getRequest(encryptedUrl)); + + assertNotNull(requestHandler); + + requestHandler = unwrapRequestHandlerDelegate(requestHandler); + + assertTrue(requestHandler instanceof ListenerInterfaceRequestHandler); + + ListenerInterfaceRequestHandler handler = (ListenerInterfaceRequestHandler) requestHandler; + assertEquals(componentPath, handler.getComponentPath()); + assertEquals(Page1.class, handler.getPageClass()); + + /* + * We anticipate that sometimes multiple cryptomappers will be used. It should still work in these situations. + */ + requestHandler = new CryptoMapper(mapper, tester.getApplication()) + .mapRequest(getRequest(encryptedUrl)); + + assertNotNull(requestHandler); + + requestHandler = unwrapRequestHandlerDelegate(requestHandler); + + assertTrue(requestHandler instanceof ListenerInterfaceRequestHandler); + + handler = (ListenerInterfaceRequestHandler) requestHandler; + assertEquals(componentPath, handler.getComponentPath()); + assertEquals(Page1.class, handler.getPageClass()); + } + + /** + * Tests that the compatability score is correctly calculated from wrapped mapper. + */ + @Test + public void compatabilityScore() + { + int delegateHomePageScore = mapper.getDelegateMapper().getCompatibilityScore( + getRequest(Url.parse(""))); + int cryptoHomePageScore = mapper.getCompatibilityScore( + getRequest(Url.parse(""))); + assertEquals(delegateHomePageScore, cryptoHomePageScore); + + int delegateBookmarkableScore = mapper.getDelegateMapper().getCompatibilityScore( + getRequest(Url.parse(PLAIN_BOOKMARKABLE_URL))); + int cryptoBookmarkableScore = mapper.getCompatibilityScore( + getRequest(Url.parse(ENCRYPTED_BOOKMARKABLE_URL))); + assertEquals(delegateBookmarkableScore, cryptoBookmarkableScore); + + int delegatePageInstanceScore = mapper.getDelegateMapper().getCompatibilityScore( + getRequest(Url.parse(PLAIN_PAGE_INSTANCE_URL))); + int cryptoPageInstanceScore = mapper.getCompatibilityScore( + getRequest(Url.parse(ENCRYPTED_PAGE_INSTANCE_URL))); + assertEquals(delegatePageInstanceScore, cryptoPageInstanceScore); } /** @@ -151,7 +410,7 @@ public class CryptoMapperTest extends AbstractMapperTest @Test public void pageParameters() { - String expectedEncrypted = "ywKWg-Qpk7YQBiYCmj9MaAJSIV1gtssNinjiALijtet62VMQc2-sMK_RchttkidUpYM_cplXKeZSfGxBkvWzH_E_zWv4Ii7MNSm5nXKno7o/ywK6c/MK_c0/nji3c/Qpk1b/XKnba/c2-cd"; + String expectedEncrypted = "L7ExSNbPC4sb6TPJDblCAopL53TWmZP5y7BQEaJSJAC05HXod5M5U7gT2yNT0lK5L6L09ZAOoZkGyUhseyPrC4S5tqUUrV6zipc4_Ni877FDOOoE5C_Cd7YCyK1xSScpVhno6LeBz2wiu5oWyf7hB1RKcv6zkhEBmbx8vU7K7-e4xe1_LO8Y3fhEjMSQyU9BVh7Uz4HKzkR2OxFo5LaDzQ/L7E59/yPr6a/5L6ae/OxF2c"; PageParameters expectedParameters = new PageParameters(); expectedParameters.add("namedKey1", "namedValue1"); @@ -159,49 +418,33 @@ public class CryptoMapperTest extends AbstractMapperTest expectedParameters.set(0, "indexedValue1"); expectedParameters.set(1, "indexedValue2"); RenderPageRequestHandler renderPageRequestHandler = new RenderPageRequestHandler( - new PageProvider(DummyHomePage.class, expectedParameters)); + new PageProvider(Page2.class, expectedParameters)); Url url = mapper.mapHandler(renderPageRequestHandler); -// System.err.println(url.toString()); assertEquals(expectedEncrypted, url.toString()); Request request = getRequest(url); IRequestHandler requestHandler = mapper.mapRequest(request); - assertTrue(requestHandler instanceof RequestSettingRequestHandler); - requestHandler = ((RequestSettingRequestHandler)requestHandler).getDelegateHandler(); + assertNotNull(requestHandler); + requestHandler = unwrapRequestHandlerDelegate(requestHandler); assertTrue(requestHandler instanceof RenderPageRequestHandler); - RenderPageRequestHandler handler = (RenderPageRequestHandler)requestHandler; - assertEquals(DummyHomePage.class, handler.getPageClass()); + RenderPageRequestHandler handler = (RenderPageRequestHandler) requestHandler; + assertEquals(Page2.class, handler.getPageClass()); PageParameters actualParameters = handler.getPageParameters(); assertEquals(expectedParameters, actualParameters); } /** - * When the home page url is requested, with parameters, the url will contain only page - * parameters. It should not be encrypted, otherwise we get needless redirects. + * UrlResourceReferences, WICKET-5319 */ @Test - public void homePageWithParameters() + public void urlResourceReference() { - String expectedEncrypted = "?namedKey1=namedValue1"; - PageParameters expectedParameters = new PageParameters(); - expectedParameters.add("namedKey1", "namedValue1"); - - RenderPageRequestHandler renderPageRequestHandler = new RenderPageRequestHandler( - new PageProvider(tester.getApplication().getHomePage(), expectedParameters)); - Url url = mapper.mapHandler(renderPageRequestHandler); - assertEquals(expectedEncrypted, url.toString()); - - Request request = getRequest(url); - IRequestHandler requestHandler = mapper.mapRequest(request); - assertTrue(requestHandler instanceof RequestSettingRequestHandler); - requestHandler = ((RequestSettingRequestHandler)requestHandler).getDelegateHandler(); - assertTrue(requestHandler instanceof RenderPageRequestHandler); + UrlResourceReference resource = new UrlResourceReference( + Url.parse("http://wicket.apache.org/")); + Url url = mapper.mapHandler(new ResourceReferenceRequestHandler(resource)); - RenderPageRequestHandler handler = (RenderPageRequestHandler)requestHandler; - assertEquals(tester.getApplication().getHomePage(), handler.getPageClass()); - PageParameters actualParameters = handler.getPageParameters(); - assertEquals(expectedParameters, actualParameters); + assertEquals("http://wicket.apache.org/", url.toString(StringMode.FULL)); } /** @@ -218,29 +461,16 @@ public class CryptoMapperTest extends AbstractMapperTest IRequestHandler requestHandler = mapper.mapRequest(request); - assertTrue(requestHandler instanceof RequestSettingRequestHandler); - requestHandler = ((RequestSettingRequestHandler)requestHandler).getDelegateHandler(); + assertNotNull(requestHandler); + requestHandler = unwrapRequestHandlerDelegate(requestHandler); assertTrue(requestHandler instanceof ResourceReferenceRequestHandler); - ResourceReferenceRequestHandler handler = (ResourceReferenceRequestHandler)requestHandler; + ResourceReferenceRequestHandler handler = (ResourceReferenceRequestHandler) requestHandler; assertEquals(getClass(), handler.getResourceReference().getScope()); assertEquals("crypt/crypt.txt", handler.getResourceReference().getName()); } /** - * UrlResourceReferences, WICKET-5319 - */ - @Test - public void urlResourceReference() - { - UrlResourceReference resource = new UrlResourceReference( - Url.parse("http://wicket.apache.org/")); - Url url = mapper.mapHandler(new ResourceReferenceRequestHandler(resource)); - - assertEquals("http://wicket.apache.org/", url.toString(StringMode.FULL)); - } - - /** * Relative ResourceReferences, WICKET-3514 */ @Test @@ -256,10 +486,10 @@ public class CryptoMapperTest extends AbstractMapperTest IRequestHandler requestHandler = mapper.mapRequest(request); - assertTrue(requestHandler instanceof RequestSettingRequestHandler); - requestHandler = ((RequestSettingRequestHandler)requestHandler).getDelegateHandler(); + assertNotNull(requestHandler); + requestHandler = unwrapRequestHandlerDelegate(requestHandler); assertTrue(requestHandler instanceof ResourceReferenceRequestHandler); - ResourceReferenceRequestHandler handler = (ResourceReferenceRequestHandler)requestHandler; + ResourceReferenceRequestHandler handler = (ResourceReferenceRequestHandler) requestHandler; assertEquals(getClass(), handler.getResourceReference().getScope()); assertEquals("crypt/modified-crypt.txt", handler.getResourceReference().getName()); @@ -282,10 +512,9 @@ public class CryptoMapperTest extends AbstractMapperTest IRequestHandler requestHandler = mapper.mapRequest(request); - assertTrue(requestHandler instanceof RequestSettingRequestHandler); - requestHandler = ((RequestSettingRequestHandler)requestHandler).getDelegateHandler(); - assertTrue(requestHandler instanceof ResourceReferenceRequestHandler); - ResourceReferenceRequestHandler handler = (ResourceReferenceRequestHandler)requestHandler; + assertNotNull(requestHandler); + requestHandler = unwrapRequestHandlerDelegate(requestHandler); + ResourceReferenceRequestHandler handler = (ResourceReferenceRequestHandler) requestHandler; assertEquals(getClass(), handler.getResourceReference().getScope()); assertEquals("crypt/more/more-crypt.txt", handler.getResourceReference().getName()); @@ -308,10 +537,10 @@ public class CryptoMapperTest extends AbstractMapperTest IRequestHandler requestHandler = mapper.mapRequest(request); - assertTrue(requestHandler instanceof RequestSettingRequestHandler); - requestHandler = ((RequestSettingRequestHandler)requestHandler).getDelegateHandler(); + assertNotNull(requestHandler); + requestHandler = unwrapRequestHandlerDelegate(requestHandler); assertTrue(requestHandler instanceof ResourceReferenceRequestHandler); - ResourceReferenceRequestHandler handler = (ResourceReferenceRequestHandler)requestHandler; + ResourceReferenceRequestHandler handler = (ResourceReferenceRequestHandler) requestHandler; assertEquals(getClass(), handler.getResourceReference().getScope()); assertEquals("less-crypt.txt", handler.getResourceReference().getName()); @@ -338,9 +567,43 @@ public class CryptoMapperTest extends AbstractMapperTest assertTrue(requestHandler instanceof RequestSettingRequestHandler); - assertEquals("foo", ((RequestSettingRequestHandler)requestHandler).getRequest() + assertEquals("foo", ((RequestSettingRequestHandler) requestHandler).getRequest() .getUrl() .getQueryParameterValue("q") .toString()); } -} \ No newline at end of file + + private static IRequestHandler unwrapRequestHandlerDelegate(IRequestHandler handler) + { + while (handler instanceof IRequestHandlerDelegate) + { + handler = ((IRequestHandlerDelegate) handler).getDelegateHandler(); + } + + return handler; + } + + /** + * Page that is mounted + */ + public static class Page1 extends WebPage + { + @Override + public IMarkupFragment getMarkup() + { + return Markup.of("<html><body></body></html>"); + } + } + + /** + * Page that is not mounted + */ + public static class Page2 extends WebPage + { + @Override + public IMarkupFragment getMarkup() + { + return Markup.of("<html><body></body></html>"); + } + } +} http://git-wip-us.apache.org/repos/asf/wicket/blob/c6398901/wicket-core/src/test/java/org/apache/wicket/settings/ISecuritySettingsTest.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/settings/ISecuritySettingsTest.java b/wicket-core/src/test/java/org/apache/wicket/settings/ISecuritySettingsTest.java index f13db98..6ac1e7e 100644 --- a/wicket-core/src/test/java/org/apache/wicket/settings/ISecuritySettingsTest.java +++ b/wicket-core/src/test/java/org/apache/wicket/settings/ISecuritySettingsTest.java @@ -20,6 +20,8 @@ import junit.framework.Assert; import org.apache.wicket.MockPageWithLink; import org.apache.wicket.WicketTestCase; +import org.apache.wicket.core.request.handler.BookmarkablePageRequestHandler; +import org.apache.wicket.core.request.handler.PageProvider; import org.apache.wicket.core.request.mapper.CryptoMapper; import org.apache.wicket.markup.IMarkupFragment; import org.apache.wicket.markup.Markup; @@ -28,6 +30,7 @@ import org.apache.wicket.markup.html.link.Link; import org.apache.wicket.protocol.http.WebApplication; import org.apache.wicket.protocol.https.HttpsConfig; import org.apache.wicket.protocol.https.HttpsMapper; +import org.apache.wicket.request.IRequestHandler; import org.apache.wicket.request.flow.RedirectToUrlException; import org.junit.Test; @@ -92,9 +95,46 @@ public class ISecuritySettingsTest extends WicketTestCase @Test public void enforceMountsWithCryptoMapper() { - WebApplication app = tester.getApplication(); - app.setRootRequestMapper(new CryptoMapper(app.getRootRequestMapper(), app)); - enforceMounts(); + WebApplication app = tester.getApplication(); + + IRequestHandler handler = new BookmarkablePageRequestHandler(new PageProvider(UnknownPage.class)); + + String plainTextNonMountedUrl = tester.urlFor(handler).toString(); + + assertTrue("Plain text non mounted url should start with wicket/bookmarkable/: " + plainTextNonMountedUrl, plainTextNonMountedUrl.startsWith("wicket/bookmarkable/")); + + tester.executeUrl(plainTextNonMountedUrl); + tester.assertRenderedPage(UnknownPage.class); + + app.setRootRequestMapper(new CryptoMapper(app.getRootRequestMapper(), app)); + + /* + * Execute dummy request to get WicketTester to re-initialise with CryptoMapper in place. + */ + tester.executeUrl(""); + + String encryptedNonMountedUrl = tester.urlFor(handler).toString(); + + assertFalse("Encrypted URL should not start with wicket/bookmarkable/" + encryptedNonMountedUrl, encryptedNonMountedUrl.startsWith("wicket/bookmarkable/")); + + tester.executeUrl(plainTextNonMountedUrl); + assertNull(tester.getLastRenderedPage()); + tester.executeUrl(encryptedNonMountedUrl); + tester.assertRenderedPage(UnknownPage.class); + + app.mountPackage("unknown", UnknownPage.class); + + tester.executeUrl(plainTextNonMountedUrl); + assertNull(tester.getLastRenderedPage()); + tester.executeUrl(encryptedNonMountedUrl); + tester.assertRenderedPage(UnknownPage.class); + + app.getSecuritySettings().setEnforceMounts(true); + + tester.executeUrl(plainTextNonMountedUrl); + assertNull(tester.getLastRenderedPage()); + tester.executeUrl(encryptedNonMountedUrl); + assertNull(tester.getLastRenderedPage()); } /**
