From a50e8713b0c400b63da07229b98849cfcbd1e1c7 Mon Sep 17 00:00:00 2001 From: Michal Hlavac Date: Wed, 16 Jun 2021 12:17:02 +0200 Subject: [PATCH 1/2] fix ModelEncryptionSupport serializer/deserializer used in DefaultEncryptingOAuthDataProvider --- .../utils/crypto/ModelEncryptionSupport.java | 68 ++++++++++--------- .../oauth2/utils/crypto/CryptoUtilsTest.java | 15 ++++ 2 files changed, 50 insertions(+), 33 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 e5c7d3e13e3..98dc37e43d2 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,6 +21,7 @@ import java.security.Key; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.LinkedList; @@ -40,6 +41,7 @@ import org.apache.cxf.rt.security.crypto.CryptoUtils; import org.apache.cxf.rt.security.crypto.KeyProperties; +import static java.util.stream.Collectors.toList; /** * Default Model Encryption helpers @@ -279,7 +281,7 @@ private static ServerAccessToken recreateAccessToken(OAuthDataProvider provider, // Permissions if (!parts[9].trim().isEmpty()) { List 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])); @@ -328,16 +330,14 @@ private static String tokenizeServerToken(ServerAccessToken token) { 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 { + if (!token.getScopes().isEmpty()) { for (OAuthPermission p : token.getScopes()) { // 9.1 state.append(tokenizeString(p.getPermission())); @@ -349,10 +349,10 @@ private static String tokenizeServerToken(ServerAccessToken token) { state.append(p.isDefaultPermission()); state.append('.'); // 9.4 - state.append(p.getHttpVerbs().toString()); + state.append(tokenizeCollection(p.getHttpVerbs())); state.append('.'); // 9.5 - state.append(p.getUris().toString()); + state.append(tokenizeCollection(p.getUris())); } } state.append(SEP); @@ -364,15 +364,15 @@ private static String tokenizeServerToken(ServerAccessToken token) { // 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 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 +410,22 @@ private static String tokenizeClient(Client client) { 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()); @@ -478,24 +478,19 @@ private static String tokenizeCodeGrant(ServerAuthorizationCodeGrant grant) { // 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 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 parseSimpleMap(String mapStr) { @@ -509,13 +504,13 @@ public static Map parseSimpleMap(String mapStr) { } 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])); @@ -534,16 +529,23 @@ private static void tokenizeUserSubject(StringBuilder state, UserSubject subject 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 : ""; + } + + public static String tokenizeCollection(Collection col) { + return col != null ? String.join(",", col) : ""; + } + + public static String tokenizeMap(Map map) { + List entryStrings = map.entrySet().stream().map(String::valueOf).collect(toList()); + return map != null ? String.join(",", entryStrings) : ""; } } 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 a4378d58c5b..f852aa6c74d 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.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; @@ -191,6 +192,20 @@ public void testClientJSON() throws Exception { assertEquals("id", c2.getSubject().getId()); } + @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()); + assertFalse(c2.isConfidential()); + assertEquals("subject", c2.getSubject().getLogin()); + assertEquals("id", c2.getSubject().getId()); + } @Test public void testCodeGrantJSON() throws Exception { From 5102ba1fd83ee70b54d8f9f5aab521681bba4c37 Mon Sep 17 00:00:00 2001 From: Michal Hlavac Date: Wed, 30 Jun 2021 13:29:03 +0200 Subject: [PATCH 2/2] fix scope parser --- .../utils/crypto/ModelEncryptionSupport.java | 163 +++++++++--------- 1 file changed, 86 insertions(+), 77 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 98dc37e43d2..e2b45bb0ff8 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 @@ -27,6 +27,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.StringJoiner; import javax.crypto.SecretKey; import javax.security.auth.DestroyFailedException; @@ -56,7 +57,7 @@ public static String encryptClient(Client client, Key secretKey) throws Security } public static String encryptClient(Client client, Key secretKey, - KeyProperties props) throws SecurityException { + KeyProperties props) throws SecurityException { String tokenSequence = tokenizeClient(client); return CryptoUtils.encryptSequence(tokenSequence, secretKey, props); } @@ -66,7 +67,7 @@ public static String encryptAccessToken(ServerAccessToken token, Key secretKey) } public static String encryptAccessToken(ServerAccessToken token, Key secretKey, - KeyProperties props) throws SecurityException { + KeyProperties props) throws SecurityException { String tokenSequence = tokenizeServerToken(token); return CryptoUtils.encryptSequence(tokenSequence, secretKey, props); } @@ -76,31 +77,31 @@ public static String encryptRefreshToken(RefreshToken token, Key secretKey) thro } public static String encryptRefreshToken(RefreshToken token, Key secretKey, - KeyProperties props) throws SecurityException { + KeyProperties props) throws SecurityException { String tokenSequence = tokenizeRefreshToken(token); return CryptoUtils.encryptSequence(tokenSequence, secretKey, props); } public static String encryptCodeGrant(ServerAuthorizationCodeGrant grant, Key secretKey) - throws SecurityException { + throws SecurityException { return encryptCodeGrant(grant, secretKey, null); } public static String encryptCodeGrant(ServerAuthorizationCodeGrant grant, Key secretKey, - KeyProperties props) throws SecurityException { + KeyProperties props) throws SecurityException { String tokenSequence = tokenizeCodeGrant(grant); return CryptoUtils.encryptSequence(tokenSequence, secretKey, props); } public static Client decryptClient(String encodedSequence, String encodedSecretKey) - throws SecurityException { + throws SecurityException { return decryptClient(encodedSequence, encodedSecretKey, new KeyProperties("AES")); } public static Client decryptClient(String encodedSequence, String encodedSecretKey, - KeyProperties props) throws SecurityException { + KeyProperties props) throws SecurityException { SecretKey key = CryptoUtils.decodeSecretKey(encodedSecretKey, props.getKeyAlgo()); Client client = decryptClient(encodedSequence, key, props); @@ -119,21 +120,21 @@ public static Client decryptClient(String encodedSequence, Key secretKey) throws } public static Client decryptClient(String encodedData, Key secretKey, - KeyProperties props) throws SecurityException { + KeyProperties props) throws SecurityException { String decryptedSequence = CryptoUtils.decryptSequence(encodedData, secretKey, props); return recreateClient(decryptedSequence); } public static ServerAccessToken decryptAccessToken(OAuthDataProvider provider, - String encodedToken, - String encodedSecretKey) throws SecurityException { + String encodedToken, + String encodedSecretKey) throws SecurityException { return decryptAccessToken(provider, encodedToken, encodedSecretKey, new KeyProperties("AES")); } public static ServerAccessToken decryptAccessToken(OAuthDataProvider provider, - String encodedToken, - String encodedSecretKey, - KeyProperties props) throws SecurityException { + String encodedToken, + String encodedSecretKey, + KeyProperties props) throws SecurityException { SecretKey key = CryptoUtils.decodeSecretKey(encodedSecretKey, props.getKeyAlgo()); ServerAccessToken serverAccessToken = decryptAccessToken(provider, encodedToken, key, props); @@ -148,29 +149,29 @@ public static ServerAccessToken decryptAccessToken(OAuthDataProvider provider, } public static ServerAccessToken decryptAccessToken(OAuthDataProvider provider, - String encodedToken, - Key secretKey) throws SecurityException { + String encodedToken, + Key secretKey) throws SecurityException { return decryptAccessToken(provider, encodedToken, secretKey, null); } public static ServerAccessToken decryptAccessToken(OAuthDataProvider provider, - String encodedData, - Key secretKey, - KeyProperties props) throws SecurityException { + String encodedData, + Key secretKey, + KeyProperties props) throws SecurityException { String decryptedSequence = CryptoUtils.decryptSequence(encodedData, secretKey, props); return recreateAccessToken(provider, encodedData, decryptedSequence); } public static RefreshToken decryptRefreshToken(OAuthDataProvider provider, - String encodedToken, - String encodedSecretKey) throws SecurityException { + String encodedToken, + String encodedSecretKey) throws SecurityException { return decryptRefreshToken(provider, encodedToken, encodedSecretKey, new KeyProperties("AES")); } public static RefreshToken decryptRefreshToken(OAuthDataProvider provider, - String encodedToken, - String encodedSecretKey, - KeyProperties props) throws SecurityException { + String encodedToken, + String encodedSecretKey, + KeyProperties props) throws SecurityException { SecretKey key = CryptoUtils.decodeSecretKey(encodedSecretKey, props.getKeyAlgo()); RefreshToken refreshToken = decryptRefreshToken(provider, encodedToken, key, props); @@ -185,29 +186,29 @@ public static RefreshToken decryptRefreshToken(OAuthDataProvider provider, } public static RefreshToken decryptRefreshToken(OAuthDataProvider provider, - String encodedToken, - Key key) throws SecurityException { + String encodedToken, + Key key) throws SecurityException { return decryptRefreshToken(provider, encodedToken, key, null); } public static RefreshToken decryptRefreshToken(OAuthDataProvider provider, - String encodedData, - Key key, - KeyProperties props) throws SecurityException { + String encodedData, + Key key, + KeyProperties props) throws SecurityException { String decryptedSequence = CryptoUtils.decryptSequence(encodedData, key, props); return recreateRefreshToken(provider, encodedData, decryptedSequence); } public static ServerAuthorizationCodeGrant decryptCodeGrant(OAuthDataProvider provider, - String encodedToken, - String encodedSecretKey) throws SecurityException { + String encodedToken, + String encodedSecretKey) throws SecurityException { return decryptCodeGrant(provider, encodedToken, encodedSecretKey, new KeyProperties("AES")); } public static ServerAuthorizationCodeGrant decryptCodeGrant(OAuthDataProvider provider, - String encodedToken, - String encodedSecretKey, - KeyProperties props) throws SecurityException { + String encodedToken, + String encodedSecretKey, + KeyProperties props) throws SecurityException { SecretKey key = CryptoUtils.decodeSecretKey(encodedSecretKey, props.getKeyAlgo()); ServerAuthorizationCodeGrant authzCodeGrant = decryptCodeGrant(provider, encodedToken, key, props); @@ -222,37 +223,37 @@ public static ServerAuthorizationCodeGrant decryptCodeGrant(OAuthDataProvider pr } public static ServerAuthorizationCodeGrant decryptCodeGrant(OAuthDataProvider provider, - String encodedToken, - Key key) throws SecurityException { + String encodedToken, + Key key) throws SecurityException { return decryptCodeGrant(provider, encodedToken, key, null); } public static ServerAuthorizationCodeGrant decryptCodeGrant(OAuthDataProvider provider, - String encodedData, - Key key, - KeyProperties props) throws SecurityException { + String encodedData, + Key key, + KeyProperties props) throws SecurityException { String decryptedSequence = CryptoUtils.decryptSequence(encodedData, key, props); return recreateCodeGrant(provider, decryptedSequence); } public static ServerAccessToken recreateAccessToken(OAuthDataProvider provider, - String newTokenKey, - String decryptedSequence) throws SecurityException { + String newTokenKey, + String decryptedSequence) throws SecurityException { return recreateAccessToken(provider, newTokenKey, getParts(decryptedSequence)); } public static RefreshToken recreateRefreshToken(OAuthDataProvider provider, - String newTokenKey, - String decryptedSequence) throws SecurityException { + String newTokenKey, + String decryptedSequence) throws SecurityException { String[] parts = getParts(decryptedSequence); ServerAccessToken token = recreateAccessToken(provider, newTokenKey, parts); return new RefreshToken(token, - newTokenKey, - parseSimpleList(parts[parts.length - 1])); + newTokenKey, + parseSimpleList(parts[parts.length - 1])); } public static ServerAuthorizationCodeGrant recreateCodeGrant(OAuthDataProvider provider, - String decryptedSequence) throws SecurityException { + String decryptedSequence) throws SecurityException { return recreateCodeGrantInternal(provider, decryptedSequence); } @@ -261,16 +262,16 @@ public static Client recreateClient(String sequence) throws SecurityException { } private static ServerAccessToken recreateAccessToken(OAuthDataProvider provider, - String newTokenKey, - String[] parts) { + String newTokenKey, + String[] parts) { @SuppressWarnings("serial") final ServerAccessToken newToken = new ServerAccessToken(provider.getClient(parts[4]), - parts[1], - newTokenKey == null ? parts[0] : newTokenKey, - Long.parseLong(parts[2]), - Long.parseLong(parts[3])) { + parts[1], + newTokenKey == null ? parts[0] : newTokenKey, + Long.parseLong(parts[2]), + Long.parseLong(parts[3])) { }; newToken.setRefreshToken(getStringPart(parts[5])); @@ -303,7 +304,7 @@ private static ServerAccessToken recreateAccessToken(OAuthDataProvider provider, 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) { @@ -338,22 +339,7 @@ private static String tokenizeServerToken(ServerAccessToken token) { // 9: permissions state.append(SEP); if (!token.getScopes().isEmpty()) { - 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(tokenizeCollection(p.getHttpVerbs())); - state.append('.'); - // 9.5 - state.append(tokenizeCollection(p.getUris())); - } + state.append(tokenizePermissions(token.getScopes())); } state.append(SEP); // 10: code verifier @@ -368,13 +354,35 @@ private static String tokenizeServerToken(ServerAccessToken token) { return state.toString(); } + private static String tokenizePermissions(List perms) { + StringJoiner joiner = new StringJoiner("."); + for (OAuthPermission p : perms) { + StringBuilder sb = new StringBuilder(); + // 9.1 + sb.append(tokenizeString(p.getPermission())); + sb.append('.'); + // 9.2 + sb.append(tokenizeString(p.getDescription())); + sb.append('.'); + // 9.3 + sb.append(p.isDefaultPermission()); + sb.append('.'); + // 9.4 + sb.append(tokenizeCollection(p.getHttpVerbs())); + sb.append('.'); + // 9.5 + 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(getStringPart(parts[0]), - getStringPart(parts[1]), - Boolean.parseBoolean(parts[2]), - getStringPart(parts[3]), getStringPart(parts[4])); + getStringPart(parts[1]), + Boolean.parseBoolean(parts[2]), + getStringPart(parts[3]), getStringPart(parts[4])); c.setApplicationDescription(getStringPart(parts[5])); c.setApplicationLogoUri(getStringPart(parts[6])); c.setApplicationCertificates(parseSimpleList(parts[7])); @@ -386,6 +394,7 @@ private static Client recreateClientInternal(String sequence) { c.setSubject(recreateUserSubject(parts[13])); return c; } + private static String tokenizeClient(Client client) { StringBuilder state = new StringBuilder(); // 0: id @@ -432,13 +441,14 @@ private static String tokenizeClient(Client client) { return state.toString(); } + private static ServerAuthorizationCodeGrant recreateCodeGrantInternal(OAuthDataProvider provider, - String sequence) { + String sequence) { String[] parts = getParts(sequence); ServerAuthorizationCodeGrant grant = new ServerAuthorizationCodeGrant(provider.getClient(parts[0]), - parts[1], - Long.parseLong(parts[2]), - Long.parseLong(parts[3])); + parts[1], + Long.parseLong(parts[2]), + Long.parseLong(parts[3])); grant.setRedirectUri(getStringPart(parts[4])); grant.setAudience(getStringPart(parts[5])); grant.setClientCodeChallenge(getStringPart(parts[6])); @@ -447,6 +457,7 @@ private static ServerAuthorizationCodeGrant recreateCodeGrantInternal(OAuthDataP grant.setExtraProperties(parseSimpleMap(parts[9])); return grant; } + private static String tokenizeCodeGrant(ServerAuthorizationCodeGrant grant) { StringBuilder state = new StringBuilder(); // 0: client id @@ -516,8 +527,6 @@ private static UserSubject recreateUserSubject(String sequence) { subject.setProperties(parseSimpleMap(subjectParts[3])); } return subject; - - } private static void tokenizeUserSubject(StringBuilder state, UserSubject subject) {