Move migration of admin flags to own update step

scm-webapp/src/main/java/sonia/scm/group/update/XmlGroupV1UpdateStep.java

@@ -3,15 +3,12 @@ package sonia.scm.group.update;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import sonia.scm.SCMContextProvider;
+import sonia.scm.group.Group;
 import sonia.scm.group.xml.XmlGroupDAO;
 import sonia.scm.migration.UpdateException;
 import sonia.scm.migration.UpdateStep;
 import sonia.scm.plugin.Extension;
-import sonia.scm.security.AssignedPermission;
-import sonia.scm.store.ConfigurationEntryStore;
-import sonia.scm.store.ConfigurationEntryStoreFactory;
 import sonia.scm.store.StoreConstants;
-import sonia.scm.group.Group;
 import sonia.scm.version.Version;
 
 import javax.inject.Inject;
@@ -25,16 +22,12 @@ import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.Arrays;
-import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 
-import static java.util.Collections.emptyList;
 import static java.util.Optional.empty;
 import static java.util.Optional.of;
-import static java.util.Optional.ofNullable;
 import static sonia.scm.version.Version.parse;
 
 @Extension
@@ -44,13 +37,11 @@ public class XmlGroupV1UpdateStep implements UpdateStep {
 
   private final SCMContextProvider contextProvider;
   private final XmlGroupDAO groupDAO;
-  private final ConfigurationEntryStoreFactory configurationEntryStoreFactory;
 
   @Inject
-  public XmlGroupV1UpdateStep(SCMContextProvider contextProvider, XmlGroupDAO groupDAO, ConfigurationEntryStoreFactory configurationEntryStoreFactory) {
+  public XmlGroupV1UpdateStep(SCMContextProvider contextProvider, XmlGroupDAO groupDAO) {
     this.contextProvider = contextProvider;
     this.groupDAO = groupDAO;
-    this.configurationEntryStoreFactory = configurationEntryStoreFactory;
   }
 
   @Override
@@ -60,30 +51,8 @@ public class XmlGroupV1UpdateStep implements UpdateStep {
       LOG.info("no v1 file for groups found");
       return;
     }
-    Collection<String> adminGroups = determineAdminGroups();
-    LOG.debug("found the following admin groups from global config: {}", adminGroups);
     XmlGroupV1UpdateStep.V1GroupDatabase v1Database = readV1Database(v1GroupsFile.get());
-    ConfigurationEntryStore<AssignedPermission> securityStore = createSecurityStore();
+    v1Database.groupList.groups.forEach(group -> update(group));
-    v1Database.groupList.groups.forEach(group -> update(group, adminGroups, securityStore));
-  }
-
-  private Collection<String> determineAdminGroups() throws JAXBException {
-    Path configDirectory = determineConfigDirectory();
-    Path existingConfigFile = configDirectory.resolve("config" + StoreConstants.FILE_EXTENSION);
-    if (existingConfigFile.toFile().exists()) {
-      return extractAdminGroupsFromConfigFile(existingConfigFile);
-    } else {
-      return emptyList();
-    }
-  }
-
-  private Collection<String> extractAdminGroupsFromConfigFile(Path existingConfigFile) throws JAXBException {
-    JAXBContext jaxbContext = JAXBContext.newInstance(XmlGroupV1UpdateStep.V1Configuration.class);
-    V1Configuration v1Configuration = (V1Configuration) jaxbContext.createUnmarshaller().unmarshal(existingConfigFile.toFile());
-    return ofNullable(v1Configuration.adminGroups)
-      .map(groupList -> groupList.split(","))
-      .map(Arrays::asList)
-      .orElse(emptyList());
   }
 
   @Override
@@ -96,7 +65,7 @@ public class XmlGroupV1UpdateStep implements UpdateStep {
     return "sonia.scm.group.xml";
   }
 
-  private void update(V1Group v1Group, Collection<String> adminGroups, ConfigurationEntryStore<AssignedPermission> securityStore) {
+  private void update(V1Group v1Group) {
     LOG.debug("updating group {}", v1Group.name);
     Group group = new Group(
       v1Group.type,
@@ -106,11 +75,6 @@ public class XmlGroupV1UpdateStep implements UpdateStep {
     group.setCreationDate(v1Group.creationDate);
     group.setLastModified(v1Group.lastModified);
     groupDAO.add(group);
-
-    if (adminGroups.contains(v1Group.name)) {
-      LOG.debug("setting admin permissions for group {}", v1Group.name);
-      securityStore.put(new AssignedPermission(v1Group.name, true, "*"));
-    }
   }
 
   private XmlGroupV1UpdateStep.V1GroupDatabase readV1Database(Path v1GroupsFile) throws JAXBException {
@@ -118,10 +82,6 @@ public class XmlGroupV1UpdateStep implements UpdateStep {
     return (XmlGroupV1UpdateStep.V1GroupDatabase) jaxbContext.createUnmarshaller().unmarshal(v1GroupsFile.toFile());
   }
 
-  private ConfigurationEntryStore<AssignedPermission> createSecurityStore() {
-    return configurationEntryStoreFactory.withType(AssignedPermission.class).withName("security").build();
-  }
-
   private Optional<Path> determineV1File() {
     Path configDirectory = determineConfigDirectory();
     Path existingGroupsFile = configDirectory.resolve("groups" + StoreConstants.FILE_EXTENSION);
@@ -167,13 +127,6 @@ public class XmlGroupV1UpdateStep implements UpdateStep {
     }
   }
 
-  @XmlAccessorType(XmlAccessType.FIELD)
-  @XmlRootElement(name = "scm-config")
-  private static class V1Configuration {
-    @XmlElement(name = "admin-groups")
-    private String adminGroups;
-  }
-
   private static class GroupList {
     @XmlElement(name = "group")
     private List<V1Group> groups;

scm-webapp/src/main/java/sonia/scm/security/update/XmlSecurityV1UpdateStep.java

@@ -0,0 +1,105 @@
+package sonia.scm.security.update;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import sonia.scm.SCMContextProvider;
+import sonia.scm.migration.UpdateStep;
+import sonia.scm.plugin.Extension;
+import sonia.scm.security.AssignedPermission;
+import sonia.scm.store.ConfigurationEntryStore;
+import sonia.scm.store.ConfigurationEntryStoreFactory;
+import sonia.scm.store.StoreConstants;
+import sonia.scm.version.Version;
+
+import javax.inject.Inject;
+import javax.xml.bind.JAXBContext;
+import javax.xml.bind.JAXBException;
+import javax.xml.bind.annotation.XmlAccessType;
+import javax.xml.bind.annotation.XmlAccessorType;
+import javax.xml.bind.annotation.XmlElement;
+import javax.xml.bind.annotation.XmlRootElement;
+import java.io.File;
+import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.function.Consumer;
+
+import static java.util.Optional.ofNullable;
+import static sonia.scm.version.Version.parse;
+
+@Extension
+public class XmlSecurityV1UpdateStep implements UpdateStep {
+
+  private static final Logger LOG = LoggerFactory.getLogger(XmlSecurityV1UpdateStep.class);
+
+  private final SCMContextProvider contextProvider;
+  private final ConfigurationEntryStoreFactory configurationEntryStoreFactory;
+
+  @Inject
+  public XmlSecurityV1UpdateStep(SCMContextProvider contextProvider, ConfigurationEntryStoreFactory configurationEntryStoreFactory) {
+    this.contextProvider = contextProvider;
+    this.configurationEntryStoreFactory = configurationEntryStoreFactory;
+  }
+
+  @Override
+  public void doUpdate() throws JAXBException {
+    ConfigurationEntryStore<AssignedPermission> securityStore = createSecurityStore();
+
+    forAllAdmins(user -> createSecurityEntry(user, false, securityStore),
+        group -> createSecurityEntry(group, true, securityStore));
+  }
+
+  private void forAllAdmins(Consumer<String> userConsumer, Consumer<String> groupConsumer) throws JAXBException {
+    Path configDirectory = determineConfigDirectory();
+    Path existingConfigFile = configDirectory.resolve("config" + StoreConstants.FILE_EXTENSION);
+    if (existingConfigFile.toFile().exists()) {
+      forAllAdmins(existingConfigFile, userConsumer, groupConsumer);
+    }
+  }
+
+  private void forAllAdmins(
+    Path existingConfigFile, Consumer<String> userConsumer, Consumer<String> groupConsumer
+  ) throws JAXBException {
+    JAXBContext jaxbContext = JAXBContext.newInstance(XmlSecurityV1UpdateStep.V1Configuration.class);
+    V1Configuration v1Configuration = (V1Configuration) jaxbContext.createUnmarshaller().unmarshal(existingConfigFile.toFile());
+
+    ofNullable(v1Configuration.adminUsers).ifPresent(users -> forAll(users, userConsumer));
+    ofNullable(v1Configuration.adminGroups).ifPresent(groups -> forAll(groups, groupConsumer));
+  }
+
+  private void forAll(String entries, Consumer<String> consumer) {
+    Arrays.stream(entries.split(",")).forEach(consumer);
+  }
+
+
+  @Override
+  public Version getTargetVersion() {
+    return parse("2.0.0");
+  }
+
+  @Override
+  public String getAffectedDataType() {
+    return "sonia.scm.security.xml";
+  }
+
+  private void createSecurityEntry(String name, boolean group, ConfigurationEntryStore<AssignedPermission> securityStore) {
+    LOG.debug("setting admin permissions for {} {}", group? "group": "user", name);
+    securityStore.put(new AssignedPermission(name, group, "*"));
+  }
+
+  private ConfigurationEntryStore<AssignedPermission> createSecurityStore() {
+    return configurationEntryStoreFactory.withType(AssignedPermission.class).withName("security").build();
+  }
+
+  private Path determineConfigDirectory() {
+    return new File(contextProvider.getBaseDirectory(), StoreConstants.CONFIG_DIRECTORY_NAME).toPath();
+  }
+
+  @XmlAccessorType(XmlAccessType.FIELD)
+  @XmlRootElement(name = "scm-config")
+  private static class V1Configuration {
+    @XmlElement(name = "admin-users")
+    private String adminUsers;
+    @XmlElement(name = "admin-groups")
+    private String adminGroups;
+  }
+}

scm-webapp/src/main/java/sonia/scm/user/update/XmlUserV1UpdateStep.java

@@ -25,16 +25,12 @@ import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.Arrays;
-import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 
-import static java.util.Collections.emptyList;
 import static java.util.Optional.empty;
 import static java.util.Optional.of;
-import static java.util.Optional.ofNullable;
 import static sonia.scm.version.Version.parse;
 
 @Extension
@@ -60,30 +56,9 @@ public class XmlUserV1UpdateStep implements UpdateStep {
       LOG.info("no v1 file for users found");
       return;
     }
-    Collection<String> adminUsers = determineAdminUsers();
-    LOG.debug("found the following admin users from global config: {}", adminUsers);
     XmlUserV1UpdateStep.V1UserDatabase v1Database = readV1Database(v1UsersFile.get());
     ConfigurationEntryStore<AssignedPermission> securityStore = createSecurityStore();
-    v1Database.userList.users.forEach(user -> update(user, adminUsers, securityStore));
+    v1Database.userList.users.forEach(user -> update(user, securityStore));
-  }
-
-  private Collection<String> determineAdminUsers() throws JAXBException {
-    Path configDirectory = determineConfigDirectory();
-    Path existingConfigFile = configDirectory.resolve("config" + StoreConstants.FILE_EXTENSION);
-    if (existingConfigFile.toFile().exists()) {
-      return extractAdminUsersFromConfigFile(existingConfigFile);
-    } else {
-      return emptyList();
-    }
-  }
-
-  private Collection<String> extractAdminUsersFromConfigFile(Path existingConfigFile) throws JAXBException {
-    JAXBContext jaxbContext = JAXBContext.newInstance(XmlUserV1UpdateStep.V1Configuration.class);
-    V1Configuration v1Configuration = (V1Configuration) jaxbContext.createUnmarshaller().unmarshal(existingConfigFile.toFile());
-    return ofNullable(v1Configuration.adminUsers)
-      .map(userList -> userList.split(","))
-      .map(Arrays::asList)
-      .orElse(emptyList());
   }
 
   @Override
@@ -96,7 +71,7 @@ public class XmlUserV1UpdateStep implements UpdateStep {
     return "sonia.scm.user.xml";
   }
 
-  private void update(V1User v1User, Collection<String> adminUsers, ConfigurationEntryStore<AssignedPermission> securityStore) {
+  private void update(V1User v1User, ConfigurationEntryStore<AssignedPermission> securityStore) {
     LOG.debug("updating user {}", v1User.name);
     User user = new User(
       v1User.name,
@@ -109,7 +84,7 @@ public class XmlUserV1UpdateStep implements UpdateStep {
     user.setLastModified(v1User.lastModified);
     userDAO.add(user);
 
-    if (v1User.admin || adminUsers.contains(v1User.name)) {
+    if (v1User.admin) {
       LOG.debug("setting admin permissions for user {}", v1User.name);
       securityStore.put(new AssignedPermission(v1User.name, "*"));
     }
@@ -174,13 +149,6 @@ public class XmlUserV1UpdateStep implements UpdateStep {
     }
   }
 
-  @XmlAccessorType(XmlAccessType.FIELD)
-  @XmlRootElement(name = "scm-config")
-  private static class V1Configuration {
-    @XmlElement(name = "admin-users")
-    private String adminUsers;
-  }
-
   private static class UserList {
     @XmlElement(name = "user")
     private List<V1User> users;

scm-webapp/src/test/java/sonia/scm/group/update/XmlGroupV1UpdateStepTest.java

@@ -11,13 +11,11 @@ import org.mockito.Captor;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import sonia.scm.SCMContextProvider;
-import sonia.scm.security.AssignedPermission;
-import sonia.scm.store.ConfigurationEntryStore;
-import sonia.scm.store.ConfigurationEntryStoreFactory;
-import sonia.scm.store.InMemoryConfigurationEntryStore;
-import sonia.scm.store.InMemoryConfigurationEntryStoreFactory;
 import sonia.scm.group.Group;
 import sonia.scm.group.xml.XmlGroupDAO;
+import sonia.scm.security.AssignedPermission;
+import sonia.scm.store.ConfigurationEntryStore;
+import sonia.scm.store.InMemoryConfigurationEntryStore;
 
 import javax.xml.bind.JAXBException;
 import java.io.IOException;
@@ -53,8 +51,7 @@ class XmlGroupV1UpdateStepTest {
   void mockScmHome(@TempDirectory.TempDir Path tempDir) {
     when(contextProvider.getBaseDirectory()).thenReturn(tempDir.toFile());
     assignedPermissionStore = new InMemoryConfigurationEntryStore<>();
-    ConfigurationEntryStoreFactory inMemoryConfigurationEntryStoreFactory = new InMemoryConfigurationEntryStoreFactory(assignedPermissionStore);
+    updateStep = new XmlGroupV1UpdateStep(contextProvider, groupDAO);
-    updateStep = new XmlGroupV1UpdateStep(contextProvider, groupDAO, inMemoryConfigurationEntryStoreFactory);
   }
 
   @Nested
@@ -70,7 +67,6 @@ class XmlGroupV1UpdateStepTest {
       Path configDir = tempDir.resolve("config");
       Files.createDirectories(configDir);
       copyTestDatabaseFile(configDir, "groups.xml");
-      copyTestDatabaseFile(configDir, "config.xml");
     }
 
     @Test
@@ -92,14 +88,6 @@ class XmlGroupV1UpdateStepTest {
         .hasFieldOrPropertyWithValue("lastModified", 1559550955883L)
         .hasFieldOrPropertyWithValue("creationDate", 1559548942457L);
     }
-
-    @Test
-    void shouldCreatePermissionForGroupsConfiguredAsAdminInConfig() throws JAXBException {
-      updateStep.doUpdate();
-      Optional<AssignedPermission> assignedPermission = assignedPermissionStore.getAll().values().stream().filter(a -> a.getName().equals("vogons")).findFirst();
-      assertThat(assignedPermission.get().getPermission().getValue()).contains("*");
-      assertThat(assignedPermission.get().isGroupPermission()).isTrue();
-    }
   }
 
   private void copyTestDatabaseFile(Path configDir, String groupsFileName) throws IOException {

scm-webapp/src/test/java/sonia/scm/security/update/XmlSecurityV1UpdateStepTest.java

@@ -0,0 +1,93 @@
+package sonia.scm.security.update;
+
+import com.google.common.io.Resources;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junitpioneer.jupiter.TempDirectory;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import sonia.scm.SCMContextProvider;
+import sonia.scm.security.AssignedPermission;
+import sonia.scm.store.ConfigurationEntryStore;
+import sonia.scm.store.ConfigurationEntryStoreFactory;
+import sonia.scm.store.InMemoryConfigurationEntryStore;
+import sonia.scm.store.InMemoryConfigurationEntryStoreFactory;
+
+import javax.xml.bind.JAXBException;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.List;
+
+import static java.util.stream.Collectors.toList;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.when;
+
+@ExtendWith(MockitoExtension.class)
+@ExtendWith(TempDirectory.class)
+class XmlSecurityV1UpdateStepTest {
+
+  @Mock
+  SCMContextProvider contextProvider;
+
+  XmlSecurityV1UpdateStep updateStep;
+  ConfigurationEntryStore<AssignedPermission> assignedPermissionStore;
+
+  @BeforeEach
+  void mockScmHome(@TempDirectory.TempDir Path tempDir) {
+    when(contextProvider.getBaseDirectory()).thenReturn(tempDir.toFile());
+    assignedPermissionStore = new InMemoryConfigurationEntryStore<>();
+    ConfigurationEntryStoreFactory inMemoryConfigurationEntryStoreFactory = new InMemoryConfigurationEntryStoreFactory(assignedPermissionStore);
+    updateStep = new XmlSecurityV1UpdateStep(contextProvider, inMemoryConfigurationEntryStoreFactory);
+  }
+
+  @Nested
+  class WithExistingDatabase {
+
+    @BeforeEach
+    void createConfigV1XML(@TempDirectory.TempDir Path tempDir) throws IOException {
+      Path configDir = tempDir.resolve("config");
+      Files.createDirectories(configDir);
+      copyTestDatabaseFile(configDir, "config.xml");
+    }
+
+    @Test
+    void shouldCreatePermissionForUsersConfiguredAsAdmin() throws JAXBException {
+      updateStep.doUpdate();
+      List<String> assignedPermission =
+        assignedPermissionStore.getAll().values()
+          .stream()
+          .filter(a -> a.getPermission().getValue().equals("*"))
+          .filter(a -> !a.isGroupPermission())
+          .map(AssignedPermission::getName)
+          .collect(toList());
+      assertThat(assignedPermission).contains("arthur", "dent", "ldap-admin");
+    }
+
+    @Test
+    void shouldCreatePermissionForGroupsConfiguredAsAdmin() throws JAXBException {
+      updateStep.doUpdate();
+      List<String> assignedPermission =
+        assignedPermissionStore.getAll().values()
+          .stream()
+          .filter(a -> a.getPermission().getValue().equals("*"))
+          .filter(AssignedPermission::isGroupPermission)
+          .map(AssignedPermission::getName)
+          .collect(toList());
+      assertThat(assignedPermission).contains("admins", "vogons");
+    }
+  }
+
+  private void copyTestDatabaseFile(Path configDir, String fileName) throws IOException {
+    URL url = Resources.getResource("sonia/scm/security/update/" + fileName);
+    Files.copy(url.openStream(), configDir.resolve(fileName));
+  }
+
+  @Test
+  void shouldNotFailForMissingConfigDir() throws JAXBException {
+    updateStep.doUpdate();
+  }
+}

scm-webapp/src/test/java/sonia/scm/user/update/XmlUserV1UpdateStepTest.java

@@ -69,7 +69,6 @@ class XmlUserV1UpdateStepTest {
       Path configDir = tempDir.resolve("config");
       Files.createDirectories(configDir);
       copyTestDatabaseFile(configDir, "users.xml");
-      copyTestDatabaseFile(configDir, "config.xml");
     }
 
     @Test
@@ -101,14 +100,6 @@ class XmlUserV1UpdateStepTest {
         .hasFieldOrPropertyWithValue("lastModified", 1558597367492L)
         .hasFieldOrPropertyWithValue("creationDate", 1558597074732L);
     }
-
-    @Test
-    void shouldCreatePermissionForUsersConfiguredAsAdminInConfig() throws JAXBException {
-      updateStep.doUpdate();
-      Optional<AssignedPermission> assignedPermission = assignedPermissionStore.getAll().values().stream().filter(a -> a.getName().equals("dent")).findFirst();
-      assertThat(assignedPermission.get().getPermission().getValue()).contains("*");
-      assertThat(assignedPermission.get().isGroupPermission()).isFalse();
-    }
   }
 
   private void copyTestDatabaseFile(Path configDir, String usersFileName) throws IOException {

scm-webapp/src/test/resources/sonia/scm/security/update/config.xml

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
 <scm-config>
     <admin-groups>admins,vogons</admin-groups>
-    <admin-users>arthur,dent</admin-users>
+    <admin-users>arthur,dent,ldap-admin</admin-users>
     <base-url>http://localhost:8081/scm</base-url>
     <enableProxy>false</enableProxy>
     <force-base-url>false</force-base-url>

scm-webapp/src/test/resources/sonia/scm/user/update/config.xml

@@ -1,20 +0,0 @@
-<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
-<scm-config>
-    <admin-groups>admins,vogons</admin-groups>
-    <admin-users>arthur,dent</admin-users>
-    <base-url>http://localhost:8081/scm</base-url>
-    <enableProxy>false</enableProxy>
-    <force-base-url>false</force-base-url>
-    <forwardPort>80</forwardPort>
-    <plugin-url>http://plugins.scm-manager.org/scm-plugin-backend/api/{version}/plugins?os={os}&amp;arch={arch}&amp;snapshot=false</plugin-url>
-    <proxyPort>8080</proxyPort>
-    <proxyServer>proxy.mydomain.com</proxyServer>
-    <servername>localhost</servername>
-    <enableSSL>false</enableSSL>
-    <enablePortForward>false</enablePortForward>
-    <sslPort>8181</sslPort>
-    <enableRepositoryArchive>false</enableRepositoryArchive>
-    <disableGroupingGrid>false</disableGroupingGrid>
-    <dateFormat>Y-m-d H:i:s</dateFormat>
-    <anonymousAccessEnabled>false</anonymousAccessEnabled>
-</scm-config>