improve error handling of permission system

scm-webapp/src/main/java/sonia/scm/security/DefaultSecuritySystem.java

@@ -30,11 +30,14 @@
  */
 
 
+
 package sonia.scm.security;
 
 //~--- non-JDK imports --------------------------------------------------------
 
+import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
+import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableList.Builder;
 import com.google.common.eventbus.Subscribe;
@@ -125,6 +128,7 @@ public class DefaultSecuritySystem implements SecuritySystem
   public StoredAssignedPermission addPermission(AssignedPermission permission)
   {
     assertIsAdmin();
+    validatePermission(permission);
 
     String id = store.put(permission);
 
@@ -234,6 +238,7 @@ public class DefaultSecuritySystem implements SecuritySystem
   public void modifyPermission(StoredAssignedPermission permission)
   {
     assertIsAdmin();
+    validatePermission(permission);
 
     synchronized (store)
     {
@@ -424,6 +429,20 @@ public class DefaultSecuritySystem implements SecuritySystem
     availablePermissions = builder.build();
   }
 
+  /**
+   * Method description
+   *
+   *
+   * @param perm
+   */
+  private void validatePermission(AssignedPermission perm)
+  {
+    Preconditions.checkArgument(!Strings.isNullOrEmpty(perm.getName()),
+      "name is required");
+    Preconditions.checkArgument(!Strings.isNullOrEmpty(perm.getPermission()),
+      "permission is required");
+  }
+
   //~--- get methods ----------------------------------------------------------
 
   /**
@@ -445,7 +464,7 @@ public class DefaultSecuritySystem implements SecuritySystem
 
     return classLoader;
   }
-  
+
   //~--- inner classes --------------------------------------------------------
 
   /**

scm-webapp/src/main/java/sonia/scm/security/RepositoryPermissionResolver.java

@@ -36,6 +36,7 @@ package sonia.scm.security;
 //~--- non-JDK imports --------------------------------------------------------
 
 import com.google.common.base.Splitter;
+import com.google.common.base.Strings;
 
 import org.apache.shiro.authz.permission.PermissionResolver;
 
@@ -76,24 +77,33 @@ public class RepositoryPermissionResolver implements PermissionResolver
   public RepositoryPermission resolvePermission(String permissionString)
   {
     RepositoryPermission permission = null;
-    Iterator<String> permissionIt =
-      Splitter.on(':').omitEmptyStrings().trimResults().split(
-        permissionString).iterator();
 
-    if (permissionIt.hasNext())
+    if (!Strings.isNullOrEmpty(permissionString))
     {
-      String type = permissionIt.next();
+      Iterator<String> permissionIt =
+        Splitter.on(':').omitEmptyStrings().trimResults().split(
+          permissionString).iterator();
 
-      if (type.equals(RepositoryPermission.TYPE))
+      if (permissionIt.hasNext())
       {
-        permission = createRepositoryPermission(permissionIt);
+        String type = permissionIt.next();
-      }
+
-      else if (logger.isWarnEnabled())
+        if (type.equals(RepositoryPermission.TYPE))
-      {
+        {
-        logger.warn("permission '{}' is not a repository permission",
+          permission = createRepositoryPermission(permissionIt);
-          permissionString);
+        }
+        else if (logger.isWarnEnabled())
+        {
+          logger.warn("permission '{}' is not a repository permission",
+            permissionString);
+        }
       }
     }
+    else
+    {
+      logger.warn(
+        "permision string is empty, could not resolve empty permission");
+    }
 
     return permission;
   }