From 5d8658280f4aa8ffab5cd524b443d0e28a914f1d Mon Sep 17 00:00:00 2001 From: Igor Vinokur Date: Tue, 21 Feb 2023 16:30:52 +0200 Subject: [PATCH 1/5] Support Basic authentication for devfile factory URL Signed-off-by: Igor Vinokur --- .../bitbucket/BitbucketServerURLParser.java | 2 +- .../server/bitbucket/BitbucketServerUrl.java | 4 +- .../server/bitbucket/BitbucketURLParser.java | 3 +- .../server/bitbucket/BitbucketUrl.java | 4 +- .../server/github/GithubURLParser.java | 3 +- .../api/factory/server/github/GithubUrl.java | 4 +- .../api/factory/server/gitlab/GitlabUrl.java | 4 +- .../server/gitlab/GitlabUrlParser.java | 2 +- .../DefaultFactoryParameterResolver.java | 4 +- .../scm/AuthorizingFileContentProvider.java | 35 ++++++++++-- .../server/urlfactory/DefaultFactoryUrl.java | 40 +++++++++++++- .../server/urlfactory/RemoteFactoryUrl.java | 5 +- .../server/urlfactory/URLFactoryBuilder.java | 3 +- .../DefaultFactoryParameterResolverTest.java | 6 +- .../urlfactory/DefaultFactoryUrlTest.java | 55 +++++++++++++++++++ .../urlfactory/URLFactoryBuilderTest.java | 32 +++++++---- .../server/devfile/FileContentProvider.java | 25 +++++++-- .../workspace/server/devfile/URLFetcher.java | 11 +++- .../devfile/URLFileContentProvider.java | 21 +++++-- .../devfile/URLFileContentProviderTest.java | 7 ++- 20 files changed, 221 insertions(+), 49 deletions(-) create mode 100644 wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java diff --git a/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerURLParser.java b/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerURLParser.java index 782e91776e1..e1828820d3e 100644 --- a/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerURLParser.java +++ b/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerURLParser.java @@ -169,7 +169,7 @@ public BitbucketServerUrl parse(String url) { format( "The given url %s is not a valid Bitbucket server URL. Check either URL or server configuration.", url))); - return parse(matcher); + return parse(matcher).withUrl(url); } private BitbucketServerUrl parse(Matcher matcher) { diff --git a/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerUrl.java b/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerUrl.java index 26c98db4b22..d965fb0b1e7 100644 --- a/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerUrl.java +++ b/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerUrl.java @@ -19,10 +19,10 @@ import java.util.Optional; import java.util.StringJoiner; import java.util.stream.Collectors; -import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl; +import org.eclipse.che.api.factory.server.urlfactory.DefaultFactoryUrl; /** Representation of a bitbucket Server URL, allowing to get details from it. */ -public class BitbucketServerUrl implements RemoteFactoryUrl { +public class BitbucketServerUrl extends DefaultFactoryUrl { private final String NAME = "bitbucket"; diff --git a/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketURLParser.java b/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketURLParser.java index 6c4494e9d6a..fb70810d699 100644 --- a/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketURLParser.java +++ b/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketURLParser.java @@ -61,6 +61,7 @@ public BitbucketUrl parse(String url) { .withRepository(repoName) .withBranch(branchName) .withWorkspaceId(workspaceId) - .withDevfileFilenames(devfileFilenamesProvider.getConfiguredDevfileFilenames()); + .withDevfileFilenames(devfileFilenamesProvider.getConfiguredDevfileFilenames()) + .withUrl(url); } } diff --git a/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketUrl.java b/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketUrl.java index 962533ac959..3c669b6f450 100644 --- a/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketUrl.java +++ b/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketUrl.java @@ -19,14 +19,14 @@ import java.util.Optional; import java.util.StringJoiner; import java.util.stream.Collectors; -import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl; +import org.eclipse.che.api.factory.server.urlfactory.DefaultFactoryUrl; /** * Representation of a bitbucket URL, allowing to get details from it. * *

like https://@bitbucket.org//.git */ -public class BitbucketUrl implements RemoteFactoryUrl { +public class BitbucketUrl extends DefaultFactoryUrl { private final String NAME = "bitbucket"; diff --git a/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubURLParser.java b/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubURLParser.java index 6f53018c57d..bd71791a6a0 100644 --- a/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubURLParser.java +++ b/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubURLParser.java @@ -174,7 +174,8 @@ private GithubUrl parse(String url, boolean authenticationRequired) throws ApiEx .withBranch(branchName) .withLatestCommit(latestCommit) .withSubfolder(matcher.group("subFolder")) - .withDevfileFilenames(devfileFilenamesProvider.getConfiguredDevfileFilenames()); + .withDevfileFilenames(devfileFilenamesProvider.getConfiguredDevfileFilenames()) + .withUrl(url); } private GithubPullRequest getPullRequest( diff --git a/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubUrl.java b/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubUrl.java index 4ae96649779..d00cd7c7f54 100644 --- a/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubUrl.java +++ b/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubUrl.java @@ -18,7 +18,7 @@ import java.util.Optional; import java.util.StringJoiner; import java.util.stream.Collectors; -import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl; +import org.eclipse.che.api.factory.server.urlfactory.DefaultFactoryUrl; /** * Representation of a github URL, allowing to get details from it. @@ -28,7 +28,7 @@ * * @author Florent Benoit */ -public class GithubUrl implements RemoteFactoryUrl { +public class GithubUrl extends DefaultFactoryUrl { private final String NAME = "github"; diff --git a/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabUrl.java b/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabUrl.java index 222f528dcfa..22edaa74518 100644 --- a/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabUrl.java +++ b/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabUrl.java @@ -20,7 +20,7 @@ import java.util.Optional; import java.util.StringJoiner; import java.util.stream.Collectors; -import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl; +import org.eclipse.che.api.factory.server.urlfactory.DefaultFactoryUrl; /** * Representation of a gitlab URL, allowing to get details from it. @@ -30,7 +30,7 @@ * * @author Max Shaposhnyk */ -public class GitlabUrl implements RemoteFactoryUrl { +public class GitlabUrl extends DefaultFactoryUrl { private final String NAME = "gitlab"; diff --git a/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabUrlParser.java b/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabUrlParser.java index 2609a0bc399..c35eef3f825 100644 --- a/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabUrlParser.java +++ b/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabUrlParser.java @@ -153,7 +153,7 @@ public GitlabUrl parse(String url) { .findFirst() .or(() -> getPatternMatcherByUrl(url)); if (matcherOptional.isPresent()) { - return parse(matcherOptional.get()); + return parse(matcherOptional.get()).withUrl(url); } else { throw new UnsupportedOperationException( "The gitlab integration is not configured properly and cannot be used at this moment." 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 2778100797a..dc136cc01ae 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 @@ -88,7 +88,9 @@ public FactoryMetaDto createFactory(@NotNull final Map factoryPa } return urlFactoryBuilder .createFactoryFromDevfile( - new DefaultFactoryUrl().withDevfileFileLocation(devfileLocation), + new DefaultFactoryUrl() + .withDevfileFileLocation(devfileLocation) + .withUrl(devfileLocation), new URLFileContentProvider(devfileURI, urlFetcher), extractOverrideParams(factoryParameters), false) 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 a30d92348c3..701d12021da 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 @@ -11,10 +11,13 @@ */ package org.eclipse.che.api.factory.server.scm; +import static com.google.common.base.Strings.isNullOrEmpty; + import java.io.FileNotFoundException; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; +import java.util.Base64; import javax.net.ssl.SSLException; import org.eclipse.che.api.factory.server.scm.exception.ScmCommunicationException; import org.eclipse.che.api.factory.server.scm.exception.ScmConfigurationPersistenceException; @@ -25,6 +28,7 @@ import org.eclipse.che.api.workspace.server.devfile.FileContentProvider; import org.eclipse.che.api.workspace.server.devfile.URLFetcher; import org.eclipse.che.api.workspace.server.devfile.exception.DevfileException; +import org.eclipse.che.commons.annotation.Nullable; /** * Common implementation of file content provider which is able to access content of private @@ -48,16 +52,23 @@ public AuthorizingFileContentProvider( @Override public String fetchContent(String fileURL) throws IOException, DevfileException { - return fetchContent(fileURL, false); + return fetchContent(fileURL, false, null); + } + + @Override + public String fetchContent(String fileURL, String credentials) + throws IOException, DevfileException { + return fetchContent(fileURL, false, credentials); } @Override public String fetchContentWithoutAuthentication(String fileURL) throws IOException, DevfileException { - return fetchContent(fileURL, true); + return fetchContent(fileURL, true, null); } - protected String fetchContent(String fileURL, boolean skipAuthentication) + private String fetchContent( + String fileURL, boolean skipAuthentication, @Nullable String credentials) throws IOException, DevfileException { final String requestURL = formatUrl(fileURL); try { @@ -65,9 +76,17 @@ protected String fetchContent(String fileURL, boolean skipAuthentication) return urlFetcher.fetch(requestURL); } else { // try to authenticate for the given URL - PersonalAccessToken token = - personalAccessTokenManager.getAndStore(remoteFactoryUrl.getHostName()); - return urlFetcher.fetch(requestURL, formatAuthorization(token.getToken())); + String authorization; + if (isNullOrEmpty(credentials)) { + authorization = + formatAuthorization( + personalAccessTokenManager + .getAndStore(remoteFactoryUrl.getHostName()) + .getToken()); + } else { + authorization = getCredentialsAuthorization(credentials); + } + return urlFetcher.fetch(requestURL, authorization); } } catch (UnknownScmProviderException e) { return fetchContentWithoutToken(requestURL, e); @@ -143,4 +162,8 @@ protected String formatUrl(String fileURL) throws DevfileException { protected String formatAuthorization(String token) { return "Bearer " + token; } + + private String getCredentialsAuthorization(String credentials) { + return "Basic " + new String(Base64.getEncoder().encode(credentials.getBytes())); + } } diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrl.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrl.java index a4df1a09002..b8f9498f024 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrl.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrl.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/ @@ -11,11 +11,16 @@ */ package org.eclipse.che.api.factory.server.urlfactory; +import static com.google.common.base.Strings.isNullOrEmpty; +import static java.lang.String.format; import static java.util.Collections.singletonList; +import static java.util.regex.Pattern.compile; import java.net.URI; import java.util.List; import java.util.Optional; +import java.util.regex.Matcher; +import org.eclipse.che.commons.annotation.Nullable; /** * Default implementation of {@link RemoteFactoryUrl} which used with all factory URL's until there @@ -24,6 +29,7 @@ public class DefaultFactoryUrl implements RemoteFactoryUrl { private String devfileFileLocation; + private String url; @Override public String getProviderName() { @@ -61,6 +67,38 @@ public String getBranch() { return null; } + @Override + public @Nullable String getCredentials() { + if (isNullOrEmpty(url)) { + return null; + } + Matcher matcher = + compile("https?://((?[^:|@]+)?:?(?[^@]+)?@)?.*").matcher(url); + String password = null; + String username = null; + try { + username = matcher.matches() ? matcher.group("username") : null; + } catch (IllegalArgumentException e) { + // no such group + } + try { + password = matcher.matches() ? matcher.group("password") : null; + } catch (IllegalArgumentException e) { + // no such group + } + if (!isNullOrEmpty(username) || !isNullOrEmpty(password)) { + return format( + "%s:%s", + isNullOrEmpty(username) ? "" : username, isNullOrEmpty(password) ? "" : password); + } + return null; + } + + public T withUrl(String url) { + this.url = url; + return (T) this; + } + public DefaultFactoryUrl withDevfileFileLocation(String devfileFileLocation) { this.devfileFileLocation = devfileFileLocation; return this; diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/RemoteFactoryUrl.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/RemoteFactoryUrl.java index 794cc963d8c..20d69b7b339 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/RemoteFactoryUrl.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/RemoteFactoryUrl.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/ @@ -42,6 +42,9 @@ public interface RemoteFactoryUrl { /** Remote branch */ String getBranch(); + /** Credentials in format : */ + String getCredentials(); + /** Describes devfile location, including filename if any. */ interface DevfileLocation { Optional filename(); 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 4b58f8e1c6c..fe03ea61605 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 @@ -115,7 +115,8 @@ public Optional createFactoryFromDevfile( devfileYamlContent = skipAuthentication ? fileContentProvider.fetchContentWithoutAuthentication(location.location()) - : fileContentProvider.fetchContent(location.location()); + : fileContentProvider.fetchContent( + location.location(), remoteFactoryUrl.getCredentials()); } catch (IOException ex) { // try next location LOG.debug( 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 f2a5f32836a..942f1a8ce90 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 @@ -91,14 +91,14 @@ public void shouldResolveRelativeFiles() throws Exception { // set up our factory with the location of our devfile that is referencing our localfile Map factoryParameters = new HashMap<>(); factoryParameters.put(URL_PARAMETER_NAME, "http://myloc.com/aa/bb/devfile"); - doReturn(DEVFILE).when(urlFetcher).fetch(eq("http://myloc.com/aa/bb/devfile")); - doReturn("localfile").when(urlFetcher).fetch("http://myloc.com/aa/localfile"); + doReturn(DEVFILE).when(urlFetcher).fetch(eq("http://myloc.com/aa/bb/devfile"), eq(null)); + doReturn("localfile").when(urlFetcher).fetch("http://myloc.com/aa/localfile", null); // when res.createFactory(factoryParameters); // then - verify(urlFetcher).fetch(eq("http://myloc.com/aa/localfile")); + verify(urlFetcher).fetch(eq("http://myloc.com/aa/localfile"), eq(null)); } @Test diff --git a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java new file mode 100644 index 00000000000..20ceeff3f3d --- /dev/null +++ b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java @@ -0,0 +1,55 @@ +/* + * 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.urlfactory; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNull; + +import org.mockito.testng.MockitoTestNGListener; +import org.testng.annotations.Listeners; +import org.testng.annotations.Test; + +/** Testing {@link DefaultFactoryUrl} */ +@Listeners(MockitoTestNGListener.class) +public class DefaultFactoryUrlTest { + @Test + public void shouldCredentials() { + // given + DefaultFactoryUrl factoryUrl = + new DefaultFactoryUrl().withUrl("https://username:password@hostname/path"); + // when + String credentials = factoryUrl.getCredentials(); + // then + assertEquals(credentials, "username:password"); + } + + @Test + public void shouldCredentialsWithoutPassword() { + // given + DefaultFactoryUrl factoryUrl = + new DefaultFactoryUrl().withUrl("https://username@hostname/path"); + // when + String credentials = factoryUrl.getCredentials(); + // then + assertEquals(credentials, "username:"); + } + + @Test + public void shouldGetNullCredentials() { + // given + DefaultFactoryUrl factoryUrl = new DefaultFactoryUrl().withUrl("https://hostname/path"); + // when + String credentials = factoryUrl.getCredentials(); + // then + assertNull(credentials); + } +} 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 80158e2c126..c660f577341 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 @@ -21,6 +21,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; @@ -127,12 +128,12 @@ public void checkWithCustomDevfileAndRecipe() throws Exception { .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"); + when(fileContentProvider.fetchContent(anyString(), eq(null))).thenReturn("content"); FactoryMetaDto factory = urlFactoryBuilder .createFactoryFromDevfile( - new DefaultFactoryUrl().withDevfileFileLocation(myLocation), + new DefaultFactoryUrl().withDevfileFileLocation(myLocation).withUrl("url"), fileContentProvider, emptyMap(), false) @@ -152,12 +153,12 @@ public void testDevfileV2() throws ApiException, DevfileException, IOException { when(devfileParser.parseYamlRaw(anyString())).thenReturn(devfile); when(devfileParser.convertYamlToMap(devfile)).thenReturn(devfileAsMap); when(devfileVersionDetector.devfileMajorVersion(devfile)).thenReturn(2); - when(fileContentProvider.fetchContent(anyString())).thenReturn("content"); + when(fileContentProvider.fetchContent(anyString(), eq(null))).thenReturn("content"); FactoryMetaDto factory = urlFactoryBuilder .createFactoryFromDevfile( - new DefaultFactoryUrl().withDevfileFileLocation(myLocation), + new DefaultFactoryUrl().withDevfileFileLocation(myLocation).withUrl("url"), fileContentProvider, emptyMap(), false) @@ -178,7 +179,7 @@ public void testDevfileV2WithFilename() throws ApiException, DevfileException, I when(devfileParser.parseYamlRaw(anyString())).thenReturn(devfile); when(devfileParser.convertYamlToMap(devfile)).thenReturn(devfileAsMap); when(devfileVersionDetector.devfileMajorVersion(devfile)).thenReturn(2); - when(fileContentProvider.fetchContent(anyString())).thenReturn("content"); + when(fileContentProvider.fetchContent(anyString(), eq(null))).thenReturn("content"); RemoteFactoryUrl githubLikeRemoteUrl = new RemoteFactoryUrl() { @@ -218,6 +219,11 @@ public String getBranch() { return null; } + @Override + public String getCredentials() { + return null; + } + @Override public void setDevfileFilename(String devfileName) {} }; @@ -242,7 +248,7 @@ public void testDevfileSpecifyingFilename() throws ApiException, DevfileExceptio when(devfileParser.parseYamlRaw(anyString())).thenReturn(devfile); when(devfileParser.convertYamlToMap(devfile)).thenReturn(devfileAsMap); when(devfileVersionDetector.devfileMajorVersion(devfile)).thenReturn(2); - when(fileContentProvider.fetchContent(anyString())).thenReturn("content"); + when(fileContentProvider.fetchContent(anyString(), eq(null))).thenReturn("content"); RemoteFactoryUrl githubLikeRemoteUrl = new RemoteFactoryUrl() { @@ -285,6 +291,11 @@ public String getBranch() { return null; } + @Override + public String getCredentials() { + return null; + } + @Override public void setDevfileFilename(String devfileName) { this.devfileName = devfileName; @@ -317,7 +328,7 @@ public void testShouldReturnV2WithDevworkspacesDisabled() when(devfileParser.parseYamlRaw(anyString())).thenReturn(devfile); when(devfileParser.convertYamlToMap(devfile)).thenReturn(devfileAsMap); when(devfileVersionDetector.devfileMajorVersion(devfile)).thenReturn(2); - when(fileContentProvider.fetchContent(anyString())).thenReturn("content"); + when(fileContentProvider.fetchContent(anyString(), eq(null))).thenReturn("content"); URLFactoryBuilder localUrlFactoryBuilder = new URLFactoryBuilder( @@ -326,7 +337,7 @@ public void testShouldReturnV2WithDevworkspacesDisabled() FactoryMetaDto factory = localUrlFactoryBuilder .createFactoryFromDevfile( - new DefaultFactoryUrl().withDevfileFileLocation(myLocation), + new DefaultFactoryUrl().withDevfileFileLocation(myLocation).withUrl("url"), fileContentProvider, emptyMap(), false) @@ -381,7 +392,7 @@ public String location() { return "http://foo.bar/anything"; } })); - when(fileContentProvider.fetchContent(anyString())).thenReturn("anything"); + when(fileContentProvider.fetchContent(anyString(), eq(null))).thenReturn("anything"); when(devfileParser.parseYamlRaw("anything")) .thenReturn(new ObjectNode(JsonNodeFactory.instance)); when(devfileParser.parseJsonNode(any(JsonNode.class), anyMap())).thenReturn(devfile); @@ -420,7 +431,8 @@ public String location() { } })); - when(fileContentProvider.fetchContent(anyString())).thenThrow(new DevfileException("", cause)); + when(fileContentProvider.fetchContent(anyString(), eq(null))) + .thenThrow(new DevfileException("", cause)); // when try { 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 ce52dd50360..3d6e663b350 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 @@ -37,6 +37,18 @@ 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 + * @param credentials a string representation of credentials in format: : + * @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 fetchContent(String fileURL, String credentials) throws IOException, DevfileException; + /** * Fetches content of the specified file without the authentication step. * @@ -78,8 +90,7 @@ public CachingProvider(FileContentProvider provider) { this.provider = provider; } - private String fetchContent(String fileURL, boolean skipAuthentication) - throws IOException, DevfileException { + private String fetchContentInternal(String fileURL) throws IOException, DevfileException { SoftReference ref = cache.get(fileURL); String ret = ref == null ? null : ref.get(); @@ -93,13 +104,19 @@ private String fetchContent(String fileURL, boolean skipAuthentication) @Override public String fetchContent(String fileURL) throws IOException, DevfileException { - return fetchContent(fileURL, false); + return fetchContentInternal(fileURL); + } + + @Override + public String fetchContent(String fileURL, String credentials) + throws IOException, DevfileException { + return fetchContentInternal(fileURL); } @Override public String fetchContentWithoutAuthentication(String fileURL) throws IOException, DevfileException { - return fetchContent(fileURL, true); + return fetchContentInternal(fileURL); } } } diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFetcher.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFetcher.java index 9a44ec34eaa..936c0d5b503 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFetcher.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFetcher.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/ @@ -31,6 +31,7 @@ import javax.inject.Inject; import javax.inject.Named; import javax.inject.Singleton; +import org.eclipse.che.commons.annotation.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -91,10 +92,12 @@ public String fetch(@NotNull final String url) throws IOException { * URLFetcher#CONNECTION_READ_TIMEOUT} * * @param url the URL to fetch + * @param authorization Authorisation string or {@code null} * @return content of the requested URL * @throws IOException if fetch error occurs */ - public String fetch(@NotNull final String url, String authorization) throws IOException { + public String fetch(@NotNull final String url, @Nullable String authorization) + throws IOException { return fetch(url, CONNECTION_READ_TIMEOUT, authorization); } @@ -115,12 +118,14 @@ String fetch(@NotNull final String url, int timeout) throws IOException { * Fetches the url provided and return its content. * * @param url the URL to fetch + * @param authorization Authorisation string or {@code null} * @param timeout read and connection timeout (see {@link URLConnection#setConnectTimeout(int)} * and {@link URLConnection#setReadTimeout(int)} * @return content of the requested URL * @throws IOException if fetch error occurs */ - String fetch(@NotNull final String url, int timeout, String authorization) throws IOException { + String fetch(@NotNull final String url, int timeout, @Nullable String authorization) + throws IOException { requireNonNull(url, "url parameter can't be null"); URLConnection connection = new URL(sanitized(url)).openConnection(); connection.setConnectTimeout(timeout); 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 aa8f4fb7f23..0dadfe54c9e 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 @@ -11,11 +11,13 @@ */ package org.eclipse.che.api.workspace.server.devfile; +import static com.google.common.base.Strings.isNullOrEmpty; import static java.lang.String.format; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; +import java.util.Base64; import org.eclipse.che.api.workspace.server.devfile.exception.DevfileException; import org.eclipse.che.commons.annotation.Nullable; @@ -33,7 +35,7 @@ public URLFileContentProvider(@Nullable URI devfileLocation, URLFetcher urlFetch this.urlFetcher = urlFetcher; } - private String fetchContent(String fileURL, boolean skipAuthentication) + private String fetchContentInternal(String fileURL, @Nullable String credentials) throws IOException, DevfileException { URI fileURI; String requestURL; @@ -56,7 +58,8 @@ private String fetchContent(String fileURL, boolean skipAuthentication) requestURL = devfileLocation.resolve(fileURI).toString(); } try { - return urlFetcher.fetch(requestURL); + return urlFetcher.fetch( + requestURL, isNullOrEmpty(credentials) ? null : getCredentialsAuthorization(credentials)); } catch (IOException e) { throw new IOException( format( @@ -72,14 +75,24 @@ private String fetchContent(String fileURL, boolean skipAuthentication) } } + private String getCredentialsAuthorization(String credentials) { + return "Basic " + new String(Base64.getEncoder().encode(credentials.getBytes())); + } + @Override public String fetchContent(String fileURL) throws IOException, DevfileException { - return fetchContent(fileURL, false); + return fetchContentInternal(fileURL, null); } @Override public String fetchContentWithoutAuthentication(String fileURL) throws IOException, DevfileException { - return fetchContent(fileURL, true); + return fetchContentInternal(fileURL, null); + } + + @Override + public String fetchContent(String fileURL, String credentials) + throws IOException, DevfileException { + return fetchContentInternal(fileURL, credentials); } } diff --git a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProviderTest.java b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProviderTest.java index c00badbc176..f067c3ed43c 100644 --- a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProviderTest.java +++ b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProviderTest.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/ @@ -11,6 +11,7 @@ */ package org.eclipse.che.api.workspace.server.devfile; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.verify; import static org.testng.Assert.assertEquals; @@ -42,7 +43,7 @@ public void shouldFetchByAbsoluteURL() throws Exception { URLFileContentProvider provider = new URLFileContentProvider(null, urlFetcher); ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); provider.fetchContent(url); - verify(urlFetcher).fetch(captor.capture()); + verify(urlFetcher).fetch(captor.capture(), eq(null)); assertEquals(captor.getValue(), url); } @@ -53,7 +54,7 @@ public void shouldMergeDevfileLocationAndRelativeURL() throws Exception { URLFileContentProvider provider = new URLFileContentProvider(new URI(devfileUrl), urlFetcher); ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); provider.fetchContent(relativeUrl); - verify(urlFetcher).fetch(captor.capture()); + verify(urlFetcher).fetch(captor.capture(), eq(null)); assertEquals(captor.getValue(), "http://myhost.com/relative/relative.yaml"); } } From 419ec92d65bda1b1a4aec84cca50743e94646224 Mon Sep 17 00:00:00 2001 From: Igor Vinokur Date: Wed, 22 Feb 2023 14:49:19 +0200 Subject: [PATCH 2/5] fixup! Support Basic authentication for devfile factory URL --- .../server/urlfactory/DefaultFactoryUrl.java | 13 +++++----- .../server/urlfactory/RemoteFactoryUrl.java | 4 ++-- .../server/urlfactory/URLFactoryBuilder.java | 22 ++++++++++------- .../urlfactory/DefaultFactoryUrlTest.java | 20 +++++++++------- .../urlfactory/URLFactoryBuilderTest.java | 24 +++++++++---------- 5 files changed, 45 insertions(+), 38 deletions(-) diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrl.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrl.java index b8f9498f024..67b9bca638e 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrl.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrl.java @@ -68,9 +68,9 @@ public String getBranch() { } @Override - public @Nullable String getCredentials() { + public @Nullable Optional getCredentials() { if (isNullOrEmpty(url)) { - return null; + return Optional.empty(); } Matcher matcher = compile("https?://((?[^:|@]+)?:?(?[^@]+)?@)?.*").matcher(url); @@ -87,11 +87,12 @@ public String getBranch() { // no such group } if (!isNullOrEmpty(username) || !isNullOrEmpty(password)) { - return format( - "%s:%s", - isNullOrEmpty(username) ? "" : username, isNullOrEmpty(password) ? "" : password); + return Optional.of( + format( + "%s:%s", + isNullOrEmpty(username) ? "" : username, isNullOrEmpty(password) ? "" : password)); } - return null; + return Optional.empty(); } public T withUrl(String url) { diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/RemoteFactoryUrl.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/RemoteFactoryUrl.java index 20d69b7b339..864496ae42a 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/RemoteFactoryUrl.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/RemoteFactoryUrl.java @@ -42,8 +42,8 @@ public interface RemoteFactoryUrl { /** Remote branch */ String getBranch(); - /** Credentials in format : */ - String getCredentials(); + /** Optional of Credentials in format : */ + Optional getCredentials(); /** Describes devfile location, including filename if any. */ interface DevfileLocation { 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 fe03ea61605..098500b15dc 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 @@ -111,18 +111,22 @@ public Optional createFactoryFromDevfile( } for (DevfileLocation location : remoteFactoryUrl.devfileFileLocations()) { + String devfileLocation = location.location(); try { - devfileYamlContent = - skipAuthentication - ? fileContentProvider.fetchContentWithoutAuthentication(location.location()) - : fileContentProvider.fetchContent( - location.location(), remoteFactoryUrl.getCredentials()); + Optional credentialsOptional = remoteFactoryUrl.getCredentials(); + if (skipAuthentication) { + devfileYamlContent = + fileContentProvider.fetchContentWithoutAuthentication(devfileLocation); + } else if (credentialsOptional.isPresent()) { + devfileYamlContent = + fileContentProvider.fetchContent(devfileLocation, credentialsOptional.get()); + } else { + devfileYamlContent = fileContentProvider.fetchContent(devfileLocation); + } } catch (IOException ex) { // try next location LOG.debug( - "Unreachable devfile location met: {}. Error is: {}", - location.location(), - ex.getMessage()); + "Unreachable devfile location met: {}. Error is: {}", devfileLocation, ex.getMessage()); continue; } catch (DevfileException e) { LOG.debug("Unexpected devfile exception: {}", e.getMessage()); @@ -183,7 +187,7 @@ private FactoryMetaDto createFactory( /** * Creates devfile with only `generateName` and no `name`. We take `generateName` with precedence. * See doc of {@link URLFactoryBuilder#createFactoryFromDevfile(RemoteFactoryUrl, - * FileContentProvider, Map, Boolean)} 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/urlfactory/DefaultFactoryUrlTest.java b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java index 20ceeff3f3d..ccf9716d100 100644 --- a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java +++ b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java @@ -12,8 +12,10 @@ package org.eclipse.che.api.factory.server.urlfactory; import static org.testng.Assert.assertEquals; -import static org.testng.Assert.assertNull; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; +import java.util.Optional; import org.mockito.testng.MockitoTestNGListener; import org.testng.annotations.Listeners; import org.testng.annotations.Test; @@ -27,9 +29,10 @@ public void shouldCredentials() { DefaultFactoryUrl factoryUrl = new DefaultFactoryUrl().withUrl("https://username:password@hostname/path"); // when - String credentials = factoryUrl.getCredentials(); + Optional credentialsOptional = factoryUrl.getCredentials(); // then - assertEquals(credentials, "username:password"); + assertTrue(credentialsOptional.isPresent()); + assertEquals(credentialsOptional.get(), "username:password"); } @Test @@ -38,18 +41,19 @@ public void shouldCredentialsWithoutPassword() { DefaultFactoryUrl factoryUrl = new DefaultFactoryUrl().withUrl("https://username@hostname/path"); // when - String credentials = factoryUrl.getCredentials(); + Optional credentialsOptional = factoryUrl.getCredentials(); // then - assertEquals(credentials, "username:"); + assertTrue(credentialsOptional.isPresent()); + assertEquals(credentialsOptional.get(), "username:"); } @Test - public void shouldGetNullCredentials() { + public void shouldGetEmptyCredentials() { // given DefaultFactoryUrl factoryUrl = new DefaultFactoryUrl().withUrl("https://hostname/path"); // when - String credentials = factoryUrl.getCredentials(); + Optional credentialsOptional = factoryUrl.getCredentials(); // then - assertNull(credentials); + assertFalse(credentialsOptional.isPresent()); } } 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 c660f577341..36cd5966adf 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 @@ -21,7 +21,6 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; @@ -128,7 +127,7 @@ public void checkWithCustomDevfileAndRecipe() throws Exception { .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(), eq(null))).thenReturn("content"); + when(fileContentProvider.fetchContent(anyString())).thenReturn("content"); FactoryMetaDto factory = urlFactoryBuilder @@ -153,7 +152,7 @@ public void testDevfileV2() throws ApiException, DevfileException, IOException { when(devfileParser.parseYamlRaw(anyString())).thenReturn(devfile); when(devfileParser.convertYamlToMap(devfile)).thenReturn(devfileAsMap); when(devfileVersionDetector.devfileMajorVersion(devfile)).thenReturn(2); - when(fileContentProvider.fetchContent(anyString(), eq(null))).thenReturn("content"); + when(fileContentProvider.fetchContent(anyString())).thenReturn("content"); FactoryMetaDto factory = urlFactoryBuilder @@ -179,7 +178,7 @@ public void testDevfileV2WithFilename() throws ApiException, DevfileException, I when(devfileParser.parseYamlRaw(anyString())).thenReturn(devfile); when(devfileParser.convertYamlToMap(devfile)).thenReturn(devfileAsMap); when(devfileVersionDetector.devfileMajorVersion(devfile)).thenReturn(2); - when(fileContentProvider.fetchContent(anyString(), eq(null))).thenReturn("content"); + when(fileContentProvider.fetchContent(anyString())).thenReturn("content"); RemoteFactoryUrl githubLikeRemoteUrl = new RemoteFactoryUrl() { @@ -220,8 +219,8 @@ public String getBranch() { } @Override - public String getCredentials() { - return null; + public Optional getCredentials() { + return Optional.empty(); } @Override @@ -248,7 +247,7 @@ public void testDevfileSpecifyingFilename() throws ApiException, DevfileExceptio when(devfileParser.parseYamlRaw(anyString())).thenReturn(devfile); when(devfileParser.convertYamlToMap(devfile)).thenReturn(devfileAsMap); when(devfileVersionDetector.devfileMajorVersion(devfile)).thenReturn(2); - when(fileContentProvider.fetchContent(anyString(), eq(null))).thenReturn("content"); + when(fileContentProvider.fetchContent(anyString())).thenReturn("content"); RemoteFactoryUrl githubLikeRemoteUrl = new RemoteFactoryUrl() { @@ -292,8 +291,8 @@ public String getBranch() { } @Override - public String getCredentials() { - return null; + public Optional getCredentials() { + return Optional.empty(); } @Override @@ -328,7 +327,7 @@ public void testShouldReturnV2WithDevworkspacesDisabled() when(devfileParser.parseYamlRaw(anyString())).thenReturn(devfile); when(devfileParser.convertYamlToMap(devfile)).thenReturn(devfileAsMap); when(devfileVersionDetector.devfileMajorVersion(devfile)).thenReturn(2); - when(fileContentProvider.fetchContent(anyString(), eq(null))).thenReturn("content"); + when(fileContentProvider.fetchContent(anyString())).thenReturn("content"); URLFactoryBuilder localUrlFactoryBuilder = new URLFactoryBuilder( @@ -392,7 +391,7 @@ public String location() { return "http://foo.bar/anything"; } })); - when(fileContentProvider.fetchContent(anyString(), eq(null))).thenReturn("anything"); + when(fileContentProvider.fetchContent(anyString())).thenReturn("anything"); when(devfileParser.parseYamlRaw("anything")) .thenReturn(new ObjectNode(JsonNodeFactory.instance)); when(devfileParser.parseJsonNode(any(JsonNode.class), anyMap())).thenReturn(devfile); @@ -431,8 +430,7 @@ public String location() { } })); - when(fileContentProvider.fetchContent(anyString(), eq(null))) - .thenThrow(new DevfileException("", cause)); + when(fileContentProvider.fetchContent(anyString())).thenThrow(new DevfileException("", cause)); // when try { From 27452304d50f15cea9927a8ddf0ccc6097f62e42 Mon Sep 17 00:00:00 2001 From: Igor Vinokur Date: Wed, 22 Feb 2023 15:24:41 +0200 Subject: [PATCH 3/5] fixup! Support Basic authentication for devfile factory URL --- .../urlfactory/DefaultFactoryUrlTest.java | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java index ccf9716d100..794e3473788 100644 --- a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java +++ b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java @@ -17,34 +17,38 @@ import java.util.Optional; import org.mockito.testng.MockitoTestNGListener; +import org.testng.annotations.DataProvider; import org.testng.annotations.Listeners; import org.testng.annotations.Test; /** Testing {@link DefaultFactoryUrl} */ @Listeners(MockitoTestNGListener.class) public class DefaultFactoryUrlTest { - @Test - public void shouldCredentials() { + @Test(dataProvider = "urlsProvider") + public void shouldGetCredentials(String url, String credentials) { // given - DefaultFactoryUrl factoryUrl = - new DefaultFactoryUrl().withUrl("https://username:password@hostname/path"); + DefaultFactoryUrl factoryUrl = new DefaultFactoryUrl().withUrl(url); // when Optional credentialsOptional = factoryUrl.getCredentials(); // then assertTrue(credentialsOptional.isPresent()); - assertEquals(credentialsOptional.get(), "username:password"); + assertEquals(credentialsOptional.get(), credentials); } - @Test - public void shouldCredentialsWithoutPassword() { - // given - DefaultFactoryUrl factoryUrl = - new DefaultFactoryUrl().withUrl("https://username@hostname/path"); - // when - Optional credentialsOptional = factoryUrl.getCredentials(); - // then - assertTrue(credentialsOptional.isPresent()); - assertEquals(credentialsOptional.get(), "username:"); + @DataProvider(name = "urlsProvider") + private Object[][] urlsProvider() { + return new Object[][] { + {"https://username:password@hostname/path", "username:password"}, + {"https://token@hostname/path/user/repo/", "token:"}, + {"http://token@hostname/path/user/repo/", "token:"}, + {"https://token@dev.azure.com/user/repo/", "token:"}, + { + "https://personal-access-token@raw.githubusercontent.com/user/repo/branch/.devfile.yaml", + "personal-access-token:" + }, + {"https://token@gitlub.com/user/repo/", "token:"}, + {"https://token@bitbucket.org/user/repo/", "token:"}, + }; } @Test From 81cceea9c28ceb19f1ceb8130278eb2b69215e03 Mon Sep 17 00:00:00 2001 From: Igor Vinokur Date: Wed, 22 Feb 2023 17:06:41 +0200 Subject: [PATCH 4/5] fixup! Support Basic authentication for devfile factory URL --- .../server/urlfactory/DefaultFactoryUrl.java | 39 ++++++++----------- .../server/urlfactory/RemoteFactoryUrl.java | 2 +- .../urlfactory/DefaultFactoryUrlTest.java | 6 +-- 3 files changed, 20 insertions(+), 27 deletions(-) diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrl.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrl.java index 67b9bca638e..86b3674c888 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrl.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrl.java @@ -14,13 +14,12 @@ import static com.google.common.base.Strings.isNullOrEmpty; import static java.lang.String.format; import static java.util.Collections.singletonList; -import static java.util.regex.Pattern.compile; +import java.net.MalformedURLException; import java.net.URI; +import java.net.URL; import java.util.List; import java.util.Optional; -import java.util.regex.Matcher; -import org.eclipse.che.commons.annotation.Nullable; /** * Default implementation of {@link RemoteFactoryUrl} which used with all factory URL's until there @@ -29,7 +28,7 @@ public class DefaultFactoryUrl implements RemoteFactoryUrl { private String devfileFileLocation; - private String url; + private URL url; @Override public String getProviderName() { @@ -68,24 +67,14 @@ public String getBranch() { } @Override - public @Nullable Optional getCredentials() { - if (isNullOrEmpty(url)) { + public Optional getCredentials() { + if (url == null || isNullOrEmpty(url.getUserInfo())) { return Optional.empty(); } - Matcher matcher = - compile("https?://((?[^:|@]+)?:?(?[^@]+)?@)?.*").matcher(url); - String password = null; - String username = null; - try { - username = matcher.matches() ? matcher.group("username") : null; - } catch (IllegalArgumentException e) { - // no such group - } - try { - password = matcher.matches() ? matcher.group("password") : null; - } catch (IllegalArgumentException e) { - // no such group - } + String userInfo = url.getUserInfo(); + String[] credentials = userInfo.split(":"); + String username = credentials[0]; + String password = credentials.length == 2 ? credentials[1] : null; if (!isNullOrEmpty(username) || !isNullOrEmpty(password)) { return Optional.of( format( @@ -95,9 +84,13 @@ public String getBranch() { return Optional.empty(); } - public T withUrl(String url) { - this.url = url; - return (T) this; + public U withUrl(String url) { + try { + this.url = new URL(url); + } catch (MalformedURLException e) { + // Do nothing, wrong URL. + } + return (U) this; } public DefaultFactoryUrl withDevfileFileLocation(String devfileFileLocation) { diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/RemoteFactoryUrl.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/RemoteFactoryUrl.java index 864496ae42a..213be2cbd23 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/RemoteFactoryUrl.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/RemoteFactoryUrl.java @@ -42,7 +42,7 @@ public interface RemoteFactoryUrl { /** Remote branch */ String getBranch(); - /** Optional of Credentials in format : */ + /** Optional of credentials in format : or : */ Optional getCredentials(); /** Describes devfile location, including filename if any. */ diff --git a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java index 794e3473788..0d27af89952 100644 --- a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java +++ b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java @@ -42,12 +42,12 @@ private Object[][] urlsProvider() { {"https://token@hostname/path/user/repo/", "token:"}, {"http://token@hostname/path/user/repo/", "token:"}, {"https://token@dev.azure.com/user/repo/", "token:"}, + {"https://token@gitlub.com/user/repo/", "token:"}, + {"https://token@bitbucket.org/user/repo/", "token:"}, { "https://personal-access-token@raw.githubusercontent.com/user/repo/branch/.devfile.yaml", "personal-access-token:" - }, - {"https://token@gitlub.com/user/repo/", "token:"}, - {"https://token@bitbucket.org/user/repo/", "token:"}, + } }; } From 25474b5d5f37428ce15addcbf9fb73ce52db7bb0 Mon Sep 17 00:00:00 2001 From: Igor Vinokur Date: Thu, 23 Feb 2023 09:54:22 +0200 Subject: [PATCH 5/5] Update wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java Co-authored-by: Ilya Buziuk --- .../che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java index 0d27af89952..3d3010d9f88 100644 --- a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java +++ b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/DefaultFactoryUrlTest.java @@ -42,6 +42,7 @@ private Object[][] urlsProvider() { {"https://token@hostname/path/user/repo/", "token:"}, {"http://token@hostname/path/user/repo/", "token:"}, {"https://token@dev.azure.com/user/repo/", "token:"}, + {"https://token@dev.azure.com/user/repo?a=&b=b&c=/.devfile.yaml&api-version=7.0", "token:"}, {"https://token@gitlub.com/user/repo/", "token:"}, {"https://token@bitbucket.org/user/repo/", "token:"}, {