From b2ff110da5d227ca6b55669f8fbbe854e5c22d65 Mon Sep 17 00:00:00 2001
From: Igor Vinokur <ivinokur@redhat.com>
Date: Wed, 27 Sep 2023 14:20:27 +0300
Subject: [PATCH] Encode the authentication reject error to build a proper
 callback url (#568)

Encode the &error_code=access_denied query param for the callback url in order to fix the bug when the authentication request appears again if it was rejected.
---
 .../che/security/oauth/EmbeddedOAuthAPI.java  | 24 +++++++++++++++---
 .../security/oauth/EmbeddedOAuthAPITest.java  | 25 ++++++++++++++++++-
 2 files changed, 45 insertions(+), 4 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 35e0b4a874..f04d2832e5 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
@@ -12,6 +12,7 @@
 package org.eclipse.che.security.oauth;
 
 import static com.google.common.base.Strings.isNullOrEmpty;
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Collections.emptyList;
 import static org.eclipse.che.commons.lang.UrlUtils.*;
 import static org.eclipse.che.commons.lang.UrlUtils.getParameter;
@@ -24,8 +25,10 @@ import jakarta.ws.rs.core.Response;
 import jakarta.ws.rs.core.UriBuilder;
 import jakarta.ws.rs.core.UriInfo;
 import java.io.IOException;
+import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URL;
+import java.net.URLEncoder;
 import java.util.*;
 import javax.inject.Inject;
 import javax.inject.Named;
@@ -84,9 +87,7 @@ public class EmbeddedOAuthAPI implements OAuthAPI, OAuthTokenFetcher {
     if (!isNullOrEmpty(redirectAfterLogin)
         && errorValues != null
         && errorValues.contains("access_denied")) {
-      return Response.temporaryRedirect(
-              URI.create(redirectAfterLogin + "&error_code=access_denied"))
-          .build();
+      return Response.temporaryRedirect(URI.create(encodeRedirectUrl())).build();
     }
     final String providerName = getParameter(params, "oauth_provider");
     OAuthAuthenticator oauth = getAuthenticator(providerName);
@@ -104,6 +105,23 @@ public class EmbeddedOAuthAPI implements OAuthAPI, OAuthTokenFetcher {
     return Response.temporaryRedirect(URI.create(redirectAfterLogin)).build();
   }
 
+  /**
+   * Encode the redirect URL query parameters to avoid the error when the redirect URL contains
+   * JSON, as a query parameter. This prevents passing unsupported characters, like '{' and '}' to
+   * the {@link URI#create(String)} method.
+   */
+  private String encodeRedirectUrl() {
+    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);
+    } catch (MalformedURLException e) {
+      LOG.error(e.getMessage(), e);
+      throw new RuntimeException(e);
+    }
+  }
+
   @Override
   public Set<OAuthAuthenticatorDescriptor> getRegisteredAuthenticators(UriInfo uriInfo) {
     Set<OAuthAuthenticatorDescriptor> result = new HashSet<>();
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 74269621d3..53670aa6e4 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
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2018 Red Hat, Inc.
+ * Copyright (c) 2012-2023 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/
@@ -11,6 +11,7 @@
  */
 package org.eclipse.che.security.oauth;
 
+import static java.util.Collections.singletonList;
 import static org.eclipse.che.dto.server.DtoFactory.newDto;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
@@ -18,6 +19,10 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 import static org.testng.Assert.assertEquals;
 
+import jakarta.ws.rs.core.Response;
+import jakarta.ws.rs.core.UriInfo;
+import java.lang.reflect.Field;
+import java.net.URI;
 import org.eclipse.che.api.auth.shared.dto.OAuthToken;
 import org.eclipse.che.api.core.NotFoundException;
 import org.mockito.InjectMocks;
@@ -54,4 +59,22 @@ public class EmbeddedOAuthAPITest {
 
     assertEquals(result.getToken(), token);
   }
+
+  @Test
+  public void shouldEncodeRejectErrorForRedirectUrl() throws Exception {
+    // given
+    UriInfo uriInfo = mock(UriInfo.class);
+    when(uriInfo.getRequestUri()).thenReturn(new URI("http://eclipse.che"));
+    Field redirectAfterLogin = EmbeddedOAuthAPI.class.getDeclaredField("redirectAfterLogin");
+    redirectAfterLogin.setAccessible(true);
+    redirectAfterLogin.set(embeddedOAuthAPI, "http://eclipse.che?quary=param");
+
+    // when
+    Response callback = embeddedOAuthAPI.callback(uriInfo, singletonList("access_denied"));
+
+    // then
+    assertEquals(
+        callback.getLocation().toString(),
+        "http://eclipse.che?quary%3Dparam%26error_code%3Daccess_denied");
+  }
 }