Fix RoleBinding created when triming suffix is not valid (#191)

docs/region-configuration.md

@@ -30,8 +30,7 @@ See [regions.json](/onyxia-api/src/main/resources/regions.json) for a complete e
 | `location` | Geographical position of the datacenter on which the region is supposed to run. | {lat: 48.864716, longitude: 2.349014, name: "Paris" } |
 | `includedGroupPattern` | Pattern of user groups considered for the user in the region. Patterns are case sensitive. | ".*_Onyxia" |
 | `excludedGroupPattern` | Pattern of user groups that will not be considered for the user in the region. Patterns are case sensitive. | ".*_BadGroup" |
-| `transformGroupPattern` | Indicate how to transform a group based on `includedGroupPattern`. For example with a `includedGroupPattern` of "(.*)_Onxyia" and a `transformGroupPattern` of "$1-k8s", a mygroup_Onyxia will be considered by Onyxia as mygroup-k8s. | "$1-k8s" |
-proposition : removeSharedPattern
+| `transformGroupPattern` | Indicate how to transform a group based on `includedGroupPattern` to make a project name used for namespace or S3 bucket for example. For example with a `includedGroupPattern` of "(.*)_Onxyia" and a `transformGroupPattern` of "$1-k8s", a mygroup_Onyxia will generate a mygroup-k8s namespace. | "$1-k8s" |
 | `onyxiaAPI` | Contains the base url of an onyxia api | {baseURL: "http://localhost:8080"} |
 | `services` | Configuration of Onyxia services provider platform | See [Service](##services-properties) |
 | `data` | Configuration of the S3 Object Storage | See [S3](#data-properties) |

onyxia-api/src/main/java/fr/insee/onyxia/api/security/KeycloakUserProvider.java

@@ -26,8 +26,6 @@ public class KeycloakUserProvider {
 
     @Autowired private HttpRequestUtils httpRequestUtils;
 
-    Pattern rfc1123Pattern = Pattern.compile("[a-z0-9]([-a-z0-9]*[a-z0-9])?");
-
     @Bean
     @Scope(
             scopeName = WebApplicationContext.SCOPE_REQUEST,
@@ -92,33 +90,6 @@ private List<String> getGroupsFromToken(Region region, final AccessToken token)
             Pattern includePattern = Pattern.compile(region.getIncludedGroupPattern());
             groups.removeIf(group -> !includePattern.matcher(group).matches());
         }
-        if (region.getIncludedGroupPattern() != null && region.getTransformGroupPattern() != null) {
-            Pattern includePattern = Pattern.compile(region.getIncludedGroupPattern());
-            groups =
-                    groups.stream()
-                            .map(
-                                    group ->
-                                            transformGroupFromProviderGroup(
-                                                    includePattern,
-                                                    region.getTransformGroupPattern(),
-                                                    group))
-                            .collect(Collectors.toList());
-        }
-        return groups.stream()
-                .filter(this::isRespectingRFC1123LabelName)
-                .collect(Collectors.toList());
-    }
-
-    private String transformGroupFromProviderGroup(
-            Pattern includePattern, String extractPattern, String providerGroup) {
-        try {
-            return includePattern.matcher(providerGroup).replaceAll(extractPattern);
-        } catch (Exception e) {
-            return null;
-        }
-    }
-
-    private boolean isRespectingRFC1123LabelName(String string) {
-        return string != null && string.length() <= 63 && rfc1123Pattern.matcher(string).matches();
+        return groups;
     }
 }

onyxia-api/src/main/java/fr/insee/onyxia/api/user/OnyxiaUserProvider.java

@@ -5,6 +5,9 @@
 import fr.insee.onyxia.model.OnyxiaUser;
 import fr.insee.onyxia.model.project.Project;
 import fr.insee.onyxia.model.region.Region;
+import java.util.regex.Pattern;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 
@@ -15,33 +18,52 @@ public class OnyxiaUserProvider {
 
     @Autowired private KubernetesService kubernetesService; // TODO : cleanup
 
+    private Logger logger = LoggerFactory.getLogger(OnyxiaUserProvider.class);
+
+    Pattern rfc1123Pattern = Pattern.compile("[a-z0-9]([-a-z0-9]*[a-z0-9])?");
+
     public OnyxiaUser getUser(Region region) {
         OnyxiaUser user = new OnyxiaUser(userProvider.getUser(region));
 
         Project userProject = getUserProject(region, user);
         user.getProjects().add(userProject);
 
+        Pattern includedGroupPattern = getPrecompiledIncludedGroupPattern(region);
+
         if (!region.getServices().isSingleNamespace()) {
             userProvider.getUser(region).getGroups().stream()
                     .forEach(
                             group -> {
-                                Project project = new Project();
-                                project.setId(
-                                        region.getServices().getGroupNamespacePrefix() + group);
-                                project.setGroup(group);
-                                project.setVaultTopDir(
-                                        region.getServices().getGroupNamespacePrefix() + group);
-                                if (region.getData() != null && region.getData().getS3() != null) {
-                                    project.setBucket(
-                                            region.getData().getS3().getGroupBucketPrefix()
-                                                    + group);
+                                String projectBaseName =
+                                        getProjectBaseNameFromGroup(
+                                                group,
+                                                includedGroupPattern,
+                                                region.getTransformGroupPattern());
+                                if (projectBaseName != null) {
+                                    Project project = new Project();
+                                    project.setGroup(group);
+                                    project.setId(
+                                            region.getServices().getGroupNamespacePrefix()
+                                                    + projectBaseName);
+                                    project.setVaultTopDir(
+                                            region.getServices().getGroupNamespacePrefix()
+                                                    + projectBaseName);
+                                    if (region.getData() != null
+                                            && region.getData().getS3() != null) {
+                                        project.setBucket(
+                                                region.getData().getS3().getGroupBucketPrefix()
+                                                        + projectBaseName);
+                                    }
+                                    project.setNamespace(
+                                            region.getServices().getGroupNamespacePrefix()
+                                                    + projectBaseName);
+                                    user.getProjects().add(project);
                                 }
-                                project.setNamespace(
-                                        region.getServices().getGroupNamespacePrefix() + group);
-                                user.getProjects().add(project);
                             });
         }
 
+        user.getProjects().removeIf(p -> !isRespectingRFC1123LabelName(p.getNamespace()));
+
         return user;
     }
 
@@ -79,4 +101,30 @@ private Project getUserProject(Region region, OnyxiaUser user) {
         }
         return userProject;
     }
+
+    private Pattern getPrecompiledIncludedGroupPattern(Region region) {
+        if (region.getIncludedGroupPattern() != null && region.getTransformGroupPattern() != null) {
+            return Pattern.compile(region.getIncludedGroupPattern());
+        } else return null;
+    }
+
+    private String getProjectBaseNameFromGroup(
+            String group, Pattern includePattern, String extractPattern) {
+        if (includePattern != null) {
+            try {
+                return includePattern.matcher(group).replaceAll(extractPattern);
+            } catch (Exception e) {
+                logger.error(
+                        "Failed to transform group project with include pattern : {} "
+                                + "and transform pattern : {} . Returning non transformed group.",
+                        includePattern.pattern(),
+                        extractPattern);
+                return null;
+            }
+        } else return group;
+    }
+
+    private boolean isRespectingRFC1123LabelName(String string) {
+        return string != null && string.length() <= 63 && rfc1123Pattern.matcher(string).matches();
+    }
 }

onyxia-api/src/test/java/fr/insee/onyxia/api/user/OnyxiaUserProviderTest.java

@@ -0,0 +1,116 @@
+package fr.insee.onyxia.api.user;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.when;
+
+import fr.insee.onyxia.api.services.UserProvider;
+import fr.insee.onyxia.api.services.impl.kubernetes.KubernetesService;
+import fr.insee.onyxia.model.OnyxiaUser;
+import fr.insee.onyxia.model.User;
+import fr.insee.onyxia.model.project.Project;
+import fr.insee.onyxia.model.region.Region;
+import fr.insee.onyxia.model.region.Region.Services;
+import java.util.List;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.boot.test.mock.mockito.MockBean;
+
+@SpringBootTest(classes = {OnyxiaUserProvider.class, UserProvider.class})
+public class OnyxiaUserProviderTest {
+
+    @Autowired OnyxiaUserProvider onyxiaUserProvider;
+
+    @MockBean private KubernetesService kubernetesService;
+
+    @MockBean private UserProvider userProvider;
+
+    Region region;
+    User user;
+
+    @BeforeEach
+    public void setUp() {
+        region = new Region();
+        user = new User();
+        Services servicesConfiguration = new Services();
+        servicesConfiguration.setSingleNamespace(false);
+        region.setServices(servicesConfiguration);
+        when(userProvider.getUser(any())).thenReturn(user);
+    }
+
+    @DisplayName(
+            "Given a multi namespace region with a group pattern set "
+                    + "and group transformation enabled, when we ask for an invalid transformation, "
+                    + "then the user should not have group projects")
+    @Test
+    public void shouldNotHaveProjectAddedWhenConfWrong() {
+        region.setIncludedGroupPattern("(.*)-onyxia");
+        region.setTransformGroupPattern("$2");
+        user.setGroups(List.of("group2-onyxia"));
+        OnyxiaUser simpleUser = onyxiaUserProvider.getUser(region);
+        assertGroupDoesntBelongToUserProjects(simpleUser, "group2-onyxia");
+    }
+
+    @Test
+    public void namespaceNotRespectingRFC1123ShouldBeRejected() {
+        user.setGroups(
+                List.of(
+                        "group1",
+                        "toto_onyxia",
+                        "titi-Onyxia",
+                        "-titi",
+                        "morethan64chargroupmorethan64chargroupmorethan64chargroupmorethan",
+                        "groupwith@"));
+        OnyxiaUser simpleUser = onyxiaUserProvider.getUser(region);
+        assertGroupBelongsToUserProjects(simpleUser, "group1");
+        assertGroupDoesntBelongToUserProjects(simpleUser, "titi-Onyxia");
+        assertGroupBelongsToUserProjects(simpleUser, "-titi");
+        assertGroupDoesntBelongToUserProjects(simpleUser, "group@with");
+        assertGroupDoesntBelongToUserProjects(simpleUser, "toto_onyxia");
+        assertGroupDoesntBelongToUserProjects(
+                simpleUser, "morethan64chargroupmorethan64chargroupmorethan64chargroupmorethan");
+    }
+
+    @DisplayName(
+            "Given a multi namespace region with a group pattern set "
+                    + "and group transformation enabled, when we ask for the user groups, "
+                    + "then the user should get a project for transformed groups")
+    @Test
+    public void shouldHaveAProjectWithTransformedNamespace() {
+        region.setIncludedGroupPattern("(.*)_Onyxia");
+        region.setTransformGroupPattern("$1-k8s");
+        user.setGroups(List.of("group2_Onyxia"));
+        OnyxiaUser simpleUser = onyxiaUserProvider.getUser(region);
+        assertThat("The user should have 2 projects", simpleUser.getProjects().size(), is(2));
+        assertThat(
+                "One of this project is personal, meaning it is not associated to a group",
+                simpleUser.getProjects().stream().anyMatch(p -> p.getGroup() == null));
+        Project group2Project =
+                simpleUser.getProjects().stream()
+                        .filter(p -> p.getGroup() == "group2_Onyxia")
+                        .findFirst()
+                        .get();
+        assertThat(
+                "The group project should have a transformed namespace",
+                group2Project.getNamespace(),
+                is("projet-group2-k8s"));
+    }
+
+    private void assertGroupBelongsToUserProjects(OnyxiaUser user, String testedGroup) {
+        assertThat(
+                testedGroup + " should belong to user projects",
+                user.getProjects().stream()
+                        .anyMatch(p -> p.getGroup() != null && p.getGroup().equals(testedGroup)));
+    }
+
+    private void assertGroupDoesntBelongToUserProjects(OnyxiaUser user, String testedGroup) {
+        assertThat(
+                testedGroup + " should not belong to user projects",
+                user.getProjects().stream()
+                        .noneMatch(p -> p.getGroup() != null && p.getGroup().equals(testedGroup)));
+    }
+}

onyxia-api/src/test/java/fr/insee/onyxia/api/user/OnyxiaUserProviderWithKeycloakTest.java

@@ -60,10 +60,11 @@ public void setUp() {
                     + "then the user should have all groups coming from the access token")
     @Test
     public void shouldReturnGroupsFromAccessToken() {
-        setGroupsInAccessTokenTo(List.of("group1", "group2-onyxia"));
+        setGroupsInAccessTokenTo(List.of("group1", "group2-onyxia", "group3-INVALID"));
         OnyxiaUser simpleUser = onyxiaUserProvider.getUser(region);
         assertGroupBelongsToUser(simpleUser, "group1");
         assertGroupBelongsToUser(simpleUser, "group2-onyxia");
+        assertGroupBelongsToUser(simpleUser, "group3-INVALID");
     }
 
     @DisplayName(
@@ -79,57 +80,6 @@ public void shouldOnlyReturnGroupMatchingWhenRegionRule() {
         assertGroupBelongsToUser(simpleUser, "group2-onyxia");
     }
 
-    @DisplayName(
-            "Given a multi namespace region with a group pattern set "
-                    + "and group transformation enabled, when we ask for the user groups, "
-                    + "then the user should get transformed groups")
-    @Test
-    public void shouldHaveTheTransformedGroups() {
-        setGroupsInAccessTokenTo(List.of("group1", "group2_Onyxia"));
-        region.setIncludedGroupPattern("(.*)_Onyxia");
-        region.setTransformGroupPattern("$1-k8s");
-        OnyxiaUser simpleUser = onyxiaUserProvider.getUser(region);
-        assertGroupDoesntBelongToUser(simpleUser, "group1");
-        assertGroupDoesntBelongToUser(simpleUser, "group2_Onyxia");
-        assertGroupDoesntBelongToUser(simpleUser, "group1-k8s");
-        assertGroupBelongsToUser(simpleUser, "group2-k8s");
-    }
-
-    @DisplayName(
-            "Given a multi namespace region with a group pattern set "
-                    + "and group transformation enabled, when we ask for the an invalid transformation, "
-                    + "then the user should have no groups")
-    @Test
-    public void shouldHaveNoGroupsWhenTransformPatternConfIsWrong() {
-        setGroupsInAccessTokenTo(List.of("group1", "group2_Onyxia"));
-        region.setIncludedGroupPattern("(.*)_Onyxia");
-        region.setTransformGroupPattern("$2");
-        OnyxiaUser simpleUser = onyxiaUserProvider.getUser(region);
-        assertThat(
-                "Invalid transformation should return no groups",
-                simpleUser.getUser().getGroups().isEmpty());
-    }
-
-    @Test
-    public void groupsNotRespectingRFC1223ShouldBeRejected() {
-        setGroupsInAccessTokenTo(
-                List.of(
-                        "group1",
-                        "toto_onyxia",
-                        "titi-Onyxia",
-                        "-titi",
-                        "morethan64chargroupmorethan64chargroupmorethan64chargroupmorethan",
-                        "groupwith@"));
-        OnyxiaUser simpleUser = onyxiaUserProvider.getUser(region);
-        assertGroupBelongsToUser(simpleUser, "group1");
-        assertGroupDoesntBelongToUser(simpleUser, "titi-Onyxia");
-        assertGroupDoesntBelongToUser(simpleUser, "-titi");
-        assertGroupDoesntBelongToUser(simpleUser, "group@with");
-        assertGroupDoesntBelongToUser(simpleUser, "toto_onyxia");
-        assertGroupDoesntBelongToUser(
-                simpleUser, "morethan64chargroupmorethan64chargroupmorethan64chargroupmorethan");
-    }
-
     @DisplayName(
             "Given a multi namespace region with a group pattern to exclude set, "
                     + "when we ask for the user groups, "