SAK-40828 Added url validation of site ids on creation (sakaiproject#…

…6239)

config/configuration/bundles/src/bundle/org/sakaiproject/config/bundle/default.sakai.properties

@@ -3090,6 +3090,10 @@
 # DEFAULT: true
 # site.setup.copy.template.description = false
 
+# SAK-40828: Prevent creating sites with ids that contain spaces or other characters
+# DEFAULT: false
+# site.api.allow_malformed_ids = true
+
 # Comma separated list of site types to hide PageOrder tab, 
 # e.g. if set to "course,project", the PageOrder tool tab will be hidden for all course sites and project sites.
 # DEFAULT: none (null)

deploy/pom.xml

@@ -491,6 +491,11 @@
             <artifactId>commons-lang3</artifactId>
             <scope>compile</scope>
         </dependency>
+        <dependency>
+            <groupId>commons-validator</groupId>
+            <artifactId>commons-validator</artifactId>
+            <scope>compile</scope>
+        </dependency>
         <dependency>
             <groupId>org.apache.httpcomponents</groupId>
             <artifactId>httpcore</artifactId>

kernel/kernel-impl/src/main/java/org/sakaiproject/site/impl/BaseSiteService.java

@@ -31,6 +31,7 @@
 import lombok.extern.slf4j.Slf4j;
 
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.validator.routines.UrlValidator;
 
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
@@ -94,6 +95,9 @@ public SecurityAdvice isAllowed(String userId, String function, String reference
 	private static final String RESOURCEBUNDLE = "resource.bundle.siteimpl";
 	private static final String ORIGINAL_SITE_ID_PROPERTY = "original-site-id";
 
+	private final UrlValidator siteIdValidator
+		= new UrlValidator(UrlValidator.ALLOW_LOCAL_URLS | UrlValidator.ALLOW_2_SLASHES);
+
 	private ResourceLoader rb = null;
 	// protected ResourceLoader rb = new ResourceLoader("site-impl");
 
@@ -1268,6 +1272,14 @@ public Site addSite(String id, String type) throws IdInvalidException, IdUsedExc
 
 		id = Validator.escapeResourceName(id);
 
+		if (!serverConfigurationService().getBoolean("site.api.allow_malformed_ids", false)
+				&& !siteIdValidator.isValid("http://localhost/portal/site/" + id)) {
+			String message
+				= "Id " + id + " is not a valid id format. It can only contain url friendly characters";
+			log.warn(".addSite(): " + message);
+			throw new IdInvalidException(message);
+		}
+
 		// check for a valid site type
 		if (!Validator.checkSiteType(type)) {
 			throw new IdInvalidException("Type " + type + " is not a valid type format");			

kernel/kernel-impl/src/test/java/org/sakaiproject/site/impl/test/SiteServiceTest.java

@@ -79,10 +79,28 @@ public void testNullSiteId() {
 
 	}
 
+	@Test
+	public void testSiteIdWithSpaces() {
+
+		SiteService siteService = getService(SiteService.class);
+		workAsAdmin();
+
+		try {
+			siteService.addSite("abc def ghi", "other");
+			Assert.fail();
+		} catch (IdInvalidException e) {
+			log.info("when passed a null id the test correctly responded with an IdInvalidException");
+		} catch (IdUsedException e) {
+			log.error(e.getMessage(), e);
+		} catch (PermissionException e) {
+			log.error(e.getMessage(), e);
+		}
+	}
+
 	private void workAsAdmin() {
 		workAsUser("admin", "admin");
 	}
-	
+
 	private void workAsUser(String eid, String id) {
 		SessionManager sessionManager = getService(SessionManager.class);
 		Session session = sessionManager.getCurrentSession();