diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/authorization/OpenShiftAuthorizationCheckerImpl.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/authorization/OpenShiftAuthorizationCheckerImpl.java index ac8816b4ec..356572982f 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/authorization/OpenShiftAuthorizationCheckerImpl.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/authorization/OpenShiftAuthorizationCheckerImpl.java @@ -13,6 +13,7 @@ import static org.eclipse.che.commons.lang.StringUtils.strToSet; +import com.google.common.net.UrlEscapers; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.openshift.api.model.Group; import java.util.List; @@ -70,7 +71,7 @@ private boolean isAllowedUser(KubernetesClient client, String username) { } for (String groupName : allowGroups) { - Group group = client.resources(Group.class).withName(groupName).get(); + Group group = getGroup(client, groupName); if (group != null) { List users = group.getUsers(); if (users != null && users.contains(username)) { @@ -93,7 +94,7 @@ private boolean isDeniedUser(KubernetesClient client, String username) { } for (String groupName : denyGroups) { - Group group = client.resources(Group.class).withName(groupName).get(); + Group group = getGroup(client, groupName); if (group != null) { List users = group.getUsers(); if (users != null && users.contains(username)) { @@ -104,4 +105,17 @@ private boolean isDeniedUser(KubernetesClient client, String username) { return false; } + + /** + * Looks up an OpenShift Group by name. Group names are path-segment encoded so names containing + * spaces or other reserved URI characters (common for Active Directory groups) can be requested + * without triggering {@link java.net.URISyntaxException}. + */ + private static Group getGroup(KubernetesClient client, String groupName) { + return client.resources(Group.class).withName(encodeGroupName(groupName)).get(); + } + + static String encodeGroupName(String groupName) { + return UrlEscapers.urlPathSegmentEscaper().escape(groupName); + } } diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/authorization/OpenShiftAuthorizationCheckerTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/authorization/OpenShiftAuthorizationCheckerTest.java index 875c7cde6b..5a8406bbe2 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/authorization/OpenShiftAuthorizationCheckerTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/authorization/OpenShiftAuthorizationCheckerTest.java @@ -15,6 +15,8 @@ import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.dsl.MixedOperation; +import io.fabric8.kubernetes.client.dsl.Resource; import io.fabric8.kubernetes.client.server.mock.KubernetesMixedDispatcher; import io.fabric8.kubernetes.client.server.mock.KubernetesMockServer; import io.fabric8.mockwebserver.Context; @@ -143,4 +145,37 @@ public static Object[][] advancedAuthorizationData() { }, }; } + + @Test + public void encodeGroupNamePercentEncodesSpaces() { + Assert.assertEquals( + OpenShiftAuthorizationCheckerImpl.encodeGroupName("LD EIDP PTEC 3P40"), + "LD%20EIDP%20PTEC%203P40"); + Assert.assertEquals( + OpenShiftAuthorizationCheckerImpl.encodeGroupName("groupWithUser1"), "groupWithUser1"); + } + + @Test + @SuppressWarnings("unchecked") + public void shouldAuthorizeUserWhenAllowGroupNameContainsSpaces() throws InfrastructureException { + Group groupWithSpaces = + new Group( + "v1", + "Group", + new ObjectMetaBuilder().withName("LD EIDP PTEC 3P40").build(), + List.of("user1")); + KubernetesClient mockClient = mock(KubernetesClient.class); + MixedOperation resources = mock(MixedOperation.class); + Resource resource = mock(Resource.class); + when(clientFactory.create()).thenReturn(mockClient); + when(mockClient.resources(Group.class)).thenReturn(resources); + when(resources.withName("LD%20EIDP%20PTEC%203P40")).thenReturn(resource); + when(resource.get()).thenReturn(groupWithSpaces); + + OpenShiftAuthorizationCheckerImpl authorizationChecker = + new OpenShiftAuthorizationCheckerImpl("", "LD EIDP PTEC 3P40", "", "", ",", clientFactory); + + Assert.assertTrue(authorizationChecker.isAuthorized(user1)); + Assert.assertFalse(authorizationChecker.isAuthorized(user2)); + } }