diff --git a/scm-webapp/src/main/java/sonia/scm/update/security/XmlSecurityV1UpdateStep.java b/scm-webapp/src/main/java/sonia/scm/update/security/XmlSecurityV1UpdateStep.java index fba3e6a929..8dac2b5073 100644 --- a/scm-webapp/src/main/java/sonia/scm/update/security/XmlSecurityV1UpdateStep.java +++ b/scm-webapp/src/main/java/sonia/scm/update/security/XmlSecurityV1UpdateStep.java @@ -23,6 +23,8 @@ import java.nio.file.Path; import java.util.Arrays; import java.util.List; import java.util.function.Consumer; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import static java.util.Optional.ofNullable; import static sonia.scm.version.Version.parse; @@ -30,6 +32,8 @@ import static sonia.scm.version.Version.parse; @Extension public class XmlSecurityV1UpdateStep implements UpdateStep { + private static final Pattern v1PermissionPattern = Pattern.compile("^repository:\\*:(READ|WRITE|OWNER)$"); + private static final Logger LOG = LoggerFactory.getLogger(XmlSecurityV1UpdateStep.class); private final SCMContextProvider contextProvider; @@ -63,30 +67,36 @@ public class XmlSecurityV1UpdateStep implements UpdateStep { V1Security v1Security = (V1Security) jaxbContext.createUnmarshaller().unmarshal(v1SecurityFile.toFile()); v1Security.entries.forEach(assignedPermission -> { - - String newPermission = ""; - if (assignedPermission.value.permission != null && !assignedPermission.value.permission.isEmpty()) { - String[] splitPermission = assignedPermission.value.permission.split(":"); - switch(splitPermission[2]) { - case "OWNER": - newPermission = "repository:*"; - break; - case "WRITE": - newPermission = "repository:read,pull,push:*"; - break; - case "READ": - newPermission = "repository:read,pull:*"; - } + Matcher matcher = v1PermissionPattern.matcher(assignedPermission.value.permission); + if (matcher.matches()) { + String newPermission = convertRole(matcher.group(1)); + securityStore.put(new AssignedPermission( + assignedPermission.value.name, + Boolean.parseBoolean(assignedPermission.value.groupPermission), + newPermission + )); } - - securityStore.put(new AssignedPermission( - assignedPermission.value.name, - Boolean.parseBoolean(assignedPermission.value.groupPermission), - newPermission - )); }); } + private String convertRole(String role) { + String newPermission; + switch (role) { + case "OWNER": + newPermission = "repository:*"; + break; + case "WRITE": + newPermission = "repository:read,pull,push:*"; + break; + case "READ": + newPermission = "repository:read,pull:*"; + break; + default: + newPermission = ""; + } + return newPermission; + } + private void forAllAdmins(Consumer userConsumer, Consumer groupConsumer) throws JAXBException { Path configDirectory = determineConfigDirectory(); Path existingConfigFile = configDirectory.resolve("config" + StoreConstants.FILE_EXTENSION); diff --git a/scm-webapp/src/test/java/sonia/scm/update/security/XmlSecurityV1UpdateStepTest.java b/scm-webapp/src/test/java/sonia/scm/update/security/XmlSecurityV1UpdateStepTest.java index 76914d3bfa..73c7fe6aca 100644 --- a/scm-webapp/src/test/java/sonia/scm/update/security/XmlSecurityV1UpdateStepTest.java +++ b/scm-webapp/src/test/java/sonia/scm/update/security/XmlSecurityV1UpdateStepTest.java @@ -56,13 +56,6 @@ class XmlSecurityV1UpdateStepTest { copyTestDatabaseFile(configDir, "config.xml"); } - @BeforeEach - void createSecurityV1XML(@TempDirectory.TempDir Path tempDir) throws IOException { - Path configDir = tempDir.resolve("config"); - Files.createDirectories(configDir); - copyTestDatabaseFile(configDir, "securityV1.xml"); - } - @Test void shouldCreatePermissionForUsersConfiguredAsAdmin() throws JAXBException { updateStep.doUpdate(); @@ -89,6 +82,18 @@ class XmlSecurityV1UpdateStepTest { assertThat(assignedPermission).contains("admins", "vogons"); } + } + + @Nested + class WithExistingSecurityXml { + + @BeforeEach + void createSecurityV1XML(@TempDirectory.TempDir Path tempDir) throws IOException { + Path configDir = tempDir.resolve("config"); + Files.createDirectories(configDir); + copyTestDatabaseFile(configDir, "securityV1.xml"); + } + @Test void shouldMapV1PermissionsFromSecurityV1XML() throws JAXBException { updateStep.doUpdate(); @@ -101,6 +106,7 @@ class XmlSecurityV1UpdateStepTest { assertThat(assignedPermission).contains("scmadmin"); assertThat(assignedPermission).contains("test"); } + } private void copyTestDatabaseFile(Path configDir, String fileName) throws IOException { diff --git a/scm-webapp/src/test/resources/sonia/scm/update/security/securityV1.xml b/scm-webapp/src/test/resources/sonia/scm/update/security/securityV1.xml index 2f23062f42..8de82f88d9 100644 --- a/scm-webapp/src/test/resources/sonia/scm/update/security/securityV1.xml +++ b/scm-webapp/src/test/resources/sonia/scm/update/security/securityV1.xml @@ -1,19 +1,35 @@ - - 4lRWOA7DH1 - - false - scmadmin - repository:*:READ - - - - CfRWOAANM2 - - true - test - repository:*:OWNER - - + + 4lRWOA7DH1 + + false + scmadmin + repository:*:READ + + + + CfRWOAANM2 + + true + test + repository:*:OWNER + + + + CfRWOAANM2 + + true + test + invalid:permission + + + + CfRWOAANM2 + + true + test + repository:*:STRANGE + +