logging non-existent projects/groups in forProjects/forGroups (#1582)

tulinkry · Vladimir Kotal · commit 40f24dbc265e · 2017-05-10T14:21:26.000+02:00
fixes #1572
diff --git a/src/org/opensolaris/opengrok/authorization/AuthorizationEntity.java b/src/org/opensolaris/opengrok/authorization/AuthorizationEntity.java
@@ -28,9 +28,13 @@
 import java.util.TreeMap;
 import java.util.TreeSet;
 import java.util.function.Predicate;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import org.opensolaris.opengrok.configuration.Group;
 import org.opensolaris.opengrok.configuration.Nameable;
+import org.opensolaris.opengrok.configuration.Project;
+import org.opensolaris.opengrok.logger.LoggerFactory;
 
 /**
  * This class covers authorization entities used in opengrok.
@@ -57,6 +61,8 @@
  */
 public abstract class AuthorizationEntity implements Nameable, Serializable, Cloneable {
 
+    private static final Logger LOGGER = LoggerFactory.getLogger(AuthorizationEntity.class);
+
     /**
      * Predicate specialized for the the plugin decisions. The caller should
      * implement the <code>decision</code> method. Returning true if the plugin
@@ -369,25 +375,49 @@ public void setForGroups(String[] groups) {
      * the group</li>
      * <li>add to the {@link #forProjects()} all projects and repositories which
      * are in the descendant groups or in the group itself</li>
+     * <li>issue a warning for non-existent groups</li>
+     * <li>issue a warning for non-existent projects</li>
      * </ul>
      */
-    protected void discoverGroups() {
+    protected void processTargetGroupsAndProjects() {
         Set<String> groups = new TreeSet<>();
+
         for (String x : forGroups()) {
             /**
              * Full group discovery takes place here. All projects/repositories
              * in the group are added into "forProjects" and all subgroups
              * (including projects/repositories) and parent groups (excluding
              * the projects/repositories) are added into "forGroups".
+             *
+             * If the group does not exist then a warning is issued.
              */
             Group g;
             if ((g = Group.getByName(x)) != null) {
                 forProjects().addAll(g.getAllProjects().stream().map((t) -> t.getName()).collect(Collectors.toSet()));
                 groups.addAll(g.getRelatedGroups().stream().map((t) -> t.getName()).collect(Collectors.toSet()));
                 groups.add(x);
+            } else {
+                LOGGER.log(Level.WARNING, "Configured group \"{0}\" in forGroups section"
+                        + " for name \"{1}\" does not exist",
+                        new Object[]{x, getName()});
             }
         }
         setForGroups(groups);
+
+        forProjects().removeIf((t) -> {
+            /**
+             * Check the existence of the projects and issue a warning if there
+             * is no such project.
+             */
+            Project p;
+            if ((p = Project.getByName(t)) == null) {
+                LOGGER.log(Level.WARNING, "Configured project \"{0}\" in forProjects"
+                        + " section for name \"{1}\" does not exist",
+                        new Object[]{t, getName()});
+                return true;
+            }
+            return false;
+        });
     }
 
     /**
diff --git a/src/org/opensolaris/opengrok/authorization/AuthorizationPlugin.java b/src/org/opensolaris/opengrok/authorization/AuthorizationPlugin.java
@@ -94,6 +94,9 @@ public AuthorizationPlugin(AuthControlFlag flag, String name, IAuthorizationPlug
      */
     @Override
     public synchronized void load(Map<String, Object> parameters) {
+        // fill properly the "forGroups" and "forProjects" fields
+        processTargetGroupsAndProjects();
+
         if (!hasPlugin()) {
             LOGGER.log(Level.SEVERE, "Configured plugin \"{0}\" has not been loaded into JVM (missing file?). "
                     + "This can cause the authorization to fail always.",
@@ -112,9 +115,6 @@ public synchronized void load(Map<String, Object> parameters) {
         s.putAll(parameters);
         s.putAll(getSetup());
 
-        // fill properly the "forGroups" and "forProjects" fields
-        discoverGroups();
-
         try {
             plugin.load(s);
             setWorking();
diff --git a/src/org/opensolaris/opengrok/authorization/AuthorizationStack.java b/src/org/opensolaris/opengrok/authorization/AuthorizationStack.java
@@ -134,7 +134,7 @@ public void load(Map<String, Object> parameters) {
                     getName()});
 
         // fill properly the "forGroups" and "forProjects" fields
-        discoverGroups();
+        processTargetGroupsAndProjects();
 
         setWorking();
 
diff --git a/test/org/opensolaris/opengrok/authorization/AuthorizationEntityTest.java b/test/org/opensolaris/opengrok/authorization/AuthorizationEntityTest.java
@@ -0,0 +1,232 @@
+/*
+ * CDDL HEADER START
+ *
+ * The contents of this file are subject to the terms of the
+ * Common Development and Distribution License (the "License").
+ * You may not use this file except in compliance with the License.
+ *
+ * See LICENSE.txt included in this distribution for the specific
+ * language governing permissions and limitations under the License.
+ *
+ * When distributing Covered Code, include this CDDL HEADER in each
+ * file and include the License file at LICENSE.txt.
+ * If applicable, add the following below this CDDL HEADER, with the
+ * fields enclosed by brackets "[]" replaced with your own identifying
+ * information: Portions Copyright [yyyy] [name of copyright owner]
+ *
+ * CDDL HEADER END
+ */
+
+ /*
+ * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+ */
+package org.opensolaris.opengrok.authorization;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
+import java.util.TreeSet;
+import java.util.function.Function;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.opensolaris.opengrok.configuration.Group;
+import org.opensolaris.opengrok.configuration.Project;
+import org.opensolaris.opengrok.configuration.RuntimeEnvironment;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ *
+ * @author Krystof Tulinger
+ */
+@RunWith(Parameterized.class)
+public class AuthorizationEntityTest {
+
+    private Set<Group> envGroups;
+    private Map<String, Project> envProjects;
+
+    private final Function<Void, AuthorizationEntity> authEntityFactory;
+
+    private static final Function<Void, AuthorizationEntity> PLUGIN_FACTORY = new Function<Void, AuthorizationEntity>() {
+        @Override
+        public AuthorizationEntity apply(Void t) {
+            return new AuthorizationPlugin(AuthControlFlag.REQUIRED, "");
+        }
+    };
+
+    private static final Function<Void, AuthorizationEntity> STACK_FACTORY = new Function<Void, AuthorizationEntity>() {
+        @Override
+        public AuthorizationEntity apply(Void t) {
+            return new AuthorizationStack(AuthControlFlag.REQUIRED, "");
+        }
+    };
+
+    @Parameterized.Parameters
+    public static Collection<Function<Void, AuthorizationEntity>> parameters() {
+        List<Function<Void, AuthorizationEntity>> l = new ArrayList<>();
+        l.add(PLUGIN_FACTORY);
+        l.add(STACK_FACTORY);
+        return l;
+    }
+
+    public AuthorizationEntityTest(Function<Void, AuthorizationEntity> authEntityFactory) {
+        this.authEntityFactory = authEntityFactory;
+    }
+
+    @Before
+    public void setUp() {
+        RuntimeEnvironment env = RuntimeEnvironment.getInstance();
+        envGroups = env.getGroups();
+        envProjects = env.getProjects();
+        env.setGroups(new TreeSet<>());
+        env.setProjects(new TreeMap<>());
+    }
+
+    @After
+    public void tearDown() {
+        RuntimeEnvironment.getInstance().setGroups(envGroups);
+        RuntimeEnvironment.getInstance().setProjects(envProjects);
+    }
+
+    @Test
+    public void testForGroupsAndForProjectsDiscovery() {
+        Group g1, g2, g3;
+        AuthorizationEntity authEntity;
+
+        RuntimeEnvironment env = RuntimeEnvironment.getInstance();
+
+        env.getProjects().put("project 1", new Project("project 1"));
+        env.getProjects().put("project 2", new Project("project 2"));
+        env.getProjects().put("project 3", new Project("project 3"));
+        env.getProjects().put("project 4", new Project("project 4"));
+        env.getProjects().put("project 5", new Project("project 5"));
+        env.getProjects().put("project 6", new Project("project 6"));
+        env.getProjects().put("project 7", new Project("project 7"));
+        env.getProjects().put("project 8", new Project("project 8"));
+        env.getProjects().put("project 9", new Project("project 9"));
+
+        /**
+         * Structure<br>
+         * <pre>
+         * G1 + P1
+         *    + P2
+         *    + P3
+         *    + G2
+         *       + P4
+         *       + P5
+         *       + P6
+         *       + P7
+         * G3 + P8
+         *    + P9
+         * </pre>
+         */
+        g1 = new Group();
+        g1.setName("group 1");
+        g1.addProject(env.getProjects().get("project 1"));
+        g1.addProject(env.getProjects().get("project 2"));
+        g1.addProject(env.getProjects().get("project 3"));
+        env.getGroups().add(g1);
+        g2 = new Group();
+        g2.setName("group 2");
+        g2.addProject(env.getProjects().get("project 4"));
+        g2.addProject(env.getProjects().get("project 5"));
+        g2.addProject(env.getProjects().get("project 6"));
+        g2.addProject(env.getProjects().get("project 7"));
+        g1.addGroup(g2);
+        env.getGroups().add(g2);
+        g3 = new Group();
+        g3.setName("group 3");
+        g3.addProject(env.getProjects().get("project 8"));
+        g3.addProject(env.getProjects().get("project 9"));
+        env.getGroups().add(g3);
+
+        // add g1 and all descendants their projects
+        authEntity = authEntityFactory.apply(null);
+        authEntity.setForGroups(new TreeSet<>());
+        authEntity.setForGroups("group 1");
+        authEntity.load(new TreeMap<>());
+
+        assertEquals(new TreeSet<>(Arrays.asList(new String[]{"group 1", "group 2"})), authEntity.forGroups());
+        assertEquals(new TreeSet<>(Arrays.asList(new String[]{"project 1", "project 2", "project 3",
+            "project 4", "project 5", "project 6", "project 7"})), authEntity.forProjects());
+
+        // add group2, its parent g1 and g2 projects
+        authEntity = authEntityFactory.apply(null);
+        authEntity.setForGroups(new TreeSet<>());
+        authEntity.setForGroups("group 2");
+        authEntity.load(new TreeMap<>());
+
+        assertEquals(new TreeSet<>(Arrays.asList(new String[]{"group 1", "group 2"})), authEntity.forGroups());
+        assertEquals(new TreeSet<>(Arrays.asList(new String[]{"project 4", "project 5", "project 6", "project 7"})), authEntity.forProjects());
+
+        // add only g3 and its projects
+        authEntity = authEntityFactory.apply(null);
+        authEntity.setForGroups(new TreeSet<>());
+        authEntity.setForGroups("group 3");
+        authEntity.load(new TreeMap<>());
+
+        assertEquals(new TreeSet<>(Arrays.asList(new String[]{"group 3"})), authEntity.forGroups());
+        assertEquals(new TreeSet<>(Arrays.asList(new String[]{"project 8", "project 9"})), authEntity.forProjects());
+    }
+
+    /**
+     * Listed projects don't exist.
+     */
+    @Test
+    public void testForGroupsAndForProjectsDiscoveryInvalidProject() {
+        AuthorizationEntity authEntity = authEntityFactory.apply(null);
+
+        authEntity.setForProjects(new TreeSet<>(Arrays.asList(new String[]{"project 1", "project 2", "project 3",
+            "project 4", "project 5", "project 6", "project 7"})));
+
+        authEntity.load(new TreeMap<>());
+
+        assertEquals(new TreeSet<>(), authEntity.forGroups());
+        assertEquals(new TreeSet<>(), authEntity.forProjects());
+    }
+
+    /**
+     * Listed groups don't exist.
+     */
+    @Test
+    public void testForGroupsAndForProjectsDiscoveryInvalidGroup() {
+        AuthorizationEntity authEntity = authEntityFactory.apply(null);
+
+        authEntity.setForGroups(new TreeSet<>(Arrays.asList(new String[]{"group 1", "group 2"})));
+
+        authEntity.load(new TreeMap<>());
+
+        assertEquals(new TreeSet<>(), authEntity.forGroups());
+        assertEquals(new TreeSet<>(), authEntity.forProjects());
+    }
+
+    /**
+     * Listed projects in the group don't exist.
+     */
+    @Test
+    public void testForGroupsAndForProjectsDiscoveryInvalidProjectInGroup() {
+        AuthorizationEntity authEntity = authEntityFactory.apply(null);
+
+        authEntity.setForGroups(new TreeSet<>(Arrays.asList(new String[]{"group 1", "group 2"})));
+        RuntimeEnvironment env = RuntimeEnvironment.getInstance();
+
+        Group g1 = new Group();
+        g1.setName("group 1");
+        g1.addProject(new Project("project 1"));
+        g1.addProject(new Project("project 2"));
+        g1.addProject(new Project("project 3"));
+        env.getGroups().add(g1);
+
+        authEntity.load(new TreeMap<>());
+
+        assertEquals(new TreeSet<>(Arrays.asList(new String[]{"group 1"})), authEntity.forGroups());
+        assertEquals(new TreeSet<>(), authEntity.forProjects());
+    }
+}
diff --git a/test/org/opensolaris/opengrok/authorization/AuthorizationStackTest.java b/test/org/opensolaris/opengrok/authorization/AuthorizationStackTest.java
