From 148415c69160f091e604bc283ef3554504ea1ce4 Mon Sep 17 00:00:00 2001 From: Igor Vinokur Date: Thu, 12 Oct 2023 15:59:42 +0300 Subject: [PATCH] Respect authorisation request opt-out on workspace start (#576) If user rejects an scm provider authorisation request while creating or starting existed workspace store the name of the scm provider in the workspace-preferences config-map. The workspace create/start step must proceed without token fetch step. If user creates another workspace or starts existed workspace from an scm provider which name is stored in the config-map, do not ask the authorisation as it was already rejected once. --- .../che/api/deploy/WsMasterModule.java | 2 - .../infrastructure-factory/pom.xml | 12 ++ .../server/scm/KubernetesScmModule.java | 5 +- ...KubernetesAuthorisationRequestManager.java | 145 +++++++++++++ ...rnetesAuthorisationRequestManagerTest.java | 197 ++++++++++++++++++ wsmaster/che-core-api-auth/pom.xml | 4 - .../oauth/AuthorisationRequestManager.java | 47 +++++ .../che/security/oauth/EmbeddedOAuthAPI.java | 3 +- .../oauth/OAuthAuthenticationService.java | 4 +- .../AzureDevOpsFactoryParametersResolver.java | 37 ++-- .../devops/AzureDevOpsUserDataFetcher.java | 4 +- ...rAuthorizingFactoryParametersResolver.java | 40 ++-- ...horizingFactoryParametersResolverTest.java | 9 +- .../BitbucketFactoryParametersResolver.java | 36 ++-- ...itbucketFactoryParametersResolverTest.java | 6 +- wsmaster/che-core-api-factory-git-ssh/pom.xml | 4 + .../ssh/GitSshFactoryParametersResolver.java | 16 +- .../GithubFactoryParametersResolver.java | 44 ++-- .../server/github/GithubUserDataFetcher.java | 6 +- .../GithubFactoryParametersResolverTest.java | 47 +++++ .../github/GithubGitUserDataFetcherTest.java | 4 +- .../GitlabFactoryParametersResolver.java | 36 ++-- .../server/gitlab/GitlabUserDataFetcher.java | 3 +- .../GitlabFactoryParametersResolverTest.java | 9 +- .../gitlab/GitlabUserDataFetcherTest.java | 4 +- wsmaster/che-core-api-factory/pom.xml | 4 + .../server/BaseFactoryParameterResolver.java | 121 +++++++++++ .../server/FactoryParametersResolver.java | 49 +---- .../api/factory/server/FactoryService.java | 12 +- ...RawDevfileUrlFactoryParameterResolver.java | 11 +- .../scm/AbstractGitUserDataFetcher.java | 5 +- .../factory/server/scm/OAuthTokenFetcher.java | 34 --- .../BaseFactoryParameterResolverTest.java | 76 +++++++ .../factory/server/FactoryServiceTest.java | 53 ++++- 34 files changed, 886 insertions(+), 203 deletions(-) create mode 100644 infrastructures/infrastructure-factory/src/main/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesAuthorisationRequestManager.java create mode 100644 infrastructures/infrastructure-factory/src/test/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesAuthorisationRequestManagerTest.java create mode 100644 wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/AuthorisationRequestManager.java create mode 100644 wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/BaseFactoryParameterResolver.java delete mode 100644 wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/OAuthTokenFetcher.java create mode 100644 wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/BaseFactoryParameterResolverTest.java diff --git a/assembly/assembly-wsmaster-war/src/main/java/org/eclipse/che/api/deploy/WsMasterModule.java b/assembly/assembly-wsmaster-war/src/main/java/org/eclipse/che/api/deploy/WsMasterModule.java index 93379d7a999..08acaa2eafc 100644 --- a/assembly/assembly-wsmaster-war/src/main/java/org/eclipse/che/api/deploy/WsMasterModule.java +++ b/assembly/assembly-wsmaster-war/src/main/java/org/eclipse/che/api/deploy/WsMasterModule.java @@ -51,7 +51,6 @@ import org.eclipse.che.api.factory.server.github.GithubScmFileResolver; import org.eclipse.che.api.factory.server.gitlab.GitlabFactoryParametersResolver; import org.eclipse.che.api.factory.server.gitlab.GitlabScmFileResolver; -import org.eclipse.che.api.factory.server.scm.OAuthTokenFetcher; import org.eclipse.che.api.metrics.WsMasterMetricsModule; import org.eclipse.che.api.system.server.ServiceTermination; import org.eclipse.che.api.system.server.SystemModule; @@ -413,7 +412,6 @@ private void configureMultiUserMode( bind(TokenValidator.class).to(NotImplementedTokenValidator.class); bind(ProfileDao.class).to(JpaProfileDao.class); bind(OAuthAPI.class).to(EmbeddedOAuthAPI.class).asEagerSingleton(); - bind(OAuthTokenFetcher.class).to(EmbeddedOAuthAPI.class).asEagerSingleton(); } bind(AdminPermissionInitializer.class).asEagerSingleton(); diff --git a/infrastructures/infrastructure-factory/pom.xml b/infrastructures/infrastructure-factory/pom.xml index 809d2d42b52..2593e203fcb 100644 --- a/infrastructures/infrastructure-factory/pom.xml +++ b/infrastructures/infrastructure-factory/pom.xml @@ -23,6 +23,10 @@ jar Infrastructure :: Factory components + + com.google.code.gson + gson + com.google.guava guava @@ -43,6 +47,14 @@ jakarta.inject jakarta.inject-api + + jakarta.ws.rs + jakarta.ws.rs-api + + + org.eclipse.che.core + che-core-api-auth + org.eclipse.che.core che-core-api-core diff --git a/infrastructures/infrastructure-factory/src/main/java/org/eclipse/che/api/factory/server/scm/KubernetesScmModule.java b/infrastructures/infrastructure-factory/src/main/java/org/eclipse/che/api/factory/server/scm/KubernetesScmModule.java index 014a68dc35d..c7e0f79dd48 100644 --- a/infrastructures/infrastructure-factory/src/main/java/org/eclipse/che/api/factory/server/scm/KubernetesScmModule.java +++ b/infrastructures/infrastructure-factory/src/main/java/org/eclipse/che/api/factory/server/scm/KubernetesScmModule.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2023 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/ @@ -12,13 +12,16 @@ package org.eclipse.che.api.factory.server.scm; import com.google.inject.AbstractModule; +import org.eclipse.che.api.factory.server.scm.kubernetes.KubernetesAuthorisationRequestManager; import org.eclipse.che.api.factory.server.scm.kubernetes.KubernetesGitCredentialManager; import org.eclipse.che.api.factory.server.scm.kubernetes.KubernetesPersonalAccessTokenManager; +import org.eclipse.che.security.oauth.AuthorisationRequestManager; public class KubernetesScmModule extends AbstractModule { @Override protected void configure() { bind(GitCredentialManager.class).to(KubernetesGitCredentialManager.class); bind(PersonalAccessTokenManager.class).to(KubernetesPersonalAccessTokenManager.class); + bind(AuthorisationRequestManager.class).to(KubernetesAuthorisationRequestManager.class); } } diff --git a/infrastructures/infrastructure-factory/src/main/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesAuthorisationRequestManager.java b/infrastructures/infrastructure-factory/src/main/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesAuthorisationRequestManager.java new file mode 100644 index 00000000000..51dd6e62588 --- /dev/null +++ b/infrastructures/infrastructure-factory/src/main/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesAuthorisationRequestManager.java @@ -0,0 +1,145 @@ +/* + * Copyright (c) 2012-2023 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.kubernetes; + +import static com.google.common.base.Strings.isNullOrEmpty; +import static org.eclipse.che.commons.lang.UrlUtils.getParameter; +import static org.eclipse.che.commons.lang.UrlUtils.getQueryParametersFromState; +import static org.eclipse.che.commons.lang.UrlUtils.getRequestUrl; +import static org.eclipse.che.commons.lang.UrlUtils.getState; +import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.PREFERENCES_CONFIGMAP_NAME; + +import com.google.gson.Gson; +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.client.KubernetesClient; +import jakarta.ws.rs.core.UriInfo; +import java.net.URL; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import javax.inject.Inject; +import javax.inject.Singleton; +import org.eclipse.che.api.factory.server.scm.exception.ScmConfigurationPersistenceException; +import org.eclipse.che.api.factory.server.scm.exception.UnsatisfiedScmPreconditionException; +import org.eclipse.che.api.workspace.server.spi.InfrastructureException; +import org.eclipse.che.security.oauth.AuthorisationRequestManager; +import org.eclipse.che.workspace.infrastructure.kubernetes.CheServerKubernetesClientFactory; +import org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta; +import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespaceFactory; + +/** Store and retrieve rejected authorisation requests in the Kubernetes ConfigMap. */ +@Singleton +public class KubernetesAuthorisationRequestManager implements AuthorisationRequestManager { + private final KubernetesNamespaceFactory namespaceFactory; + private final CheServerKubernetesClientFactory cheServerKubernetesClientFactory; + private static final String SKIP_AUTHORISATION_MAP_KEY = "skip-authorisation"; + + @Inject + public KubernetesAuthorisationRequestManager( + KubernetesNamespaceFactory namespaceFactory, + CheServerKubernetesClientFactory cheServerKubernetesClientFactory) { + this.namespaceFactory = namespaceFactory; + this.cheServerKubernetesClientFactory = cheServerKubernetesClientFactory; + } + + @Override + public void store(String scmProviderName) { + if (isStored(scmProviderName)) { + return; + } + ConfigMap configMap = getConfigMap(); + HashSet fromJson = getSkipAuthorisationValues(); + fromJson.add(scmProviderName); + + configMap.setData(Map.of(SKIP_AUTHORISATION_MAP_KEY, fromJson.toString())); + + patchConfigMap(configMap); + } + + @Override + public void remove(String scmProviderName) { + if (!isStored(scmProviderName)) { + return; + } + ConfigMap configMap = getConfigMap(); + HashSet fromJson = getSkipAuthorisationValues(); + fromJson.remove(scmProviderName); + + configMap.setData(Map.of(SKIP_AUTHORISATION_MAP_KEY, fromJson.toString())); + + patchConfigMap(configMap); + } + + @Override + public boolean isStored(String scmProviderName) { + return getSkipAuthorisationValues().contains(scmProviderName); + } + + @Override + public void callback(UriInfo uriInfo, List errorValues) { + URL requestUrl = getRequestUrl(uriInfo); + Map> params = getQueryParametersFromState(getState(requestUrl)); + errorValues = errorValues == null ? uriInfo.getQueryParameters().get("error") : errorValues; + if (errorValues != null && errorValues.contains("access_denied")) { + store(getParameter(params, "oauth_provider")); + } + } + + private ConfigMap getConfigMap() { + try (KubernetesClient kubernetesClient = cheServerKubernetesClientFactory.create()) { + String namespace = getFirstNamespace(); + return kubernetesClient + .configMaps() + .inNamespace(namespace) + .withName(PREFERENCES_CONFIGMAP_NAME) + .get(); + } catch (UnsatisfiedScmPreconditionException + | ScmConfigurationPersistenceException + | InfrastructureException e) { + throw new RuntimeException(e); + } + } + + private void patchConfigMap(ConfigMap configMap) { + try (KubernetesClient kubernetesClient = cheServerKubernetesClientFactory.create()) { + kubernetesClient + .configMaps() + .inNamespace(getFirstNamespace()) + .withName(PREFERENCES_CONFIGMAP_NAME) + .patch(configMap); + } catch (UnsatisfiedScmPreconditionException + | ScmConfigurationPersistenceException + | InfrastructureException e) { + throw new RuntimeException(e); + } + } + + private HashSet getSkipAuthorisationValues() { + String data = getConfigMap().getData().get(SKIP_AUTHORISATION_MAP_KEY); + return new Gson().fromJson(isNullOrEmpty(data) ? "[]" : data, HashSet.class); + } + + private String getFirstNamespace() + throws UnsatisfiedScmPreconditionException, ScmConfigurationPersistenceException { + try { + return namespaceFactory.list().stream() + .map(KubernetesNamespaceMeta::getName) + .findFirst() + .orElseThrow( + () -> + new UnsatisfiedScmPreconditionException( + "No user namespace found. Cannot read SCM credentials.")); + } catch (InfrastructureException e) { + throw new ScmConfigurationPersistenceException(e.getMessage(), e); + } + } +} diff --git a/infrastructures/infrastructure-factory/src/test/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesAuthorisationRequestManagerTest.java b/infrastructures/infrastructure-factory/src/test/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesAuthorisationRequestManagerTest.java new file mode 100644 index 00000000000..f585bb678a5 --- /dev/null +++ b/infrastructures/infrastructure-factory/src/test/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesAuthorisationRequestManagerTest.java @@ -0,0 +1,197 @@ +/* + * Copyright (c) 2012-2023 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.kubernetes; + +import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.PREFERENCES_CONFIGMAP_NAME; +import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +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; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ConfigMapList; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.dsl.MixedOperation; +import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation; +import io.fabric8.kubernetes.client.dsl.Resource; +import java.util.Collections; +import java.util.Map; +import org.eclipse.che.workspace.infrastructure.kubernetes.CheServerKubernetesClientFactory; +import org.eclipse.che.workspace.infrastructure.kubernetes.api.server.impls.KubernetesNamespaceMetaImpl; +import org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta; +import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespaceFactory; +import org.mockito.ArgumentCaptor; +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(MockitoTestNGListener.class) +public class KubernetesAuthorisationRequestManagerTest { + @Mock private KubernetesNamespaceFactory namespaceFactory; + + @Mock + private MixedOperation> configMapsMixedOperation; + + @Mock NonNamespaceOperation> nonNamespaceOperation; + + @Mock private Resource configMapResource; + + @Mock private ConfigMap configMap; + + @Mock private KubernetesClient kubeClient; + + @Mock private CheServerKubernetesClientFactory cheServerKubernetesClientFactory; + + private KubernetesAuthorisationRequestManager authorisationRequestManager; + + @BeforeMethod + protected void init() throws Exception { + KubernetesNamespaceMeta meta = new KubernetesNamespaceMetaImpl("test"); + when(namespaceFactory.list()).thenReturn(Collections.singletonList(meta)); + + when(cheServerKubernetesClientFactory.create()).thenReturn(kubeClient); + when(kubeClient.configMaps()).thenReturn(configMapsMixedOperation); + when(configMapsMixedOperation.inNamespace(eq(meta.getName()))) + .thenReturn(nonNamespaceOperation); + when(nonNamespaceOperation.withName(eq(PREFERENCES_CONFIGMAP_NAME))) + .thenReturn(configMapResource); + when(configMapResource.get()).thenReturn(configMap); + + authorisationRequestManager = + new KubernetesAuthorisationRequestManager( + namespaceFactory, cheServerKubernetesClientFactory); + } + + @Test + public void shouldStoreAuthorisationRequestToEmptyConfigMap() { + // given + ArgumentCaptor> captor = ArgumentCaptor.forClass(Map.class); + + // when + authorisationRequestManager.store("test-provider"); + + // then + verify(configMap).setData(captor.capture()); + Map value = captor.getValue(); + assertEquals(value.get("skip-authorisation"), "[test-provider]"); + } + + @Test + public void shouldStoreAuthorisationRequestToNonEmptyConfigMap() { + // given + when(configMap.getData()).thenReturn(Map.of("skip-authorisation", "[existing-provider]")); + ArgumentCaptor> captor = ArgumentCaptor.forClass(Map.class); + + // when + authorisationRequestManager.store("test-provider"); + + // then + verify(configMap).setData(captor.capture()); + Map value = captor.getValue(); + assertEquals(value.get("skip-authorisation"), "[test-provider, existing-provider]"); + } + + @Test + public void shouldNotStoreAuthorisationRequestIfAlreadyStored() { + // given + when(configMap.getData()).thenReturn(Map.of("skip-authorisation", "[test-provider]")); + + // when + authorisationRequestManager.store("test-provider"); + + // then + verify(configMap, never()).setData(anyMap()); + } + + @Test + public void shouldRemoveAuthorisationRequestFromNonEmptyConfigMap() { + // given + when(configMap.getData()) + .thenReturn(Map.of("skip-authorisation", "[test-provider, existing-provider]")); + ArgumentCaptor> captor = ArgumentCaptor.forClass(Map.class); + + // when + authorisationRequestManager.remove("test-provider"); + + // then + verify(configMap).setData(captor.capture()); + Map value = captor.getValue(); + assertEquals(value.get("skip-authorisation"), "[existing-provider]"); + } + + @Test + public void shouldRemoveAuthorisationRequestFromSingleValueConfigMap() { + // given + when(configMap.getData()).thenReturn(Map.of("skip-authorisation", "[test-provider]")); + ArgumentCaptor> captor = ArgumentCaptor.forClass(Map.class); + + // when + authorisationRequestManager.remove("test-provider"); + + // then + verify(configMap).setData(captor.capture()); + Map value = captor.getValue(); + assertEquals(value.get("skip-authorisation"), "[]"); + } + + @Test + public void shouldNotRemoveAuthorisationRequestIfAlreadyRemoved() { + // given + when(configMap.getData()).thenReturn(Map.of("skip-authorisation", "[]")); + + // when + authorisationRequestManager.remove("test-provider"); + + // then + verify(configMap, never()).setData(anyMap()); + } + + @Test + public void shouldReturnFalseAuthorisationRequestStateFromEmptyConfigMap() { + // when + boolean stored = authorisationRequestManager.isStored("test-provider"); + + // then + assertFalse(stored); + } + + @Test + public void shouldReturnFalseAuthorisationRequestStateFromNonEmptyConfigMap() { + // given + when(configMap.getData()).thenReturn(Map.of("skip-authorisation", "[existing-provider]")); + + // when + boolean stored = authorisationRequestManager.isStored("test-provider"); + + // then + assertFalse(stored); + } + + @Test + public void shouldReturnTrueAuthorisationRequestStateFromNonEmptyConfigMap() { + // given + when(configMap.getData()) + .thenReturn(Map.of("skip-authorisation", "[existing-provider, test-provider]")); + + // when + boolean stored = authorisationRequestManager.isStored("test-provider"); + + // then + assertTrue(stored); + } +} diff --git a/wsmaster/che-core-api-auth/pom.xml b/wsmaster/che-core-api-auth/pom.xml index 1e841b160fd..c39ddd12298 100644 --- a/wsmaster/che-core-api-auth/pom.xml +++ b/wsmaster/che-core-api-auth/pom.xml @@ -59,10 +59,6 @@ org.eclipse.che.core che-core-api-dto - - org.eclipse.che.core - che-core-api-factory - org.eclipse.che.core che-core-commons-annotations diff --git a/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/AuthorisationRequestManager.java b/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/AuthorisationRequestManager.java new file mode 100644 index 00000000000..d2492a65970 --- /dev/null +++ b/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/AuthorisationRequestManager.java @@ -0,0 +1,47 @@ +/* + * Copyright (c) 2012-2023 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.security.oauth; + +import jakarta.ws.rs.core.UriInfo; +import java.util.List; + +/** + * Manager for storing and retrieving rejected authorisation requests. This is used to prevent from + * asking the user to grant access to the SCM provider after the user has already rejected the + * request. + */ +public interface AuthorisationRequestManager { + /** + * Store the reject flag for the given SCM provider name. + * + * @param scmProviderName the SCM provider name + */ + void store(String scmProviderName); + + /** + * Remove the reject flag for the given SCM provider name. + * + * @param scmProviderName the SCM provider name + */ + void remove(String scmProviderName); + + /** + * Check if the reject flag is stored for the given SCM provider name. + * + * @param scmProviderName the SCM provider name to check + * @return {@code true} if the reject flag is stored, {@code false} otherwise + */ + boolean isStored(String scmProviderName); + + /** This method must be called on the Oauth callback from the SCM provider. */ + void callback(UriInfo uriInfo, List errorValues); +} 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 f04d2832e50..2ba960cb0eb 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 @@ -40,7 +40,6 @@ import org.eclipse.che.api.core.rest.shared.dto.Link; import org.eclipse.che.api.core.rest.shared.dto.LinkParameter; import org.eclipse.che.api.core.util.LinksHelper; -import org.eclipse.che.api.factory.server.scm.OAuthTokenFetcher; import org.eclipse.che.commons.env.EnvironmentContext; import org.eclipse.che.commons.subject.Subject; import org.eclipse.che.security.oauth.shared.dto.OAuthAuthenticatorDescriptor; @@ -54,7 +53,7 @@ * @author Mykhailo Kuznietsov */ @Singleton -public class EmbeddedOAuthAPI implements OAuthAPI, OAuthTokenFetcher { +public class EmbeddedOAuthAPI implements OAuthAPI { private static final Logger LOG = LoggerFactory.getLogger(EmbeddedOAuthAPI.class); @Inject diff --git a/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/OAuthAuthenticationService.java b/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/OAuthAuthenticationService.java index 66e03434d5c..eb135c99b91 100644 --- a/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/OAuthAuthenticationService.java +++ b/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/OAuthAuthenticationService.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2023 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/ @@ -38,6 +38,7 @@ public class OAuthAuthenticationService extends Service { @Context protected SecurityContext security; @Inject private OAuthAPI oAuthAPI; + @Inject private AuthorisationRequestManager authorisationRequestManager; /** * Redirect request to OAuth provider site for authentication|authorization. Client must provide @@ -66,6 +67,7 @@ public Response authenticate( /** Process OAuth callback */ public Response callback(@QueryParam("errorValues") List errorValues) throws OAuthAuthenticationException, NotFoundException, ForbiddenException { + authorisationRequestManager.callback(uriInfo, errorValues); return oAuthAPI.callback(uriInfo, errorValues); } diff --git a/wsmaster/che-core-api-factory-azure-devops/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsFactoryParametersResolver.java b/wsmaster/che-core-api-factory-azure-devops/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsFactoryParametersResolver.java index e98e35d7e63..60f307ad1ab 100644 --- a/wsmaster/che-core-api-factory-azure-devops/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsFactoryParametersResolver.java +++ b/wsmaster/che-core-api-factory-azure-devops/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsFactoryParametersResolver.java @@ -11,10 +11,8 @@ */ package org.eclipse.che.api.factory.server.azure.devops; -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 com.google.common.base.Strings; import jakarta.validation.constraints.NotNull; @@ -23,6 +21,7 @@ import javax.inject.Inject; import javax.inject.Singleton; import org.eclipse.che.api.core.ApiException; +import org.eclipse.che.api.factory.server.BaseFactoryParameterResolver; import org.eclipse.che.api.factory.server.FactoryParametersResolver; import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; import org.eclipse.che.api.factory.server.urlfactory.ProjectConfigDtoMerger; @@ -38,6 +37,7 @@ import org.eclipse.che.api.workspace.shared.dto.SourceStorageDto; import org.eclipse.che.api.workspace.shared.dto.devfile.ProjectDto; import org.eclipse.che.api.workspace.shared.dto.devfile.SourceDto; +import org.eclipse.che.security.oauth.AuthorisationRequestManager; /** * Provides Factory Parameters resolver for Azure DevOps repositories. @@ -45,7 +45,10 @@ * @author Anatolii Bazko */ @Singleton -public class AzureDevOpsFactoryParametersResolver implements FactoryParametersResolver { +public class AzureDevOpsFactoryParametersResolver extends BaseFactoryParameterResolver + implements FactoryParametersResolver { + + private static final String PROVIDER_NAME = "azure-devops"; /** Parser which will allow to check validity of URLs and create objects. */ private final AzureDevOpsURLParser azureDevOpsURLParser; @@ -61,7 +64,9 @@ public AzureDevOpsFactoryParametersResolver( AzureDevOpsURLParser azureDevOpsURLParser, URLFetcher urlFetcher, URLFactoryBuilder urlFactoryBuilder, - PersonalAccessTokenManager personalAccessTokenManager) { + PersonalAccessTokenManager personalAccessTokenManager, + AuthorisationRequestManager authorisationRequestManager) { + super(authorisationRequestManager, urlFactoryBuilder, PROVIDER_NAME); this.azureDevOpsURLParser = azureDevOpsURLParser; this.urlFetcher = urlFetcher; this.urlFactoryBuilder = urlFactoryBuilder; @@ -75,27 +80,25 @@ public boolean accept(@NotNull final Map factoryParameters) { && azureDevOpsURLParser.isValid(factoryParameters.get(URL_PARAMETER_NAME)); } + @Override + public String getProviderName() { + return PROVIDER_NAME; + } + @Override public FactoryMetaDto createFactory(@NotNull final Map factoryParameters) throws ApiException { - boolean skipAuthentication = - factoryParameters.get(ERROR_QUERY_NAME) != null - && factoryParameters.get(ERROR_QUERY_NAME).equals("access_denied"); - // no need to check null value of url parameter as accept() method has performed the check final AzureDevOpsUrl azureDevOpsUrl = azureDevOpsURLParser.parse(factoryParameters.get(URL_PARAMETER_NAME)); // create factory from the following location if location exists, else create default factory - return urlFactoryBuilder - .createFactoryFromDevfile( - azureDevOpsUrl, - new AzureDevOpsAuthorizingFileContentProvider( - azureDevOpsUrl, urlFetcher, personalAccessTokenManager), - extractOverrideParams(factoryParameters), - skipAuthentication) - .orElseGet(() -> newDto(FactoryDto.class).withV(CURRENT_VERSION).withSource("repo")) - .acceptVisitor(new AzureDevOpsFactoryVisitor(azureDevOpsUrl)); + return createFactory( + factoryParameters, + azureDevOpsUrl, + new AzureDevOpsFactoryVisitor(azureDevOpsUrl), + new AzureDevOpsAuthorizingFileContentProvider( + azureDevOpsUrl, urlFetcher, personalAccessTokenManager)); } /** diff --git a/wsmaster/che-core-api-factory-azure-devops/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsUserDataFetcher.java b/wsmaster/che-core-api-factory-azure-devops/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsUserDataFetcher.java index 75f126570db..5e8f23ac517 100644 --- a/wsmaster/che-core-api-factory-azure-devops/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsUserDataFetcher.java +++ b/wsmaster/che-core-api-factory-azure-devops/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsUserDataFetcher.java @@ -18,12 +18,12 @@ import org.eclipse.che.api.auth.shared.dto.OAuthToken; import org.eclipse.che.api.factory.server.scm.AbstractGitUserDataFetcher; import org.eclipse.che.api.factory.server.scm.GitUserData; -import org.eclipse.che.api.factory.server.scm.OAuthTokenFetcher; import org.eclipse.che.api.factory.server.scm.PersonalAccessToken; import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; import org.eclipse.che.api.factory.server.scm.exception.ScmBadRequestException; import org.eclipse.che.api.factory.server.scm.exception.ScmCommunicationException; import org.eclipse.che.api.factory.server.scm.exception.ScmItemNotFoundException; +import org.eclipse.che.security.oauth.OAuthAPI; /** * Azure DevOps user data fetcher. @@ -37,7 +37,7 @@ public class AzureDevOpsUserDataFetcher extends AbstractGitUserDataFetcher { @Inject public AzureDevOpsUserDataFetcher( - OAuthTokenFetcher oAuthTokenFetcher, + OAuthAPI oAuthTokenFetcher, PersonalAccessTokenManager personalAccessTokenManager, AzureDevOpsApiClient azureDevOpsApiClient, @Named("che.api") String cheApiEndpoint, 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 7aafa4baec8..e53a93b40cf 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 @@ -11,10 +11,8 @@ */ package org.eclipse.che.api.factory.server.bitbucket; -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; @@ -22,6 +20,7 @@ import javax.inject.Singleton; import org.eclipse.che.api.core.ApiException; import org.eclipse.che.api.core.BadRequestException; +import org.eclipse.che.api.factory.server.BaseFactoryParameterResolver; import org.eclipse.che.api.factory.server.FactoryParametersResolver; import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl; @@ -31,10 +30,10 @@ import org.eclipse.che.api.factory.shared.dto.FactoryMetaDto; import org.eclipse.che.api.factory.shared.dto.FactoryVisitor; import org.eclipse.che.api.factory.shared.dto.ScmInfoDto; -import org.eclipse.che.api.workspace.server.devfile.FileContentProvider; import org.eclipse.che.api.workspace.server.devfile.URLFetcher; import org.eclipse.che.api.workspace.shared.dto.devfile.ProjectDto; import org.eclipse.che.api.workspace.shared.dto.devfile.SourceDto; +import org.eclipse.che.security.oauth.AuthorisationRequestManager; /** * Provides Factory Parameters resolver for both public and private bitbucket repositories. @@ -43,7 +42,9 @@ */ @Singleton public class BitbucketServerAuthorizingFactoryParametersResolver - implements FactoryParametersResolver { + extends BaseFactoryParameterResolver implements FactoryParametersResolver { + + private static final String PROVIDER_NAME = "bitbucket-server"; private final URLFactoryBuilder urlFactoryBuilder; private final URLFetcher urlFetcher; @@ -57,7 +58,9 @@ public BitbucketServerAuthorizingFactoryParametersResolver( URLFactoryBuilder urlFactoryBuilder, URLFetcher urlFetcher, BitbucketServerURLParser bitbucketURLParser, - PersonalAccessTokenManager personalAccessTokenManager) { + PersonalAccessTokenManager personalAccessTokenManager, + AuthorisationRequestManager authorisationRequestManager) { + super(authorisationRequestManager, urlFactoryBuilder, PROVIDER_NAME); this.urlFactoryBuilder = urlFactoryBuilder; this.urlFetcher = urlFetcher; this.bitbucketURLParser = bitbucketURLParser; @@ -77,6 +80,11 @@ public boolean accept(@NotNull final Map factoryParameters) { && bitbucketURLParser.isValid(factoryParameters.get(URL_PARAMETER_NAME)); } + @Override + public String getProviderName() { + return PROVIDER_NAME; + } + /** * Create factory object based on provided parameters * @@ -90,23 +98,13 @@ public FactoryMetaDto createFactory(@NotNull final Map factoryPa // no need to check null value of url parameter as accept() method has performed the check final BitbucketServerUrl bitbucketServerUrl = bitbucketURLParser.parse(factoryParameters.get(URL_PARAMETER_NAME)); - - final FileContentProvider fileContentProvider = - new BitbucketServerAuthorizingFileContentProvider( - bitbucketServerUrl, urlFetcher, personalAccessTokenManager); - - 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 - .createFactoryFromDevfile( - bitbucketServerUrl, - fileContentProvider, - extractOverrideParams(factoryParameters), - skipAuthentication) - .orElseGet(() -> newDto(FactoryDto.class).withV(CURRENT_VERSION).withSource("repo")) - .acceptVisitor(new BitbucketFactoryVisitor(bitbucketServerUrl)); + return createFactory( + factoryParameters, + bitbucketServerUrl, + new BitbucketFactoryVisitor(bitbucketServerUrl), + new BitbucketServerAuthorizingFileContentProvider( + bitbucketServerUrl, urlFetcher, personalAccessTokenManager)); } /** 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 bd1160dc10c..664d0749d28 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 @@ -45,6 +45,7 @@ import org.eclipse.che.api.workspace.shared.dto.devfile.DevfileDto; import org.eclipse.che.api.workspace.shared.dto.devfile.MetadataDto; import org.eclipse.che.api.workspace.shared.dto.devfile.SourceDto; +import org.eclipse.che.security.oauth.AuthorisationRequestManager; import org.eclipse.che.security.oauth.OAuthAPI; import org.mockito.Mock; import org.mockito.testng.MockitoTestNGListener; @@ -62,6 +63,8 @@ public class BitbucketServerAuthorizingFactoryParametersResolverTest { @Mock private DevfileFilenamesProvider devfileFilenamesProvider; + @Mock private AuthorisationRequestManager authorisationRequestManager; + BitbucketServerURLParser bitbucketURLParser; @Mock private PersonalAccessTokenManager personalAccessTokenManager; @@ -79,7 +82,11 @@ protected void init() { assertNotNull(this.bitbucketURLParser); bitbucketServerFactoryParametersResolver = new BitbucketServerAuthorizingFactoryParametersResolver( - urlFactoryBuilder, urlFetcher, bitbucketURLParser, personalAccessTokenManager); + urlFactoryBuilder, + urlFetcher, + bitbucketURLParser, + personalAccessTokenManager, + authorisationRequestManager); assertNotNull(this.bitbucketServerFactoryParametersResolver); } 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 86b37a7cb01..9c67f50f228 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 @@ -11,10 +11,8 @@ */ package org.eclipse.che.api.factory.server.bitbucket; -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; @@ -22,6 +20,7 @@ import javax.inject.Singleton; import org.eclipse.che.api.core.ApiException; import org.eclipse.che.api.core.BadRequestException; +import org.eclipse.che.api.factory.server.BaseFactoryParameterResolver; import org.eclipse.che.api.factory.server.FactoryParametersResolver; import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; import org.eclipse.che.api.factory.server.urlfactory.ProjectConfigDtoMerger; @@ -35,10 +34,14 @@ import org.eclipse.che.api.workspace.server.devfile.URLFetcher; import org.eclipse.che.api.workspace.shared.dto.ProjectConfigDto; import org.eclipse.che.api.workspace.shared.dto.devfile.ProjectDto; +import org.eclipse.che.security.oauth.AuthorisationRequestManager; /** Provides Factory Parameters resolver for bitbucket repositories. */ @Singleton -public class BitbucketFactoryParametersResolver implements FactoryParametersResolver { +public class BitbucketFactoryParametersResolver extends BaseFactoryParameterResolver + implements FactoryParametersResolver { + + private static final String PROVIDER_NAME = "bitbucket"; /** Parser which will allow to check validity of URLs and create objects. */ private final BitbucketURLParser bitbucketURLParser; @@ -64,7 +67,9 @@ public BitbucketFactoryParametersResolver( URLFactoryBuilder urlFactoryBuilder, ProjectConfigDtoMerger projectConfigDtoMerger, PersonalAccessTokenManager personalAccessTokenManager, - BitbucketApiClient bitbucketApiClient) { + BitbucketApiClient bitbucketApiClient, + AuthorisationRequestManager authorisationRequestManager) { + super(authorisationRequestManager, urlFactoryBuilder, PROVIDER_NAME); this.bitbucketURLParser = bitbucketURLParser; this.urlFetcher = urlFetcher; this.bitbucketSourceStorageBuilder = bitbucketSourceStorageBuilder; @@ -88,6 +93,11 @@ public boolean accept(@NotNull final Map factoryParameters) { && bitbucketURLParser.isValid(factoryParameters.get(URL_PARAMETER_NAME)); } + @Override + public String getProviderName() { + return PROVIDER_NAME; + } + /** * Create factory object based on provided parameters * @@ -100,19 +110,13 @@ public FactoryMetaDto createFactory(@NotNull final Map factoryPa // no need to check null value of url parameter as accept() method has performed the check final BitbucketUrl bitbucketUrl = bitbucketURLParser.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 - .createFactoryFromDevfile( - bitbucketUrl, - new BitbucketAuthorizingFileContentProvider( - bitbucketUrl, urlFetcher, personalAccessTokenManager, bitbucketApiClient), - extractOverrideParams(factoryParameters), - skipAuthentication) - .orElseGet(() -> newDto(FactoryDto.class).withV(CURRENT_VERSION).withSource("repo")) - .acceptVisitor(new BitbucketFactoryVisitor(bitbucketUrl)); + return createFactory( + factoryParameters, + bitbucketUrl, + new BitbucketFactoryVisitor(bitbucketUrl), + new BitbucketAuthorizingFileContentProvider( + bitbucketUrl, urlFetcher, personalAccessTokenManager, bitbucketApiClient)); } /** 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 16a977f21e6..7d6e2ae754f 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 @@ -49,6 +49,7 @@ import org.eclipse.che.api.workspace.shared.dto.devfile.MetadataDto; import org.eclipse.che.api.workspace.shared.dto.devfile.ProjectDto; import org.eclipse.che.api.workspace.shared.dto.devfile.SourceDto; +import org.eclipse.che.security.oauth.AuthorisationRequestManager; import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.Mock; @@ -66,6 +67,8 @@ public class BitbucketFactoryParametersResolverTest { @Mock private DevfileFilenamesProvider devfileFilenamesProvider; + @Mock private AuthorisationRequestManager authorisationRequestManager; + /** Parser which will allow to check validity of URLs and create objects. */ private BitbucketURLParser bitbucketURLParser; @@ -104,7 +107,8 @@ protected void init() { urlFactoryBuilder, projectConfigDtoMerger, personalAccessTokenManager, - bitbucketApiClient); + bitbucketApiClient, + authorisationRequestManager); assertNotNull(this.bitbucketFactoryParametersResolver); } diff --git a/wsmaster/che-core-api-factory-git-ssh/pom.xml b/wsmaster/che-core-api-factory-git-ssh/pom.xml index e55b8ea1775..674fadee55f 100644 --- a/wsmaster/che-core-api-factory-git-ssh/pom.xml +++ b/wsmaster/che-core-api-factory-git-ssh/pom.xml @@ -34,6 +34,10 @@ jakarta.validation jakarta.validation-api + + org.eclipse.che.core + che-core-api-auth + org.eclipse.che.core che-core-api-core diff --git a/wsmaster/che-core-api-factory-git-ssh/src/main/java/org/eclipse/che/api/factory/server/git/ssh/GitSshFactoryParametersResolver.java b/wsmaster/che-core-api-factory-git-ssh/src/main/java/org/eclipse/che/api/factory/server/git/ssh/GitSshFactoryParametersResolver.java index 685976aef1d..97698512d90 100644 --- a/wsmaster/che-core-api-factory-git-ssh/src/main/java/org/eclipse/che/api/factory/server/git/ssh/GitSshFactoryParametersResolver.java +++ b/wsmaster/che-core-api-factory-git-ssh/src/main/java/org/eclipse/che/api/factory/server/git/ssh/GitSshFactoryParametersResolver.java @@ -21,6 +21,7 @@ import javax.inject.Inject; import javax.inject.Singleton; import org.eclipse.che.api.core.ApiException; +import org.eclipse.che.api.factory.server.BaseFactoryParameterResolver; import org.eclipse.che.api.factory.server.FactoryParametersResolver; import org.eclipse.che.api.factory.server.FactoryResolverPriority; import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; @@ -34,6 +35,7 @@ import org.eclipse.che.api.workspace.server.devfile.URLFetcher; import org.eclipse.che.api.workspace.shared.dto.devfile.ProjectDto; import org.eclipse.che.api.workspace.shared.dto.devfile.SourceDto; +import org.eclipse.che.security.oauth.AuthorisationRequestManager; /** * Provides Factory Parameters resolver for Git Ssh repositories. @@ -41,7 +43,10 @@ * @author Anatolii Bazko */ @Singleton -public class GitSshFactoryParametersResolver implements FactoryParametersResolver { +public class GitSshFactoryParametersResolver extends BaseFactoryParameterResolver + implements FactoryParametersResolver { + + private static final String PROVIDER_NAME = "git-ssh"; private final GitSshURLParser gitSshURLParser; @@ -54,7 +59,9 @@ public GitSshFactoryParametersResolver( GitSshURLParser gitSshURLParser, URLFetcher urlFetcher, URLFactoryBuilder urlFactoryBuilder, - PersonalAccessTokenManager personalAccessTokenManager) { + PersonalAccessTokenManager personalAccessTokenManager, + AuthorisationRequestManager authorisationRequestManager) { + super(authorisationRequestManager, urlFactoryBuilder, PROVIDER_NAME); this.gitSshURLParser = gitSshURLParser; this.urlFetcher = urlFetcher; this.urlFactoryBuilder = urlFactoryBuilder; @@ -67,6 +74,11 @@ public boolean accept(@NotNull final Map factoryParameters) { && gitSshURLParser.isValid(factoryParameters.get(URL_PARAMETER_NAME)); } + @Override + public String getProviderName() { + return PROVIDER_NAME; + } + @Override public FactoryMetaDto createFactory(@NotNull final Map factoryParameters) throws ApiException { 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 4c96fd82446..f769f8fb325 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 @@ -11,10 +11,9 @@ */ package org.eclipse.che.api.factory.server.github; -import static org.eclipse.che.api.factory.shared.Constants.CURRENT_VERSION; +import static java.util.Collections.emptyMap; 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; @@ -22,6 +21,7 @@ import javax.inject.Singleton; import org.eclipse.che.api.core.ApiException; import org.eclipse.che.api.core.BadRequestException; +import org.eclipse.che.api.factory.server.BaseFactoryParameterResolver; import org.eclipse.che.api.factory.server.FactoryParametersResolver; import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; import org.eclipse.che.api.factory.server.urlfactory.ProjectConfigDtoMerger; @@ -35,6 +35,7 @@ import org.eclipse.che.api.workspace.server.devfile.URLFetcher; import org.eclipse.che.api.workspace.shared.dto.ProjectConfigDto; import org.eclipse.che.api.workspace.shared.dto.devfile.ProjectDto; +import org.eclipse.che.security.oauth.AuthorisationRequestManager; /** * Provides Factory Parameters resolver for github repositories. @@ -42,7 +43,10 @@ * @author Florent Benoit */ @Singleton -public class GithubFactoryParametersResolver implements FactoryParametersResolver { +public class GithubFactoryParametersResolver extends BaseFactoryParameterResolver + implements FactoryParametersResolver { + + private static final String PROVIDER_NAME = "github"; /** Parser which will allow to check validity of URLs and create objects. */ private final GithubURLParser githubUrlParser; @@ -64,9 +68,11 @@ public GithubFactoryParametersResolver( GithubURLParser githubUrlParser, URLFetcher urlFetcher, GithubSourceStorageBuilder githubSourceStorageBuilder, + AuthorisationRequestManager authorisationRequestManager, URLFactoryBuilder urlFactoryBuilder, ProjectConfigDtoMerger projectConfigDtoMerger, PersonalAccessTokenManager personalAccessTokenManager) { + super(authorisationRequestManager, urlFactoryBuilder, PROVIDER_NAME); this.githubUrlParser = githubUrlParser; this.urlFetcher = urlFetcher; this.githubSourceStorageBuilder = githubSourceStorageBuilder; @@ -89,6 +95,11 @@ public boolean accept(@NotNull final Map factoryParameters) { && githubUrlParser.isValid(factoryParameters.get(URL_PARAMETER_NAME)); } + @Override + public String getProviderName() { + return PROVIDER_NAME; + } + /** * Create factory object based on provided parameters * @@ -98,28 +109,21 @@ public boolean accept(@NotNull final Map factoryParameters) { @Override public FactoryMetaDto createFactory(@NotNull final Map factoryParameters) throws ApiException { - boolean skipAuthentication = - factoryParameters.get(ERROR_QUERY_NAME) != null - && factoryParameters.get(ERROR_QUERY_NAME).equals("access_denied"); // no need to check null value of url parameter as accept() method has performed the check final GithubUrl githubUrl; - if (skipAuthentication) { + if (getSkipAuthorisation(factoryParameters)) { githubUrl = githubUrlParser.parseWithoutAuthentication(factoryParameters.get(URL_PARAMETER_NAME)); } else { githubUrl = githubUrlParser.parse(factoryParameters.get(URL_PARAMETER_NAME)); } - // create factory from the following location if location exists, else create default factory - return urlFactoryBuilder - .createFactoryFromDevfile( - githubUrl, - new GithubAuthorizingFileContentProvider( - githubUrl, urlFetcher, personalAccessTokenManager), - extractOverrideParams(factoryParameters), - skipAuthentication) - .orElseGet(() -> newDto(FactoryDto.class).withV(CURRENT_VERSION).withSource("repo")) - .acceptVisitor(new GithubFactoryVisitor(githubUrl)); + return createFactory( + factoryParameters, + githubUrl, + new GithubFactoryVisitor(githubUrl), + new GithubAuthorizingFileContentProvider( + githubUrl, urlFetcher, personalAccessTokenManager)); } /** @@ -182,6 +186,10 @@ public FactoryDto visit(FactoryDto factory) { @Override public RemoteFactoryUrl parseFactoryUrl(String factoryUrl) throws ApiException { - return githubUrlParser.parse(factoryUrl); + if (getSkipAuthorisation(emptyMap())) { + return githubUrlParser.parseWithoutAuthentication(factoryUrl); + } else { + return githubUrlParser.parse(factoryUrl); + } } } diff --git a/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubUserDataFetcher.java b/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubUserDataFetcher.java index e05c48b21c5..3b319cc9885 100644 --- a/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubUserDataFetcher.java +++ b/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubUserDataFetcher.java @@ -19,13 +19,13 @@ import org.eclipse.che.api.auth.shared.dto.OAuthToken; import org.eclipse.che.api.factory.server.scm.AbstractGitUserDataFetcher; import org.eclipse.che.api.factory.server.scm.GitUserData; -import org.eclipse.che.api.factory.server.scm.OAuthTokenFetcher; import org.eclipse.che.api.factory.server.scm.PersonalAccessToken; import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; import org.eclipse.che.api.factory.server.scm.exception.ScmBadRequestException; import org.eclipse.che.api.factory.server.scm.exception.ScmCommunicationException; import org.eclipse.che.api.factory.server.scm.exception.ScmItemNotFoundException; import org.eclipse.che.commons.annotation.Nullable; +import org.eclipse.che.security.oauth.OAuthAPI; /** GitHub user data retriever. */ public class GithubUserDataFetcher extends AbstractGitUserDataFetcher { @@ -44,7 +44,7 @@ public class GithubUserDataFetcher extends AbstractGitUserDataFetcher { public GithubUserDataFetcher( @Named("che.api") String apiEndpoint, @Nullable @Named("che.integration.github.oauth_endpoint") String oauthEndpoint, - OAuthTokenFetcher oAuthTokenFetcher, + OAuthAPI oAuthTokenFetcher, PersonalAccessTokenManager personalAccessTokenManager) { this( apiEndpoint, @@ -56,7 +56,7 @@ public GithubUserDataFetcher( /** Constructor used for testing only. */ public GithubUserDataFetcher( String apiEndpoint, - OAuthTokenFetcher oAuthTokenFetcher, + OAuthAPI oAuthTokenFetcher, PersonalAccessTokenManager personalAccessTokenManager, GithubApiClient githubApiClient) { super(OAUTH_PROVIDER_NAME, personalAccessTokenManager, oAuthTokenFetcher); 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 087bf19a3e2..853ed13f610 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 @@ -51,6 +51,7 @@ import org.eclipse.che.api.workspace.shared.dto.devfile.MetadataDto; import org.eclipse.che.api.workspace.shared.dto.devfile.ProjectDto; import org.eclipse.che.api.workspace.shared.dto.devfile.SourceDto; +import org.eclipse.che.security.oauth.AuthorisationRequestManager; import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.Mock; @@ -88,6 +89,7 @@ public class GithubFactoryParametersResolverTest { private URLFactoryBuilder urlFactoryBuilder; @Mock private PersonalAccessTokenManager personalAccessTokenManager; + @Mock private AuthorisationRequestManager authorisationRequestManager; /** * Capturing the location parameter when calling {@link @@ -113,6 +115,7 @@ protected void init() throws Exception { githubUrlParser, urlFetcher, githubSourceStorageBuilder, + authorisationRequestManager, urlFactoryBuilder, projectConfigDtoMerger, personalAccessTokenManager); @@ -351,6 +354,50 @@ public void shouldCreateFactoryWithoutAuthentication() throws ApiException { eq(true)); } + @Test + public void shouldParseFactoryUrlWithAuthentication() throws Exception { + // given + githubUrlParser = mock(GithubURLParser.class); + + githubFactoryParametersResolver = + new GithubFactoryParametersResolver( + githubUrlParser, + urlFetcher, + githubSourceStorageBuilder, + authorisationRequestManager, + urlFactoryBuilder, + projectConfigDtoMerger, + personalAccessTokenManager); + when(authorisationRequestManager.isStored(eq("github"))).thenReturn(true); + // when + githubFactoryParametersResolver.parseFactoryUrl("url"); + // then + verify(githubUrlParser).parseWithoutAuthentication("url"); + verify(githubUrlParser, never()).parse("url"); + } + + @Test + public void shouldParseFactoryUrlWithOutAuthentication() throws Exception { + // given + githubUrlParser = mock(GithubURLParser.class); + + githubFactoryParametersResolver = + new GithubFactoryParametersResolver( + githubUrlParser, + urlFetcher, + githubSourceStorageBuilder, + authorisationRequestManager, + urlFactoryBuilder, + projectConfigDtoMerger, + personalAccessTokenManager); + when(authorisationRequestManager.isStored(eq("github"))).thenReturn(false); + // when + githubFactoryParametersResolver.parseFactoryUrl("url"); + // then + verify(githubUrlParser).parse("url"); + verify(githubUrlParser, never()).parseWithoutAuthentication("url"); + } + private FactoryDto generateDevfileFactory() { return newDto(FactoryDto.class) .withV(CURRENT_VERSION) diff --git a/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubGitUserDataFetcherTest.java b/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubGitUserDataFetcherTest.java index 714dd961b22..412b1e9ecf5 100644 --- a/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubGitUserDataFetcherTest.java +++ b/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubGitUserDataFetcherTest.java @@ -28,8 +28,8 @@ import com.google.common.net.HttpHeaders; import org.eclipse.che.api.auth.shared.dto.OAuthToken; import org.eclipse.che.api.factory.server.scm.GitUserData; -import org.eclipse.che.api.factory.server.scm.OAuthTokenFetcher; import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; +import org.eclipse.che.security.oauth.OAuthAPI; import org.mockito.Mock; import org.mockito.testng.MockitoTestNGListener; import org.testng.annotations.AfterMethod; @@ -40,7 +40,7 @@ @Listeners(MockitoTestNGListener.class) public class GithubGitUserDataFetcherTest { - @Mock OAuthTokenFetcher oAuthTokenFetcher; + @Mock OAuthAPI oAuthTokenFetcher; @Mock PersonalAccessTokenManager personalAccessTokenManager; GithubUserDataFetcher githubGUDFetcher; 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 efedf3f1eef..3a3efc3a669 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 @@ -11,10 +11,8 @@ */ package org.eclipse.che.api.factory.server.gitlab; -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; @@ -22,6 +20,7 @@ import javax.inject.Singleton; import org.eclipse.che.api.core.ApiException; import org.eclipse.che.api.core.BadRequestException; +import org.eclipse.che.api.factory.server.BaseFactoryParameterResolver; import org.eclipse.che.api.factory.server.FactoryParametersResolver; import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl; @@ -34,6 +33,7 @@ import org.eclipse.che.api.workspace.server.devfile.URLFetcher; import org.eclipse.che.api.workspace.shared.dto.devfile.ProjectDto; import org.eclipse.che.api.workspace.shared.dto.devfile.SourceDto; +import org.eclipse.che.security.oauth.AuthorisationRequestManager; /** * Provides Factory Parameters resolver for Gitlab repositories. @@ -41,7 +41,10 @@ * @author Max Shaposhnyk */ @Singleton -public class GitlabFactoryParametersResolver implements FactoryParametersResolver { +public class GitlabFactoryParametersResolver extends BaseFactoryParameterResolver + implements FactoryParametersResolver { + + private static final String PROVIDER_NAME = "gitlab"; private final URLFactoryBuilder urlFactoryBuilder; private final URLFetcher urlFetcher; @@ -53,7 +56,9 @@ public GitlabFactoryParametersResolver( URLFactoryBuilder urlFactoryBuilder, URLFetcher urlFetcher, GitlabUrlParser gitlabURLParser, - PersonalAccessTokenManager personalAccessTokenManager) { + PersonalAccessTokenManager personalAccessTokenManager, + AuthorisationRequestManager authorisationRequestManager) { + super(authorisationRequestManager, urlFactoryBuilder, PROVIDER_NAME); this.urlFactoryBuilder = urlFactoryBuilder; this.urlFetcher = urlFetcher; this.gitlabURLParser = gitlabURLParser; @@ -73,6 +78,11 @@ public boolean accept(@NotNull final Map factoryParameters) { && gitlabURLParser.isValid(factoryParameters.get(URL_PARAMETER_NAME)); } + @Override + public String getProviderName() { + return PROVIDER_NAME; + } + /** * Create factory object based on provided parameters * @@ -84,19 +94,13 @@ public FactoryMetaDto createFactory(@NotNull final Map factoryPa throws ApiException { // no need to check null value of url parameter as accept() method has performed the check final GitlabUrl gitlabUrl = gitlabURLParser.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 - .createFactoryFromDevfile( - gitlabUrl, - new GitlabAuthorizingFileContentProvider( - gitlabUrl, urlFetcher, personalAccessTokenManager), - extractOverrideParams(factoryParameters), - skipAuthentication) - .orElseGet(() -> newDto(FactoryDto.class).withV(CURRENT_VERSION).withSource("repo")) - .acceptVisitor(new GitlabFactoryVisitor(gitlabUrl)); + return createFactory( + factoryParameters, + gitlabUrl, + new GitlabFactoryVisitor(gitlabUrl), + new GitlabAuthorizingFileContentProvider( + gitlabUrl, urlFetcher, personalAccessTokenManager)); } /** diff --git a/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabUserDataFetcher.java b/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabUserDataFetcher.java index 2d22fafede7..a9fb66cb1c5 100644 --- a/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabUserDataFetcher.java +++ b/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabUserDataFetcher.java @@ -29,6 +29,7 @@ import org.eclipse.che.commons.annotation.Nullable; import org.eclipse.che.commons.lang.StringUtils; import org.eclipse.che.inject.ConfigurationException; +import org.eclipse.che.security.oauth.OAuthAPI; /** Gitlab OAuth token retriever. */ public class GitlabUserDataFetcher extends AbstractGitUserDataFetcher { @@ -48,7 +49,7 @@ public GitlabUserDataFetcher( @Nullable @Named("che.integration.gitlab.oauth_endpoint") String oauthEndpoint, @Named("che.api") String apiEndpoint, PersonalAccessTokenManager personalAccessTokenManager, - OAuthTokenFetcher oAuthTokenFetcher) { + OAuthAPI oAuthTokenFetcher) { super(OAUTH_PROVIDER_NAME, personalAccessTokenManager, oAuthTokenFetcher); this.apiEndpoint = apiEndpoint; if (gitlabEndpoints != null) { 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 6fa25c1497c..15daf55ff9a 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 @@ -45,6 +45,7 @@ import org.eclipse.che.api.workspace.shared.dto.devfile.DevfileDto; import org.eclipse.che.api.workspace.shared.dto.devfile.MetadataDto; import org.eclipse.che.api.workspace.shared.dto.devfile.SourceDto; +import org.eclipse.che.security.oauth.AuthorisationRequestManager; import org.mockito.Mock; import org.mockito.testng.MockitoTestNGListener; import org.testng.annotations.BeforeMethod; @@ -68,6 +69,8 @@ public class GitlabFactoryParametersResolverTest { GitlabUrlParser gitlabUrlParser; @Mock private PersonalAccessTokenManager personalAccessTokenManager; + @Mock private AuthorisationRequestManager authorisationRequestManager; + private GitlabFactoryParametersResolver gitlabFactoryParametersResolver; @BeforeMethod @@ -80,7 +83,11 @@ protected void init() { assertNotNull(this.gitlabUrlParser); gitlabFactoryParametersResolver = new GitlabFactoryParametersResolver( - urlFactoryBuilder, urlFetcher, gitlabUrlParser, personalAccessTokenManager); + urlFactoryBuilder, + urlFetcher, + gitlabUrlParser, + personalAccessTokenManager, + authorisationRequestManager); assertNotNull(this.gitlabFactoryParametersResolver); } diff --git a/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabUserDataFetcherTest.java b/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabUserDataFetcherTest.java index 953c24540e7..220f3b4b8eb 100644 --- a/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabUserDataFetcherTest.java +++ b/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabUserDataFetcherTest.java @@ -28,8 +28,8 @@ import com.google.common.net.HttpHeaders; import org.eclipse.che.api.auth.shared.dto.OAuthToken; import org.eclipse.che.api.factory.server.scm.GitUserData; -import org.eclipse.che.api.factory.server.scm.OAuthTokenFetcher; import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; +import org.eclipse.che.security.oauth.OAuthAPI; import org.mockito.Mock; import org.mockito.testng.MockitoTestNGListener; import org.testng.annotations.AfterMethod; @@ -40,7 +40,7 @@ @Listeners(MockitoTestNGListener.class) public class GitlabUserDataFetcherTest { - @Mock OAuthTokenFetcher oAuthTokenFetcher; + @Mock OAuthAPI oAuthTokenFetcher; @Mock PersonalAccessTokenManager personalAccessTokenManager; GitlabUserDataFetcher gitlabUserDataFetcher; diff --git a/wsmaster/che-core-api-factory/pom.xml b/wsmaster/che-core-api-factory/pom.xml index 4368fe77d09..062c2e0c2c3 100644 --- a/wsmaster/che-core-api-factory/pom.xml +++ b/wsmaster/che-core-api-factory/pom.xml @@ -62,6 +62,10 @@ jakarta.ws.rs jakarta.ws.rs-api + + org.eclipse.che.core + che-core-api-auth + org.eclipse.che.core che-core-api-auth-shared diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/BaseFactoryParameterResolver.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/BaseFactoryParameterResolver.java new file mode 100644 index 00000000000..652191a557b --- /dev/null +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/BaseFactoryParameterResolver.java @@ -0,0 +1,121 @@ +/* + * Copyright (c) 2012-2023 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; + +import static java.util.stream.Collectors.toMap; +import static org.eclipse.che.api.factory.shared.Constants.CURRENT_VERSION; +import static org.eclipse.che.dto.server.DtoFactory.newDto; + +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.function.Consumer; +import java.util.function.Supplier; +import org.eclipse.che.api.core.ApiException; +import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl; +import org.eclipse.che.api.factory.server.urlfactory.URLFactoryBuilder; +import org.eclipse.che.api.factory.shared.dto.FactoryDto; +import org.eclipse.che.api.factory.shared.dto.FactoryMetaDto; +import org.eclipse.che.api.factory.shared.dto.FactoryVisitor; +import org.eclipse.che.api.workspace.server.devfile.FileContentProvider; +import org.eclipse.che.api.workspace.shared.dto.devfile.DevfileDto; +import org.eclipse.che.api.workspace.shared.dto.devfile.ProjectDto; +import org.eclipse.che.security.oauth.AuthorisationRequestManager; + +public class BaseFactoryParameterResolver { + + private final AuthorisationRequestManager authorisationRequestManager; + private final URLFactoryBuilder urlFactoryBuilder; + private final String providerName; + + public BaseFactoryParameterResolver( + AuthorisationRequestManager authorisationRequestManager, + URLFactoryBuilder urlFactoryBuilder, + String providerName) { + this.authorisationRequestManager = authorisationRequestManager; + this.urlFactoryBuilder = urlFactoryBuilder; + this.providerName = providerName; + } + + protected FactoryMetaDto createFactory( + Map factoryParameters, + RemoteFactoryUrl factoryUrl, + FactoryVisitor factoryVisitor, + FileContentProvider contentProvider) + throws ApiException { + + // create factory from the following location if location exists, else create default factory + return urlFactoryBuilder + .createFactoryFromDevfile( + factoryUrl, + contentProvider, + extractOverrideParams(factoryParameters), + getSkipAuthorisation(factoryParameters)) + .orElseGet(() -> newDto(FactoryDto.class).withV(CURRENT_VERSION).withSource("repo")) + .acceptVisitor(factoryVisitor); + } + + protected boolean getSkipAuthorisation(Map factoryParameters) { + String errorCode = "error_code"; + boolean stored = authorisationRequestManager.isStored(providerName); + boolean skipAuthentication = + factoryParameters.get(errorCode) != null + && factoryParameters.get(errorCode).equals("access_denied") + || stored; + if (skipAuthentication && !stored) { + authorisationRequestManager.store(providerName); + } + return skipAuthentication; + } + + /** + * Returns priority of the resolver. Resolvers with higher priority will be used among matched + * resolvers. + */ + public FactoryResolverPriority priority() { + return FactoryResolverPriority.DEFAULT; + } + + /** + * Finds and returns devfile override parameters in general factory parameters map. + * + * @param factoryParameters map containing factory data parameters provided through URL + * @return filtered devfile values override map + */ + protected Map extractOverrideParams(Map factoryParameters) { + String overridePrefix = "override."; + return factoryParameters.entrySet().stream() + .filter(e -> e.getKey().startsWith(overridePrefix)) + .collect(toMap(e -> e.getKey().substring(overridePrefix.length()), Map.Entry::getValue)); + } + + /** + * If devfile has no projects, put there one provided by given `projectSupplier`. Otherwise update + * all projects with given `projectModifier`. + * + * @param devfile of the projects to update + * @param projectSupplier provides default project + * @param projectModifier updates existing projects + */ + protected void updateProjects( + DevfileDto devfile, + Supplier projectSupplier, + Consumer projectModifier) { + List projects = devfile.getProjects(); + if (projects.isEmpty()) { + devfile.setProjects(Collections.singletonList(projectSupplier.get())); + } else { + // update existing project with same repository, set current branch if needed + projects.forEach(projectModifier); + } + } +} diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/FactoryParametersResolver.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/FactoryParametersResolver.java index d29e982e437..ea2732cc7a9 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/FactoryParametersResolver.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/FactoryParametersResolver.java @@ -11,20 +11,12 @@ */ package org.eclipse.che.api.factory.server; -import static java.util.stream.Collectors.toMap; - import jakarta.validation.constraints.NotNull; -import java.util.Collections; -import java.util.List; import java.util.Map; -import java.util.function.Consumer; -import java.util.function.Supplier; import org.eclipse.che.api.core.ApiException; import org.eclipse.che.api.core.BadRequestException; import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl; import org.eclipse.che.api.factory.shared.dto.FactoryMetaDto; -import org.eclipse.che.api.workspace.shared.dto.devfile.DevfileDto; -import org.eclipse.che.api.workspace.shared.dto.devfile.ProjectDto; /** * Defines a resolver that will produce factories for some parameters @@ -41,6 +33,9 @@ public interface FactoryParametersResolver { */ boolean accept(@NotNull Map factoryParameters); + /** Returns the name of the provider */ + String getProviderName(); + /** * Create factory object based on provided parameters * @@ -62,41 +57,5 @@ public interface FactoryParametersResolver { * Returns priority of the resolver. Resolvers with higher priority will be used among matched * resolvers. */ - default FactoryResolverPriority priority() { - return FactoryResolverPriority.DEFAULT; - } - - /** - * Finds and returns devfile override parameters in general factory parameters map. - * - * @param factoryParameters map containing factory data parameters provided through URL - * @return filtered devfile values override map - */ - default Map extractOverrideParams(Map factoryParameters) { - String overridePrefix = "override."; - return factoryParameters.entrySet().stream() - .filter(e -> e.getKey().startsWith(overridePrefix)) - .collect(toMap(e -> e.getKey().substring(overridePrefix.length()), Map.Entry::getValue)); - } - - /** - * If devfile has no projects, put there one provided by given `projectSupplier`. Otherwise update - * all projects with given `projectModifier`. - * - * @param devfile of the projects to update - * @param projectSupplier provides default project - * @param projectModifier updates existing projects - */ - default void updateProjects( - DevfileDto devfile, - Supplier projectSupplier, - Consumer projectModifier) { - List projects = devfile.getProjects(); - if (projects.isEmpty()) { - devfile.setProjects(Collections.singletonList(projectSupplier.get())); - } else { - // update existing project with same repository, set current branch if needed - projects.forEach(projectModifier); - } - } + FactoryResolverPriority priority(); } diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/FactoryService.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/FactoryService.java index 6a5b030d3ae..1008711cf99 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/FactoryService.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/FactoryService.java @@ -42,6 +42,7 @@ import org.eclipse.che.api.factory.server.scm.exception.UnknownScmProviderException; import org.eclipse.che.api.factory.server.scm.exception.UnsatisfiedScmPreconditionException; import org.eclipse.che.api.factory.shared.dto.FactoryMetaDto; +import org.eclipse.che.security.oauth.AuthorisationRequestManager; /** * Defines Factory REST API. @@ -64,17 +65,20 @@ public class FactoryService extends Service { private final FactoryParametersResolverHolder factoryParametersResolverHolder; private final AdditionalFilenamesProvider additionalFilenamesProvider; private final PersonalAccessTokenManager personalAccessTokenManager; + private final AuthorisationRequestManager authorisationRequestManager; @Inject public FactoryService( FactoryAcceptValidator acceptValidator, FactoryParametersResolverHolder factoryParametersResolverHolder, AdditionalFilenamesProvider additionalFilenamesProvider, - PersonalAccessTokenManager personalAccessTokenManager) { + PersonalAccessTokenManager personalAccessTokenManager, + AuthorisationRequestManager authorisationRequestManager) { this.acceptValidator = acceptValidator; this.factoryParametersResolverHolder = factoryParametersResolverHolder; this.additionalFilenamesProvider = additionalFilenamesProvider; this.personalAccessTokenManager = personalAccessTokenManager; + this.authorisationRequestManager = authorisationRequestManager; } @POST @@ -147,8 +151,10 @@ public void refreshToken(@Parameter(description = "Factory url") @QueryParam("ur FactoryParametersResolver factoryParametersResolver = factoryParametersResolverHolder.getFactoryParametersResolver( singletonMap(URL_PARAMETER_NAME, url)); - personalAccessTokenManager.getAndStore( - factoryParametersResolver.parseFactoryUrl(url).getHostName()); + if (!authorisationRequestManager.isStored(factoryParametersResolver.getProviderName())) { + personalAccessTokenManager.getAndStore( + factoryParametersResolver.parseFactoryUrl(url).getHostName()); + } } catch (ScmCommunicationException | ScmConfigurationPersistenceException | UnknownScmProviderException diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/RawDevfileUrlFactoryParameterResolver.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/RawDevfileUrlFactoryParameterResolver.java index 50e5147cbe8..89ccea9aecd 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/RawDevfileUrlFactoryParameterResolver.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/RawDevfileUrlFactoryParameterResolver.java @@ -36,7 +36,10 @@ * {@link FactoryParametersResolver} implementation to resolve factory based on url parameter as a * direct URL to a devfile content. Extracts and applies devfile values override parameters. */ -public class RawDevfileUrlFactoryParameterResolver implements FactoryParametersResolver { +public class RawDevfileUrlFactoryParameterResolver extends BaseFactoryParameterResolver + implements FactoryParametersResolver { + + private static final String PROVIDER_NAME = "raw-devfile-url"; protected final URLFactoryBuilder urlFactoryBuilder; protected final URLFetcher urlFetcher; @@ -44,6 +47,7 @@ public class RawDevfileUrlFactoryParameterResolver implements FactoryParametersR @Inject public RawDevfileUrlFactoryParameterResolver( URLFactoryBuilder urlFactoryBuilder, URLFetcher urlFetcher) { + super(null, urlFactoryBuilder, PROVIDER_NAME); this.urlFactoryBuilder = urlFactoryBuilder; this.urlFetcher = urlFetcher; } @@ -61,6 +65,11 @@ public boolean accept(Map factoryParameters) { return !isNullOrEmpty(url) && (url.endsWith(".yaml") || url.endsWith(".yml")); } + @Override + public String getProviderName() { + return PROVIDER_NAME; + } + /** * Creates factory based on provided parameters. Presumes url parameter as direct URL to a devfile * content. diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AbstractGitUserDataFetcher.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AbstractGitUserDataFetcher.java index a5a25ed403a..d42eab5606a 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AbstractGitUserDataFetcher.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AbstractGitUserDataFetcher.java @@ -17,6 +17,7 @@ import org.eclipse.che.api.factory.server.scm.exception.*; import org.eclipse.che.commons.env.EnvironmentContext; import org.eclipse.che.commons.subject.Subject; +import org.eclipse.che.security.oauth.OAuthAPI; /** * Abstraction to fetch git user data from the specific git provider using OAuth 2.0 or personal @@ -27,12 +28,12 @@ public abstract class AbstractGitUserDataFetcher implements GitUserDataFetcher { protected final String oAuthProviderName; protected final PersonalAccessTokenManager personalAccessTokenManager; - protected final OAuthTokenFetcher oAuthTokenFetcher; + protected final OAuthAPI oAuthTokenFetcher; public AbstractGitUserDataFetcher( String oAuthProviderName, PersonalAccessTokenManager personalAccessTokenManager, - OAuthTokenFetcher oAuthTokenFetcher) { + OAuthAPI oAuthTokenFetcher) { this.oAuthProviderName = oAuthProviderName; this.personalAccessTokenManager = personalAccessTokenManager; this.oAuthTokenFetcher = oAuthTokenFetcher; diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/OAuthTokenFetcher.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/OAuthTokenFetcher.java deleted file mode 100644 index 833fd926266..00000000000 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/OAuthTokenFetcher.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright (c) 2012-2023 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 org.eclipse.che.api.auth.shared.dto.OAuthToken; -import org.eclipse.che.api.core.*; - -/** - * OAuth 2.0 token fetcher is designed to use a dependency in che-core-api-factory module. It is - * needed to avoid circular dependency between che-core-api-factory and che-core-api-auth modules. - * - * @author Anatolii Bazko - */ -public interface OAuthTokenFetcher { - - /** - * Fetches OAuth token for the given OAuth provider name. - * - * @param oAuthProviderName OAuth provider name (e.g. github, bitbucket) - * @return OAuth token - */ - OAuthToken getToken(String oAuthProviderName) - throws NotFoundException, UnauthorizedException, ServerException, ForbiddenException, - BadRequestException, ConflictException; -} diff --git a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/BaseFactoryParameterResolverTest.java b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/BaseFactoryParameterResolverTest.java new file mode 100644 index 00000000000..4993faeb9f3 --- /dev/null +++ b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/BaseFactoryParameterResolverTest.java @@ -0,0 +1,76 @@ +/* + * Copyright (c) 2012-2023 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; + +import static java.util.Collections.emptyMap; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +import java.util.Map; +import org.eclipse.che.api.factory.server.urlfactory.URLFactoryBuilder; +import org.eclipse.che.security.oauth.AuthorisationRequestManager; +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 BaseFactoryParameterResolverTest { + + @Mock private AuthorisationRequestManager authorisationRequestManager; + @Mock private URLFactoryBuilder urlFactoryBuilder; + + private static final String PROVIDER_NAME = "test"; + + private BaseFactoryParameterResolver baseFactoryParameterResolver; + + @BeforeMethod + protected void init() throws Exception { + baseFactoryParameterResolver = + new BaseFactoryParameterResolver( + authorisationRequestManager, urlFactoryBuilder, PROVIDER_NAME); + } + + @Test + public void shouldReturnFalseOnGetSkipAuthorisation() { + // given + when(authorisationRequestManager.isStored(eq(PROVIDER_NAME))).thenReturn(false); + // when + boolean result = baseFactoryParameterResolver.getSkipAuthorisation(emptyMap()); + // then + assertFalse(result); + } + + @Test + public void shouldReturnTrueOnGetSkipAuthorisation() { + // given + when(authorisationRequestManager.isStored(eq(PROVIDER_NAME))).thenReturn(true); + // when + boolean result = baseFactoryParameterResolver.getSkipAuthorisation(emptyMap()); + // then + assertTrue(result); + } + + @Test + public void shouldReturnTrueOnGetSkipAuthorisationFromFactoryParams() { + // given + when(authorisationRequestManager.isStored(eq(PROVIDER_NAME))).thenReturn(false); + // when + boolean result = + baseFactoryParameterResolver.getSkipAuthorisation(Map.of("error_code", "access_denied")); + // then + assertTrue(result); + } +} diff --git a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/FactoryServiceTest.java b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/FactoryServiceTest.java index 44046b990f8..56d55718812 100644 --- a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/FactoryServiceTest.java +++ b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/FactoryServiceTest.java @@ -32,6 +32,7 @@ import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -62,6 +63,7 @@ import org.eclipse.che.commons.env.EnvironmentContext; import org.eclipse.che.commons.subject.SubjectImpl; import org.eclipse.che.dto.server.DtoFactory; +import org.eclipse.che.security.oauth.AuthorisationRequestManager; import org.everrest.assured.EverrestJetty; import org.everrest.core.Filter; import org.everrest.core.GenericContainerRequest; @@ -99,6 +101,7 @@ public class FactoryServiceTest { @Mock private AdditionalFilenamesProvider additionalFilenamesProvider; @Mock private RawDevfileUrlFactoryParameterResolver rawDevfileUrlFactoryParameterResolver; @Mock private PersonalAccessTokenManager personalAccessTokenManager; + @Mock private AuthorisationRequestManager authorisationRequestManager; @InjectMocks private FactoryParametersResolverHolder factoryParametersResolverHolder; private Set specificFactoryParametersResolvers; @@ -136,7 +139,8 @@ public void setUp() throws Exception { acceptValidator, factoryParametersResolverHolder, additionalFilenamesProvider, - personalAccessTokenManager); + personalAccessTokenManager, + authorisationRequestManager); } @Filter @@ -157,7 +161,11 @@ public void shouldThrowBadRequestWhenNoURLParameterGiven() throws Exception { // service instance with dummy holder service = new FactoryService( - acceptValidator, dummyHolder, additionalFilenamesProvider, personalAccessTokenManager); + acceptValidator, + dummyHolder, + additionalFilenamesProvider, + personalAccessTokenManager, + authorisationRequestManager); // when final Map map = new HashMap<>(); @@ -184,7 +192,11 @@ public void checkValidateResolver() throws Exception { // service instance with dummy holder service = new FactoryService( - acceptValidator, dummyHolder, additionalFilenamesProvider, personalAccessTokenManager); + acceptValidator, + dummyHolder, + additionalFilenamesProvider, + personalAccessTokenManager, + authorisationRequestManager); // invalid factory final String invalidFactoryMessage = "invalid factory"; @@ -232,7 +244,11 @@ public void checkRefreshToken() throws Exception { doReturn(factoryParametersResolver).when(dummyHolder).getFactoryParametersResolver(anyMap()); service = new FactoryService( - acceptValidator, dummyHolder, additionalFilenamesProvider, personalAccessTokenManager); + acceptValidator, + dummyHolder, + additionalFilenamesProvider, + personalAccessTokenManager, + authorisationRequestManager); // when given() @@ -245,6 +261,32 @@ public void checkRefreshToken() throws Exception { verify(personalAccessTokenManager).getAndStore(eq("hostName")); } + @Test + public void shouldNotRefreshTokenIfAuthorisationRejected() throws Exception { + // given + final FactoryParametersResolverHolder dummyHolder = spy(factoryParametersResolverHolder); + FactoryParametersResolver factoryParametersResolver = mock(FactoryParametersResolver.class); + doReturn(factoryParametersResolver).when(dummyHolder).getFactoryParametersResolver(anyMap()); + when(authorisationRequestManager.isStored(any())).thenReturn(true); + service = + new FactoryService( + acceptValidator, + dummyHolder, + additionalFilenamesProvider, + personalAccessTokenManager, + authorisationRequestManager); + + // when + given() + .contentType(ContentType.JSON) + .when() + .queryParam("url", "someUrl") + .post(SERVICE_PATH + "/token/refresh"); + + // then + verify(personalAccessTokenManager, never()).getAndStore(eq("hostName")); + } + @Test public void shouldThrowBadRequestWhenRefreshTokenWithoutUrl() throws Exception { service = @@ -252,7 +294,8 @@ public void shouldThrowBadRequestWhenRefreshTokenWithoutUrl() throws Exception { acceptValidator, factoryParametersResolverHolder, additionalFilenamesProvider, - personalAccessTokenManager); + personalAccessTokenManager, + authorisationRequestManager); // when final Response response =