Encode redirect URL if needed on oauth1 callback request

wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPI.java

@@ -152,7 +152,7 @@ public class EmbeddedOAuthAPI implements OAuthAPI {
    * JSON, as a query parameter. This prevents passing unsupported characters, like '{' and '}' to
    * the {@link URI#create(String)} method.
    */
-  private String encodeRedirectUrl(String url) {
+  public static String encodeRedirectUrl(String url) {
     try {
       String query = new URL(url).getQuery();
       return url.substring(0, url.indexOf(query)) + URLEncoder.encode(query, UTF_8);

wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth1/OAuthAuthenticationService.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/
@@ -16,6 +16,7 @@ import static org.eclipse.che.commons.lang.UrlUtils.getParameter;
 import static org.eclipse.che.commons.lang.UrlUtils.getQueryParametersFromState;
 import static org.eclipse.che.commons.lang.UrlUtils.getRequestUrl;
 import static org.eclipse.che.commons.lang.UrlUtils.getState;
+import static org.eclipse.che.security.oauth.EmbeddedOAuthAPI.encodeRedirectUrl;
 
 import jakarta.ws.rs.GET;
 import jakarta.ws.rs.Path;
@@ -73,7 +74,14 @@ public class OAuthAuthenticationService extends Service {
     final Map<String, List<String>> parameters = getQueryParametersFromState(getState(requestUrl));
 
     final String providerName = getParameter(parameters, "oauth_provider");
-    final String redirectAfterLogin = getParameter(parameters, "redirect_after_login");
+    String redirectAfterLogin = getParameter(parameters, "redirect_after_login");
+
+    try {
+      URI.create(redirectAfterLogin);
+    } catch (IllegalArgumentException e) {
+      // the redirectUrl was decoded by the CSM provider, so we need to encode it back.
+      redirectAfterLogin = encodeRedirectUrl(redirectAfterLogin);
+    }
 
     UriBuilder redirectUriBuilder = UriBuilder.fromUri(redirectAfterLogin);
 

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2021 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/
@@ -12,20 +12,26 @@
 package org.eclipse.che.security.oauth1;
 
 import static io.restassured.RestAssured.given;
+import static java.net.URLEncoder.encode;
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.everrest.assured.JettyHttpServer.ADMIN_USER_NAME;
 import static org.everrest.assured.JettyHttpServer.ADMIN_USER_PASSWORD;
 import static org.everrest.assured.JettyHttpServer.SECURE_PATH;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 import static org.testng.Assert.assertEquals;
 
 import io.restassured.response.Response;
+import jakarta.ws.rs.core.UriInfo;
+import java.lang.reflect.Field;
+import java.net.URI;
 import java.net.URL;
+import org.eclipse.che.api.core.rest.Service;
 import org.everrest.assured.EverrestJetty;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.testng.MockitoTestNGListener;
-import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Listeners;
 import org.testng.annotations.Test;
 
@@ -41,17 +47,15 @@ public class OAuthAuthenticationServiceTest {
 
   @Mock private OAuthAuthenticator oAuthAuthenticator;
 
+  @Mock private UriInfo uriInfo;
+
   @Mock private OAuthAuthenticatorProvider oAuthProvider;
 
   @InjectMocks private OAuthAuthenticationService oAuthAuthenticationService;
 
-  @BeforeMethod
-  public void setUp() {
-    when(oAuthProvider.getAuthenticator("test-server")).thenReturn(oAuthAuthenticator);
-  }
-
   @Test
   public void shouldResolveCallbackWithoutError() throws OAuthAuthenticationException {
+    when(oAuthProvider.getAuthenticator("test-server")).thenReturn(oAuthAuthenticator);
     when(oAuthAuthenticator.callback(any(URL.class))).thenReturn("user1");
     final Response response =
         given()
@@ -70,6 +74,7 @@ public class OAuthAuthenticationServiceTest {
 
   @Test
   public void shouldResolveCallbackWithAccessDeniedError() throws OAuthAuthenticationException {
+    when(oAuthProvider.getAuthenticator("test-server")).thenReturn(oAuthAuthenticator);
     when(oAuthAuthenticator.callback(any(URL.class)))
         .thenThrow(new UserDeniedOAuthAuthenticationException("Access denied"));
     final Response response =
@@ -89,6 +94,7 @@ public class OAuthAuthenticationServiceTest {
 
   @Test
   public void shouldResolveCallbackWithInvalidRequestError() throws OAuthAuthenticationException {
+    when(oAuthProvider.getAuthenticator("test-server")).thenReturn(oAuthAuthenticator);
     when(oAuthAuthenticator.callback(any(URL.class)))
         .thenThrow(new OAuthAuthenticationException("Invalid request"));
     final Response response =
@@ -105,4 +111,52 @@ public class OAuthAuthenticationServiceTest {
     assertEquals(response.statusCode(), 307);
     assertEquals(response.header("Location"), REDIRECT_URI + "?error_code=invalid_request");
   }
+
+  @Test
+  public void shouldEncodeRedirectUrl() throws Exception {
+    // given
+    Field uriInfoField = Service.class.getDeclaredField("uriInfo");
+    uriInfoField.setAccessible(true);
+    uriInfoField.set(oAuthAuthenticationService, uriInfo);
+    when(uriInfo.getRequestUri())
+        .thenReturn(
+            new URI(
+                "http://eclipse.che?state=oauth_provider"
+                    + encode(
+                        "=bitbucket-server&redirect_after_login=https://redirecturl.com?params="
+                            + encode("{}", UTF_8),
+                        UTF_8)));
+    when(oAuthProvider.getAuthenticator("bitbucket-server"))
+        .thenReturn(mock(OAuthAuthenticator.class));
+
+    // when
+    jakarta.ws.rs.core.Response callback = oAuthAuthenticationService.callback();
+
+    // then
+    assertEquals(callback.getLocation().toString(), "https://redirecturl.com?params%3D%7B%7D");
+  }
+
+  @Test
+  public void shouldNotEncodeRedirectUrl() throws Exception {
+    // given
+    Field uriInfoField = Service.class.getDeclaredField("uriInfo");
+    uriInfoField.setAccessible(true);
+    uriInfoField.set(oAuthAuthenticationService, uriInfo);
+    when(uriInfo.getRequestUri())
+        .thenReturn(
+            new URI(
+                "http://eclipse.che?state=oauth_provider"
+                    + encode(
+                        "=bitbucket-server&redirect_after_login=https://redirecturl.com?params="
+                            + encode(encode("{}", UTF_8), UTF_8),
+                        UTF_8)));
+    when(oAuthProvider.getAuthenticator("bitbucket-server"))
+        .thenReturn(mock(OAuthAuthenticator.class));
+
+    // when
+    jakarta.ws.rs.core.Response callback = oAuthAuthenticationService.callback();
+
+    // then
+    assertEquals(callback.getLocation().toString(), "https://redirecturl.com?params=%7B%7D");
+  }
 }