[Wip] First set of review changes.

Moved the authentication strategies to their own packages
Added restricted where possible
Use unmodifiable set and drop the unused initializer

pom.xml

@@ -96,6 +96,7 @@
       <groupId>org.jenkins-ci.plugins</groupId>
       <artifactId>matrix-auth</artifactId>
       <version>2.3-SNAPSHOT</version>
+      <optional>true</optional>
     </dependency>
 
 

src/main/java/org/jenkinsci/plugins/casc/core/HudsonPrivateSecurityRealmConfigurator.java

@@ -42,6 +42,5 @@ public class HudsonPrivateSecurityRealmConfigurator extends DataBoundConfigurato
             this.password = password;
         }
 
-
     }
 }

src/main/java/org/jenkinsci/plugins/casc/integrations/GroupPermissionDefinition.java

@@ -1,47 +0,0 @@
-package org.jenkinsci.plugins.casc.integrations;
-
-import hudson.security.Permission;
-import org.kohsuke.stapler.DataBoundConstructor;
-
-import java.util.*;
-
-/**
- * Created by mads on 2/7/18.
- */
-public class GroupPermissionDefinition {
-
-    private String name;
-    private Collection<String> permissions = new ArrayList<>();
-
-    @DataBoundConstructor
-    public GroupPermissionDefinition(String name, Collection<String> permissions) {
-        this.name = name;
-        this.permissions = permissions;
-    }
-
-    public void grantPermission(Map<Permission,Set<String>> grantedPermissions) {
-        for(String permission : permissions) {
-            Permission pm = Permission.fromId(permission);
-            if(grantedPermissions.containsKey(pm)) {
-                grantedPermissions.get(pm).add(name);
-            } else {
-                HashSet<String> s = new HashSet<>();
-                s.add(name);
-                grantedPermissions.put(pm, s);
-            }
-        }
-    }
-
-    public String getName() {
-        return name;
-    }
-
-    public void setName(String name) {
-        this.name = name;
-    }
-
-    @Override
-    public String toString() {
-        return String.format("'%s' granted [%s]", name, permissions != null ? String.join(",", permissions) : "" );
-    }
-}

src/main/java/org/jenkinsci/plugins/casc/integrations/globalmatrixauth/GlobalMatrixAuthorizationStrategyConfigurator.java

@@ -1,31 +1,19 @@
-package org.jenkinsci.plugins.casc.integrations;
+package org.jenkinsci.plugins.casc.integrations.globalmatrixauth;
 
 import hudson.Extension;
 import hudson.security.GlobalMatrixAuthorizationStrategy;
 import hudson.security.Permission;
+import jenkins.model.Jenkins;
 import org.jenkinsci.plugins.casc.Attribute;
 import org.jenkinsci.plugins.casc.Configurator;
 import org.jenkinsci.plugins.casc.RootElementConfigurator;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 import java.util.*;
 
-/**
- * Example
- *
- * jenkins:
- *   authorizationStrategy:
- *     globalMatrix:
- *       grantedPermissions:
- *         - group:
- *             name: "anonymous"
- *             permissions:
- *               - "hudson.model.Hudson.Read"
- *         - group
- *             name: "authenticated"
- *             permissions:
- *               - "hudson.model.Hudson.Administer"
- */
 @Extension(optional = true)
