Respect the ScmComunicationException on token validation step (#715)

Throw the ScmCommunicationException on token validation step instead of returning invalid result. Propagate the ScmCommunicationException to the dashboard as an ApiException. Throw ScmUnauthorizedException on each scm api request with invalid token. Log error instead of throwing RuntimeException on workspace provision steps.
eclipse-che · Sep 13, 2024 · 6121661 · 6121661
1 parent c2a2fca
commit 6121661
Show file tree

Hide file tree

Showing 28 changed files with 211 additions and 92 deletions.
diff --git a/...g/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManager.java b/...g/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManager.java
@@ -152,7 +152,7 @@ public PersonalAccessToken fetchAndSave(Subject cheUser, String scmServerUrl)
 
   @Override
   public Optional<PersonalAccessToken> get(Subject cheUser, String scmServerUrl)
-      throws ScmConfigurationPersistenceException {
+      throws ScmConfigurationPersistenceException, ScmCommunicationException {
     return doGetPersonalAccessTokens(cheUser, null, scmServerUrl).stream().findFirst();
   }
 
@@ -174,13 +174,13 @@ public PersonalAccessToken get(String scmServerUrl)
   @Override
   public Optional<PersonalAccessToken> get(
       Subject cheUser, String oAuthProviderName, @Nullable String scmServerUrl)
-      throws ScmConfigurationPersistenceException {
+      throws ScmConfigurationPersistenceException, ScmCommunicationException {
     return doGetPersonalAccessTokens(cheUser, oAuthProviderName, scmServerUrl).stream().findFirst();
   }
 
   private List<PersonalAccessToken> doGetPersonalAccessTokens(
       Subject cheUser, @Nullable String oAuthProviderName, @Nullable String scmServerUrl)
-      throws ScmConfigurationPersistenceException {
+      throws ScmConfigurationPersistenceException, ScmCommunicationException {
     List<PersonalAccessToken> result = new ArrayList<>();
     try {
       LOG.debug(

diff --git a/...space/infrastructure/kubernetes/namespace/configurator/CredentialsSecretConfigurator.java b/...space/infrastructure/kubernetes/namespace/configurator/CredentialsSecretConfigurator.java
@@ -25,6 +25,8 @@
 import org.eclipse.che.api.workspace.server.spi.InfrastructureException;
 import org.eclipse.che.api.workspace.server.spi.NamespaceResolutionContext;
 import org.eclipse.che.workspace.infrastructure.kubernetes.CheServerKubernetesClientFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * This {@link NamespaceConfigurator} ensures that Secret {@link
@@ -45,6 +47,8 @@ public class CredentialsSecretConfigurator implements NamespaceConfigurator {
   private static final String MERGED_GIT_CREDENTIALS_SECRET_NAME =
       "devworkspace-merged-git-credentials";
 
+  private static final Logger LOG = LoggerFactory.getLogger(CredentialsSecretConfigurator.class);
+
   @Inject
   public CredentialsSecretConfigurator(
       CheServerKubernetesClientFactory cheServerKubernetesClientFactory,
@@ -79,7 +83,7 @@ public void configure(NamespaceResolutionContext namespaceResolutionContext, Str
                   | ScmConfigurationPersistenceException
                   | UnsatisfiedScmPreconditionException
                   | ScmUnauthorizedException e) {
-                throw new RuntimeException(e);
+                LOG.error(e.getMessage(), e);
               }
             });
   }

diff --git a/...space/infrastructure/kubernetes/namespace/configurator/OAuthTokenSecretsConfigurator.java b/...space/infrastructure/kubernetes/namespace/configurator/OAuthTokenSecretsConfigurator.java
@@ -17,12 +17,15 @@
 import javax.inject.Singleton;
 import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenFetcher;
 import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager;
+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.workspace.server.spi.InfrastructureException;
 import org.eclipse.che.api.workspace.server.spi.NamespaceResolutionContext;
 import org.eclipse.che.commons.env.EnvironmentContext;
 import org.eclipse.che.commons.subject.Subject;
 import org.eclipse.che.workspace.infrastructure.kubernetes.CheServerKubernetesClientFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Ensures that OAuth token that are represented by Kubernetes Secrets are valid.
@@ -44,6 +47,8 @@ public class OAuthTokenSecretsConfigurator implements NamespaceConfigurator {
           "app.kubernetes.io/part-of", "che.eclipse.org",
           "app.kubernetes.io/component", "scm-personal-access-token");
 
+  private static final Logger LOG = LoggerFactory.getLogger(OAuthTokenSecretsConfigurator.class);
+
   @Inject
   public OAuthTokenSecretsConfigurator(
       CheServerKubernetesClientFactory cheServerKubernetesClientFactory,
@@ -74,8 +79,8 @@ public void configure(NamespaceResolutionContext namespaceResolutionContext, Str
                 Subject cheSubject = EnvironmentContext.getCurrent().getSubject();
                 personalAccessTokenManager.get(
                     cheSubject, s.getMetadata().getAnnotations().get(ANNOTATION_SCM_URL));
-              } catch (ScmConfigurationPersistenceException e) {
-                throw new RuntimeException(e);
+              } catch (ScmConfigurationPersistenceException | ScmCommunicationException e) {
+                LOG.error(e.getMessage(), e);
               }
             });
   }

diff --git a/...ster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPI.java b/...ster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPI.java
@@ -252,7 +252,7 @@ public OAuthToken getOrRefreshToken(String oauthProvider)
           if (tokenOptional.isPresent()) {
             return newDto(OAuthToken.class).withToken(tokenOptional.get().getToken());
           }
-        } catch (ScmConfigurationPersistenceException e) {
+        } catch (ScmConfigurationPersistenceException | ScmCommunicationException e) {
           throw new RuntimeException(e);
         }
       }

diff --git a/...s/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsApiClient.java b/...s/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsApiClient.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2023 Red Hat, Inc.
+ * Copyright (c) 2012-2024 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/
@@ -147,7 +147,9 @@ private <T> T executeRequest(
             throw new ScmItemNotFoundException(body);
           default:
             throw new ScmCommunicationException(
-                "Unexpected status code " + response.statusCode() + " " + response.toString());
+                "Unexpected status code " + response.statusCode() + " " + response,
+                response.statusCode(),
+                "azure-devops");
         }
       }
     } catch (IOException | InterruptedException | UncheckedIOException e) {

diff --git a/...rg/eclipse/che/api/factory/server/azure/devops/AzureDevOpsPersonalAccessTokenFetcher.java b/...rg/eclipse/che/api/factory/server/azure/devops/AzureDevOpsPersonalAccessTokenFetcher.java
@@ -169,7 +169,8 @@ public Optional<Boolean> isValid(PersonalAccessToken personalAccessToken) {
   }
 
   @Override
-  public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params) {
+  public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
+      throws ScmCommunicationException {
     if (!isValidScmServerUrl(params.getScmProviderUrl())) {
       LOG.debug("not a valid url {} for current fetcher ", params.getScmProviderUrl());
       return Optional.empty();
@@ -183,7 +184,7 @@ public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
         user = azureDevOpsApiClient.getUserWithPAT(params.getToken(), params.getOrganization());
       }
       return Optional.of(Pair.of(Boolean.TRUE, user.getEmailAddress()));
-    } catch (ScmItemNotFoundException | ScmCommunicationException | ScmBadRequestException e) {
+    } catch (ScmItemNotFoundException | ScmBadRequestException e) {
       return Optional.empty();
     }
   }

