From f9f4c8e5d5a049a011ffa46393708779bbc88231 Mon Sep 17 00:00:00 2001
From: Igor Vinokur <ivinokur@redhat.com>
Date: Mon, 19 Feb 2024 19:07:07 +0200
Subject: [PATCH] Encode redirect URL if needed on oauth callback request
 (#654)

Some SCM providers like BItBucket Server decode the callback url so that cause IllegalArgumentException error. Catch the error and decode the redirect url.
---
 .../che/security/oauth/EmbeddedOAuthAPI.java  | 21 ++++++---
 .../security/oauth/EmbeddedOAuthAPITest.java  | 46 +++++++++++++++++++
 2 files changed, 60 insertions(+), 7 deletions(-)

diff --git a/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPI.java b/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPI.java
index ec317aa5c3..455f1d0160 100644
--- a/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPI.java
+++ b/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPI.java
@@ -96,7 +96,9 @@ public Response callback(UriInfo uriInfo, @Nullable List<String> errorValues)
     if (!isNullOrEmpty(redirectAfterLogin)
         && errorValues != null
         && errorValues.contains("access_denied")) {
-      return Response.temporaryRedirect(URI.create(encodeRedirectUrl())).build();
+      return Response.temporaryRedirect(
+              URI.create(encodeRedirectUrl(redirectAfterLogin + "&error_code=access_denied")))
+          .build();
     }
     final String providerName = getParameter(params, "oauth_provider");
     OAuthAuthenticator oauth = getAuthenticator(providerName);
@@ -123,7 +125,14 @@ public Response callback(UriInfo uriInfo, @Nullable List<String> errorValues)
       LOG.error(e.getMessage(), e);
     }
     final String redirectAfterLogin = getParameter(params, "redirect_after_login");
-    return Response.temporaryRedirect(URI.create(redirectAfterLogin)).build();
+    URI uri;
+    try {
+      uri = URI.create(redirectAfterLogin);
+    } catch (IllegalArgumentException e) {
+      // the redirectUrl was decoded by the CSM provider, so we need to encode it back.
+      uri = URI.create(encodeRedirectUrl(redirectAfterLogin));
+    }
+    return Response.temporaryRedirect(uri).build();
   }
 
   /**
@@ -131,12 +140,10 @@ public Response callback(UriInfo uriInfo, @Nullable List<String> errorValues)
    * JSON, as a query parameter. This prevents passing unsupported characters, like '{' and '}' to
    * the {@link URI#create(String)} method.
    */
-  private String encodeRedirectUrl() {
+  private String encodeRedirectUrl(String url) {
     try {
-      URL url = new URL(redirectAfterLogin);
-      String query = url.getQuery();
-      return redirectAfterLogin.substring(0, redirectAfterLogin.indexOf(query))
-          + URLEncoder.encode(query + "&error_code=access_denied", UTF_8);
+      String query = new URL(url).getQuery();
+      return url.substring(0, url.indexOf(query)) + URLEncoder.encode(query, UTF_8);
     } catch (MalformedURLException e) {
       LOG.error(e.getMessage(), e);
       throw new RuntimeException(e);
diff --git a/wsmaster/che-core-api-auth/src/test/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPITest.java b/wsmaster/che-core-api-auth/src/test/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPITest.java
index 5a5e6d61c6..f016473046 100644
--- a/wsmaster/che-core-api-auth/src/test/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPITest.java
+++ b/wsmaster/che-core-api-auth/src/test/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPITest.java
@@ -11,6 +11,8 @@
  */
 package org.eclipse.che.security.oauth;
 
+import static java.net.URLEncoder.encode;
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
 import static org.eclipse.che.api.factory.server.scm.PersonalAccessTokenFetcher.OAUTH_2_PREFIX;
@@ -140,4 +142,48 @@ public void shouldStoreTokenOnCallback() throws Exception {
     assertTrue(token.getScmTokenName().startsWith(OAUTH_2_PREFIX));
     assertEquals(token.getToken(), "token");
   }
+
+  @Test
+  public void shouldEncodeRedirectUrl() throws Exception {
+    // given
+    UriInfo uriInfo = mock(UriInfo.class);
+    OAuthAuthenticator authenticator = mock(OAuthAuthenticator.class);
+    when(uriInfo.getRequestUri())
+        .thenReturn(
+            new URI(
+                "http://eclipse.che?state=oauth_provider"
+                    + encode(
+                        "=github&redirect_after_login=https://redirecturl.com?params="
+                            + encode("{}", UTF_8),
+                        UTF_8)));
+    when(oauth2Providers.getAuthenticator("github")).thenReturn(authenticator);
+
+    // when
+    Response callback = embeddedOAuthAPI.callback(uriInfo, emptyList());
+
+    // then
+    assertEquals(callback.getLocation().toString(), "https://redirecturl.com?params%3D%7B%7D");
+  }
+
+  @Test
+  public void shouldNotEncodeRedirectUrl() throws Exception {
+    // given
+    UriInfo uriInfo = mock(UriInfo.class);
+    OAuthAuthenticator authenticator = mock(OAuthAuthenticator.class);
+    when(uriInfo.getRequestUri())
+        .thenReturn(
+            new URI(
+                "http://eclipse.che?state=oauth_provider"
+                    + encode(
+                        "=github&redirect_after_login=https://redirecturl.com?params="
+                            + encode(encode("{}", UTF_8), UTF_8),
+                        UTF_8)));
+    when(oauth2Providers.getAuthenticator("github")).thenReturn(authenticator);
+
+    // when
+    Response callback = embeddedOAuthAPI.callback(uriInfo, emptyList());
+
+    // then
+    assertEquals(callback.getLocation().toString(), "https://redirecturl.com?params=%7B%7D");
+  }
 }