fix: improve bitbucket OAuth flow to use more specific error messages

wsmaster/che-core-api-auth-bitbucket/src/main/java/org/eclipse/che/security/oauth1/NoopOAuthAuthenticator.java

@@ -18,32 +18,32 @@
  * integration is configured.
  */
 public class NoopOAuthAuthenticator extends OAuthAuthenticator {
-  protected NoopOAuthAuthenticator() {
+  public NoopOAuthAuthenticator() {
     super(null, null, null, null, null, null, null);
   }
 
   @Override
-  String getOAuthProvider() {
+  public String getOAuthProvider() {
     return "Noop";
   }
 
   @Override
   String getAuthenticateUrl(URL requestUrl, String requestMethod, String signatureMethod)
       throws OAuthAuthenticationException {
-    throw new RuntimeException(
+    throw new OAuthAuthenticationException(
         "The fallback noop authenticator cannot be used for authentication. Make sure OAuth is properly configured.");
   }
 
   @Override
   String callback(URL requestUrl) throws OAuthAuthenticationException {
-    throw new RuntimeException(
+    throw new OAuthAuthenticationException(
         "The fallback noop authenticator cannot be used for authentication. Make sure OAuth is properly configured.");
   }
 
   @Override
   public String computeAuthorizationHeader(String userId, String requestMethod, String requestUrl)
       throws OAuthAuthenticationException {
-    throw new RuntimeException(
+    throw new OAuthAuthenticationException(
         "The fallback noop authenticator cannot be used for authentication. Make sure OAuth is properly configured.");
   }
 

wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth1/OAuthAuthenticator.java

@@ -233,7 +233,7 @@ String callback(final URL requestUrl) throws OAuthAuthenticationException {
    *
    * @return the oauth provider name.
    */
-  abstract String getOAuthProvider();
+  public abstract String getOAuthProvider();
 
   /**
    * Returns URL to initiate authentication process using given authenticator. Typically points to

wsmaster/che-core-api-auth/src/test/java/org/eclipse/che/security/oauth1/OAuthAuthenticatorTest.java

@@ -54,7 +54,7 @@ void start() {
             null,
             "MIIEvAIBADANBgkqhkiG9w0BAQEFAASCBKYwggSiAgEAAoIBAQDZvr2tssVQ46qE1UBK1DFRZrKuIBMCL5q+cltLAVJZ7dlsdv2Yr5mgt3il2BQa0CSmwxTsMdwYqRIDchroVREs5IAfcwOFAL6OnMos/8AEg8Gamnz/Fu1/968bmlV/abKfrdlfkUNuOtpzG5PCf8UAQEt0ajMtFWKFeXPl527BTwRqz1rRVU/wqDC3nS6PM335XCr6mjdBzFDMUC1M91l34M6wPJVPcDRi3cQDb1+YMrFmuzloEs6nlUtM9gH4t9SD9bmBOOj5M7lo3jMuNJ+tE/5M/wrjs1BYbFiUKL3n/BKdyPHpGDssFwo25UBvJWPZZ/YFu6HTD85uUfsbDuTlAgMBAAECggEAA0hY27GCQAHupCoC2h3w0GVX9EAPiUzmbFCVB8BxWWG4kWYJ1K9xBXc+nmFvjCfvJYRzYEwwIT8LQnoJ5c7Cf4bCV7cIKo0kUkoS0jLY1jiWRpploALceb1mKmhdOZqCUt3wFPy/o33HpUyZIamDcsmFWa/wLZHQ9moqUSD4Dnn3Wy6mLyipQDC8LIjPQceuU96VGbZa/XJR8sVMulpgUHvQRzr9PZ1tw4yAK+tcg3rfx4XT6qZS64o4mYrNGYO3QBH3AMUZl1BVG9Q2SlrUM+RGlS5c3DYsFCD40yDBCJIvW1Tfoc4nuDn71rgDEQUZzZlP9X6q1Eh8karaHCrEAQKBgQD16HUcfEd+asDa80BZm3U9Rp0lI1JIfrF/AWR6RR1rldnZOGnQD4untYSZF8vpGdSN5t26szYGJQz9SZl1dtz5sQsyXY9TrGnSf/byoy8yJ6FU3IRaIfoxiAV1fL/QxLsyDls4G5OF9kCu4u2IDKPgDQnb1x9Kq8dQHIyB44eWJQKBgQDirmZToQNZE2GKS+GMxFeeGBTLuV2ED3YuyhRAE3v1bvMRsxcNpMWeCxPMfu8Ctn51yxvTkhRYF3vUz7HjfHYfSMFPf+DwZjOCVWMTT6d8XOcjXIq+mY2Dyek7Kagbbx2oMyBC9HCe0/iK0nOhUNfYFi8PFyDrosvmcQO3qEX3wQKBgG/+5xeKIqWYySzvDKfC/apitr9rTtZlnUFSyQhG4hdVsFoWL1rrOZewPCvdgqkvcncOZn3ZkQlLZpcVJicxc4Lk90yA//4D0E5mqXnoiF43Xmrf5AeI4gIdCR9xKYtTjk5F65WqOY4RkXQVNkl4OEqapZrSZxYDFkuONRATKHVhAoGAQSku4wVa6AUpOc78RDHAmgKEH9fmKOsk5uhSD+VJ8dB18PWRP+vIntjCVTt7y0TYb1X2ZsgMLxJ5F0Co+yKw9ec9InQ5HgHS9rlC5K82DwrJqqGUhJuxUVv+PnKID3LOjKY9tOF9ajq2rHk4ofuSQFyIJIdagEHo6RI9plKp4kECgYAtVUUoXAn5EKLuNVPzlnH+E+iBco0WaQGtsWhIlu6RVhSwJNrldxMFIuWzG56RoFV5tu+KA05RZx82cbazcJJVfwn7S6rmHCdxri3bjpnwgNHmY9E9cxBsEBW2DIYTyI8tbEytbH8syYPGSxb5+VIZEuP+8qel12mVfcoNl/oCCw==") {
           @Override
-          String getOAuthProvider() {
+          public String getOAuthProvider() {
             return null;
           }
 

wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/server/HttpBitbucketServerApiClient.java

@@ -49,8 +49,8 @@
 import org.eclipse.che.commons.env.EnvironmentContext;
 import org.eclipse.che.commons.lang.concurrent.LoggingUncaughtExceptionHandler;
 import org.eclipse.che.commons.subject.Subject;
-import org.eclipse.che.security.oauth1.BitbucketServerOAuthAuthenticator;
 import org.eclipse.che.security.oauth1.OAuthAuthenticationException;
+import org.eclipse.che.security.oauth1.OAuthAuthenticator;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -65,11 +65,10 @@ public class HttpBitbucketServerApiClient implements BitbucketServerApiClient {
   private static final Logger LOG = LoggerFactory.getLogger(HttpBitbucketServerApiClient.class);
   private static final Duration DEFAULT_HTTP_TIMEOUT = ofSeconds(10);
   private final URI serverUri;
-  private final BitbucketServerOAuthAuthenticator authenticator;
+  private final OAuthAuthenticator authenticator;
   private final HttpClient httpClient;
 
-  public HttpBitbucketServerApiClient(
-      String serverUrl, BitbucketServerOAuthAuthenticator authenticator) {
+  public HttpBitbucketServerApiClient(String serverUrl, OAuthAuthenticator authenticator) {
     this.serverUri = URI.create(serverUrl);
     this.authenticator = authenticator;
     this.httpClient =
@@ -262,7 +261,6 @@ public List<BitbucketPersonalAccessToken> getPersonalAccessTokens(String userSlu
   @Override
   public BitbucketPersonalAccessToken getPersonalAccessToken(String userSlug, Long tokenId)
       throws ScmItemNotFoundException, ScmUnauthorizedException, ScmCommunicationException {
-
     URI uri = serverUri.resolve("/rest/access-tokens/1.0/users/" + userSlug + "/" + tokenId);
     HttpRequest request =
         HttpRequest.newBuilder(uri)
@@ -325,9 +323,8 @@ private Optional<BitbucketUser> findCurrentUser(Set<String> userSlugs)
   private <T> List<T> doGetItems(Class<T> tClass, String api, String filter)
       throws ScmUnauthorizedException, ScmCommunicationException, ScmBadRequestException,
           ScmItemNotFoundException {
-    List<T> result = new ArrayList<>();
     Page<T> currentPage = doGetPage(tClass, api, 0, 25, filter);
-    result.addAll(currentPage.getValues());
+    List<T> result = new ArrayList<>(currentPage.getValues());
     while (!currentPage.isLastPage()) {
       currentPage = doGetPage(tClass, api, currentPage.getNextPageStart(), 25, filter);
       result.addAll(currentPage.getValues());

wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/security/oauth1/BitbucketServerApiProvider.java

@@ -56,46 +56,46 @@ private static BitbucketServerApiClient doGet(
       String rawBitbucketEndpoints,
       String bitbucketOauth1Endpoint,
       Set<OAuthAuthenticator> authenticators) {
-    if (isNullOrEmpty(bitbucketOauth1Endpoint)) {
+    if (isNullOrEmpty(bitbucketOauth1Endpoint) && isNullOrEmpty(rawBitbucketEndpoints)) {
       return new NoopBitbucketServerApiClient();
+    } else if (!isNullOrEmpty(bitbucketOauth1Endpoint) && isNullOrEmpty(rawBitbucketEndpoints)) {
+      throw new ConfigurationException(
+          "`che.integration.bitbucket.server_endpoints` bitbucket configuration is missing."
+              + " It should contain values from 'che.oauth1.bitbucket.endpoint'");
+    } else if (isNullOrEmpty(bitbucketOauth1Endpoint) && !isNullOrEmpty(rawBitbucketEndpoints)) {
+      return new HttpBitbucketServerApiClient(
+          sanitizedEndpoints(rawBitbucketEndpoints).get(0), new NoopOAuthAuthenticator());
     } else {
-      if (isNullOrEmpty(rawBitbucketEndpoints)) {
+      bitbucketOauth1Endpoint = StringUtils.trimEnd(bitbucketOauth1Endpoint, '/');
+      if (!sanitizedEndpoints(rawBitbucketEndpoints).contains(bitbucketOauth1Endpoint)) {
         throw new ConfigurationException(
-            "`che.integration.bitbucket.server_endpoints` bitbucket configuration is missing."
-                + " It should contain values from 'che.oauth1.bitbucket.endpoint'");
+            "`che.integration.bitbucket.server_endpoints` must contain `"
+                + bitbucketOauth1Endpoint
+                + "` value");
       } else {
-        // sanitise URL-s first
-        bitbucketOauth1Endpoint = StringUtils.trimEnd(bitbucketOauth1Endpoint, '/');
-        List<String> bitbucketEndpoints =
-            Splitter.on(",")
-                .splitToList(rawBitbucketEndpoints)
+        Optional<OAuthAuthenticator> authenticator =
+            authenticators
                 .stream()
-                .map(s -> StringUtils.trimEnd(s, '/'))
-                .collect(Collectors.toList());
-        if (bitbucketEndpoints.contains(bitbucketOauth1Endpoint)) {
-          Optional<OAuthAuthenticator> authenticator =
-              authenticators
-                  .stream()
-                  .filter(
-                      a ->
-                          a.getOAuthProvider()
-                              .equals(BitbucketServerOAuthAuthenticator.AUTHENTICATOR_NAME))
-                  .filter(
-                      a -> BitbucketServerOAuthAuthenticator.class.isAssignableFrom(a.getClass()))
-                  .findFirst();
-          if (authenticator.isEmpty()) {
-            throw new ConfigurationException(
-                "'che.oauth1.bitbucket.endpoint' is set but BitbucketServerOAuthAuthenticator is not deployed correctly");
-          }
-          return new HttpBitbucketServerApiClient(
-              bitbucketOauth1Endpoint, (BitbucketServerOAuthAuthenticator) authenticator.get());
-        } else {
+                .filter(
+                    a ->
+                        a.getOAuthProvider()
+                            .equals(BitbucketServerOAuthAuthenticator.AUTHENTICATOR_NAME))
+                .filter(a -> BitbucketServerOAuthAuthenticator.class.isAssignableFrom(a.getClass()))
+                .findFirst();
+        if (authenticator.isEmpty()) {
           throw new ConfigurationException(
-              "`che.integration.bitbucket.server_endpoints` must contain `"
-                  + bitbucketOauth1Endpoint
-                  + "` value");
+              "'che.oauth1.bitbucket.endpoint' is set but BitbucketServerOAuthAuthenticator is not deployed correctly");
         }
+        return new HttpBitbucketServerApiClient(bitbucketOauth1Endpoint, authenticator.get());
       }
     }
   }
+
+  private static List<String> sanitizedEndpoints(String rawBitbucketEndpoints) {
+    return Splitter.on(",")
+        .splitToList(rawBitbucketEndpoints)
+        .stream()
+        .map(s -> StringUtils.trimEnd(s, '/'))
+        .collect(Collectors.toList());
+  }
 }

wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/HttpBitbucketServerApiClientTest.java

@@ -43,6 +43,7 @@
 import org.eclipse.che.api.factory.server.scm.exception.ScmItemNotFoundException;
 import org.eclipse.che.api.factory.server.scm.exception.ScmUnauthorizedException;
 import org.eclipse.che.security.oauth1.BitbucketServerOAuthAuthenticator;
+import org.eclipse.che.security.oauth1.NoopOAuthAuthenticator;
 import org.eclipse.che.security.oauth1.OAuthAuthenticationException;
 import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeMethod;
@@ -57,7 +58,6 @@ public class HttpBitbucketServerApiClientTest {
   WireMockServer wireMockServer;
   WireMock wireMock;
   BitbucketServerApiClient bitbucketServer;
-  BitbucketServerOAuthAuthenticator authenticator;
 
   @BeforeMethod
   void start() {
@@ -276,4 +276,18 @@ public void shouldBeAbleToThrowScmUnauthorizedExceptionOnGePAT()
     // when
     bitbucketServer.getPersonalAccessToken("ksmster", 5L);
   }
+
+  @Test(
+      expectedExceptions = ScmCommunicationException.class,
+      expectedExceptionsMessageRegExp =
+          "The fallback noop authenticator cannot be used for authentication. Make sure OAuth is properly configured.")
+  public void shouldThrowScmCommunicationExceptionInNoOauthAuthenticator()
+      throws ScmCommunicationException, ScmUnauthorizedException, ScmItemNotFoundException {
+
+    HttpBitbucketServerApiClient localServer =
+        new HttpBitbucketServerApiClient(wireMockServer.url("/"), new NoopOAuthAuthenticator());
+
+    // when
+    localServer.getPersonalAccessToken("ksmster", 5L);
+  }
 }

wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/security/oauth1/BitbucketServerApiClientProviderTest.java

@@ -84,6 +84,22 @@ public void shouldProvideNoopOAuthAuthenticatorIfSomeConfigurationIsNotSet(
     assertTrue(NoopBitbucketServerApiClient.class.isAssignableFrom(actual.getClass()));
   }
 
+  @Test(dataProvider = "httpOnlyConfig")
+  public void shouldProvideHttpAuthenticatorIfOauthConfigurationIsNotSet(
+      @Nullable String bitbucketEndpoints,
+      @Nullable String bitbucketOauth1Endpoint,
+      Set<OAuthAuthenticator> authenticators)
+      throws IOException {
+    // given
+    BitbucketServerApiProvider bitbucketServerApiProvider =
+        new BitbucketServerApiProvider(bitbucketEndpoints, bitbucketOauth1Endpoint, authenticators);
+    // when
+    BitbucketServerApiClient actual = bitbucketServerApiProvider.get();
+    // then
+    assertNotNull(actual);
+    assertTrue(HttpBitbucketServerApiClient.class.isAssignableFrom(actual.getClass()));
+  }
+
   @Test(
       expectedExceptions = ConfigurationException.class,
       expectedExceptionsMessageRegExp =
@@ -128,6 +144,14 @@ public void shouldFailToBuildIfEndpointsAreMisconfigured3() {
   public Object[][] noopConfig() {
     return new Object[][] {
       {null, null, null},
+      {"", "", null}
+    };
+  }
+
+  @DataProvider(name = "httpOnlyConfig")
+  public Object[][] httpOnlyConfig() {
+    return new Object[][] {
+      {"https://bitbucket.server.com", null, null},
       {"https://bitbucket.server.com, https://bitbucket2.server.com", null, null},
       {
         "https://bitbucket.server.com, https://bitbucket2.server.com",

wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubPersonalAccessTokenFetcher.java

@@ -136,6 +136,11 @@ public GithubPersonalAccessTokenFetcher(@Named("che.api") String apiEndpoint, OA
   public PersonalAccessToken fetchPersonalAccessToken(Subject cheSubject, String scmServerUrl)
       throws ScmUnauthorizedException, ScmCommunicationException {
     OAuthToken oAuthToken;
+
+    if (githubApiClient == null || !githubApiClient.isConnected(scmServerUrl)) {
+      LOG.debug("not a  valid url {} for current fetcher ", scmServerUrl);
+      return null;
+    }
     try {
       oAuthToken = oAuthAPI.getToken(OAUTH_PROVIDER_NAME);
       // Find the user associated to the OAuth token by querying the GitHub API.

wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java

@@ -15,6 +15,7 @@
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.Optional;
+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;
 import org.eclipse.che.api.factory.server.scm.exception.ScmUnauthorizedException;
@@ -51,17 +52,7 @@ public AuthorizingFileContentProvider(
 
   @Override
   public String fetchContent(String fileURL) throws IOException, DevfileException {
-    String requestURL;
-    try {
-      if (new URI(fileURL).isAbsolute()) {
-        requestURL = fileURL;
-      } else {
-        // since files retrieved via REST, we cannot use path symbols like . ./ so cut them off
-        requestURL = remoteFactoryUrl.rawFileLocation(fileURL.replaceAll("^[/.]+", ""));
-      }
-    } catch (URISyntaxException e) {
-      throw new DevfileException(e.getMessage(), e);
-    }
+    final String requestURL = formatUrl(fileURL);
     try {
       Optional<PersonalAccessToken> token =
           personalAccessTokenManager.get(
@@ -76,6 +67,14 @@ public String fetchContent(String fileURL) throws IOException, DevfileException
         try {
           return urlFetcher.fetch(requestURL);
         } catch (IOException exception) {
+          if (exception instanceof SSLException) {
+            ScmCommunicationException cause =
+                new ScmCommunicationException(
+                    String.format(
+                        "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);
+          }
           // unable to determine exact cause, so let's just try to authorize...
           try {
             PersonalAccessToken personalAccessToken =
@@ -88,17 +87,18 @@ public String fetchContent(String fileURL) throws IOException, DevfileException
           } catch (ScmUnauthorizedException | UnknownScmProviderException e) {
             throw new DevfileException(e.getMessage(), e);
           } catch (ScmCommunicationException e) {
-            throw new IOException(
-                String.format(
-                    "Failed to fetch a content from URL %s. Make sure the URL"
-                        + " is correct. For private repository, make sure authentication is configured."
-                        + " Additionally, if you're using "
-                        + " relative form, make sure the referenced file are actually stored"
-                        + " relative to the devfile on the same host,"
-                        + " or try to specify URL in absolute form. The current attempt to authenticate"
-                        + " request, failed with the following error message: %s",
-                    fileURL, e.getMessage()),
-                e);
+            throw new DevfileException(
+                e.getMessage(),
+                new ScmCommunicationException(
+                    String.format(
+                        "Failed to fetch a content from URL %s. Make sure the URL"
+                            + " is correct. For private repository, make sure authentication is configured."
+                            + " Additionally, if you're using "
+                            + " relative form, make sure the referenced file are actually stored"
+                            + " relative to the devfile on the same host,"
+                            + " or try to specify URL in absolute form. The current attempt to authenticate"
+                            + " request, failed with the following error message: %s",
+                        fileURL, e.getMessage())));
           }
         }
       }
@@ -110,6 +110,21 @@ public String fetchContent(String fileURL) throws IOException, DevfileException
     }
   }
 
+  private String formatUrl(String fileURL) throws DevfileException {
+    String requestURL;
+    try {
+      if (new URI(fileURL).isAbsolute()) {
+        requestURL = fileURL;
+      } else {
+        // since files retrieved via REST, we cannot use path symbols like . ./ so cut them off
+        requestURL = remoteFactoryUrl.rawFileLocation(fileURL.replaceAll("^[/.]+", ""));
+      }
+    } catch (URISyntaxException e) {
+      throw new DevfileException(e.getMessage(), e);
+    }
+    return requestURL;
+  }
+
   protected String formatAuthorization(String token) {
     return "Bearer " + token;
   }