diff --git a/scm-webapp/src/main/java/sonia/scm/group/update/XmlGroupV1UpdateStep.java b/scm-webapp/src/main/java/sonia/scm/group/update/XmlGroupV1UpdateStep.java index 87368919bf..146a709e7f 100644 --- a/scm-webapp/src/main/java/sonia/scm/group/update/XmlGroupV1UpdateStep.java +++ b/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 adminGroups = determineAdminGroups(); - LOG.debug("found the following admin groups from global config: {}", adminGroups); XmlGroupV1UpdateStep.V1GroupDatabase v1Database = readV1Database(v1GroupsFile.get()); - ConfigurationEntryStore securityStore = createSecurityStore(); - v1Database.groupList.groups.forEach(group -> update(group, adminGroups, securityStore)); - } - - private Collection 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 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()); + v1Database.groupList.groups.forEach(group -> update(group)); } @Override @@ -96,7 +65,7 @@ public class XmlGroupV1UpdateStep implements UpdateStep { return "sonia.scm.group.xml"; } - private void update(V1Group v1Group, Collection adminGroups, ConfigurationEntryStore 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 createSecurityStore() { - return configurationEntryStoreFactory.withType(AssignedPermission.class).withName("security").build(); - } - private Optional 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 groups; diff --git a/scm-webapp/src/main/java/sonia/scm/security/update/XmlSecurityV1UpdateStep.java b/scm-webapp/src/main/java/sonia/scm/security/update/XmlSecurityV1UpdateStep.java new file mode 100644 index 0000000000..52bf8124e3 --- /dev/null +++ b/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 securityStore = createSecurityStore(); + + forAllAdmins(user -> createSecurityEntry(user, false, securityStore), + group -> createSecurityEntry(group, true, securityStore)); + } + + private void forAllAdmins(Consumer userConsumer, Consumer 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 userConsumer, Consumer 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 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 securityStore) { + LOG.debug("setting admin permissions for {} {}", group? "group": "user", name); + securityStore.put(new AssignedPermission(name, group, "*")); + } + + private ConfigurationEntryStore 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; + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/user/update/XmlUserV1UpdateStep.java b/scm-webapp/src/main/java/sonia/scm/user/update/XmlUserV1UpdateStep.java index 4a607dbac1..8c5e760710 100644 --- a/scm-webapp/src/main/java/sonia/scm/user/update/XmlUserV1UpdateStep.java +++ b/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 adminUsers = determineAdminUsers(); - LOG.debug("found the following admin users from global config: {}", adminUsers); XmlUserV1UpdateStep.V1UserDatabase v1Database = readV1Database(v1UsersFile.get()); ConfigurationEntryStore securityStore = createSecurityStore(); - v1Database.userList.users.forEach(user -> update(user, adminUsers, securityStore)); - } - - private Collection 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 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()); + v1Database.userList.users.forEach(user -> update(user, securityStore)); } @Override @@ -96,7 +71,7 @@ public class XmlUserV1UpdateStep implements UpdateStep { return "sonia.scm.user.xml"; } - private void update(V1User v1User, Collection adminUsers, ConfigurationEntryStore securityStore) { + private void update(V1User v1User, ConfigurationEntryStore 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 users; diff --git a/scm-webapp/src/test/java/sonia/scm/group/update/XmlGroupV1UpdateStepTest.java b/scm-webapp/src/test/java/sonia/scm/group/update/XmlGroupV1UpdateStepTest.java index 02a995317f..552298ed77 100644 --- a/scm-webapp/src/test/java/sonia/scm/group/update/XmlGroupV1UpdateStepTest.java +++ b/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, inMemoryConfigurationEntryStoreFactory); + updateStep = new XmlGroupV1UpdateStep(contextProvider, groupDAO); } @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 = 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 { diff --git a/scm-webapp/src/test/java/sonia/scm/security/update/XmlSecurityV1UpdateStepTest.java b/scm-webapp/src/test/java/sonia/scm/security/update/XmlSecurityV1UpdateStepTest.java new file mode 100644 index 0000000000..62e82ce168 --- /dev/null +++ b/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 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 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 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(); + } +} diff --git a/scm-webapp/src/test/java/sonia/scm/user/update/XmlUserV1UpdateStepTest.java b/scm-webapp/src/test/java/sonia/scm/user/update/XmlUserV1UpdateStepTest.java index d5d3ec93e3..764fda2c5d 100644 --- a/scm-webapp/src/test/java/sonia/scm/user/update/XmlUserV1UpdateStepTest.java +++ b/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 = 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 { diff --git a/scm-webapp/src/test/resources/sonia/scm/group/update/config.xml b/scm-webapp/src/test/resources/sonia/scm/security/update/config.xml similarity index 94% rename from scm-webapp/src/test/resources/sonia/scm/group/update/config.xml rename to scm-webapp/src/test/resources/sonia/scm/security/update/config.xml index 43a93f7d3a..a87e80859e 100644 --- a/scm-webapp/src/test/resources/sonia/scm/group/update/config.xml +++ b/scm-webapp/src/test/resources/sonia/scm/security/update/config.xml @@ -1,7 +1,7 @@ admins,vogons - arthur,dent + arthur,dent,ldap-admin http://localhost:8081/scm false false diff --git a/scm-webapp/src/test/resources/sonia/scm/user/update/config.xml b/scm-webapp/src/test/resources/sonia/scm/user/update/config.xml deleted file mode 100644 index 43a93f7d3a..0000000000 --- a/scm-webapp/src/test/resources/sonia/scm/user/update/config.xml +++ /dev/null @@ -1,20 +0,0 @@ - - - admins,vogons - arthur,dent - http://localhost:8081/scm - false - false - 80 - http://plugins.scm-manager.org/scm-plugin-backend/api/{version}/plugins?os={os}&arch={arch}&snapshot=false - 8080 - proxy.mydomain.com - localhost - false - false - 8181 - false - false - Y-m-d H:i:s - false -