Commit 8dd276f6 authored by Simon Urli's avatar Simon Urli
Browse files

XWIKI-10309: Check URL domains based on a whitelist

  * Ensure that the domain used to access the server is recorded as
    whitelisted
  * Also change the logic of the domain checks: we don't want to block
    local domains.
parent 1cdd940f
......@@ -52,6 +52,11 @@
<artifactId>xwiki-platform-wiki-api</artifactId>
<version>${project.version}</version>
</dependency>
<!-- Needed to get access to HttpServletRequest -->
<dependency>
<groupId>javax.servlet</groupId>
<artifactId>javax.servlet-api</artifactId>
</dependency>
<!-- Testing Dependencies -->
<dependency>
<groupId>org.xwiki.commons</groupId>
......
......@@ -19,12 +19,13 @@
*/
package org.xwiki.url.internal;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.HashSet;
import java.util.Set;
import java.util.regex.Pattern;
import javax.inject.Inject;
import javax.inject.Provider;
import javax.inject.Singleton;
import org.apache.commons.lang3.StringUtils;
......@@ -38,6 +39,8 @@
import org.xwiki.wiki.descriptor.WikiDescriptorManager;
import org.xwiki.wiki.manager.WikiManagerException;
import com.xpn.xwiki.XWikiContext;
/**
* Default implementation of {@link URLSecurityManager}.
* This implementation keeps a HashSet in memory containing the trusted domains defined in the configuration and
......@@ -51,7 +54,6 @@
@Singleton
public class DefaultURLSecurityManager implements URLSecurityManager
{
private static final Pattern ACCEPTED_DOMAIN_PATTERN = Pattern.compile("([^.]+\\.[^.]+)+");
private static final char DOT = '.';
@Inject
......@@ -66,32 +68,42 @@
@Inject
private Logger logger;
@Inject
private Provider<XWikiContext> contextProvider;
private Set<String> trustedDomains;
private void computeTrustedDomains()
{
Set<String> domains;
domains = new HashSet<>(this.urlConfiguration.getTrustedDomains());
this.trustedDomains = new HashSet<>(this.urlConfiguration.getTrustedDomains());
try {
for (WikiDescriptor wikiDescriptor : wikiDescriptorManager.getAll()) {
domains.addAll(wikiDescriptor.getAliases());
this.trustedDomains.addAll(wikiDescriptor.getAliases());
}
} catch (WikiManagerException e) {
logger.warn("Error while getting wiki descriptor to fill list of trusted domains: [{}]. "
+ "The subwikis won't be taken into account for the list of trusted domains.",
ExceptionUtils.getRootCauseMessage(e));
}
this.trustedDomains = new HashSet<>();
for (String domain : domains) {
if (ACCEPTED_DOMAIN_PATTERN.matcher(domain).matches()) {
this.trustedDomains.add(domain);
} else {
logger.warn("The domain [{}] specified in the trusted domains configuration won't be taken into "
+ "account since it doesn't respect the documented format.", domain);
}
private String getCurrentDomain()
{
XWikiContext context = this.contextProvider.get();
if (context.getRequest() != null && context.getRequest().getHttpServletRequest() != null) {
String request = context.getRequest().getHttpServletRequest().getRequestURL().toString();
try {
URL requestURL = new URL(request);
return requestURL.getHost();
} catch (MalformedURLException e) {
// this should never happen
throw new RuntimeException(
String.format("URL used to access the server is not a proper URL: [%s]", request));
}
}
return "";
}
@Override
......@@ -102,15 +114,18 @@ public boolean isDomainTrusted(URL urlToCheck)
computeTrustedDomains();
}
this.trustedDomains.add(this.getCurrentDomain());
String host = urlToCheck.getHost();
while (StringUtils.contains(host, DOT)) {
do {
if (trustedDomains.contains(host)) {
return true;
} else {
} else if (StringUtils.contains(host, DOT)) {
host = host.substring(host.indexOf(DOT) + 1);
} else {
host = "";
}
}
} while (!"".equals(host));
Object bypassCheckProperty = execution.getContext()
.getProperty(URLSecurityManager.BYPASS_DOMAIN_SECURITY_CHECK_CONTEXT_PROPERTY);
......
......@@ -24,6 +24,9 @@
import java.util.Arrays;
import java.util.Collections;
import javax.inject.Provider;
import javax.servlet.http.HttpServletRequest;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
......@@ -39,6 +42,9 @@
import org.xwiki.wiki.descriptor.WikiDescriptor;
import org.xwiki.wiki.descriptor.WikiDescriptorManager;
import com.xpn.xwiki.XWikiContext;
import com.xpn.xwiki.web.XWikiRequest;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.Mockito.mock;
......@@ -65,6 +71,11 @@
@MockComponent
private Execution execution;
@MockComponent
private Provider<XWikiContext> contextProvider;
private XWikiContext xWikiContext;
private ExecutionContext executionContext;
......@@ -77,6 +88,8 @@ void setup()
this.executionContext = mock(ExecutionContext.class);
when(this.execution.getContext()).thenReturn(this.executionContext);
when(this.urlConfiguration.isTrustedDomainsEnabled()).thenReturn(true);
this.xWikiContext = mock(XWikiContext.class);
when(this.contextProvider.get()).thenReturn(this.xWikiContext);
}
@Test
......@@ -84,7 +97,7 @@ void isDomainTrusted() throws Exception
{
when(urlConfiguration.getTrustedDomains()).thenReturn(Arrays.asList(
"foo.acme.org",
"com" // this should not be taken into account
"localdomain"
));
WikiDescriptor wikiDescriptor1 = mock(WikiDescriptor.class);
......@@ -98,6 +111,11 @@ void isDomainTrusted() throws Exception
"enterprise.eu"
));
XWikiRequest request = mock(XWikiRequest.class);
when(this.xWikiContext.getRequest()).thenReturn(request);
HttpServletRequest servletRequest = mock(HttpServletRequest.class);
when(request.getHttpServletRequest()).thenReturn(servletRequest);
when(servletRequest.getRequestURL()).thenReturn(new StringBuffer("http://localhost:8080/xwiki/bin/register/"));
when(this.wikiDescriptorManager.getAll()).thenReturn(Arrays.asList(wikiDescriptor1, wikiDescriptor2));
assertThat("www.xwiki.org is trusted", this.urlSecurityManager
......@@ -128,10 +146,10 @@ void isDomainTrusted() throws Exception
.isDomainTrusted(new URL("https://enterprise.eu/xwiki/")));
assertThat("enterprise.eu. is not trusted", !this.urlSecurityManager
.isDomainTrusted(new URL("https://enterprise.eu./xwiki/")));
assertEquals("The domain [com] specified in the trusted domains configuration won't be taken into account "
+ "since it doesn't respect the documented format.",
logCapture.getMessage(0));
assertThat("current domain is trusted", this.urlSecurityManager
.isDomainTrusted(new URL("https://localhost:8080/xwiki/bin/view/XWiki/Page")));
assertThat("local domain is trusted", this.urlSecurityManager
.isDomainTrusted(new URL("https://localhost.localdomain:8080/xwiki/bin/view/XWiki/Page")));
}
@Test
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment