From d6cce806ca630344027cdbf514b661d2f4d0ff4f Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Mon, 17 Jan 2022 17:09:48 +0200 Subject: [PATCH] Fix behavior for unexisting files for public repositories (#243) Signed-off-by: Max Shaposhnik --- .../GitHubOAuthAuthenticatorProvider.java | 6 +++--- .../GithubAuthorizingFileContentProvider.java | 18 +++++++++++++++++- ...thubAuthorizingFileContentProviderTest.java | 17 ++++++++++++++++- .../scm/AuthorizingFileContentProvider.java | 14 ++++++++++++-- 4 files changed, 48 insertions(+), 7 deletions(-) diff --git a/wsmaster/che-core-api-auth-github/src/main/java/org/eclipse/che/security/oauth/GitHubOAuthAuthenticatorProvider.java b/wsmaster/che-core-api-auth-github/src/main/java/org/eclipse/che/security/oauth/GitHubOAuthAuthenticatorProvider.java index 4a101737eee..c975fdd219f 100644 --- a/wsmaster/che-core-api-auth-github/src/main/java/org/eclipse/che/security/oauth/GitHubOAuthAuthenticatorProvider.java +++ b/wsmaster/che-core-api-auth-github/src/main/java/org/eclipse/che/security/oauth/GitHubOAuthAuthenticatorProvider.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -70,8 +70,8 @@ private OAuthAuthenticator getOAuthAuthenticator( && !isNullOrEmpty(tokenUri) && Objects.nonNull(redirectUris) && redirectUris.length != 0) { - String clientId = Files.readString(Path.of(clientIdPath)); - String clientSecret = Files.readString(Path.of(clientSecretPath)); + final String clientId = Files.readString(Path.of(clientIdPath)).trim(); + final String clientSecret = Files.readString(Path.of(clientSecretPath)).trim(); if (!isNullOrEmpty(clientId) && !isNullOrEmpty(clientSecret)) { return new GitHubOAuthAuthenticator( clientId, clientSecret, redirectUris, authUri, tokenUri); diff --git a/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProvider.java b/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProvider.java index d14b0f8c5bd..e2036a427e7 100644 --- a/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProvider.java +++ b/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProvider.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -11,6 +11,7 @@ */ package org.eclipse.che.api.factory.server.github; +import java.io.IOException; import org.eclipse.che.api.factory.server.scm.AuthorizingFileContentProvider; import org.eclipse.che.api.factory.server.scm.GitCredentialManager; import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; @@ -37,4 +38,19 @@ class GithubAuthorizingFileContentProvider extends AuthorizingFileContentProvide protected String formatAuthorization(String token) { return "token " + token; } + + @Override + protected boolean isPublicRepository(GithubUrl remoteFactoryUrl) { + try { + urlFetcher.fetch( + GithubApiClient.GITHUB_API_SERVER + + "/repos/" + + remoteFactoryUrl.getUsername() + + "/" + + remoteFactoryUrl.getRepository()); + return true; + } catch (IOException e) { + return false; + } + } } diff --git a/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProviderTest.java b/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProviderTest.java index 6704f678a0c..af96077cbf1 100644 --- a/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProviderTest.java +++ b/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProviderTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -13,7 +13,9 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import java.io.FileNotFoundException; import org.eclipse.che.api.factory.server.scm.GitCredentialManager; import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; import org.eclipse.che.api.workspace.server.devfile.FileContentProvider; @@ -53,4 +55,17 @@ public void shouldPreserveAbsolutePaths() throws Exception { fileContentProvider.fetchContent(url); verify(urlFetcher).fetch(eq(url)); } + + @Test(expectedExceptions = FileNotFoundException.class) + public void shouldThrowNotFoundForPublicRepos() throws Exception { + URLFetcher urlFetcher = Mockito.mock(URLFetcher.class); + String url = "https://raw.githubusercontent.com/foo/bar/devfile.yaml"; + when(urlFetcher.fetch(eq(url))).thenThrow(FileNotFoundException.class); + when(urlFetcher.fetch(eq("https://api.github.com/repos/eclipse/che"))).thenReturn("OK"); + GithubUrl githubUrl = new GithubUrl().withUsername("eclipse").withRepository("che"); + FileContentProvider fileContentProvider = + new GithubAuthorizingFileContentProvider( + githubUrl, urlFetcher, gitCredentialManager, personalAccessTokenManager); + fileContentProvider.fetchContent(url); + } } 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 a245bd404ac..7d8f8c9556a 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 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -11,6 +11,7 @@ */ package org.eclipse.che.api.factory.server.scm; +import java.io.FileNotFoundException; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; @@ -35,9 +36,9 @@ public class AuthorizingFileContentProvider implements FileContentProvider { private final T remoteFactoryUrl; - private final URLFetcher urlFetcher; private final PersonalAccessTokenManager personalAccessTokenManager; private final GitCredentialManager gitCredentialManager; + protected final URLFetcher urlFetcher; public AuthorizingFileContentProvider( T remoteFactoryUrl, @@ -74,6 +75,11 @@ public String fetchContent(String fileURL) throws IOException, DevfileException "Failed to fetch a content from URL %s due to TLS key misconfiguration. Please refer to the docs about how to correctly import it. ", requestURL)); throw new DevfileException(exception.getMessage(), cause); + } else if (exception instanceof FileNotFoundException) { + if (isPublicRepository(remoteFactoryUrl)) { + // for public repo-s return 404 as-is + throw exception; + } } // unable to determine exact cause, so let's just try to authorize... try { @@ -109,6 +115,10 @@ public String fetchContent(String fileURL) throws IOException, DevfileException } } + protected boolean isPublicRepository(T remoteFactoryUrl) { + return false; + } + private String formatUrl(String fileURL) throws DevfileException { String requestURL; try {