diff --git a/...g/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcher.java b/...g/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcher.java
@@ -158,7 +158,8 @@ public Optional<Boolean> isValid(PersonalAccessToken accessToken)
   }
 
   @Override
-  public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params) {
+  public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
+      throws ScmCommunicationException {
     if (!bitbucketServerApiClient.isConnected(params.getScmProviderUrl())) {
       // If BitBucket oAuth is not configured check the manually added user namespace token.
       HttpBitbucketServerApiClient apiClient =
@@ -180,7 +181,7 @@ public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
     try {
       BitbucketUser user = bitbucketServerApiClient.getUser(params.getToken());
       return Optional.of(Pair.of(Boolean.TRUE, user.getName()));
-    } catch (ScmItemNotFoundException | ScmUnauthorizedException | ScmCommunicationException e) {
+    } catch (ScmItemNotFoundException | ScmUnauthorizedException e) {
       return Optional.empty();
     }
   }

diff --git a/.../src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerURLParser.java b/.../src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerURLParser.java
@@ -96,7 +96,7 @@ private boolean isUserTokenPresent(String repositoryUrl) {
         Optional<PersonalAccessToken> token =
             personalAccessTokenManager.get(EnvironmentContext.getCurrent().getSubject(), serverUrl);
         return token.isPresent() && token.get().getScmTokenName().equals(OAUTH_PROVIDER_NAME);
-      } catch (ScmConfigurationPersistenceException exception) {
+      } catch (ScmConfigurationPersistenceException | ScmCommunicationException exception) {
         return false;
       }
     }

diff --git a/.../main/java/org/eclipse/che/api/factory/server/bitbucket/HttpBitbucketServerApiClient.java b/.../main/java/org/eclipse/che/api/factory/server/bitbucket/HttpBitbucketServerApiClient.java
@@ -411,7 +411,9 @@ private <T> T executeRequest(
             throw new ScmItemNotFoundException(body);
           default:
             throw new ScmCommunicationException(
-                "Unexpected status code " + response.statusCode() + " " + response.toString());
+                "Unexpected status code " + response.statusCode() + " " + response,
+                response.statusCode(),
+                "bitbucket");
         }
       }
     } catch (IOException | InterruptedException | UncheckedIOException e) {

diff --git a/...bucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketApiClient.java b/...bucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketApiClient.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2023 Red Hat, Inc.
+ * Copyright (c) 2012-2024 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/
@@ -15,6 +15,7 @@
 import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
 import static java.net.HttpURLConnection.HTTP_NO_CONTENT;
 import static java.net.HttpURLConnection.HTTP_OK;
+import static java.net.HttpURLConnection.HTTP_UNAUTHORIZED;
 import static java.time.Duration.ofSeconds;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
@@ -37,6 +38,7 @@
 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.api.factory.server.scm.exception.ScmUnauthorizedException;
 import org.eclipse.che.commons.lang.Pair;
 import org.eclipse.che.commons.lang.concurrent.LoggingUncaughtExceptionHandler;
 import org.slf4j.Logger;
@@ -100,7 +102,8 @@ public BitbucketApiClient() {
    * @throws ScmBadRequestException
    */
   public BitbucketUser getUser(String authenticationToken)
-      throws ScmItemNotFoundException, ScmCommunicationException, ScmBadRequestException {
+      throws ScmItemNotFoundException, ScmCommunicationException, ScmBadRequestException,
+          ScmUnauthorizedException {
     final URI uri = apiServerUrl.resolve("user");
     HttpRequest request = buildBitbucketApiRequest(uri, authenticationToken);
     LOG.trace("executeRequest={}", request);
@@ -120,7 +123,8 @@ public BitbucketUser getUser(String authenticationToken)
 
   public String getFileContent(
       String workspace, String repository, String source, String path, String authenticationToken)
-      throws ScmItemNotFoundException, ScmCommunicationException, ScmBadRequestException {
+      throws ScmItemNotFoundException, ScmCommunicationException, ScmBadRequestException,
+          ScmUnauthorizedException {
     final URI uri =
         apiServerUrl.resolve(
             String.format("repositories/%s/%s/src/%s/%s", workspace, repository, source, path));
@@ -148,7 +152,8 @@ public String getFileContent(
    * @throws ScmBadRequestException
    */
   public BitbucketUserEmail getEmail(String authenticationToken)
-      throws ScmItemNotFoundException, ScmCommunicationException, ScmBadRequestException {
+      throws ScmItemNotFoundException, ScmCommunicationException, ScmBadRequestException,
+          ScmUnauthorizedException {
     final URI uri = apiServerUrl.resolve("user/emails");
     HttpRequest request = buildBitbucketApiRequest(uri, authenticationToken);
     LOG.trace("executeRequest={}", request);
@@ -174,7 +179,8 @@ public BitbucketUserEmail getEmail(String authenticationToken)
    *     scopes.
    */
   public Pair<String, String[]> getTokenScopes(String authenticationToken)
-      throws ScmItemNotFoundException, ScmCommunicationException, ScmBadRequestException {
+      throws ScmItemNotFoundException, ScmCommunicationException, ScmBadRequestException,
+          ScmUnauthorizedException {
     final URI uri = apiServerUrl.resolve("user");
     HttpRequest request = buildBitbucketApiRequest(uri, authenticationToken);
     LOG.trace("executeRequest={}", request);
@@ -212,7 +218,8 @@ private <T> T executeRequest(
       HttpClient httpClient,
       HttpRequest request,
       Function<HttpResponse<InputStream>, T> responseConverter)
-      throws ScmBadRequestException, ScmItemNotFoundException, ScmCommunicationException {
+      throws ScmBadRequestException, ScmItemNotFoundException, ScmCommunicationException,
+          ScmUnauthorizedException {
     try {
       HttpResponse<InputStream> response =
           httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
@@ -228,13 +235,17 @@ private <T> T executeRequest(
             throw new ScmBadRequestException(body);
           case HTTP_NOT_FOUND:
             throw new ScmItemNotFoundException(body);
+          case HTTP_UNAUTHORIZED:
+            throw new ScmUnauthorizedException(body, "bitbucket", "v2", "");
           default:
             throw new ScmCommunicationException(
-                "Unexpected status code " + response.statusCode() + " " + response.toString());
+                "Unexpected status code " + response.statusCode() + " " + response,
+                response.statusCode(),
+                "bitbucket");
         }
       }
     } catch (IOException | InterruptedException | UncheckedIOException e) {
-      throw new ScmCommunicationException(e.getMessage(), e);
+      throw new ScmCommunicationException(e.getMessage(), e, "bitbucket");
     }
   }
 

diff --git a/...ava/org/eclipse/che/api/factory/server/bitbucket/BitbucketPersonalAccessTokenFetcher.java b/...ava/org/eclipse/che/api/factory/server/bitbucket/BitbucketPersonalAccessTokenFetcher.java
@@ -166,13 +166,17 @@ public Optional<Boolean> isValid(PersonalAccessToken personalAccessToken) {
     try {
       String[] scopes = bitbucketApiClient.getTokenScopes(personalAccessToken.getToken()).second;
       return Optional.of(isValidScope(Sets.newHashSet(scopes)));
-    } catch (ScmItemNotFoundException | ScmCommunicationException | ScmBadRequestException e) {
+    } catch (ScmItemNotFoundException
+        | ScmCommunicationException
+        | ScmBadRequestException
+        | ScmUnauthorizedException e) {
       return Optional.of(Boolean.FALSE);
     }
   }
 
   @Override
-  public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params) {
+  public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
+      throws ScmCommunicationException {
     if (!bitbucketApiClient.isConnected(params.getScmProviderUrl())) {
       LOG.debug("not a valid url {} for current fetcher ", params.getScmProviderUrl());
       return Optional.empty();
@@ -184,7 +188,7 @@ public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
           Pair.of(
               isValidScope(Sets.newHashSet(pair.second)) ? Boolean.TRUE : Boolean.FALSE,
               pair.first));
-    } catch (ScmItemNotFoundException | ScmCommunicationException | ScmBadRequestException e) {
+    } catch (ScmItemNotFoundException | ScmBadRequestException | ScmUnauthorizedException e) {
       return Optional.empty();
     }
   }

diff --git a/...org/eclipse/che/api/factory/server/bitbucket/BitbucketPersonalAccessTokenFetcherTest.java b/...org/eclipse/che/api/factory/server/bitbucket/BitbucketPersonalAccessTokenFetcherTest.java
@@ -17,7 +17,7 @@
 import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
 import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
 import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
-import static java.net.HttpURLConnection.HTTP_FORBIDDEN;
+import static java.net.HttpURLConnection.HTTP_UNAUTHORIZED;
 import static org.eclipse.che.api.factory.server.scm.PersonalAccessTokenFetcher.OAUTH_2_PREFIX;
 import static org.eclipse.che.dto.server.DtoFactory.newDto;
 import static org.mockito.ArgumentMatchers.anyString;
@@ -211,7 +211,7 @@ public void shouldValidateOauthToken() throws Exception {
 
   @Test
   public void shouldNotValidateExpiredOauthToken() throws Exception {
-    stubFor(get(urlEqualTo("/user")).willReturn(aResponse().withStatus(HTTP_FORBIDDEN)));
+    stubFor(get(urlEqualTo("/user")).willReturn(aResponse().withStatus(HTTP_UNAUTHORIZED)));
 
     PersonalAccessTokenParams params =
         new PersonalAccessTokenParams(
@@ -235,7 +235,7 @@ public void shouldThrowUnauthorizedExceptionIfTokenIsNotValid() throws Exception
     stubFor(
         get(urlEqualTo("/user"))
             .withHeader(HttpHeaders.AUTHORIZATION, equalTo("token " + bitbucketOauthToken))
-            .willReturn(aResponse().withStatus(HTTP_FORBIDDEN)));
+            .willReturn(aResponse().withStatus(HTTP_UNAUTHORIZED)));
 
     bitbucketPersonalAccessTokenFetcher.fetchPersonalAccessToken(
         subject, BitbucketApiClient.BITBUCKET_SERVER);

diff --git a/...a/org/eclipse/che/api/factory/server/github/AbstractGithubPersonalAccessTokenFetcher.java b/...a/org/eclipse/che/api/factory/server/github/AbstractGithubPersonalAccessTokenFetcher.java
@@ -215,13 +215,17 @@ public Optional<Boolean> isValid(PersonalAccessToken personalAccessToken) {
           return Optional.of(Boolean.FALSE);
         }
       }
-    } catch (ScmItemNotFoundException | ScmCommunicationException | ScmBadRequestException e) {
+    } catch (ScmItemNotFoundException
+        | ScmCommunicationException
+        | ScmBadRequestException
+        | ScmUnauthorizedException e) {
       return Optional.of(Boolean.FALSE);
     }
   }
 
   @Override
-  public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params) {
+  public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
+      throws ScmCommunicationException {
     GithubApiClient apiClient;
     if (githubApiClient.isConnected(params.getScmProviderUrl())) {
       // The url from the token has the same url as the api client, no need to create a new one.
@@ -247,7 +251,7 @@ public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
         GithubUser user = apiClient.getUser(params.getToken());
         return Optional.of(Pair.of(Boolean.TRUE, user.getLogin()));
       }
-    } catch (ScmItemNotFoundException | ScmCommunicationException | ScmBadRequestException e) {
+    } catch (ScmItemNotFoundException | ScmBadRequestException | ScmUnauthorizedException e) {
       return Optional.empty();
     }
   }