Encode redirect URL if needed on oauth1 callback request (#664)

The latest BItBucket Server decodes the callback url so that causes IllegalArgumentException error. Catch the error and decode the redirect url.

assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties

@@ -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/

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

@@ -124,15 +124,25 @@ public class EmbeddedOAuthAPI implements OAuthAPI {
       // Skip exception, the token will be stored in the next request.
       LOG.error(e.getMessage(), e);
     }
-    final String redirectAfterLogin = getParameter(params, "redirect_after_login");
-    URI uri;
+    return Response.temporaryRedirect(URI.create(getRedirectAfterLoginUrl(params))).build();
+  }
+
+  /**
+   * Returns the redirect after login URL from the query parameters. If the URL is encoded by the
+   * CSM provider, it will be decoded, to avoid unsupported characters in the URL.
+   *
+   * @param parameters the query parameters
+   * @return the redirect after login URL
+   */
+  public static String getRedirectAfterLoginUrl(Map<String, List<String>> parameters) {
+    String redirectAfterLogin = getParameter(parameters, "redirect_after_login");
     try {
-      uri = URI.create(redirectAfterLogin);
+      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));
+      redirectAfterLogin = encodeRedirectUrl(redirectAfterLogin);
     }
-    return Response.temporaryRedirect(uri).build();
+    return redirectAfterLogin;
   }
 
   /*
@@ -152,7 +162,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) {
+  private 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.getRedirectAfterLoginUrl;
 
 import jakarta.ws.rs.GET;
 import jakarta.ws.rs.Path;
@@ -73,7 +74,7 @@ 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");
+    final String redirectAfterLogin = getRedirectAfterLoginUrl(parameters);
 
     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");
+  }
 }