SONAR-4535 Validate key pattern

3 files changed, 43 insertions, 5 deletions

diff --git a/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionTemplateService.java b/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionTemplateService.java
index 7692449d6d7..71b5d3e000d 100644
--- a/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionTemplateService.java
+++ b/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionTemplateService.java
@@ -36,6 +36,8 @@ import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 
 import java.util.List;
+import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
 
 /**
  * Used by ruby code <pre>Internal.permission_templates</pre>
@@ -80,6 +82,7 @@ public class InternalPermissionTemplateService implements ServerComponent {
   public PermissionTemplate createPermissionTemplate(String name, @Nullable String description, @Nullable String keyPattern) {
     PermissionTemplateUpdater.checkSystemAdminUser();
     validateTemplateName(null, name);
+    validateKeyPattern(null, keyPattern);
     PermissionTemplateDto permissionTemplateDto = permissionTemplateDao.createPermissionTemplate(name, description, keyPattern);
     if (permissionTemplateDto.getId() == null) {
       String errorMsg = "Template creation failed";
@@ -92,6 +95,7 @@ public class InternalPermissionTemplateService implements ServerComponent {
   public void updatePermissionTemplate(Long templateId, String newName, @Nullable String newDescription, @Nullable String newKeyPattern) {
     PermissionTemplateUpdater.checkSystemAdminUser();
     validateTemplateName(templateId, newName);
+    validateKeyPattern(templateId, newKeyPattern);
     permissionTemplateDao.updatePermissionTemplate(templateId, newName, newDescription, newKeyPattern);
   }
 
@@ -144,7 +148,7 @@ public class InternalPermissionTemplateService implements ServerComponent {
     updater.executeUpdate();
   }
 
-  private void validateTemplateName(Long templateId, String templateName) {
+  private void validateTemplateName(@Nullable Long templateId, String templateName) {
     if (StringUtils.isNullOrEmpty(templateName)) {
       String errorMsg = "Name can't be blank";
       throw new BadRequestException(errorMsg);
@@ -160,4 +164,16 @@ public class InternalPermissionTemplateService implements ServerComponent {
     }
   }
 
+  private void validateKeyPattern(@Nullable Long templateId, @Nullable String keyPattern) {
+    if (StringUtils.isNullOrEmpty(keyPattern)) {
+      return;
+    }
+    try {
+      Pattern.compile(keyPattern);
+    } catch (PatternSyntaxException e) {
+      String errorMsg = "Invalid pattern: " + keyPattern + ". Should be a valid Java regular expression.";
+      throw new BadRequestException(errorMsg);
+    }
+  }
+
 }
diff --git a/sonar-server/src/main/webapp/WEB-INF/db/migrate/465_add_perm_template_key_pattern_column.rb b/sonar-server/src/main/webapp/WEB-INF/db/migrate/465_add_perm_template_key_pattern_column.rb
index 43046904cc0..818719d265a 100644
--- a/sonar-server/src/main/webapp/WEB-INF/db/migrate/465_add_perm_template_key_pattern_column.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/db/migrate/465_add_perm_template_key_pattern_column.rb
@@ -19,7 +19,8 @@
 #
 
 #
-# Sonar 4.1
+# SonarQube 4.1
+# SONAR-4535
 #
 
 class AddPermTemplateKeyPatternColumn < ActiveRecord::Migration
diff --git a/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionTemplateServiceTest.java b/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionTemplateServiceTest.java
index 843ad84d3ed..f020bcfad2f 100644
--- a/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionTemplateServiceTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionTemplateServiceTest.java
@@ -54,9 +54,10 @@ public class InternalPermissionTemplateServiceTest {
 
   private static final String DEFAULT_NAME = "my template";
   private static final String DEFAULT_DESC = "my description";
+  private static final String DEFAULT_PATTERN = "com.foo.(.*)";
   private static final String DEFAULT_PERMISSION = UserRole.USER;
   private static final PermissionTemplateDto DEFAULT_TEMPLATE =
-    new PermissionTemplateDto().setId(1L).setName(DEFAULT_NAME).setDescription(DEFAULT_DESC);
+    new PermissionTemplateDto().setId(1L).setName(DEFAULT_NAME).setDescription(DEFAULT_DESC).setKeyPattern(DEFAULT_PATTERN);
 
   private PermissionTemplateDao permissionTemplateDao;
   private UserDao userDao;
@@ -78,13 +79,14 @@ public class InternalPermissionTemplateServiceTest {
 
   @Test
   public void should_create_permission_template() throws Exception {
-    when(permissionTemplateDao.createPermissionTemplate(DEFAULT_NAME, DEFAULT_DESC, null)).thenReturn(DEFAULT_TEMPLATE);
+    when(permissionTemplateDao.createPermissionTemplate(DEFAULT_NAME, DEFAULT_DESC, DEFAULT_PATTERN)).thenReturn(DEFAULT_TEMPLATE);
 
-    PermissionTemplate permissionTemplate = permissionTemplateService.createPermissionTemplate(DEFAULT_NAME, DEFAULT_DESC, null);
+    PermissionTemplate permissionTemplate = permissionTemplateService.createPermissionTemplate(DEFAULT_NAME, DEFAULT_DESC, DEFAULT_PATTERN);
 
     assertThat(permissionTemplate.getId()).isEqualTo(1L);
     assertThat(permissionTemplate.getName()).isEqualTo(DEFAULT_NAME);
     assertThat(permissionTemplate.getDescription()).isEqualTo(DEFAULT_DESC);
+    assertThat(permissionTemplate.getKeyPattern()).isEqualTo(DEFAULT_PATTERN);
   }
 
   @Test
@@ -106,6 +108,14 @@ public class InternalPermissionTemplateServiceTest {
   }
 
   @Test
+  public void should_reject_invalid_key_pattern_on_creation() throws Exception {
+    expected.expect(BadRequestException.class);
+    expected.expectMessage("Invalid pattern: [azerty. Should be a valid Java regular expression.");
+
+    permissionTemplateService.createPermissionTemplate(DEFAULT_NAME, DEFAULT_DESC, "[azerty");
+  }
+
+  @Test
   public void should_delete_permission_template() throws Exception {
     when(permissionTemplateDao.selectTemplateByName(DEFAULT_NAME)).thenReturn(DEFAULT_TEMPLATE);
 
@@ -209,6 +219,17 @@ public class InternalPermissionTemplateServiceTest {
   }
 
   @Test
+  public void should_validate_template_key_pattern_on_update_if_applicable() throws Exception {
+    expected.expect(BadRequestException.class);
+    expected.expectMessage("Invalid pattern: [azerty. Should be a valid Java regular expression.");
+
+    PermissionTemplateDto template1 = new PermissionTemplateDto().setId(1L).setName("template1").setDescription("template1");
+    when(permissionTemplateDao.selectAllPermissionTemplates()).thenReturn(Lists.newArrayList(template1));
+
+    permissionTemplateService.updatePermissionTemplate(1L, "template1", "template1", "[azerty");
+  }
+
+  @Test
   public void should_skip_name_validation_where_not_applicable() throws Exception {
     PermissionTemplateDto template1 =
       new PermissionTemplateDto().setId(1L).setName("template1").setDescription("template1");