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 <[email protected]> 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 <[email protected]> --- .../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);
