Fix behavior for unexisting files for public repositories (#243)

Signed-off-by: Max Shaposhnik <mshaposh@redhat.com>
eclipse-che · Jan 17, 2022 · d6cce80 · d6cce80
1 parent 8de18ae
commit d6cce80
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 7 deletions.
diff --git a/...github/src/main/java/org/eclipse/che/security/oauth/GitHubOAuthAuthenticatorProvider.java b/...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/.../java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProvider.java b/.../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/...a/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProviderTest.java b/...a/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/.../src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java b/.../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<T extends RemoteFactoryUrl>
     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 {