From bd9cd9db44c2f981edf0104695e1da3ba1c6c7b8 Mon Sep 17 00:00:00 2001 From: Igor Vinokur Date: Thu, 11 Aug 2022 17:14:30 +0300 Subject: [PATCH] chore: Apply authentication status in the callback url (#338) When processing an authentication callback request set additional error query patameter to the callback url. How it works: 1. User creates a factory form dashboard. 2. Dasboard requests che-server factory API with no error param in the request url. See https://github.com/eclipse-che/che-dashboard/pull/599 3. Che-server create factory API parses the url for the error param. No error query param means `skipAuthentication=false`. Authentication in progress. https://github.com/eclipse-che/che-server/blob/74eb0a333d8a4c062fcc99253d4dc3f926c866d2/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubFactoryParametersResolver.java#L104-L106 4. GitHub shows the authentication page. If User rejects the authentication, authentication status is set to `access_denied`. 5. The error status is added to the redirect url as a query param. https://github.com/eclipse-che/che-server/blob/74eb0a333d8a4c062fcc99253d4dc3f926c866d2/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPI.java#L85-L93 6. Dasboard is loaded from the redirect url. Dashboard requests the che-server to create factory by an api request with the error status query param from the redirect url. https://github.com/eclipse-che/che-dashboard/blob/e2849d9d212c5055885b164ff05422005efc1e7d/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/FetchDevfile/index.tsx#L208-L213 7. Che-server create factory API parses the url for the status param. If `skipAuthentication=true` the authentication flow is skiped and factory creation progress goes further. If `skipAuthentication=false` factory is created in a regular way. https://github.com/eclipse-che/che-server/blob/74eb0a333d8a4c062fcc99253d4dc3f926c866d2/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java#L70-L81 --- ...rnetesComponentIntegrityValidatorTest.java | 16 ++-- ...rnetesComponentToWorkspaceApplierTest.java | 72 ++++++++++-------- ...nshiftComponentToWorkspaceApplierTest.java | 14 +++- .../che/security/oauth/EmbeddedOAuthAPI.java | 16 +++- .../oauth1/OAuthAuthenticationService.java | 4 +- ...rAuthorizingFactoryParametersResolver.java | 5 +- ...horizingFactoryParametersResolverTest.java | 10 ++- .../BitbucketFactoryParametersResolver.java | 3 +- ...itbucketFactoryParametersResolverTest.java | 19 +++-- .../GithubFactoryParametersResolver.java | 9 ++- .../server/github/GithubScmFileResolver.java | 28 +++++-- .../GithubFactoryParametersResolverTest.java | 58 +++++++++++++-- .../GitlabFactoryParametersResolver.java | 5 +- .../GitlabFactoryParametersResolverTest.java | 10 ++- .../DefaultFactoryParameterResolver.java | 5 +- .../scm/AuthorizingFileContentProvider.java | 31 ++++++-- .../server/urlfactory/URLFactoryBuilder.java | 12 ++- .../DefaultFactoryParameterResolverTest.java | 8 +- ...thorizingFactoryParameterResolverTest.java | 73 +++++++++++++++++++ .../urlfactory/URLFactoryBuilderTest.java | 44 +++++++---- .../server/devfile/FileContentProvider.java | 28 ++++++- .../devfile/URLFileContentProvider.java | 17 ++++- .../devfile/convert/CommandConverterTest.java | 10 ++- .../DevfileIntegrityValidatorTest.java | 6 +- 24 files changed, 382 insertions(+), 121 deletions(-) create mode 100644 wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/scm/AuthorizingFactoryParameterResolverTest.java diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/devfile/KubernetesComponentIntegrityValidatorTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/devfile/KubernetesComponentIntegrityValidatorTest.java index fb382838f9..da7e3309c4 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/devfile/KubernetesComponentIntegrityValidatorTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/devfile/KubernetesComponentIntegrityValidatorTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -22,6 +22,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; +import org.eclipse.che.api.workspace.server.devfile.FileContentProvider; import org.eclipse.che.api.workspace.server.devfile.exception.DevfileFormatException; import org.eclipse.che.api.workspace.server.model.impl.devfile.ComponentImpl; import org.eclipse.che.api.workspace.server.model.impl.devfile.EntrypointImpl; @@ -36,6 +37,7 @@ import org.testng.annotations.Test; public class KubernetesComponentIntegrityValidatorTest { @Mock private KubernetesRecipeParser kubernetesRecipeParser; + @Mock private FileContentProvider contentProvider; private KubernetesComponentValidator validator; @@ -69,7 +71,7 @@ public class KubernetesComponentIntegrityValidatorTest { component.setReferenceContent("content"); // when - validator.validateComponent(component, __ -> ""); + validator.validateComponent(component, contentProvider); // then no exception is thrown } @@ -105,7 +107,7 @@ public class KubernetesComponentIntegrityValidatorTest { component.setEntrypoints(Collections.singletonList(entrypoint)); // when - validator.validateComponent(component, __ -> ""); + validator.validateComponent(component, contentProvider); // then no exception is thrown } @@ -151,7 +153,7 @@ public class KubernetesComponentIntegrityValidatorTest { component.setEntrypoints(Collections.singletonList(entrypoint)); // when - validator.validateComponent(component, __ -> ""); + validator.validateComponent(component, contentProvider); // then no exception is thrown } @@ -181,7 +183,7 @@ public class KubernetesComponentIntegrityValidatorTest { component.setReferenceContent("content"); // when - validator.validateComponent(component, __ -> ""); + validator.validateComponent(component, contentProvider); // then exception is thrown } @@ -213,7 +215,7 @@ public class KubernetesComponentIntegrityValidatorTest { component.setEntrypoints(Collections.singletonList(entrypoint)); // when - validator.validateComponent(component, __ -> ""); + validator.validateComponent(component, contentProvider); // then exception is thrown } @@ -263,7 +265,7 @@ public class KubernetesComponentIntegrityValidatorTest { component.setEntrypoints(Collections.singletonList(entrypoint)); // when - validator.validateComponent(component, __ -> ""); + validator.validateComponent(component, contentProvider); // then exception is thrown } diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/devfile/KubernetesComponentToWorkspaceApplierTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/devfile/KubernetesComponentToWorkspaceApplierTest.java index a7b9cd0d58..7bc2a96120 100644 --- a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/devfile/KubernetesComponentToWorkspaceApplierTest.java +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/devfile/KubernetesComponentToWorkspaceApplierTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -32,6 +32,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNull; @@ -56,6 +57,7 @@ import java.util.stream.Stream; import org.eclipse.che.api.core.ValidationException; import org.eclipse.che.api.core.model.workspace.config.MachineConfig; import org.eclipse.che.api.core.model.workspace.config.ServerConfig; +import org.eclipse.che.api.workspace.server.devfile.FileContentProvider; import org.eclipse.che.api.workspace.server.devfile.URLFileContentProvider; import org.eclipse.che.api.workspace.server.devfile.exception.DevfileException; import org.eclipse.che.api.workspace.server.model.impl.CommandImpl; @@ -95,6 +97,7 @@ public class KubernetesComponentToWorkspaceApplierTest { @Mock private KubernetesRecipeParser k8sRecipeParser; @Mock private KubernetesEnvironmentProvisioner k8sEnvProvisioner; @Mock private EnvVars envVars; + @Mock private FileContentProvider contentProvider; @Captor private ArgumentCaptor> objectsCaptor; @@ -130,18 +133,15 @@ public class KubernetesComponentToWorkspaceApplierTest { shouldThrowExceptionWhenRecipeComponentIsPresentAndContentProviderDoesNotSupportFetching() throws Exception { // given + when(contentProvider.fetchContent(anyString())) + .thenThrow(new DevfileException("fetch is not supported")); ComponentImpl component = new ComponentImpl(); component.setType(KUBERNETES_COMPONENT_TYPE); component.setReference(REFERENCE_FILENAME); component.setAlias(COMPONENT_NAME); // when - applier.apply( - workspaceConfig, - component, - e -> { - throw new DevfileException("fetch is not supported"); - }); + applier.apply(workspaceConfig, component, contentProvider); } @Test( @@ -159,9 +159,10 @@ public class KubernetesComponentToWorkspaceApplierTest { component.setType(KUBERNETES_COMPONENT_TYPE); component.setReference(REFERENCE_FILENAME); component.setAlias(COMPONENT_NAME); + when(contentProvider.fetchContent(anyString())).thenReturn("some_non_yaml_content"); // when - applier.apply(workspaceConfig, component, s -> "some_non_yaml_content"); + applier.apply(workspaceConfig, component, contentProvider); } @Test( @@ -170,18 +171,14 @@ public class KubernetesComponentToWorkspaceApplierTest { "Error during recipe content retrieval for component 'foo' with type 'kubernetes': fetch failed") public void shouldThrowExceptionWhenExceptionHappensOnContentProvider() throws Exception { // given + when(contentProvider.fetchContent(anyString())).thenThrow(new IOException("fetch failed")); ComponentImpl component = new ComponentImpl(); component.setType(KUBERNETES_COMPONENT_TYPE); component.setReference(REFERENCE_FILENAME); component.setAlias(COMPONENT_NAME); // when - applier.apply( - workspaceConfig, - component, - e -> { - throw new IOException("fetch failed"); - }); + applier.apply(workspaceConfig, component, contentProvider); } @Test @@ -189,6 +186,7 @@ public class KubernetesComponentToWorkspaceApplierTest { throws Exception { // given String yamlRecipeContent = getResource("devfile/petclinic.yaml"); + when(contentProvider.fetchContent(anyString())).thenReturn(yamlRecipeContent); doReturn(toK8SList(yamlRecipeContent).getItems()).when(k8sRecipeParser).parse(anyString()); ComponentImpl component = new ComponentImpl(); component.setType(KUBERNETES_COMPONENT_TYPE); @@ -196,7 +194,7 @@ public class KubernetesComponentToWorkspaceApplierTest { component.setAlias(COMPONENT_NAME); // when - applier.apply(workspaceConfig, component, s -> yamlRecipeContent); + applier.apply(workspaceConfig, component, contentProvider); // then KubernetesList list = toK8SList(yamlRecipeContent); @@ -230,6 +228,7 @@ public class KubernetesComponentToWorkspaceApplierTest { public void shouldProvisionProjectVolumesIfSpecifiedIntoK8SList() throws Exception { // given String yamlRecipeContent = getResource("devfile/petclinic.yaml"); + when(contentProvider.fetchContent(anyString())).thenReturn(yamlRecipeContent); List k8sList = toK8SList(yamlRecipeContent).getItems(); doReturn(k8sList).when(k8sRecipeParser).parse(anyString()); ComponentImpl component = new ComponentImpl(); @@ -239,7 +238,7 @@ public class KubernetesComponentToWorkspaceApplierTest { component.setMountSources(true); // when - applier.apply(workspaceConfig, component, s -> yamlRecipeContent); + applier.apply(workspaceConfig, component, contentProvider); // then verify(k8sEnvProvisioner).provision(any(), any(), objectsCaptor.capture(), any()); @@ -286,6 +285,7 @@ public class KubernetesComponentToWorkspaceApplierTest { public void shouldProvisionDevfileVolumesIfSpecifiedIntoMachineConfig() throws Exception { // given String yamlRecipeContent = getResource("devfile/petclinic.yaml"); + when(contentProvider.fetchContent(anyString())).thenReturn(yamlRecipeContent); List k8sList = toK8SList(yamlRecipeContent).getItems(); doReturn(k8sList).when(k8sRecipeParser).parse(anyString()); ComponentImpl component = new ComponentImpl(); @@ -296,7 +296,7 @@ public class KubernetesComponentToWorkspaceApplierTest { ArgumentCaptor> mapCaptor = ArgumentCaptor.forClass(Map.class); // when - applier.apply(workspaceConfig, component, s -> yamlRecipeContent); + applier.apply(workspaceConfig, component, contentProvider); // then verify(k8sEnvProvisioner).provision(any(), any(), any(), mapCaptor.capture()); @@ -324,6 +324,7 @@ public class KubernetesComponentToWorkspaceApplierTest { public void shouldThrowExceptionWhenDevfileVolumeNameExists() throws Exception { // given String yamlRecipeContent = getResource("devfile/petclinic.yaml"); + when(contentProvider.fetchContent(anyString())).thenReturn(yamlRecipeContent); List k8sList = toK8SList(yamlRecipeContent).getItems(); doReturn(k8sList).when(k8sRecipeParser).parse(anyString()); ComponentImpl component = new ComponentImpl(); @@ -333,7 +334,7 @@ public class KubernetesComponentToWorkspaceApplierTest { component.setVolumes(Collections.singletonList(new VolumeImpl("foo_volume", "/foo1"))); // when - applier.apply(workspaceConfig, component, s -> yamlRecipeContent); + applier.apply(workspaceConfig, component, contentProvider); } @Test( @@ -343,6 +344,7 @@ public class KubernetesComponentToWorkspaceApplierTest { public void shouldThrowExceptionWhenDevfileVolumePathExists() throws Exception { // given String yamlRecipeContent = getResource("devfile/petclinic.yaml"); + when(contentProvider.fetchContent(anyString())).thenReturn(yamlRecipeContent); List k8sList = toK8SList(yamlRecipeContent).getItems(); doReturn(k8sList).when(k8sRecipeParser).parse(anyString()); ComponentImpl component = new ComponentImpl(); @@ -352,7 +354,7 @@ public class KubernetesComponentToWorkspaceApplierTest { component.setVolumes(Collections.singletonList(new VolumeImpl("foo", "/foo/bar"))); // when - applier.apply(workspaceConfig, component, s -> yamlRecipeContent); + applier.apply(workspaceConfig, component, contentProvider); } @Test @@ -384,9 +386,10 @@ public class KubernetesComponentToWorkspaceApplierTest { component.setAlias(COMPONENT_NAME); List envToApply = singletonList(new EnvImpl("TEST_ENV", "anyValue")); component.setEnv(envToApply); + when(contentProvider.fetchContent(anyString())).thenReturn("content"); // when - applier.apply(workspaceConfig, component, s -> "content"); + applier.apply(workspaceConfig, component, contentProvider); // then envVars.apply(new PodData(pod1), envToApply); @@ -399,6 +402,7 @@ public class KubernetesComponentToWorkspaceApplierTest { ArgumentCaptor> mapCaptor = ArgumentCaptor.forClass(Map.class); // given String yamlRecipeContent = getResource("devfile/petclinic.yaml"); + when(contentProvider.fetchContent(anyString())).thenReturn(yamlRecipeContent); List k8sList = toK8SList(yamlRecipeContent).getItems(); doReturn(k8sList).when(k8sRecipeParser).parse(anyString()); ComponentImpl component = new ComponentImpl(); @@ -408,7 +412,7 @@ public class KubernetesComponentToWorkspaceApplierTest { component.setMountSources(true); // when - applier.apply(workspaceConfig, component, s -> yamlRecipeContent); + applier.apply(workspaceConfig, component, contentProvider); // then verify(k8sEnvProvisioner).provision(any(), any(), any(), mapCaptor.capture()); @@ -427,6 +431,7 @@ public class KubernetesComponentToWorkspaceApplierTest { public void shouldFilterRecipeWithGivenSelectors() throws Exception { // given String yamlRecipeContent = getResource("devfile/petclinic.yaml"); + when(contentProvider.fetchContent(anyString())).thenReturn(yamlRecipeContent); final Map selector = singletonMap("app.kubernetes.io/component", "webapp"); ComponentImpl component = new ComponentImpl(); @@ -437,7 +442,7 @@ public class KubernetesComponentToWorkspaceApplierTest { doReturn(toK8SList(yamlRecipeContent).getItems()).when(k8sRecipeParser).parse(anyString()); // when - applier.apply(workspaceConfig, component, s -> yamlRecipeContent); + applier.apply(workspaceConfig, component, contentProvider); // then verify(k8sEnvProvisioner) @@ -468,7 +473,7 @@ public class KubernetesComponentToWorkspaceApplierTest { workspaceConfig.getCommands().add(command); // when - applier.apply(workspaceConfig, component, s -> yamlRecipeContent); + applier.apply(workspaceConfig, component, contentProvider); // then CommandImpl actualCommand = workspaceConfig.getCommands().get(0); @@ -481,6 +486,7 @@ public class KubernetesComponentToWorkspaceApplierTest { throws Exception { // given String yamlRecipeContent = getResource("devfile/petclinic.yaml"); + when(contentProvider.fetchContent(anyString())).thenReturn(yamlRecipeContent); doReturn(toK8SList(yamlRecipeContent).getItems()).when(k8sRecipeParser).parse(anyString()); ComponentImpl component = new ComponentImpl(); @@ -493,7 +499,7 @@ public class KubernetesComponentToWorkspaceApplierTest { workspaceConfig.getCommands().add(command); // when - applier.apply(workspaceConfig, component, s -> yamlRecipeContent); + applier.apply(workspaceConfig, component, contentProvider); // then CommandImpl actualCommand = workspaceConfig.getCommands().get(0); @@ -504,6 +510,7 @@ public class KubernetesComponentToWorkspaceApplierTest { public void shouldChangeEntrypointsOnMatchingContainers() throws Exception { // given String yamlRecipeContent = getResource("devfile/petclinic.yaml"); + when(contentProvider.fetchContent(anyString())).thenReturn(yamlRecipeContent); doReturn(toK8SList(yamlRecipeContent).getItems()).when(k8sRecipeParser).parse(anyString()); List command = asList("teh", "command"); @@ -518,7 +525,7 @@ public class KubernetesComponentToWorkspaceApplierTest { component.setEntrypoints(singletonList(entrypoint)); // when - applier.apply(workspaceConfig, component, s -> yamlRecipeContent); + applier.apply(workspaceConfig, component, contentProvider); // then verify(k8sEnvProvisioner).provision(any(), any(), objectsCaptor.capture(), any()); @@ -558,6 +565,7 @@ public class KubernetesComponentToWorkspaceApplierTest { MULTI_HOST_STRATEGY, k8sBasedComponents); String yamlRecipeContent = getResource("devfile/petclinic.yaml"); + when(contentProvider.fetchContent(anyString())).thenReturn(yamlRecipeContent); doReturn(toK8SList(yamlRecipeContent).getItems()).when(k8sRecipeParser).parse(anyString()); ComponentImpl component = new ComponentImpl(); component.setType(KUBERNETES_COMPONENT_TYPE); @@ -565,7 +573,7 @@ public class KubernetesComponentToWorkspaceApplierTest { component.setAlias(COMPONENT_NAME); // when - applier.apply(workspaceConfig, component, s -> yamlRecipeContent); + applier.apply(workspaceConfig, component, contentProvider); // then verify(k8sEnvProvisioner).provision(any(), any(), objectsCaptor.capture(), any()); @@ -596,6 +604,7 @@ public class KubernetesComponentToWorkspaceApplierTest { Integer endpointPort = 8081; String yamlRecipeContent = getResource("devfile/petclinicPods.yaml"); + when(contentProvider.fetchContent(anyString())).thenReturn(yamlRecipeContent); doReturn(toK8SList(yamlRecipeContent).getItems()).when(k8sRecipeParser).parse(anyString()); ComponentImpl component = new ComponentImpl(); @@ -605,7 +614,7 @@ public class KubernetesComponentToWorkspaceApplierTest { component.setEndpoints(singletonList(new EndpointImpl(endpointName, endpointPort, emptyMap()))); // when - applier.apply(workspaceConfig, component, s -> yamlRecipeContent); + applier.apply(workspaceConfig, component, contentProvider); // then @SuppressWarnings("unchecked") @@ -638,6 +647,7 @@ public class KubernetesComponentToWorkspaceApplierTest { String endpointSecure = "false"; String yamlRecipeContent = getResource("devfile/petclinicPods.yaml"); + when(contentProvider.fetchContent(anyString())).thenReturn(yamlRecipeContent); doReturn(toK8SList(yamlRecipeContent).getItems()).when(k8sRecipeParser).parse(anyString()); ComponentImpl component = new ComponentImpl(); @@ -660,7 +670,7 @@ public class KubernetesComponentToWorkspaceApplierTest { endpointSecure)))); // when - applier.apply(workspaceConfig, component, s -> yamlRecipeContent); + applier.apply(workspaceConfig, component, contentProvider); // then @SuppressWarnings("unchecked") @@ -709,6 +719,7 @@ public class KubernetesComponentToWorkspaceApplierTest { doReturn(toK8SList(yamlRecipeContent).getItems()).when(k8sRecipeParser).parse(anyString()); // given + when(contentProvider.fetchContent(anyString())).thenReturn(yamlRecipeContent); ComponentImpl component = new ComponentImpl(); component.setType(KUBERNETES_COMPONENT_TYPE); component.setReference(REFERENCE_FILENAME); @@ -718,7 +729,7 @@ public class KubernetesComponentToWorkspaceApplierTest { new EndpointImpl("e1", 1111, emptyMap()), new EndpointImpl("e2", 2222, emptyMap()))); // when - applier.apply(workspaceConfig, component, s -> yamlRecipeContent); + applier.apply(workspaceConfig, component, contentProvider); // then @SuppressWarnings("unchecked") @@ -758,6 +769,7 @@ public class KubernetesComponentToWorkspaceApplierTest { k8sBasedComponents); String yamlRecipeContent = getResource("devfile/petclinic.yaml"); + when(contentProvider.fetchContent(anyString())).thenReturn(yamlRecipeContent); doReturn(toK8SList(yamlRecipeContent).getItems()).when(k8sRecipeParser).parse(anyString()); // given @@ -770,7 +782,7 @@ public class KubernetesComponentToWorkspaceApplierTest { new EndpointImpl("e1", 1111, emptyMap()), new EndpointImpl("e2", 2222, emptyMap()))); // when - applier.apply(workspaceConfig, component, s -> yamlRecipeContent); + applier.apply(workspaceConfig, component, contentProvider); // then @SuppressWarnings("unchecked") diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/devfile/OpenshiftComponentToWorkspaceApplierTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/devfile/OpenshiftComponentToWorkspaceApplierTest.java index b5a63c99a2..425c37eb8b 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/devfile/OpenshiftComponentToWorkspaceApplierTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/devfile/OpenshiftComponentToWorkspaceApplierTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -22,6 +22,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertTrue; @@ -34,6 +35,7 @@ import java.util.Map; import java.util.Set; import org.eclipse.che.api.core.ValidationException; import org.eclipse.che.api.core.model.workspace.config.ServerConfig; +import org.eclipse.che.api.workspace.server.devfile.FileContentProvider; import org.eclipse.che.api.workspace.server.devfile.exception.DevfileException; import org.eclipse.che.api.workspace.server.model.impl.MachineConfigImpl; import org.eclipse.che.api.workspace.server.model.impl.WorkspaceConfigImpl; @@ -63,6 +65,7 @@ public class OpenshiftComponentToWorkspaceApplierTest { private KubernetesComponentToWorkspaceApplier applier; @Mock private KubernetesEnvironmentProvisioner k8sEnvProvisioner; @Mock private KubernetesRecipeParser k8sRecipeParser; + @Mock private FileContentProvider contentProvider; @Mock private EnvVars envVars; @BeforeMethod @@ -94,9 +97,10 @@ public class OpenshiftComponentToWorkspaceApplierTest { component.setType(KUBERNETES_COMPONENT_TYPE); component.setReference(REFERENCE_FILENAME); component.setAlias(COMPONENT_NAME); + when(contentProvider.fetchContent(anyString())).thenReturn("content"); // when - applier.apply(workspaceConfig, component, s -> "content"); + applier.apply(workspaceConfig, component, contentProvider); // then verify(k8sEnvProvisioner) @@ -122,6 +126,7 @@ public class OpenshiftComponentToWorkspaceApplierTest { openshiftBasedComponents); String yamlRecipeContent = getResource("devfile/petclinic.yaml"); + when(contentProvider.fetchContent(anyString())).thenReturn(yamlRecipeContent); doReturn(toK8SList(yamlRecipeContent).getItems()).when(k8sRecipeParser).parse(anyString()); // given @@ -134,7 +139,7 @@ public class OpenshiftComponentToWorkspaceApplierTest { new EndpointImpl("e1", 1111, emptyMap()), new EndpointImpl("e2", 2222, emptyMap()))); // when - applier.apply(workspaceConfig, component, s -> yamlRecipeContent); + applier.apply(workspaceConfig, component, contentProvider); // then @SuppressWarnings("unchecked") @@ -176,6 +181,7 @@ public class OpenshiftComponentToWorkspaceApplierTest { openshiftBasedComponents); String yamlRecipeContent = getResource("devfile/petclinic.yaml"); + when(contentProvider.fetchContent(anyString())).thenReturn(yamlRecipeContent); doReturn(toK8SList(yamlRecipeContent).getItems()).when(k8sRecipeParser).parse(anyString()); // given @@ -188,7 +194,7 @@ public class OpenshiftComponentToWorkspaceApplierTest { new EndpointImpl("e1", 1111, emptyMap()), new EndpointImpl("e2", 2222, emptyMap()))); // when - applier.apply(workspaceConfig, component, s -> yamlRecipeContent); + applier.apply(workspaceConfig, component, contentProvider); // then @SuppressWarnings("unchecked") diff --git a/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPI.java b/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPI.java index db685316f8..1a306a7900 100644 --- a/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPI.java +++ b/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPI.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -15,6 +15,7 @@ import static java.util.Collections.emptyList; import static org.eclipse.che.commons.lang.UrlUtils.*; import static org.eclipse.che.commons.lang.UrlUtils.getParameter; import static org.eclipse.che.dto.server.DtoFactory.newDto; +import static org.eclipse.che.security.oauth1.OAuthAuthenticationService.ERROR_QUERY_NAME; import jakarta.servlet.http.HttpServletRequest; import jakarta.ws.rs.HttpMethod; @@ -70,8 +71,7 @@ public class EmbeddedOAuthAPI implements OAuthAPI { } @Override - public Response callback(UriInfo uriInfo, List errorValues) - throws NotFoundException, OAuthAuthenticationException { + public Response callback(UriInfo uriInfo, List errorValues) throws NotFoundException { URL requestUrl = getRequestUrl(uriInfo); Map> params = getQueryParametersFromState(getState(requestUrl)); if (errorValues != null && errorValues.contains("access_denied")) { @@ -82,7 +82,15 @@ public class EmbeddedOAuthAPI implements OAuthAPI { final String providerName = getParameter(params, "oauth_provider"); OAuthAuthenticator oauth = getAuthenticator(providerName); final List scopes = params.get("scope"); - oauth.callback(requestUrl, scopes == null ? Collections.emptyList() : scopes); + try { + oauth.callback(requestUrl, scopes == null ? emptyList() : scopes); + } catch (OAuthAuthenticationException e) { + return Response.temporaryRedirect( + URI.create( + getParameter(params, "redirect_after_login") + + String.format("&%s=access_denied", ERROR_QUERY_NAME))) + .build(); + } final String redirectAfterLogin = getParameter(params, "redirect_after_login"); return Response.temporaryRedirect(URI.create(redirectAfterLogin)).build(); } diff --git a/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth1/OAuthAuthenticationService.java b/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth1/OAuthAuthenticationService.java index 08a8e7e503..01cd269368 100644 --- a/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth1/OAuthAuthenticationService.java +++ b/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth1/OAuthAuthenticationService.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -44,7 +44,7 @@ public class OAuthAuthenticationService extends Service { private static final Logger LOG = LoggerFactory.getLogger(OAuthAuthenticationService.class); private static final String UNSUPPORTED_OAUTH_PROVIDER_ERROR = "Unsupported OAuth provider: %s"; - private static final String ERROR_QUERY_NAME = "error_code"; + public static final String ERROR_QUERY_NAME = "error_code"; @Inject protected OAuthAuthenticatorProvider providers; @GET diff --git a/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFactoryParametersResolver.java b/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFactoryParametersResolver.java index 0363a89290..b064c9c125 100644 --- a/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFactoryParametersResolver.java +++ b/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFactoryParametersResolver.java @@ -97,7 +97,10 @@ public class BitbucketServerAuthorizingFactoryParametersResolver // create factory from the following location if location exists, else create default factory return urlFactoryBuilder .createFactoryFromDevfile( - bitbucketServerUrl, fileContentProvider, extractOverrideParams(factoryParameters)) + bitbucketServerUrl, + fileContentProvider, + extractOverrideParams(factoryParameters), + false) .orElseGet(() -> newDto(FactoryDto.class).withV(CURRENT_VERSION).withSource("repo")) .acceptVisitor(new BitbucketFactoryVisitor(bitbucketServerUrl)); } diff --git a/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFactoryParametersResolverTest.java b/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFactoryParametersResolverTest.java index c0d36f5f89..4962987dfb 100644 --- a/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFactoryParametersResolverTest.java +++ b/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFactoryParametersResolverTest.java @@ -17,6 +17,7 @@ import static org.eclipse.che.api.factory.shared.Constants.URL_PARAMETER_NAME; import static org.eclipse.che.api.workspace.server.devfile.Constants.CURRENT_API_VERSION; import static org.eclipse.che.dto.server.DtoFactory.newDto; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; @@ -110,7 +111,8 @@ public class BitbucketServerAuthorizingFactoryParametersResolverTest { when(urlFactoryBuilder.buildDefaultDevfile(any())).thenReturn(computedFactory.getDevfile()); - when(urlFactoryBuilder.createFactoryFromDevfile(any(RemoteFactoryUrl.class), any(), anyMap())) + when(urlFactoryBuilder.createFactoryFromDevfile( + any(RemoteFactoryUrl.class), any(), anyMap(), anyBoolean())) .thenReturn(Optional.empty()); Map params = ImmutableMap.of(URL_PARAMETER_NAME, bitbucketUrl); // when @@ -131,7 +133,8 @@ public class BitbucketServerAuthorizingFactoryParametersResolverTest { FactoryDto computedFactory = generateDevfileFactory(); - when(urlFactoryBuilder.createFactoryFromDevfile(any(RemoteFactoryUrl.class), any(), anyMap())) + when(urlFactoryBuilder.createFactoryFromDevfile( + any(RemoteFactoryUrl.class), any(), anyMap(), anyBoolean())) .thenReturn(Optional.of(computedFactory)); Map params = ImmutableMap.of(URL_PARAMETER_NAME, bitbucketUrl); @@ -152,7 +155,8 @@ public class BitbucketServerAuthorizingFactoryParametersResolverTest { FactoryDevfileV2Dto computedFactory = generateDevfileV2Factory(); - when(urlFactoryBuilder.createFactoryFromDevfile(any(RemoteFactoryUrl.class), any(), anyMap())) + when(urlFactoryBuilder.createFactoryFromDevfile( + any(RemoteFactoryUrl.class), any(), anyMap(), anyBoolean())) .thenReturn(Optional.of(computedFactory)); Map params = ImmutableMap.of(URL_PARAMETER_NAME, bitbucketUrl); diff --git a/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketFactoryParametersResolver.java b/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketFactoryParametersResolver.java index 0cefb83697..9d52ebbe89 100644 --- a/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketFactoryParametersResolver.java +++ b/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketFactoryParametersResolver.java @@ -104,7 +104,8 @@ public class BitbucketFactoryParametersResolver extends DefaultFactoryParameterR bitbucketUrl, new BitbucketAuthorizingFileContentProvider( bitbucketUrl, urlFetcher, gitCredentialManager, personalAccessTokenManager), - extractOverrideParams(factoryParameters)) + extractOverrideParams(factoryParameters), + false) .orElseGet(() -> newDto(FactoryDto.class).withV(CURRENT_VERSION).withSource("repo")) .acceptVisitor(new BitbucketFactoryVisitor(bitbucketUrl)); } diff --git a/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketFactoryParametersResolverTest.java b/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketFactoryParametersResolverTest.java index 7bff586334..e5aeb180de 100644 --- a/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketFactoryParametersResolverTest.java +++ b/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketFactoryParametersResolverTest.java @@ -17,6 +17,7 @@ import static org.eclipse.che.api.factory.shared.Constants.URL_PARAMETER_NAME; import static org.eclipse.che.api.workspace.server.devfile.Constants.CURRENT_API_VERSION; import static org.eclipse.che.dto.server.DtoFactory.newDto; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.never; @@ -145,7 +146,8 @@ public class BitbucketFactoryParametersResolverTest { when(urlFactoryBuilder.buildDefaultDevfile(any())).thenReturn(computedFactory.getDevfile()); - when(urlFactoryBuilder.createFactoryFromDevfile(any(RemoteFactoryUrl.class), any(), anyMap())) + when(urlFactoryBuilder.createFactoryFromDevfile( + any(RemoteFactoryUrl.class), any(), anyMap(), anyBoolean())) .thenReturn(Optional.empty()); Map params = ImmutableMap.of(URL_PARAMETER_NAME, bitbucketUrl); // when @@ -168,7 +170,8 @@ public class BitbucketFactoryParametersResolverTest { FactoryDto computedFactory = generateDevfileFactory(); - when(urlFactoryBuilder.createFactoryFromDevfile(any(RemoteFactoryUrl.class), any(), anyMap())) + when(urlFactoryBuilder.createFactoryFromDevfile( + any(RemoteFactoryUrl.class), any(), anyMap(), anyBoolean())) .thenReturn(Optional.of(computedFactory)); Map params = ImmutableMap.of(URL_PARAMETER_NAME, bitbucketUrl); @@ -180,7 +183,8 @@ public class BitbucketFactoryParametersResolverTest { // check we called the builder with the following devfile file verify(urlFactoryBuilder) - .createFactoryFromDevfile(factoryUrlArgumentCaptor.capture(), any(), anyMap()); + .createFactoryFromDevfile( + factoryUrlArgumentCaptor.capture(), any(), anyMap(), anyBoolean()); verify(urlFactoryBuilder, never()).buildDefaultDevfile(eq("che")); assertEquals( factoryUrlArgumentCaptor.getValue().devfileFileLocations().iterator().next().location(), @@ -194,7 +198,8 @@ public class BitbucketFactoryParametersResolverTest { FactoryDto computedFactory = generateDevfileFactory(); - when(urlFactoryBuilder.createFactoryFromDevfile(any(RemoteFactoryUrl.class), any(), anyMap())) + when(urlFactoryBuilder.createFactoryFromDevfile( + any(RemoteFactoryUrl.class), any(), anyMap(), anyBoolean())) .thenReturn(Optional.of(computedFactory)); Map params = ImmutableMap.of(URL_PARAMETER_NAME, bitbucketUrl); @@ -221,7 +226,8 @@ public class BitbucketFactoryParametersResolverTest { .withSource( newDto(SourceDto.class).withLocation("https://bitbucket.org/eclipse/che.git"))); - when(urlFactoryBuilder.createFactoryFromDevfile(any(RemoteFactoryUrl.class), any(), anyMap())) + when(urlFactoryBuilder.createFactoryFromDevfile( + any(RemoteFactoryUrl.class), any(), anyMap(), anyBoolean())) .thenReturn(Optional.of(computedFactory)); Map params = ImmutableMap.of(URL_PARAMETER_NAME, bitbucketUrl); @@ -240,7 +246,8 @@ public class BitbucketFactoryParametersResolverTest { FactoryDevfileV2Dto computedFactory = generateDevfileV2Factory(); - when(urlFactoryBuilder.createFactoryFromDevfile(any(RemoteFactoryUrl.class), any(), anyMap())) + when(urlFactoryBuilder.createFactoryFromDevfile( + any(RemoteFactoryUrl.class), any(), anyMap(), anyBoolean())) .thenReturn(Optional.of(computedFactory)); Map params = ImmutableMap.of(URL_PARAMETER_NAME, bitbucketUrl); diff --git a/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubFactoryParametersResolver.java b/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubFactoryParametersResolver.java index cfc1daa867..ed27edaf9f 100644 --- a/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubFactoryParametersResolver.java +++ b/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubFactoryParametersResolver.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -14,6 +14,7 @@ package org.eclipse.che.api.factory.server.github; import static org.eclipse.che.api.factory.shared.Constants.CURRENT_VERSION; import static org.eclipse.che.api.factory.shared.Constants.URL_PARAMETER_NAME; import static org.eclipse.che.dto.server.DtoFactory.newDto; +import static org.eclipse.che.security.oauth1.OAuthAuthenticationService.ERROR_QUERY_NAME; import jakarta.validation.constraints.NotNull; import java.util.Map; @@ -100,6 +101,9 @@ public class GithubFactoryParametersResolver extends DefaultFactoryParameterReso throws ApiException { // no need to check null value of url parameter as accept() method has performed the check final GithubUrl githubUrl = githubUrlParser.parse(factoryParameters.get(URL_PARAMETER_NAME)); + boolean skipAuthentication = + factoryParameters.get(ERROR_QUERY_NAME) != null + && factoryParameters.get(ERROR_QUERY_NAME).equals("access_denied"); // create factory from the following location if location exists, else create default factory return urlFactoryBuilder @@ -107,7 +111,8 @@ public class GithubFactoryParametersResolver extends DefaultFactoryParameterReso githubUrl, new GithubAuthorizingFileContentProvider( githubUrl, urlFetcher, gitCredentialManager, personalAccessTokenManager), - extractOverrideParams(factoryParameters)) + extractOverrideParams(factoryParameters), + skipAuthentication) .orElseGet(() -> newDto(FactoryDto.class).withV(CURRENT_VERSION).withSource("repo")) .acceptVisitor(new GithubFactoryVisitor(githubUrl)); } diff --git a/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubScmFileResolver.java b/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubScmFileResolver.java index 2c2127c1ad..5a6bdf2364 100644 --- a/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubScmFileResolver.java +++ b/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubScmFileResolver.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -55,13 +55,29 @@ public class GithubScmFileResolver implements ScmFileResolver { throws ApiException { final GithubUrl githubUrl = githubUrlParser.parse(repository); try { - return new GithubAuthorizingFileContentProvider( - githubUrl, urlFetcher, gitCredentialManager, personalAccessTokenManager) - .fetchContent(githubUrl.rawFileLocation(filePath)); + return fetchContent(githubUrl, filePath, false); + } catch (DevfileException exception) { + // This catch might mean that the authentication was rejected by user, try to repeat the fetch + // without authentication flow. + try { + return fetchContent(githubUrl, filePath, true); + } catch (DevfileException devfileException) { + throw toApiException(devfileException); + } + } + } + + private String fetchContent(GithubUrl githubUrl, String filePath, boolean skipAuthentication) + throws DevfileException, NotFoundException { + try { + GithubAuthorizingFileContentProvider contentProvider = + new GithubAuthorizingFileContentProvider( + githubUrl, urlFetcher, gitCredentialManager, personalAccessTokenManager); + return skipAuthentication + ? contentProvider.fetchContentWithoutAuthentication(filePath) + : contentProvider.fetchContent(filePath); } catch (IOException e) { throw new NotFoundException(e.getMessage()); - } catch (DevfileException devfileException) { - throw toApiException(devfileException); } } } diff --git a/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubFactoryParametersResolverTest.java b/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubFactoryParametersResolverTest.java index e7328559ca..8b22271456 100644 --- a/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubFactoryParametersResolverTest.java +++ b/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubFactoryParametersResolverTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -16,7 +16,9 @@ import static org.eclipse.che.api.factory.shared.Constants.CURRENT_VERSION; import static org.eclipse.che.api.factory.shared.Constants.URL_PARAMETER_NAME; import static org.eclipse.che.api.workspace.server.devfile.Constants.CURRENT_API_VERSION; import static org.eclipse.che.dto.server.DtoFactory.newDto; +import static org.eclipse.che.security.oauth1.OAuthAuthenticationService.ERROR_QUERY_NAME; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.never; @@ -88,7 +90,8 @@ public class GithubFactoryParametersResolverTest { /** * Capturing the location parameter when calling {@link - * URLFactoryBuilder#createFactoryFromDevfile(RemoteFactoryUrl, FileContentProvider, Map)} + * URLFactoryBuilder#createFactoryFromDevfile(RemoteFactoryUrl, FileContentProvider, Map, + * Boolean)} */ @Captor private ArgumentCaptor factoryUrlArgumentCaptor; @@ -148,7 +151,8 @@ public class GithubFactoryParametersResolverTest { when(urlFactoryBuilder.buildDefaultDevfile(any())).thenReturn(computedFactory.getDevfile()); - when(urlFactoryBuilder.createFactoryFromDevfile(any(RemoteFactoryUrl.class), any(), anyMap())) + when(urlFactoryBuilder.createFactoryFromDevfile( + any(RemoteFactoryUrl.class), any(), anyMap(), anyBoolean())) .thenReturn(Optional.empty()); Map params = ImmutableMap.of(URL_PARAMETER_NAME, githubUrl); // when @@ -161,6 +165,39 @@ public class GithubFactoryParametersResolverTest { assertEquals(source.getBranch(), null); } + @Test + public void shouldSkipAuthenticationWhenAccessDenied() throws Exception { + // given + when(urlFactoryBuilder.buildDefaultDevfile(any())) + .thenReturn(generateDevfileFactory().getDevfile()); + // when + Map params = + ImmutableMap.of( + URL_PARAMETER_NAME, + "https://github.com/eclipse/che", + ERROR_QUERY_NAME, + "access_denied"); + githubFactoryParametersResolver.createFactory(params); + // then + verify(urlFactoryBuilder) + .createFactoryFromDevfile( + any(RemoteFactoryUrl.class), any(FileContentProvider.class), anyMap(), eq(true)); + } + + @Test + public void shouldNotSkipAuthenticationWhenNoErrorParameterPassed() throws Exception { + // given + when(urlFactoryBuilder.buildDefaultDevfile(any())) + .thenReturn(generateDevfileFactory().getDevfile()); + // when + githubFactoryParametersResolver.createFactory( + ImmutableMap.of(URL_PARAMETER_NAME, "https://github.com/eclipse/che")); + // then + verify(urlFactoryBuilder) + .createFactoryFromDevfile( + any(RemoteFactoryUrl.class), any(FileContentProvider.class), anyMap(), eq(false)); + } + @Test public void shouldReturnFactoryFromRepositoryWithDevfile() throws Exception { @@ -171,7 +208,8 @@ public class GithubFactoryParametersResolverTest { FactoryDto computedFactory = generateDevfileFactory(); - when(urlFactoryBuilder.createFactoryFromDevfile(any(RemoteFactoryUrl.class), any(), anyMap())) + when(urlFactoryBuilder.createFactoryFromDevfile( + any(RemoteFactoryUrl.class), any(), anyMap(), anyBoolean())) .thenReturn(Optional.of(computedFactory)); Map params = ImmutableMap.of(URL_PARAMETER_NAME, githubUrl); @@ -183,7 +221,8 @@ public class GithubFactoryParametersResolverTest { // check we called the builder with the following devfile file verify(urlFactoryBuilder) - .createFactoryFromDevfile(factoryUrlArgumentCaptor.capture(), any(), anyMap()); + .createFactoryFromDevfile( + factoryUrlArgumentCaptor.capture(), any(), anyMap(), anyBoolean()); verify(urlFactoryBuilder, never()).buildDefaultDevfile(eq("che")); assertEquals( factoryUrlArgumentCaptor.getValue().devfileFileLocations().iterator().next().location(), @@ -197,7 +236,8 @@ public class GithubFactoryParametersResolverTest { FactoryDto computedFactory = generateDevfileFactory(); - when(urlFactoryBuilder.createFactoryFromDevfile(any(RemoteFactoryUrl.class), any(), anyMap())) + when(urlFactoryBuilder.createFactoryFromDevfile( + any(RemoteFactoryUrl.class), any(), anyMap(), anyBoolean())) .thenReturn(Optional.of(computedFactory)); Map params = ImmutableMap.of(URL_PARAMETER_NAME, githubUrl); @@ -224,7 +264,8 @@ public class GithubFactoryParametersResolverTest { .withSource( newDto(SourceDto.class).withLocation("https://github.com/eclipse/che.git"))); - when(urlFactoryBuilder.createFactoryFromDevfile(any(RemoteFactoryUrl.class), any(), anyMap())) + when(urlFactoryBuilder.createFactoryFromDevfile( + any(RemoteFactoryUrl.class), any(), anyMap(), anyBoolean())) .thenReturn(Optional.of(computedFactory)); Map params = ImmutableMap.of(URL_PARAMETER_NAME, githubUrl); @@ -243,7 +284,8 @@ public class GithubFactoryParametersResolverTest { FactoryDevfileV2Dto computedFactory = generateDevfileV2Factory(); - when(urlFactoryBuilder.createFactoryFromDevfile(any(RemoteFactoryUrl.class), any(), anyMap())) + when(urlFactoryBuilder.createFactoryFromDevfile( + any(RemoteFactoryUrl.class), any(), anyMap(), anyBoolean())) .thenReturn(Optional.of(computedFactory)); Map params = ImmutableMap.of(URL_PARAMETER_NAME, githubUrl); diff --git a/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabFactoryParametersResolver.java b/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabFactoryParametersResolver.java index 729614c5c1..ebc74fdf82 100644 --- a/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabFactoryParametersResolver.java +++ b/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabFactoryParametersResolver.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -91,7 +91,8 @@ public class GitlabFactoryParametersResolver extends DefaultFactoryParameterReso gitlabUrl, new GitlabAuthorizingFileContentProvider( gitlabUrl, urlFetcher, gitCredentialManager, personalAccessTokenManager), - extractOverrideParams(factoryParameters)) + extractOverrideParams(factoryParameters), + false) .orElseGet(() -> newDto(FactoryDto.class).withV(CURRENT_VERSION).withSource("repo")) .acceptVisitor(new GitlabFactoryVisitor(gitlabUrl)); } diff --git a/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabFactoryParametersResolverTest.java b/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabFactoryParametersResolverTest.java index 5498524255..7c7ac72303 100644 --- a/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabFactoryParametersResolverTest.java +++ b/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabFactoryParametersResolverTest.java @@ -17,6 +17,7 @@ import static org.eclipse.che.api.factory.shared.Constants.URL_PARAMETER_NAME; import static org.eclipse.che.api.workspace.server.devfile.Constants.CURRENT_API_VERSION; import static org.eclipse.che.dto.server.DtoFactory.newDto; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; @@ -114,7 +115,8 @@ public class GitlabFactoryParametersResolverTest { when(urlFactoryBuilder.buildDefaultDevfile(any())).thenReturn(computedFactory.getDevfile()); - when(urlFactoryBuilder.createFactoryFromDevfile(any(RemoteFactoryUrl.class), any(), anyMap())) + when(urlFactoryBuilder.createFactoryFromDevfile( + any(RemoteFactoryUrl.class), any(), anyMap(), anyBoolean())) .thenReturn(Optional.empty()); Map params = ImmutableMap.of(URL_PARAMETER_NAME, gitlabUrl); // when @@ -134,7 +136,8 @@ public class GitlabFactoryParametersResolverTest { FactoryDto computedFactory = generateDevfileFactory(); - when(urlFactoryBuilder.createFactoryFromDevfile(any(RemoteFactoryUrl.class), any(), anyMap())) + when(urlFactoryBuilder.createFactoryFromDevfile( + any(RemoteFactoryUrl.class), any(), anyMap(), anyBoolean())) .thenReturn(Optional.of(computedFactory)); Map params = ImmutableMap.of(URL_PARAMETER_NAME, gitlabUrl); @@ -154,7 +157,8 @@ public class GitlabFactoryParametersResolverTest { FactoryDevfileV2Dto computedFactory = generateDevfileV2Factory(); - when(urlFactoryBuilder.createFactoryFromDevfile(any(RemoteFactoryUrl.class), any(), anyMap())) + when(urlFactoryBuilder.createFactoryFromDevfile( + any(RemoteFactoryUrl.class), any(), anyMap(), anyBoolean())) .thenReturn(Optional.of(computedFactory)); Map params = ImmutableMap.of(URL_PARAMETER_NAME, gitlabUrl); diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/DefaultFactoryParameterResolver.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/DefaultFactoryParameterResolver.java index bda8073e35..f765820b61 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/DefaultFactoryParameterResolver.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/DefaultFactoryParameterResolver.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -89,7 +89,8 @@ public class DefaultFactoryParameterResolver implements FactoryParametersResolve .createFactoryFromDevfile( new DefaultFactoryUrl().withDevfileFileLocation(devfileLocation), new URLFileContentProvider(devfileURI, urlFetcher), - extractOverrideParams(factoryParameters)) + extractOverrideParams(factoryParameters), + false) .orElse(null); } diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java index f02c327e58..f6177472a3 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java @@ -53,6 +53,17 @@ public class AuthorizingFileContentProvider @Override public String fetchContent(String fileURL) throws IOException, DevfileException { + return fetchContent(fileURL, false); + } + + @Override + public String fetchContentWithoutAuthentication(String fileURL) + throws IOException, DevfileException { + return fetchContent(fileURL, true); + } + + private String fetchContent(String fileURL, boolean skipAuthentication) + throws IOException, DevfileException { final String requestURL = formatUrl(fileURL); try { Optional token = @@ -65,15 +76,19 @@ public class AuthorizingFileContentProvider gitCredentialManager.createOrReplace(personalAccessToken); return content; } else { - // try to authenticate for the given URL try { - PersonalAccessToken personalAccessToken = - personalAccessTokenManager.fetchAndSave( - EnvironmentContext.getCurrent().getSubject(), remoteFactoryUrl.getHostName()); - String content = - urlFetcher.fetch(requestURL, formatAuthorization(personalAccessToken.getToken())); - gitCredentialManager.createOrReplace(personalAccessToken); - return content; + if (skipAuthentication) { + return urlFetcher.fetch(requestURL); + } else { + // try to authenticate for the given URL + PersonalAccessToken personalAccessToken = + personalAccessTokenManager.fetchAndSave( + EnvironmentContext.getCurrent().getSubject(), remoteFactoryUrl.getHostName()); + String content = + urlFetcher.fetch(requestURL, formatAuthorization(personalAccessToken.getToken())); + gitCredentialManager.createOrReplace(personalAccessToken); + return content; + } } catch (UnknownScmProviderException e) { // we don't have any provider matching this SCM provider // so try without secrets being configured diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilder.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilder.java index 014ba402b2..b21ded50d3 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilder.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -100,7 +100,8 @@ public class URLFactoryBuilder { public Optional createFactoryFromDevfile( RemoteFactoryUrl remoteFactoryUrl, FileContentProvider fileContentProvider, - Map overrideProperties) + Map overrideProperties, + boolean skipAuthentication) throws ApiException { String devfileYamlContent; @@ -111,7 +112,10 @@ public class URLFactoryBuilder { for (DevfileLocation location : remoteFactoryUrl.devfileFileLocations()) { try { - devfileYamlContent = fileContentProvider.fetchContent(location.location()); + devfileYamlContent = + skipAuthentication + ? fileContentProvider.fetchContentWithoutAuthentication(location.location()) + : fileContentProvider.fetchContent(location.location()); } catch (IOException ex) { // try next location LOG.debug( @@ -178,7 +182,7 @@ public class URLFactoryBuilder { /** * Creates devfile with only `generateName` and no `name`. We take `generateName` with precedence. * See doc of {@link URLFactoryBuilder#createFactoryFromDevfile(RemoteFactoryUrl, - * FileContentProvider, Map)} for explanation why. + * FileContentProvider, Map, Boolean)} for explanation why. */ private DevfileImpl ensureToUseGenerateName(DevfileImpl devfile) { MetadataImpl devfileMetadata = new MetadataImpl(devfile.getMetadata()); diff --git a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/DefaultFactoryParameterResolverTest.java b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/DefaultFactoryParameterResolverTest.java index 3585ae5a0e..a9eaca9bb4 100644 --- a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/DefaultFactoryParameterResolverTest.java +++ b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/DefaultFactoryParameterResolverTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -18,6 +18,7 @@ import static org.eclipse.che.api.workspace.server.devfile.Constants.KUBERNETES_ import static org.eclipse.che.api.workspace.server.devfile.Constants.OPENSHIFT_COMPONENT_TYPE; import static org.eclipse.che.api.workspace.server.devfile.Constants.PLUGIN_COMPONENT_TYPE; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -121,7 +122,10 @@ public class DefaultFactoryParameterResolverTest { verify(urlFactoryBuilder) .createFactoryFromDevfile( - any(RemoteFactoryUrl.class), any(URLFileContentProvider.class), captor.capture()); + any(RemoteFactoryUrl.class), + any(URLFileContentProvider.class), + captor.capture(), + anyBoolean()); Map filteredOverrides = captor.getValue(); assertEquals(2, filteredOverrides.size()); assertEquals("bar", filteredOverrides.get("param.foo")); diff --git a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/scm/AuthorizingFactoryParameterResolverTest.java b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/scm/AuthorizingFactoryParameterResolverTest.java new file mode 100644 index 0000000000..e06603005b --- /dev/null +++ b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/scm/AuthorizingFactoryParameterResolverTest.java @@ -0,0 +1,73 @@ +/* + * Copyright (c) 2012-2022 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.api.factory.server.scm; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl; +import org.eclipse.che.api.workspace.server.devfile.URLFetcher; +import org.eclipse.che.commons.subject.Subject; +import org.mockito.Mock; +import org.mockito.testng.MockitoTestNGListener; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Listeners; +import org.testng.annotations.Test; + +@Listeners(value = {MockitoTestNGListener.class}) +public class AuthorizingFactoryParameterResolverTest { + @Mock private RemoteFactoryUrl remoteFactoryUrl; + @Mock private URLFetcher urlFetcher; + @Mock private PersonalAccessTokenManager personalAccessTokenManager; + @Mock private GitCredentialManager gitCredentialManager; + @Mock private PersonalAccessToken personalAccessToken; + + private AuthorizingFileContentProvider provider; + + @BeforeMethod + public void setUp() throws Exception { + provider = + new AuthorizingFileContentProvider( + remoteFactoryUrl, urlFetcher, personalAccessTokenManager, gitCredentialManager); + when(remoteFactoryUrl.getHostName()).thenReturn("hostName"); + when(remoteFactoryUrl.rawFileLocation(anyString())).thenReturn("rawFileLocation"); + } + + @Test + public void shouldFetchContentWithAuthentication() throws Exception { + // given + when(urlFetcher.fetch(anyString(), anyString())).thenReturn("content"); + when(personalAccessTokenManager.fetchAndSave(any(Subject.class), anyString())) + .thenReturn(personalAccessToken); + + // when + provider.fetchContent("url"); + + // then + verify(personalAccessTokenManager).fetchAndSave(any(Subject.class), anyString()); + } + + @Test + public void shouldFetchContentWithoutAuthentication() throws Exception { + // given + when(urlFetcher.fetch(anyString())).thenReturn("content"); + + // when + provider.fetchContentWithoutAuthentication("url"); + + // then + verify(personalAccessTokenManager, never()).fetchAndSave(any(Subject.class), anyString()); + } +} diff --git a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilderTest.java b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilderTest.java index 8c320b21fd..7fbdf2f445 100644 --- a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilderTest.java +++ b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilderTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -85,11 +85,13 @@ public class URLFactoryBuilderTest { @Mock private DevfileVersionDetector devfileVersionDetector; + @Mock private FileContentProvider fileContentProvider; + /** Tested instance. */ private URLFactoryBuilder urlFactoryBuilder; @BeforeMethod - public void setUp() { + public void setUp() throws IOException, DevfileException { this.urlFactoryBuilder = new URLFactoryBuilder( defaultEditor, defaultPlugin, true, devfileParser, devfileVersionDetector); @@ -125,13 +127,15 @@ public class URLFactoryBuilderTest { .thenReturn(new ObjectNode(JsonNodeFactory.instance)); when(devfileParser.parseJsonNode(any(JsonNode.class), anyMap())).thenReturn(devfile); when(devfileVersionDetector.devfileMajorVersion(any(JsonNode.class))).thenReturn(1); + when(fileContentProvider.fetchContent(anyString())).thenReturn("content"); FactoryMetaDto factory = urlFactoryBuilder .createFactoryFromDevfile( new DefaultFactoryUrl().withDevfileFileLocation(myLocation), - s -> myLocation + ".list", - emptyMap()) + fileContentProvider, + emptyMap(), + false) .get(); assertNotNull(factory); @@ -140,7 +144,7 @@ public class URLFactoryBuilderTest { } @Test - public void testDevfileV2() throws ApiException, DevfileException { + public void testDevfileV2() throws ApiException, DevfileException, IOException { String myLocation = "http://foo-location/"; Map devfileAsMap = Map.of("hello", "there", "how", "are", "you", "?"); @@ -148,13 +152,15 @@ public class URLFactoryBuilderTest { when(devfileParser.parseYamlRaw(anyString())).thenReturn(devfile); when(devfileParser.convertYamlToMap(devfile)).thenReturn(devfileAsMap); when(devfileVersionDetector.devfileMajorVersion(devfile)).thenReturn(2); + when(fileContentProvider.fetchContent(anyString())).thenReturn("content"); FactoryMetaDto factory = urlFactoryBuilder .createFactoryFromDevfile( new DefaultFactoryUrl().withDevfileFileLocation(myLocation), - s -> myLocation + ".list", - emptyMap()) + fileContentProvider, + emptyMap(), + false) .get(); assertNotNull(factory); @@ -164,7 +170,7 @@ public class URLFactoryBuilderTest { } @Test - public void testDevfileV2WithFilename() throws ApiException, DevfileException { + public void testDevfileV2WithFilename() throws ApiException, DevfileException, IOException { String myLocation = "http://foo-location/"; Map devfileAsMap = Map.of("hello", "there", "how", "are", "you", "?"); @@ -172,6 +178,7 @@ public class URLFactoryBuilderTest { when(devfileParser.parseYamlRaw(anyString())).thenReturn(devfile); when(devfileParser.convertYamlToMap(devfile)).thenReturn(devfileAsMap); when(devfileVersionDetector.devfileMajorVersion(devfile)).thenReturn(2); + when(fileContentProvider.fetchContent(anyString())).thenReturn("content"); RemoteFactoryUrl githubLikeRemoteUrl = new RemoteFactoryUrl() { @@ -217,7 +224,7 @@ public class URLFactoryBuilderTest { FactoryMetaDto factory = urlFactoryBuilder - .createFactoryFromDevfile(githubLikeRemoteUrl, s -> myLocation + ".list", emptyMap()) + .createFactoryFromDevfile(githubLikeRemoteUrl, fileContentProvider, emptyMap(), false) .get(); assertNotNull(factory); @@ -227,7 +234,7 @@ public class URLFactoryBuilderTest { } @Test - public void testDevfileSpecifyingFilename() throws ApiException, DevfileException { + public void testDevfileSpecifyingFilename() throws ApiException, DevfileException, IOException { String myLocation = "http://foo-location/"; Map devfileAsMap = Map.of("hello", "there", "how", "are", "you", "?"); @@ -235,6 +242,7 @@ public class URLFactoryBuilderTest { when(devfileParser.parseYamlRaw(anyString())).thenReturn(devfile); when(devfileParser.convertYamlToMap(devfile)).thenReturn(devfileAsMap); when(devfileVersionDetector.devfileMajorVersion(devfile)).thenReturn(2); + when(fileContentProvider.fetchContent(anyString())).thenReturn("content"); RemoteFactoryUrl githubLikeRemoteUrl = new RemoteFactoryUrl() { @@ -288,7 +296,8 @@ public class URLFactoryBuilderTest { singletonMap(URLFactoryBuilder.DEVFILE_FILENAME, pathToDevfile); FactoryMetaDto factory = urlFactoryBuilder - .createFactoryFromDevfile(githubLikeRemoteUrl, s -> myLocation + ".list", propertiesMap) + .createFactoryFromDevfile( + githubLikeRemoteUrl, fileContentProvider, propertiesMap, false) .get(); assertNotNull(factory); @@ -299,7 +308,8 @@ public class URLFactoryBuilderTest { } @Test - public void testShouldReturnV2WithDevworkspacesDisabled() throws ApiException, DevfileException { + public void testShouldReturnV2WithDevworkspacesDisabled() + throws ApiException, DevfileException, IOException { String myLocation = "http://foo-location/"; Map devfileAsMap = Map.of("hello", "there", "how", "are", "you", "?"); @@ -307,6 +317,7 @@ public class URLFactoryBuilderTest { when(devfileParser.parseYamlRaw(anyString())).thenReturn(devfile); when(devfileParser.convertYamlToMap(devfile)).thenReturn(devfileAsMap); when(devfileVersionDetector.devfileMajorVersion(devfile)).thenReturn(2); + when(fileContentProvider.fetchContent(anyString())).thenReturn("content"); URLFactoryBuilder localUrlFactoryBuilder = new URLFactoryBuilder( @@ -316,8 +327,9 @@ public class URLFactoryBuilderTest { localUrlFactoryBuilder .createFactoryFromDevfile( new DefaultFactoryUrl().withDevfileFileLocation(myLocation), - s -> myLocation + ".list", - emptyMap()) + fileContentProvider, + emptyMap(), + false) .get(); assertNotNull(factory); assertTrue(factory instanceof FactoryDevfileV2Dto); @@ -377,7 +389,7 @@ public class URLFactoryBuilderTest { FactoryDto factory = (FactoryDto) urlFactoryBuilder - .createFactoryFromDevfile(defaultFactoryUrl, fileContentProvider, emptyMap()) + .createFactoryFromDevfile(defaultFactoryUrl, fileContentProvider, emptyMap(), false) .get(); assertNull(factory.getDevfile().getMetadata().getName()); @@ -413,7 +425,7 @@ public class URLFactoryBuilderTest { // when try { urlFactoryBuilder.createFactoryFromDevfile( - defaultFactoryUrl, fileContentProvider, emptyMap()); + defaultFactoryUrl, fileContentProvider, emptyMap(), false); } catch (ApiException e) { assertTrue(e.getClass().isAssignableFrom(expectedClass)); assertEquals(e.getMessage(), expectedMessage); diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/FileContentProvider.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/FileContentProvider.java index e850ee8439..b4401c7c53 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/FileContentProvider.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/FileContentProvider.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -37,6 +37,17 @@ public interface FileContentProvider { */ String fetchContent(String fileURL) throws IOException, DevfileException; + /** + * Fetches content of the specified file without the authentication step. + * + * @param fileURL absolute or devfile-relative file URL to fetch content + * @return content of the specified file + * @throws IOException when there is an error during content retrieval + * @throws DevfileException when implementation does not support fetching of additional files + * content + */ + String fetchContentWithoutAuthentication(String fileURL) throws IOException, DevfileException; + /** * Short for {@code new CachingProvider(contentProvider);}. If the {@code contentProvider} is * itself an instance of the {@link CachingProvider}, no new instance is produced. @@ -67,8 +78,8 @@ public interface FileContentProvider { this.provider = provider; } - @Override - public String fetchContent(String fileURL) throws IOException, DevfileException { + private String fetchContent(String fileURL, boolean skipAuthentication) + throws IOException, DevfileException { SoftReference ref = cache.get(fileURL); String ret = ref == null ? null : ref.get(); @@ -79,5 +90,16 @@ public interface FileContentProvider { return ret; } + + @Override + public String fetchContent(String fileURL) throws IOException, DevfileException { + return fetchContent(fileURL, false); + } + + @Override + public String fetchContentWithoutAuthentication(String fileURL) + throws IOException, DevfileException { + return fetchContent(fileURL, true); + } } } diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProvider.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProvider.java index a561bda40d..2f37f2c7b3 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProvider.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProvider.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -33,8 +33,8 @@ public class URLFileContentProvider implements FileContentProvider { this.urlFetcher = urlFetcher; } - @Override - public String fetchContent(String fileURL) throws IOException, DevfileException { + private String fetchContent(String fileURL, boolean skipAuthentication) + throws IOException, DevfileException { URI fileURI; String requestURL; try { @@ -71,4 +71,15 @@ public class URLFileContentProvider implements FileContentProvider { e); } } + + @Override + public String fetchContent(String fileURL) throws IOException, DevfileException { + return fetchContent(fileURL, false); + } + + @Override + public String fetchContentWithoutAuthentication(String fileURL) + throws IOException, DevfileException { + return fetchContent(fileURL, true); + } } diff --git a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/convert/CommandConverterTest.java b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/convert/CommandConverterTest.java index b244e945ad..bf0f5c3437 100644 --- a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/convert/CommandConverterTest.java +++ b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/convert/CommandConverterTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -14,6 +14,9 @@ package org.eclipse.che.api.workspace.server.devfile.convert; import static org.eclipse.che.api.core.model.workspace.config.Command.WORKING_DIRECTORY_ATTRIBUTE; import static org.eclipse.che.api.workspace.server.devfile.Constants.COMPONENT_ALIAS_COMMAND_ATTRIBUTE; import static org.eclipse.che.api.workspace.server.devfile.Constants.EXEC_ACTION_TYPE; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNull; @@ -21,6 +24,7 @@ import static org.testng.Assert.assertNull; import com.google.common.collect.ImmutableMap; import java.util.HashMap; import org.eclipse.che.api.core.model.workspace.config.Command; +import org.eclipse.che.api.workspace.server.devfile.FileContentProvider; import org.eclipse.che.api.workspace.server.devfile.exception.DevfileFormatException; import org.eclipse.che.api.workspace.server.devfile.exception.WorkspaceExportException; import org.eclipse.che.api.workspace.server.model.impl.devfile.ActionImpl; @@ -171,9 +175,11 @@ public class CommandConverterTest { action.setReference("blah"); devfileCommand.getActions().add(action); + FileContentProvider fileContentProvider = mock(FileContentProvider.class); + when(fileContentProvider.fetchContent(anyString())).thenReturn("content"); // when - Command command = commandConverter.toWorkspaceCommand(devfileCommand, fileURL -> "content"); + Command command = commandConverter.toWorkspaceCommand(devfileCommand, fileContentProvider); // then assertNull(command.getCommandLine()); diff --git a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/validator/DevfileIntegrityValidatorTest.java b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/validator/DevfileIntegrityValidatorTest.java index 9e6ad587ba..6e2f014fa8 100644 --- a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/validator/DevfileIntegrityValidatorTest.java +++ b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/validator/DevfileIntegrityValidatorTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -16,6 +16,7 @@ import static org.eclipse.che.api.workspace.server.devfile.Constants.EDITOR_COMP import static org.eclipse.che.api.workspace.server.devfile.Constants.KUBERNETES_COMPONENT_TYPE; import static org.eclipse.che.api.workspace.server.devfile.Constants.OPENSHIFT_COMPONENT_TYPE; import static org.eclipse.che.api.workspace.server.devfile.Constants.PLUGIN_COMPONENT_TYPE; +import static org.mockito.Mockito.mock; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; @@ -24,6 +25,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.Map; +import org.eclipse.che.api.workspace.server.devfile.FileContentProvider; import org.eclipse.che.api.workspace.server.devfile.exception.DevfileFormatException; import org.eclipse.che.api.workspace.server.model.impl.devfile.ActionImpl; import org.eclipse.che.api.workspace.server.model.impl.devfile.CommandImpl; @@ -221,7 +223,7 @@ public class DevfileIntegrityValidatorTest { } // when - integrityValidator.validateContentReferences(devfile, __ -> ""); + integrityValidator.validateContentReferences(devfile, mock(FileContentProvider.class)); // then // no exception is thrown