Bugfix for double URL encoding

The serialization of the EnrollmentRequest was twice URL encoded. Too avoid any problems with URL encoding / decoding we now use the org.apache.commons.codec.binary.Base64.encodeBase64 method with the urlSafe safe option set to true.
SURFnet · Jan 19, 2024 · f771fba · f771fba
1 parent f4b7050
commit f771fba
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 19 deletions.
diff --git a/pom.xml b/pom.xml
@@ -10,7 +10,7 @@
     </parent>
     <groupId>org.surfnet</groupId>
     <artifactId>student-mobility-inteken-ontvanger-generiek</artifactId>
-    <version>0.2.11</version>
+    <version>0.2.12</version>
     <name>inteken-ontvanger-generiek</name>
     <description>inteken-ontvanger-generiek</description>
 
@@ -61,6 +61,12 @@
             <groupId>org.springframework.boot</groupId>
             <artifactId>spring-boot-starter-security</artifactId>
         </dependency>
+        <!-- https://mvnrepository.com/artifact/commons-codec/commons-codec -->
+        <dependency>
+            <groupId>commons-codec</groupId>
+            <artifactId>commons-codec</artifactId>
+            <version>1.16.0</version>
+        </dependency>
         <dependency>
             <groupId>org.projectlombok</groupId>
             <artifactId>lombok</artifactId>

diff --git a/src/main/java/generiek/model/EnrollmentRequest.java b/src/main/java/generiek/model/EnrollmentRequest.java
@@ -13,9 +13,6 @@
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.Serializable;
-import java.net.URLDecoder;
-import java.net.URLEncoder;
-import java.nio.charset.Charset;
 import java.time.Instant;
 import java.util.*;
 import java.util.zip.GZIPInputStream;
@@ -91,8 +88,8 @@ public String serializeToBase64(ObjectMapper objectMapper) throws IOException {
         GZIPOutputStream gout = new GZIPOutputStream(bos);
         gout.write(bytes);
         gout.finish();
-
-        return URLEncoder.encode(Base64.getEncoder().encodeToString(bos.toByteArray()), Charset.defaultCharset().name());
+        //Avoid decoding / encoding as URL parameter problems
+        return new String(org.apache.commons.codec.binary.Base64.encodeBase64(bos.toByteArray(), false, true));
     }
 
     public String toString() {
@@ -110,8 +107,9 @@ public String toString() {
     }
 
     @SuppressWarnings("unchecked")
-    public static EnrollmentRequest serializeFromBase64(ObjectMapper objectMapper, String base64) throws IOException {
-        byte[] decoded = Base64.getDecoder().decode(URLDecoder.decode(base64, Charset.defaultCharset().name()));
+    public static EnrollmentRequest serializeFromBase64(ObjectMapper objectMapper,
+                                                        String base64) throws IOException {
+        byte[] decoded = org.apache.commons.codec.binary.Base64.decodeBase64(base64);
         //Equal or more than 42 KB is considered a gzip bomb attack
         if (decoded.length / 1024 >= 42) {
             throw new IllegalArgumentException("GZip bomb detected");

diff --git a/src/test/java/generiek/api/EnrollmentEndpointTest.java b/src/test/java/generiek/api/EnrollmentEndpointTest.java
@@ -29,7 +29,6 @@
 
 import java.io.IOException;
 import java.net.URLDecoder;
-import java.nio.charset.Charset;
 import java.security.*;
 import java.security.interfaces.RSAPrivateKey;
 import java.security.interfaces.RSAPublicKey;
@@ -610,7 +609,7 @@ private String doAuthorize(String personAuth) {
         MultiValueMap<String, String> params = UriComponentsBuilder.fromHttpUrl(location).build().getQueryParams();
         String scope = params.getFirst("scope");
         assertEquals("openid write", URLDecoder.decode(scope, "UTF-8"));
-        return URLDecoder.decode(params.getFirst("state"), Charset.defaultCharset().name());
+        return params.getFirst("state");
     }
 
 
@@ -630,7 +629,8 @@ private String doToken(String state) throws NoSuchProviderException, NoSuchAlgor
                 .withHeader("Content-Type", "application/json")
                 .withBody(objectMapper.writeValueAsString(tokenResult))));
 
-        String location = given().redirects().follow(false)
+        String location = given()
+                .redirects().follow(false)
                 .when()
                 .queryParam("code", "123456")
                 .queryParam("state", state)

diff --git a/src/test/java/generiek/model/EnrollmentRequestTest.java b/src/test/java/generiek/model/EnrollmentRequestTest.java
@@ -2,10 +2,12 @@
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.apache.commons.lang3.RandomStringUtils;
-import org.jetbrains.annotations.TestOnly;
 import org.junit.jupiter.api.Test;
 
 import java.io.IOException;
+import java.net.URLDecoder;
+import java.net.URLEncoder;
+import java.nio.charset.Charset;
 import java.util.Base64;
 import java.util.HashSet;
 
@@ -22,7 +24,7 @@ void serialization() throws IOException {
         enrollmentRequest.setEduid("eduID");
         enrollmentRequest.setRefreshToken("refreshToken");
         String randomString = RandomStringUtils.randomAscii(500);
-        enrollmentRequest.setHomeInstitution("uu.utrecht" + randomString);
+        enrollmentRequest.setHomeInstitution("uu+ utrecht" + randomString);
         enrollmentRequest.setPersonAuth(PersonAuthentication.HEADER.name());
         enrollmentRequest.setPersonURI("https://results.uu.university.com" + randomString);
         enrollmentRequest.setScope("https://long.scope.uri.at.somewhere" + randomString);
@@ -37,6 +39,12 @@ void serialization() throws IOException {
         //Ensure we don't max out on the query param size - which we won't for the GZIP compression
         assertTrue(base64.length() < 1024);
 
+        //Ensure URL decoding / encoding does not change the base64
+        String encoded = URLEncoder.encode(base64, Charset.defaultCharset().name());
+        assertEquals(base64, encoded);
+        String decoded = URLDecoder.decode(encoded, Charset.defaultCharset().name());
+        assertEquals(encoded, decoded);
+
         EnrollmentRequest newEnrollmentRequest = EnrollmentRequest.serializeFromBase64(objectMapper, base64);
 
         assertEquals(enrollmentRequest.getPersonURI(), newEnrollmentRequest.getPersonURI());
@@ -53,10 +61,5 @@ void serializeFromBase64GZipBomb() {
         assertThrows(IllegalArgumentException.class, () -> EnrollmentRequest.serializeFromBase64(objectMapper, s));
     }
 
-    @Test
-    void serializeStateTest() throws IOException {
-        String s = "H4sIAAAAAAAAAH2MsQ5AQBAFf0W2xlaa6yQkan%2BgOHFxbi%2F7DoX4d1doKSeZmYsiGVpSijDMKiGdFXada4FTh8q7sNbBs8gUHR8NR6uQAN4slTTldujbrh8zIMP34O2Kb0Mtdp%2BQT8vfie4HirfhR7QAAAA%3D";
-        EnrollmentRequest enrollmentRequest = EnrollmentRequest.serializeFromBase64(objectMapper, s);
-        assertEquals("rontw-surf.osiris-link.nl", enrollmentRequest.getHomeInstitution());
-    }
+
 }