fix: fixed multiple encoding issue during authz (#2152)

JanssenProject · Sep 1, 2022 · fb0b6d7 · fb0b6d7
1 parent de70cde
commit fb0b6d7
Show file tree

Hide file tree

Showing 26 changed files with 172 additions and 103 deletions.
diff --git a/jans-auth-server/client/src/main/java/io/jans/as/client/AuthorizationResponse.java b/jans-auth-server/client/src/main/java/io/jans/as/client/AuthorizationResponse.java
@@ -14,6 +14,7 @@
 import io.jans.as.model.jwe.Jwe;
 import io.jans.as.model.jwt.Jwt;
 import io.jans.as.model.util.JwtUtil;
+import io.jans.as.model.util.QueryStringDecoder;
 import io.jans.as.model.util.Util;
 import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;

diff --git a/jans-auth-server/client/src/main/java/io/jans/as/client/EndSessionClient.java b/jans-auth-server/client/src/main/java/io/jans/as/client/EndSessionClient.java
@@ -10,6 +10,7 @@
 import io.jans.as.model.session.EndSessionErrorResponseType;
 import io.jans.as.model.session.EndSessionRequestParam;
 import io.jans.as.model.session.EndSessionResponseParam;
+import io.jans.as.model.util.QueryStringDecoder;
 import io.jans.as.model.util.Util;
 import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;

diff --git a/...io/jans/as/client/QueryStringDecoder.java → ...ans/as/model/util/QueryStringDecoder.java b/...io/jans/as/client/QueryStringDecoder.java → ...ans/as/model/util/QueryStringDecoder.java
@@ -4,25 +4,25 @@
  * Copyright (c) 2020, Janssen Project
  */
 
-package io.jans.as.client;
+package io.jans.as.model.util;
 
 import org.apache.commons.lang.StringUtils;
+import org.apache.log4j.Logger;
 
+import java.io.UnsupportedEncodingException;
+import java.net.URLDecoder;
 import java.util.HashMap;
 import java.util.Map;
 
 /**
  * Provides functionality to parse query strings.
  *
- * @author Javier Rojas Blum Date: 09.29.2011
+ * @author Javier Rojas Blum
+ * @version November 24, 2017
  */
 public class QueryStringDecoder {
 
-    /**
-     * Avoid instance creation
-     */
-    private QueryStringDecoder() {
-    }
+    private static final Logger log = Logger.getLogger(QueryStringDecoder.class);
 
     /**
      * Decodes a query string and returns a map with the parsed query string
@@ -41,7 +41,11 @@ public static Map<String, String> decode(String queryString) {
                 String name = nameValue.length > 0 ? nameValue[0] : "";
                 String value = nameValue.length > 1 ? nameValue[1] : "";
                 if (StringUtils.isNotBlank(name)) {
-                    map.put(name, value);
+                    try {
+                        map.put(name, URLDecoder.decode(value, Util.UTF8_STRING_ENCODING));
+                    } catch (Exception e) {
+                        log.error(String.format("Error encoding query param, key: '%s', value: '%s'", name, value), e);
+                    }
                 }
             }
         }

diff --git a/jans-auth-server/model/src/test/java/io/jans/as/model/util/QueryStringDecoderTest.java b/jans-auth-server/model/src/test/java/io/jans/as/model/util/QueryStringDecoderTest.java
@@ -0,0 +1,97 @@
+package io.jans.as.model.util;
+
+import io.jans.as.model.BaseTest;
+import org.testng.annotations.Test;
+
+import java.util.Map;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+
+public class QueryStringDecoderTest extends BaseTest {
+
+    @Test
+    public void decode_nullParam_emptyMap() {
+        showTitle("decode_nullParam_emptyMap");
+
+        Map<String, String> queryParamMap = QueryStringDecoder.decode(null);
+
+        assertTrue(queryParamMap.isEmpty());
+    }
+
+    @Test
+    public void decode_emptyParam_emptyMap() {
+        showTitle("decode_emptyParam_emptyMap");
+
+        Map<String, String> queryParamMap = QueryStringDecoder.decode("");
+
+        assertTrue(queryParamMap.isEmpty());
+    }
+
+    @Test
+    public void decode_simpleParam_validParam() {
+        showTitle("decode_simpleParam_validParam");
+
+        String key1 = "key1";
+        String key2 = "key2";
+        String simpleValue = "SIMPLE";
+        String urlValue = "http://localhost:9000";
+        String queryParamString = String.format("%s=%s&%s=%s", key1, simpleValue, key2, urlValue);
+
+        Map<String, String> queryParamMap = QueryStringDecoder.decode(queryParamString);
+
+        assertEquals(queryParamMap.size(), 2);
+        assertEquals(queryParamMap.get(key1), simpleValue);
+        assertEquals(queryParamMap.get(key2), urlValue);
+    }
+
+    @Test
+    public void decode_encodedParam_validParam() {
+        showTitle("decode_encodedParam_validParam");
+
+        String key1 = "key1";
+        String key3 = "key3";
+        String urlValue = "http://localhost:9000";
+        String encodedUrlValue = "http%3A%2F%2Flocalhost%3A9000";
+        String queryParamString = String.format("%s=%s&%s=%s", key1, urlValue, key3, encodedUrlValue);
+
+        Map<String, String> queryParamMap = QueryStringDecoder.decode(queryParamString);
+
+        assertEquals(queryParamMap.size(), 2);
+        assertEquals(queryParamMap.get(key1), urlValue);
+        assertEquals(queryParamMap.get(key3), urlValue);
+    }
+
+    @Test
+    public void decode_emptyKeyAndValueParam_validParam() {
+        showTitle("decode_emptyKeyAndValueParam_validParam");
+
+        String emptyKey = "";
+        String key2 = "key2";
+        String simpleValue = "SIMPLE";
+        String emptyValue = "";
+        String queryParamString = String.format("%s=%s&%s=%s", emptyKey, simpleValue, key2, emptyValue);
+
+        Map<String, String> queryParamMap = QueryStringDecoder.decode(queryParamString);
+
+        assertEquals(queryParamMap.size(), 1);
+        assertEquals(queryParamMap.get(key2), emptyValue);
+    }
+
+    @Test
+    public void decode_unsupportedDecodedParam_validParam() {
+        showTitle("decode_unsupportedDecodedParam_validParam");
+
+        String key1 = "key1";
+        String key2 = "key2";
+        String urlValue = "http://localhost:9000";
+        String encodedUrlValue = "http%3A%2F%2Flocalhost%3A9000";
+        String unsupportedValue = "http%3A%2F%2Flocalhost%3A9000%GG";
+        String queryParamString = String.format("%s=%s&%s=%s", key1, encodedUrlValue, key2, unsupportedValue);
+
+        Map<String, String> queryParamMap = QueryStringDecoder.decode(queryParamString);
+
+        assertEquals(queryParamMap.size(), 1);
+        assertEquals(queryParamMap.get(key1), urlValue);
+    }
+}
diff --git a/...r/server/src/main/java/io/jans/as/server/authorize/ws/rs/AuthorizeRestWebServiceImpl.java b/...r/server/src/main/java/io/jans/as/server/authorize/ws/rs/AuthorizeRestWebServiceImpl.java
@@ -25,6 +25,7 @@
 import io.jans.as.model.crypto.binding.TokenBindingParseException;
 import io.jans.as.model.error.ErrorResponseFactory;
 import io.jans.as.model.token.JsonWebResponse;
+import io.jans.as.model.util.QueryStringDecoder;
 import io.jans.as.model.util.Util;
 import io.jans.as.server.audit.ApplicationAuditLogger;
 import io.jans.as.server.ciba.CIBAPingCallbackService;
@@ -71,7 +72,6 @@
 import io.jans.as.server.service.external.context.ExternalUpdateTokenContext;
 import io.jans.as.server.service.external.session.SessionEvent;
 import io.jans.as.server.service.external.session.SessionEventType;
-import io.jans.as.server.util.QueryStringDecoder;
 import io.jans.as.server.util.RedirectUtil;
 import io.jans.as.server.util.ServerUtil;
 import io.jans.orm.exception.EntryPersistenceException;

diff --git a/jans-auth-server/server/src/main/java/io/jans/as/server/par/ws/rs/ParRestWebService.java b/jans-auth-server/server/src/main/java/io/jans/as/server/par/ws/rs/ParRestWebService.java
@@ -12,12 +12,12 @@
 import io.jans.as.model.error.ErrorResponse;
 import io.jans.as.model.error.ErrorResponseFactory;
 import io.jans.as.model.jwt.Jwt;
+import io.jans.as.model.util.QueryStringDecoder;
 import io.jans.as.model.util.Util;
 import io.jans.as.persistence.model.Par;
 import io.jans.as.server.authorize.ws.rs.AuthorizeRestWebServiceValidator;
 import io.jans.as.server.service.RedirectUriResponse;
 import io.jans.as.server.service.RequestParameterService;
-import io.jans.as.server.util.QueryStringDecoder;
 import io.jans.as.server.util.ServerUtil;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.ThreadContext;

diff --git a/jans-auth-server/server/src/main/java/io/jans/as/server/service/RedirectionUriService.java b/jans-auth-server/server/src/main/java/io/jans/as/server/service/RedirectionUriService.java
@@ -9,11 +9,11 @@
 import com.google.common.base.Strings;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
-import io.jans.as.client.QueryStringDecoder;
 import io.jans.as.common.model.registration.Client;
 import io.jans.as.model.configuration.AppConfiguration;
 import io.jans.as.model.error.ErrorResponseFactory;
 import io.jans.as.model.session.EndSessionErrorResponseType;
+import io.jans.as.model.util.QueryStringDecoder;
 import io.jans.as.model.util.Util;
 import io.jans.as.common.model.session.SessionId;
 import org.apache.commons.lang.StringUtils;

diff --git a/jans-auth-server/server/src/main/java/io/jans/as/server/util/QueryStringDecoder.java b/jans-auth-server/server/src/main/java/io/jans/as/server/util/QueryStringDecoder.java
diff --git a/jans-auth-server/server/src/test/java/io/jans/as/server/model/uma/TTokenRequest.java b/jans-auth-server/server/src/test/java/io/jans/as/server/model/uma/TTokenRequest.java
@@ -7,7 +7,6 @@
 package io.jans.as.server.model.uma;
 
 import io.jans.as.client.AuthorizationRequest;
-import io.jans.as.client.QueryStringDecoder;
 import io.jans.as.client.TokenRequest;
 import io.jans.as.model.common.AuthenticationMethod;
 import io.jans.as.model.common.GrantType;
@@ -20,6 +19,7 @@
 import io.jans.as.model.uma.UmaScopeType;
 import io.jans.as.model.uma.UmaTestUtil;
 import io.jans.as.model.uma.wrapper.Token;
+import io.jans.as.model.util.QueryStringDecoder;
 import io.jans.as.server.BaseTest;
 import io.jans.as.server.util.ServerUtil;
 import org.apache.commons.lang.StringUtils;

diff --git a/...erver/server/src/test/java/io/jans/as/server/ws/rs/AuthorizationCodeFlowEmbeddedTest.java b/...erver/server/src/test/java/io/jans/as/server/ws/rs/AuthorizationCodeFlowEmbeddedTest.java
@@ -7,6 +7,7 @@
 package io.jans.as.server.ws.rs;
 
 import io.jans.as.client.RegisterRequest;
+import io.jans.as.model.util.QueryStringDecoder;
 import io.jans.as.server.util.TestUtil;
 import io.jans.as.model.authorize.AuthorizeResponseParam;
 import io.jans.as.model.common.GrantType;
@@ -144,7 +145,7 @@ public void completeFlowStep1(final String authorizePath, final String userId, f
                 URI uri = new URI(response.getLocation().toString());
                 assertNotNull(uri.getQuery(), "The query string is null");
 
-                Map<String, String> params = io.jans.as.client.QueryStringDecoder.decode(uri.getQuery());
+                Map<String, String> params = QueryStringDecoder.decode(uri.getQuery());
 
                 assertNotNull(params.get(AuthorizeResponseParam.CODE), "The code is null");
                 assertNotNull(params.get(AuthorizeResponseParam.SCOPE), "The scope is null");
@@ -284,7 +285,7 @@ public void completeFlowWithOptionalNonceStep1(final String authorizePath, final
                 URI uri = new URI(response.getLocation().toString());
                 assertNotNull(uri.getQuery(), "The query string is null");
 
-                Map<String, String> params = io.jans.as.client.QueryStringDecoder.decode(uri.getQuery());
+                Map<String, String> params = QueryStringDecoder.decode(uri.getQuery());
 
                 assertNotNull(params.get(AuthorizeResponseParam.CODE), "The code is null");
                 assertNotNull(params.get(AuthorizeResponseParam.SCOPE), "The scope is null");
@@ -436,7 +437,7 @@ public void revokeTokensStep1(final String authorizePath, final String userId, f
                 URI uri = new URI(response.getLocation().toString());
                 assertNotNull(uri.getQuery(), "The query string is null");
 
-                Map<String, String> params = io.jans.as.client.QueryStringDecoder.decode(uri.getQuery());
+                Map<String, String> params = QueryStringDecoder.decode(uri.getQuery());
 
                 assertNotNull(params.get(AuthorizeResponseParam.CODE), "The code is null");
                 assertNotNull(params.get(AuthorizeResponseParam.SCOPE), "The scope is null");
@@ -636,7 +637,7 @@ public void tokenExpirationStep1(final String authorizePath, final String userId
                     URI uri = new URI(response.getLocation().toString());
                     assertNotNull(uri.getQuery(), "The query string is null");
 
-                    Map<String, String> params = io.jans.as.client.QueryStringDecoder.decode(uri.getQuery());
+                    Map<String, String> params = QueryStringDecoder.decode(uri.getQuery());
 
                     assertNotNull(params.get(AuthorizeResponseParam.CODE), "The code is null");
                     assertNotNull(params.get(AuthorizeResponseParam.SCOPE), "The scope is null");