You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@archiva.apache.org by ma...@apache.org on 2019/04/13 09:59:50 UTC
[archiva] branch archiva-2.x updated: Improving URL check for
organisation info
This is an automated email from the ASF dual-hosted git repository.
martin_s pushed a commit to branch archiva-2.x
in repository https://gitbox.apache.org/repos/asf/archiva.git
The following commit(s) were added to refs/heads/archiva-2.x by this push:
new 796716d Improving URL check for organisation info
796716d is described below
commit 796716d44183bd315dd20184a66b39ae533eb747
Author: Martin Stockhammer <ma...@apache.org>
AuthorDate: Sat Apr 13 11:59:29 2019 +0200
Improving URL check for organisation info
---
.../admin/DefaultArchivaAdministration.java | 28 +++++------
.../admin/ArchivaAdministrationTest.java | 4 +-
.../DefaultArchivaAdministrationService.java | 8 +---
.../services/ArchivaAdministrationServiceTest.java | 56 ++++++++++++++++++++++
4 files changed, 74 insertions(+), 22 deletions(-)
diff --git a/archiva-modules/archiva-base/archiva-repository-admin/archiva-repository-admin-default/src/main/java/org/apache/archiva/admin/repository/admin/DefaultArchivaAdministration.java b/archiva-modules/archiva-base/archiva-repository-admin/archiva-repository-admin-default/src/main/java/org/apache/archiva/admin/repository/admin/DefaultArchivaAdministration.java
index 1ba1048..3be9f58 100644
--- a/archiva-modules/archiva-base/archiva-repository-admin/archiva-repository-admin-default/src/main/java/org/apache/archiva/admin/repository/admin/DefaultArchivaAdministration.java
+++ b/archiva-modules/archiva-base/archiva-repository-admin/archiva-repository-admin-default/src/main/java/org/apache/archiva/admin/repository/admin/DefaultArchivaAdministration.java
@@ -21,20 +21,14 @@ package org.apache.archiva.admin.repository.admin;
import org.apache.archiva.admin.model.AuditInformation;
import org.apache.archiva.admin.model.RepositoryAdminException;
import org.apache.archiva.admin.model.admin.ArchivaAdministration;
-import org.apache.archiva.admin.model.beans.FileType;
-import org.apache.archiva.admin.model.beans.LegacyArtifactPath;
-import org.apache.archiva.admin.model.beans.NetworkConfiguration;
-import org.apache.archiva.admin.model.beans.OrganisationInformation;
-import org.apache.archiva.admin.model.beans.UiConfiguration;
+import org.apache.archiva.admin.model.beans.*;
import org.apache.archiva.admin.repository.AbstractRepositoryAdmin;
import org.apache.archiva.configuration.Configuration;
import org.apache.archiva.configuration.UserInterfaceOptions;
import org.apache.archiva.configuration.WebappConfiguration;
import org.apache.archiva.metadata.model.facets.AuditEvent;
-import org.apache.commons.codec.net.URLCodec;
import org.apache.commons.lang.StringEscapeUtils;
import org.apache.commons.lang.StringUtils;
-import org.apache.http.impl.conn.PoolingClientConnectionManager;
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
import org.apache.maven.wagon.providers.http.HttpWagon;
import org.springframework.stereotype.Service;
@@ -42,10 +36,8 @@ import org.springframework.util.ResourceUtils;
import javax.annotation.PostConstruct;
import javax.annotation.PreDestroy;
-import java.io.UnsupportedEncodingException;
-import java.net.MalformedURLException;
-import java.net.URL;
-import java.net.URLEncoder;
+import java.net.URI;
+import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@@ -328,14 +320,21 @@ public class DefaultArchivaAdministration
return getModelMapper().map( organisationInformation, OrganisationInformation.class );
}
- private void checkUrl(String url, String propertyName) throws RepositoryAdminException {
+ private String fixUrl(String url, String propertyName) throws RepositoryAdminException {
if ( StringUtils.isNotEmpty( url ) )
{
if ( !ResourceUtils.isUrl( url ) )
{
throw new RepositoryAdminException( "Bad URL in " + propertyName + ": " + url );
}
+ try {
+ URI urlToCheck = new URI(url);
+ return urlToCheck.toString();
+ } catch (URISyntaxException e) {
+ throw new RepositoryAdminException( "Bad URL in " + propertyName + ": " + url );
+ }
}
+ return url;
}
@@ -347,8 +346,9 @@ public class DefaultArchivaAdministration
public void setOrganisationInformation( OrganisationInformation organisationInformation )
throws RepositoryAdminException
{
- checkUrl(organisationInformation.getUrl(), "url");
- checkUrl( organisationInformation.getLogoLocation(), "logoLocation" );
+
+ organisationInformation.setUrl(fixUrl(organisationInformation.getUrl(), "url"));
+ organisationInformation.setLogoLocation(fixUrl( organisationInformation.getLogoLocation(), "logoLocation" ));
Configuration configuration = getArchivaConfiguration( ).getConfiguration( );
if ( organisationInformation != null )
{
diff --git a/archiva-modules/archiva-base/archiva-repository-admin/archiva-repository-admin-default/src/test/java/org/apache/archiva/admin/repository/admin/ArchivaAdministrationTest.java b/archiva-modules/archiva-base/archiva-repository-admin/archiva-repository-admin-default/src/test/java/org/apache/archiva/admin/repository/admin/ArchivaAdministrationTest.java
index 9bb9ed4..e597de4 100644
--- a/archiva-modules/archiva-base/archiva-repository-admin/archiva-repository-admin-default/src/test/java/org/apache/archiva/admin/repository/admin/ArchivaAdministrationTest.java
+++ b/archiva-modules/archiva-base/archiva-repository-admin/archiva-repository-admin-default/src/test/java/org/apache/archiva/admin/repository/admin/ArchivaAdministrationTest.java
@@ -222,7 +222,7 @@ public class ArchivaAdministrationTest
try
{
OrganisationInformation newOrganisationInformation = new OrganisationInformation( );
- newOrganisationInformation.setLogoLocation( "'/><svg/onload=alert(/logoLocation_xss/)>" );
+ newOrganisationInformation.setLogoLocation( "http://www.foo.com'/><svg/onload=alert(/logoLocation_xss/)>" );
newOrganisationInformation.setName( "foo org" );
newOrganisationInformation.setUrl( "http://foo.com" );
archivaAdministration.setOrganisationInformation( newOrganisationInformation );
@@ -240,7 +240,7 @@ public class ArchivaAdministrationTest
try
{
OrganisationInformation newOrganisationInformation = new OrganisationInformation( );
- newOrganisationInformation.setUrl( "'/><svg/onload=alert(/url_xss/)>" );
+ newOrganisationInformation.setUrl( "http://foo.com'/><svg/onload=alert(/url_xss/)>" );
newOrganisationInformation.setName( "foo org" );
newOrganisationInformation.setLogoLocation( "http://foo.com/bar.png" );
archivaAdministration.setOrganisationInformation( newOrganisationInformation );
diff --git a/archiva-modules/archiva-web/archiva-rest/archiva-rest-services/src/main/java/org/apache/archiva/rest/services/DefaultArchivaAdministrationService.java b/archiva-modules/archiva-web/archiva-rest/archiva-rest-services/src/main/java/org/apache/archiva/rest/services/DefaultArchivaAdministrationService.java
index ff1d3fd..51144e8 100644
--- a/archiva-modules/archiva-web/archiva-rest/archiva-rest-services/src/main/java/org/apache/archiva/rest/services/DefaultArchivaAdministrationService.java
+++ b/archiva-modules/archiva-web/archiva-rest/archiva-rest-services/src/main/java/org/apache/archiva/rest/services/DefaultArchivaAdministrationService.java
@@ -20,11 +20,7 @@ package org.apache.archiva.rest.services;
import org.apache.archiva.admin.model.RepositoryAdminException;
import org.apache.archiva.admin.model.admin.ArchivaAdministration;
-import org.apache.archiva.admin.model.beans.FileType;
-import org.apache.archiva.admin.model.beans.LegacyArtifactPath;
-import org.apache.archiva.admin.model.beans.NetworkConfiguration;
-import org.apache.archiva.admin.model.beans.OrganisationInformation;
-import org.apache.archiva.admin.model.beans.UiConfiguration;
+import org.apache.archiva.admin.model.beans.*;
import org.apache.archiva.repository.scanner.RepositoryContentConsumers;
import org.apache.archiva.rest.api.model.AdminRepositoryConsumer;
import org.apache.archiva.rest.api.services.ArchivaAdministrationService;
@@ -319,7 +315,7 @@ public class DefaultArchivaAdministrationService
}
catch ( RepositoryAdminException e )
{
- throw new ArchivaRestServiceException( e.getMessage(), e );
+ throw new ArchivaRestServiceException( e.getMessage(), 400, e );
}
}
diff --git a/archiva-modules/archiva-web/archiva-rest/archiva-rest-services/src/test/java/org/apache/archiva/rest/services/ArchivaAdministrationServiceTest.java b/archiva-modules/archiva-web/archiva-rest/archiva-rest-services/src/test/java/org/apache/archiva/rest/services/ArchivaAdministrationServiceTest.java
index e3377f2..e6a87b0 100644
--- a/archiva-modules/archiva-web/archiva-rest/archiva-rest-services/src/test/java/org/apache/archiva/rest/services/ArchivaAdministrationServiceTest.java
+++ b/archiva-modules/archiva-web/archiva-rest/archiva-rest-services/src/test/java/org/apache/archiva/rest/services/ArchivaAdministrationServiceTest.java
@@ -23,9 +23,11 @@ import org.apache.archiva.admin.model.beans.LegacyArtifactPath;
import org.apache.archiva.admin.model.beans.OrganisationInformation;
import org.apache.archiva.admin.model.beans.UiConfiguration;
import org.apache.archiva.rest.api.model.AdminRepositoryConsumer;
+import org.apache.archiva.rest.api.services.ArchivaRestServiceException;
import org.apache.commons.lang.StringUtils;
import org.junit.Test;
+import javax.ws.rs.BadRequestException;
import java.util.Arrays;
import java.util.List;
@@ -93,6 +95,60 @@ public class ArchivaAdministrationServiceTest
}
@Test
+ public void badOrganizationLogoLocation()
+ throws Exception
+ {
+ OrganisationInformation organisationInformation =
+ getArchivaAdministrationService().getOrganisationInformation();
+
+ // rest return an empty bean
+ assertNotNull( organisationInformation );
+ assertTrue( StringUtils.isBlank( organisationInformation.getLogoLocation() ) );
+ assertTrue( StringUtils.isBlank( organisationInformation.getName() ) );
+ assertTrue( StringUtils.isBlank( organisationInformation.getUrl() ) );
+
+ organisationInformation = new OrganisationInformation();
+ organisationInformation.setLogoLocation( "http://foo.com'/><svg/onload=alert(/logoLocation_xss/)>" );
+ organisationInformation.setName( "foo org" );
+ organisationInformation.setUrl( "http://foo.com" );
+
+ try {
+ getArchivaAdministrationService().setOrganisationInformation(organisationInformation);
+ fail("RepositoryAdminException expected. Bad URL content should not be allowed for logo location.");
+ } catch (BadRequestException e) {
+ // OK
+ }
+
+ }
+
+ @Test
+ public void badOrganizationUrl()
+ throws Exception
+ {
+ OrganisationInformation organisationInformation =
+ getArchivaAdministrationService().getOrganisationInformation();
+
+ // rest return an empty bean
+ assertNotNull( organisationInformation );
+ assertTrue( StringUtils.isBlank( organisationInformation.getLogoLocation() ) );
+ assertTrue( StringUtils.isBlank( organisationInformation.getName() ) );
+ assertTrue( StringUtils.isBlank( organisationInformation.getUrl() ) );
+
+ organisationInformation = new OrganisationInformation();
+ organisationInformation.setLogoLocation( "http://foo.com/logo.jpg" );
+ organisationInformation.setName( "foo org" );
+ organisationInformation.setUrl( "http://foo.com'/><svg/onload=alert(/url_xss/)>" );
+
+ try {
+ getArchivaAdministrationService().setOrganisationInformation(organisationInformation);
+ fail("RepositoryAdminException expected. Bad URL content should not be allowed for logo location.");
+ } catch (BadRequestException e) {
+ // OK
+ }
+
+ }
+
+ @Test
public void uiConfigurationReadUpdate()
throws Exception
{