You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by gn...@apache.org on 2026/03/12 08:31:08 UTC
(cxf) 01/01: Fix ModelEncryptionSupport serialization/deserialization bugs
This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch fix/model-encryption-support
in repository https://gitbox.apache.org/repos/asf/cxf.git
commit 623906aa059fa8678775eff862f91993c2177930
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Thu Mar 12 09:30:50 2026 +0100
Fix ModelEncryptionSupport serialization/deserialization bugs
- Use empty string instead of space for null values in tokenizeString
- Use tokenizeCollection/tokenizeMap instead of toString() to avoid
brackets and spaces in serialized collections/maps
- Use split with -1 limit to preserve trailing empty strings
- Add empty check in parseSimpleMap to handle empty map strings
- Apply getStringPart to client id and secret in recreateClient
- Add testPublicClient test verifying null secret round-trips correctly
Closes #816
Co-Authored-By: Claude Opus 4.6 <no...@anthropic.com>
---
.../utils/crypto/ModelEncryptionSupport.java | 116 +++++++++++----------
.../oauth2/utils/crypto/CryptoUtilsTest.java | 17 +++
2 files changed, 79 insertions(+), 54 deletions(-)
diff --git a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/utils/crypto/ModelEncryptionSupport.java b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/utils/crypto/ModelEncryptionSupport.java
index e5c7d3e13e..18466d8a48 100644
--- a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/utils/crypto/ModelEncryptionSupport.java
+++ b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/utils/crypto/ModelEncryptionSupport.java
@@ -21,11 +21,14 @@ package org.apache.cxf.rs.security.oauth2.utils.crypto;
import java.security.Key;
import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
+import java.util.StringJoiner;
+import java.util.stream.Collectors;
import javax.crypto.SecretKey;
import javax.security.auth.DestroyFailedException;
@@ -279,7 +282,7 @@ public final class ModelEncryptionSupport {
// Permissions
if (!parts[9].trim().isEmpty()) {
List<OAuthPermission> perms = new LinkedList<>();
- String[] allPermParts = parts[9].split("\\.");
+ String[] allPermParts = parts[9].split("\\.", -1);
for (int i = 0; i + 4 < allPermParts.length; i = i + 5) {
OAuthPermission perm = new OAuthPermission(allPermParts[i], allPermParts[i + 1]);
perm.setDefaultPermission(Boolean.parseBoolean(allPermParts[i + 2]));
@@ -301,7 +304,7 @@ public final class ModelEncryptionSupport {
private static String tokenizeRefreshToken(RefreshToken token) {
String seq = tokenizeServerToken(token);
- return seq + SEP + token.getAccessTokens().toString();
+ return seq + SEP + tokenizeCollection(token.getAccessTokens());
}
private static String tokenizeServerToken(ServerAccessToken token) {
@@ -328,32 +331,14 @@ public final class ModelEncryptionSupport {
state.append(tokenizeString(token.getGrantType()));
// 7: audience
state.append(SEP);
- state.append(token.getAudiences().toString());
+ state.append(tokenizeCollection(token.getAudiences()));
// 8: other parameters
state.append(SEP);
- // {key=value, key=value}
- state.append(token.getParameters().toString());
+ state.append(tokenizeMap(token.getParameters()));
// 9: permissions
state.append(SEP);
- if (token.getScopes().isEmpty()) {
- state.append(' ');
- } else {
- for (OAuthPermission p : token.getScopes()) {
- // 9.1
- state.append(tokenizeString(p.getPermission()));
- state.append('.');
- // 9.2
- state.append(tokenizeString(p.getDescription()));
- state.append('.');
- // 9.3
- state.append(p.isDefaultPermission());
- state.append('.');
- // 9.4
- state.append(p.getHttpVerbs().toString());
- state.append('.');
- // 9.5
- state.append(p.getUris().toString());
- }
+ if (!token.getScopes().isEmpty()) {
+ state.append(tokenizePermissions(token.getScopes()));
}
state.append(SEP);
// 10: code verifier
@@ -363,16 +348,33 @@ public final class ModelEncryptionSupport {
tokenizeUserSubject(state, token.getSubject());
// 13: extra properties
state.append(SEP);
- // {key=value, key=value}
- state.append(token.getExtraProperties().toString());
+ state.append(tokenizeMap(token.getExtraProperties()));
return state.toString();
}
+ private static String tokenizePermissions(List<OAuthPermission> perms) {
+ StringJoiner joiner = new StringJoiner(".");
+ for (OAuthPermission p : perms) {
+ StringBuilder sb = new StringBuilder();
+ sb.append(tokenizeString(p.getPermission()));
+ sb.append('.');
+ sb.append(tokenizeString(p.getDescription()));
+ sb.append('.');
+ sb.append(p.isDefaultPermission());
+ sb.append('.');
+ sb.append(tokenizeCollection(p.getHttpVerbs()));
+ sb.append('.');
+ sb.append(tokenizeCollection(p.getUris()));
+ joiner.add(sb);
+ }
+ return joiner.toString();
+ }
+
private static Client recreateClientInternal(String sequence) {
String[] parts = getParts(sequence);
- Client c = new Client(parts[0],
- parts[1],
+ Client c = new Client(getStringPart(parts[0]),
+ getStringPart(parts[1]),
Boolean.parseBoolean(parts[2]),
getStringPart(parts[3]), getStringPart(parts[4]));
c.setApplicationDescription(getStringPart(parts[5]));
@@ -410,22 +412,22 @@ public final class ModelEncryptionSupport {
state.append(tokenizeString(client.getApplicationLogoUri()));
state.append(SEP);
// 7: app certificates
- state.append(client.getApplicationCertificates());
+ state.append(tokenizeCollection(client.getApplicationCertificates()));
state.append(SEP);
// 8: grants
- state.append(client.getAllowedGrantTypes().toString());
+ state.append(tokenizeCollection(client.getAllowedGrantTypes()));
state.append(SEP);
// 9: redirect URIs
- state.append(client.getRedirectUris().toString());
+ state.append(tokenizeCollection(client.getRedirectUris()));
state.append(SEP);
// 10: registered scopes
- state.append(client.getRegisteredScopes().toString());
+ state.append(tokenizeCollection(client.getRegisteredScopes()));
state.append(SEP);
// 11: registered audiences
- state.append(client.getRegisteredAudiences().toString());
+ state.append(tokenizeCollection(client.getRegisteredAudiences()));
state.append(SEP);
// 12: properties
- state.append(client.getProperties().toString());
+ state.append(tokenizeMap(client.getProperties()));
state.append(SEP);
// 13: subject
tokenizeUserSubject(state, client.getSubject());
@@ -471,34 +473,31 @@ public final class ModelEncryptionSupport {
state.append(tokenizeString(grant.getClientCodeChallenge()));
state.append(SEP);
// 7: approved scopes
- state.append(grant.getApprovedScopes().toString());
+ state.append(tokenizeCollection(grant.getApprovedScopes()));
state.append(SEP);
// 8: subject
tokenizeUserSubject(state, grant.getSubject());
// 9: extra properties
state.append(SEP);
- // {key=value, key=value}
- state.append(grant.getExtraProperties().toString());
+ state.append(tokenizeMap(grant.getExtraProperties()));
return state.toString();
}
public static String getStringPart(String str) {
- return " ".equals(str) ? null : str;
- }
-
- private static String prepareSimpleString(String str) {
- return str.trim().isEmpty() ? "" : str.substring(1, str.length() - 1);
+ return "".equals(str) ? null : str;
}
private static List<String> parseSimpleList(String listStr) {
- String pureStringList = prepareSimpleString(listStr);
- if (pureStringList.isEmpty()) {
+ if (listStr.isEmpty()) {
return Collections.emptyList();
}
- return Arrays.asList(pureStringList.split(","));
+ return Arrays.asList(listStr.split(",", -1));
}
public static Map<String, String> parseSimpleMap(String mapStr) {
+ if (mapStr.isEmpty()) {
+ return new HashMap<>();
+ }
Map<String, String> props = new HashMap<>();
List<String> entries = parseSimpleList(mapStr);
for (String entry : entries) {
@@ -509,20 +508,18 @@ public final class ModelEncryptionSupport {
}
public static String[] getParts(String sequence) {
- return sequence.split("\\" + SEP);
+ return sequence.split("\\" + SEP, -1);
}
private static UserSubject recreateUserSubject(String sequence) {
UserSubject subject = null;
if (!sequence.trim().isEmpty()) {
- String[] subjectParts = sequence.split("\\.");
+ String[] subjectParts = sequence.split("\\.", -1);
subject = new UserSubject(getStringPart(subjectParts[0]), getStringPart(subjectParts[1]));
subject.setRoles(parseSimpleList(subjectParts[2]));
subject.setProperties(parseSimpleMap(subjectParts[3]));
}
return subject;
-
-
}
private static void tokenizeUserSubject(StringBuilder state, UserSubject subject) {
@@ -534,16 +531,27 @@ public final class ModelEncryptionSupport {
state.append(tokenizeString(subject.getId()));
state.append('.');
// 3
- state.append(subject.getRoles().toString());
+ state.append(tokenizeCollection(subject.getRoles()));
state.append('.');
// 4
- state.append(subject.getProperties().toString());
- } else {
- state.append(' ');
+ state.append(tokenizeMap(subject.getProperties()));
}
}
public static String tokenizeString(String str) {
- return str != null ? str : " ";
+ return str != null ? str : "";
+ }
+
+ private static String tokenizeCollection(Collection<? extends String> col) {
+ return col != null ? String.join(",", col) : "";
+ }
+
+ private static String tokenizeMap(Map<String, String> map) {
+ if (map == null || map.isEmpty()) {
+ return "";
+ }
+ return map.entrySet().stream()
+ .map(String::valueOf)
+ .collect(Collectors.joining(","));
}
}
diff --git a/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/utils/crypto/CryptoUtilsTest.java b/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/utils/crypto/CryptoUtilsTest.java
index dd529d45ad..afafe94e4f 100644
--- a/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/utils/crypto/CryptoUtilsTest.java
+++ b/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/utils/crypto/CryptoUtilsTest.java
@@ -50,6 +50,7 @@ import org.junit.Before;
import org.junit.Test;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
@@ -192,6 +193,22 @@ public class CryptoUtilsTest {
}
+ @Test
+ public void testPublicClient() throws Exception {
+ Client c = new Client("client", null, false);
+ c.setSubject(new UserSubject("subject", "id"));
+
+ String encryptedClient = ModelEncryptionSupport.encryptClient(c, p.key);
+ Client c2 = ModelEncryptionSupport.decryptClient(encryptedClient, p.key);
+
+ assertEquals(c.getClientId(), c2.getClientId());
+ assertEquals(c.getClientSecret(), c2.getClientSecret());
+ assertNull(c2.getClientSecret());
+ assertFalse(c2.isConfidential());
+ assertEquals("subject", c2.getSubject().getLogin());
+ assertEquals("id", c2.getSubject().getId());
+ }
+
@Test
public void testCodeGrantJSON() throws Exception {
Client c = new Client("client", "secret", true);