+@Restricted(NoExternalUse.class)
 public class GlobalMatrixAuthorizationStrategyConfigurator extends Configurator<GlobalMatrixAuthorizationStrategy> implements RootElementConfigurator {
 
     @Override

src/main/java/org/jenkinsci/plugins/casc/integrations/globalmatrixauth/GroupPermissionDefinition.java

@@ -0,0 +1,58 @@
+package org.jenkinsci.plugins.casc.integrations.globalmatrixauth;
+
+import hudson.security.Permission;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
+import org.kohsuke.stapler.DataBoundConstructor;
+
+import java.util.*;
+import java.util.logging.Logger;
+
+/**
+ * Created by mads on 2/7/18.
+ */
+@Restricted(NoExternalUse.class)
+public class GroupPermissionDefinition {
+
+    private String name;
+    private Collection<String> permissions;
+
+    private static final Logger LOGGER = Logger.getLogger(GroupPermissionDefinition.class.getName());
+
+    @DataBoundConstructor
+    public GroupPermissionDefinition(String name, Collection<String> permissions) {
+        this.name = name;
+        this.permissions = permissions != null ? Collections.unmodifiableCollection(permissions) : Collections.EMPTY_SET;
+    }
+
+    public void grantPermission(Map<Permission,Set<String>> grantedPermissions) {
+
+        for(String permission : permissions) {
+            Permission pm = Permission.fromId(permission);
+            if(pm != null) {
+                if (grantedPermissions.containsKey(pm)) {
+                    grantedPermissions.get(pm).add(name);
+                } else {
+                    HashSet<String> s = new HashSet<>();
+                    s.add(name);
+                    grantedPermissions.put(pm, s);
+                }
+            } else {
+                LOGGER.warning(String.format("Ignoring unknown permission with id: '%s'", permission));
+            }
+        }
+    }
+
+    public String getName() {
+        return name;
+    }
+
+    public void setName(String name) {
+        this.name = name;
+    }
+
+    @Override
+    public String toString() {
+        return String.format("'%s' granted [%s]", name, permissions != null ? String.join(",", permissions) : "" );
+    }
+}

src/main/java/org/jenkinsci/plugins/casc/integrations/rolebasedauth/RoleBasedAuthorizationStrategyConfigurator.java

@@ -1,7 +1,6 @@
-package org.jenkinsci.plugins.casc.integrations;
+package org.jenkinsci.plugins.casc.integrations.rolebasedauth;
 
 
-import com.cloudbees.plugins.credentials.domains.Domain;
 import com.michelin.cio.hudson.plugins.rolestrategy.Role;
 import com.michelin.cio.hudson.plugins.rolestrategy.RoleBasedAuthorizationStrategy;
 import com.michelin.cio.hudson.plugins.rolestrategy.RoleMap;
@@ -21,7 +20,6 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
-import java.util.SortedMap;
 import java.util.TreeMap;
 
 /**

src/main/java/org/jenkinsci/plugins/casc/integrations/rolebasedauth/RoleDefinition.java

@@ -1,4 +1,4 @@
-package org.jenkinsci.plugins.casc.integrations;
+package org.jenkinsci.plugins.casc.integrations.rolebasedauth;
 
 import com.michelin.cio.hudson.plugins.rolestrategy.Role;
 import org.kohsuke.accmod.Restricted;

src/test/java/org/jenkinsci/plugins/casc/integrations/globalmatrixauth/GlobalMatrixAuthorizationTest.java

@@ -1,8 +1,7 @@
-package org.jenkinsci.plugins.casc.integrations;
+package org.jenkinsci.plugins.casc.integrations.globalmatrixauth;
 
 import hudson.security.AuthorizationStrategy;
 import hudson.security.GlobalMatrixAuthorizationStrategy;
-import hudson.security.Permission;
 import jenkins.model.Jenkins;
 import org.jenkinsci.plugins.casc.ConfigurationAsCode;
 import org.jenkinsci.plugins.casc.Configurator;
@@ -38,14 +37,14 @@ public class GlobalMatrixAuthorizationTest {
 
     @Test
     public void checkCorrectlyConfiguredPermissions() throws Exception {
-        ConfigurationAsCode.configure(getClass().getResourceAsStream("global-matrix-auth/GlobalMatrixStrategy.yml"));
+        ConfigurationAsCode.configure(getClass().getResourceAsStream("GlobalMatrixStrategy.yml"));
         assertEquals("The configured instance must use the Global Matrix Authentication Strategy", GlobalMatrixAuthorizationStrategy.class, Jenkins.getInstance().getAuthorizationStrategy().getClass());
         GlobalMatrixAuthorizationStrategy gms = (GlobalMatrixAuthorizationStrategy)Jenkins.getInstance().getAuthorizationStrategy();
 
-        List<String> adminPermission = new ArrayList<>(gms.getGrantedPermissions().get(Permission.fromId("hudson.model.Hudson.Administer")));
+        List<String> adminPermission = new ArrayList<>(gms.getGrantedPermissions().get(Jenkins.ADMINISTER));
         assertEquals("authenticated", adminPermission.get(0));
 
-        List<String> readPermission = new ArrayList<>(gms.getGrantedPermissions().get(Permission.fromId("hudson.model.Hudson.Read")));
+        List<String> readPermission = new ArrayList<>(gms.getGrantedPermissions().get(Jenkins.READ));
         assertEquals("anonymous", readPermission.get(0));
     }
 }

src/test/java/org/jenkinsci/plugins/casc/integrations/rolebasedauth/RoleStrategyTest.java

@@ -1,15 +1,15 @@
-package org.jenkinsci.plugins.casc.integrations;
+package org.jenkinsci.plugins.casc.integrations.rolebasedauth;
 
 import com.michelin.cio.hudson.plugins.rolestrategy.Role;
 import com.michelin.cio.hudson.plugins.rolestrategy.RoleBasedAuthorizationStrategy;
-import com.michelin.cio.hudson.plugins.rolestrategy.RoleMap;
 import hudson.security.AuthorizationStrategy;
+import hudson.security.Permission;
 import jenkins.model.Jenkins;
 import static org.hamcrest.CoreMatchers.*;
 import org.jenkinsci.plugins.casc.ConfigurationAsCode;
 import org.jenkinsci.plugins.casc.Configurator;
+import org.jenkinsci.plugins.casc.integrations.rolebasedauth.RoleBasedAuthorizationStrategyConfigurator;
 import org.junit.ClassRule;
-import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
@@ -50,9 +50,10 @@ public class RoleStrategyTest {
     @Test
     @Issue("Issue #48")
     public void shouldReadRolesCorrectly() throws Exception {
-        ConfigurationAsCode.configure(getClass().getResourceAsStream("role-strategy/GlobalMatrixStrategy.yml"));
+        ConfigurationAsCode.configure(getClass().getResourceAsStream("GlobalMatrixStrategy.yml"));
 
         final Jenkins jenkins = Jenkins.getInstance();
+
         AuthorizationStrategy s = jenkins.getAuthorizationStrategy();
         assertThat("Authorization Strategy has been read incorrectly",
                 s, instanceOf(RoleBasedAuthorizationStrategy.class));