Specify Bitbucket Server provider naem, fix GitHub url parser

Signed-off-by: ivinokur <[email protected]>
eclipse-che · May 31, 2024 · fa88e1e · fa88e1e
1 parent aa213c3
commit fa88e1e
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 11 deletions.
diff --git a/...h-bitbucket/src/main/java/org/eclipse/che/security/oauth/BitbucketOAuthAuthenticator.java b/...h-bitbucket/src/main/java/org/eclipse/che/security/oauth/BitbucketOAuthAuthenticator.java
@@ -29,6 +29,8 @@
 public class BitbucketOAuthAuthenticator extends OAuthAuthenticator {
   private final String bitbucketEndpoint;
 
+  private static final String BITBUCKET_CLOUD_ENDPOINT = "https://bitbucket.org";
+
   public BitbucketOAuthAuthenticator(
       String bitbucketEndpoint,
       String clientId,
@@ -52,7 +54,7 @@ public String getAuthenticateUrl(URL requestUrl, List<String> scopes) {
 
   @Override
   public final String getOAuthProvider() {
-    return "bitbucket";
+    return BITBUCKET_CLOUD_ENDPOINT.equals(bitbucketEndpoint) ? "bitbucket" : "bitbucket-server";
   }
 
   @Override
@@ -76,7 +78,7 @@ public OAuthToken getToken(String userId) throws IOException {
    * @return Bitbucket Cloud or Server API request URL
    */
   private String getTestRequestUrl() {
-    return "https://bitbucket.org".equals(bitbucketEndpoint)
+    return BITBUCKET_CLOUD_ENDPOINT.equals(bitbucketEndpoint)
         ? "https://api.bitbucket.org/2.0/user"
         : bitbucketEndpoint + "/plugins/servlet/applinks/whoami";
   }

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
@@ -421,7 +421,7 @@ private <T> T executeRequest(
 
   private @Nullable String getToken() throws ScmUnauthorizedException {
     try {
-      OAuthToken token = oAuthAPI.getToken("bitbucket");
+      OAuthToken token = oAuthAPI.getToken("bitbucket-server");
       return token.getToken();
     } catch (NotFoundException
         | ServerException
@@ -459,7 +459,7 @@ private ScmUnauthorizedException buildScmUnauthorizedException() {
         "bitbucket",
         authenticator instanceof NoopOAuthAuthenticator ? "2.0" : "1.0",
         authenticator instanceof NoopOAuthAuthenticator
-            ? apiEndpoint + "/oauth/authenticate?oauth_provider=bitbucket&scope=ADMIN_WRITE"
+            ? apiEndpoint + "/oauth/authenticate?oauth_provider=bitbucket-server&scope=ADMIN_WRITE"
             : authenticator.getLocalAuthenticateUrl());
   }
 }
diff --git a/...t/java/org/eclipse/che/api/factory/server/bitbucket/HttpBitbucketServerApiClientTest.java b/...t/java/org/eclipse/che/api/factory/server/bitbucket/HttpBitbucketServerApiClientTest.java
@@ -394,7 +394,7 @@ public void shouldThrowScmCommunicationExceptionInNoOauthAuthenticator()
           NotFoundException, BadRequestException {
 
     // given
-    when(oAuthAPI.getToken(eq("bitbucket"))).thenReturn(mock(OAuthToken.class));
+    when(oAuthAPI.getToken(eq("bitbucket-server"))).thenReturn(mock(OAuthToken.class));
     HttpBitbucketServerApiClient localServer =
         new HttpBitbucketServerApiClient(
             wireMockServer.url("/"), new NoopOAuthAuthenticator(), oAuthAPI, apiEndpoint);
@@ -411,7 +411,7 @@ public void shouldGetOauth2Token()
     // given
     OAuthToken token = mock(OAuthToken.class);
     when(token.getToken()).thenReturn("token");
-    when(oAuthAPI.getToken(eq("bitbucket"))).thenReturn(token);
+    when(oAuthAPI.getToken(eq("bitbucket-server"))).thenReturn(token);
     bitbucketServer =
         new HttpBitbucketServerApiClient(
             wireMockServer.url("/"), new NoopOAuthAuthenticator(), oAuthAPI, apiEndpoint);
@@ -437,6 +437,6 @@ public void shouldGetOauth2Token()
     bitbucketServer.getUser();
 
     // then
-    verify(oAuthAPI, times(2)).getToken(eq("bitbucket"));
+    verify(oAuthAPI, times(2)).getToken(eq("bitbucket-server"));
   }
 }
diff --git a/...mmon/src/main/java/org/eclipse/che/api/factory/server/github/AbstractGithubURLParser.java b/...mmon/src/main/java/org/eclipse/che/api/factory/server/github/AbstractGithubURLParser.java
@@ -63,6 +63,7 @@ public abstract class AbstractGithubURLParser {
   private final boolean disableSubdomainIsolation;
 
   private final String providerName;
+  private final String endpoint;
 
   /** Constructor used for testing only. */
   AbstractGithubURLParser(
@@ -78,8 +79,7 @@ public abstract class AbstractGithubURLParser {
     this.disableSubdomainIsolation = disableSubdomainIsolation;
     this.providerName = providerName;
 
-    String endpoint =
-        isNullOrEmpty(oauthEndpoint) ? GITHUB_SAAS_ENDPOINT : trimEnd(oauthEndpoint, '/');
+    endpoint = isNullOrEmpty(oauthEndpoint) ? GITHUB_SAAS_ENDPOINT : trimEnd(oauthEndpoint, '/');
 
     this.githubPattern = compile(format(githubPatternTemplate, endpoint));
     this.githubSSHPattern =
@@ -93,8 +93,8 @@ public boolean isValid(@NotNull String url) {
         // If the GitHub URL is not configured, try to find it in a manually added user namespace
         // token.
         || isUserTokenPresent(trimmedUrl)
-        // Try to call an API request to see if the URL matches GitHub.
-        || isApiRequestRelevant(trimmedUrl);
+        // Try to call an API request to see if the URL matches self-hosted GitHub Enterprise.
+        || (!GITHUB_SAAS_ENDPOINT.equals(endpoint) && isApiRequestRelevant(trimmedUrl));
   }
 
   private boolean isUserTokenPresent(String repositoryUrl) {

diff --git a/...y-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubURLParserTest.java b/...y-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubURLParserTest.java
@@ -31,6 +31,7 @@
 import com.github.tomakehurst.wiremock.WireMockServer;
 import com.github.tomakehurst.wiremock.client.WireMock;
 import com.github.tomakehurst.wiremock.common.Slf4jNotifier;
+import java.lang.reflect.Field;
 import java.util.Optional;
 import org.eclipse.che.api.core.ApiException;
 import org.eclipse.che.api.factory.server.scm.PersonalAccessToken;
@@ -336,6 +337,9 @@ public void shouldParseServerUrWithPullRequestId() throws Exception {
   @Test
   public void shouldValidateOldVersionGitHubServerUrl() throws Exception {
     // given
+    Field endpoint = AbstractGithubURLParser.class.getDeclaredField("endpoint");
+    endpoint.setAccessible(true);
+    endpoint.set(githubUrlParser, wireMockServer.baseUrl());
     String url = wireMockServer.url("/user/repo");
     stubFor(
         get(urlEqualTo("/api/v3/user"))
@@ -354,6 +358,9 @@ public void shouldValidateOldVersionGitHubServerUrl() throws Exception {
   @Test
   public void shouldValidateGitHubServerUrl() throws Exception {
     // given
+    Field endpoint = AbstractGithubURLParser.class.getDeclaredField("endpoint");
+    endpoint.setAccessible(true);
+    endpoint.set(githubUrlParser, wireMockServer.baseUrl());
     String url = wireMockServer.url("/user/repo");
     stubFor(
         get(urlEqualTo("/api/v3/user"))
@@ -368,4 +375,13 @@ public void shouldValidateGitHubServerUrl() throws Exception {
     // then
     assertTrue(valid);
   }
+
+  @Test
+  public void shouldNotRequestGitHubSAASUrl() throws Exception {
+    // when
+    githubUrlParser.isValid("https:github.com/repo/user.git");
+
+    // then
+    verify(githubApiClient, never()).getUser(anyString());
+  }
 }