diff --git a/core/src/main/java/org/keycloak/crypto/PublicKeysWrapper.java b/core/src/main/java/org/keycloak/crypto/PublicKeysWrapper.java new file mode 100644 index 00000000000..d2cab90c5d1 --- /dev/null +++ b/core/src/main/java/org/keycloak/crypto/PublicKeysWrapper.java @@ -0,0 +1,55 @@ +/* + * Copyright 2022 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.keycloak.crypto; + +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + +/** + * @author Marek Posolda + */ +public class PublicKeysWrapper { + + private final List keys; + + public static final PublicKeysWrapper EMPTY = new PublicKeysWrapper(Collections.emptyList()); + + public PublicKeysWrapper(List keys) { + this.keys = keys; + } + + public List getKeys() { + return keys; + } + + public List getKids() { + return keys.stream() + .map(KeyWrapper::getKid) + .collect(Collectors.toList()); + } + + public KeyWrapper getKeyByKidAndAlg(String kid, String alg) { + return keys.stream() + .filter(keyWrapper -> kid == null || kid.equals(keyWrapper.getKid())) + .filter(keyWrapper -> alg == null || alg.equals(keyWrapper.getAlgorithmOrDefault()) || (keyWrapper.getAlgorithm() == null && kid != null)) + .findFirst().orElse(null); + } +} diff --git a/core/src/main/java/org/keycloak/util/JWKSUtils.java b/core/src/main/java/org/keycloak/util/JWKSUtils.java index fc352bae4ac..60b9ea83011 100644 --- a/core/src/main/java/org/keycloak/util/JWKSUtils.java +++ b/core/src/main/java/org/keycloak/util/JWKSUtils.java @@ -17,17 +17,19 @@ package org.keycloak.util; +import org.jboss.logging.Logger; import org.keycloak.crypto.KeyUse; import org.keycloak.crypto.KeyWrapper; +import org.keycloak.crypto.PublicKeysWrapper; import org.keycloak.jose.jwk.JSONWebKeySet; import org.keycloak.jose.jwk.JWK; import org.keycloak.jose.jwk.JWKParser; import java.security.PublicKey; -import java.util.HashMap; +import java.util.ArrayList; +import java.util.List; import java.util.Map; -import java.util.logging.Level; -import java.util.logging.Logger; +import java.util.stream.Collectors; /** * @author Marek Posolda @@ -36,27 +38,22 @@ public class JWKSUtils { private static final Logger logger = Logger.getLogger(JWKSUtils.class.getName()); + /** + * @deprecated Use {@link #getKeyWrappersForUse(JSONWebKeySet, JWK.Use)} + **/ + @Deprecated public static Map getKeysForUse(JSONWebKeySet keySet, JWK.Use requestedUse) { - Map result = new HashMap<>(); - - for (JWK jwk : keySet.getKeys()) { - JWKParser parser = JWKParser.create(jwk); - if (jwk.getPublicKeyUse() == null) { - logger.log(Level.FINE, "Ignoring JWK key '%s'. Missing required field 'use'.", jwk.getKeyId()); - } else if (requestedUse.asString().equals(jwk.getPublicKeyUse()) && parser.isKeyTypeSupported(jwk.getKeyType())) { - result.put(jwk.getKeyId(), parser.toPublicKey()); - } - } - - return result; + return getKeyWrappersForUse(keySet, requestedUse).getKeys() + .stream() + .collect(Collectors.toMap(KeyWrapper::getKid, keyWrapper -> (PublicKey) keyWrapper.getPublicKey())); } - public static Map getKeyWrappersForUse(JSONWebKeySet keySet, JWK.Use requestedUse) { - Map result = new HashMap<>(); + public static PublicKeysWrapper getKeyWrappersForUse(JSONWebKeySet keySet, JWK.Use requestedUse) { + List result = new ArrayList<>(); for (JWK jwk : keySet.getKeys()) { JWKParser parser = JWKParser.create(jwk); if (jwk.getPublicKeyUse() == null) { - logger.log(Level.FINE, "Ignoring JWK key '%s'. Missing required field 'use'.", jwk.getKeyId()); + logger.debugf("Ignoring JWK key '%s'. Missing required field 'use'.", jwk.getKeyId()); } else if (requestedUse.asString().equals(jwk.getPublicKeyUse()) && parser.isKeyTypeSupported(jwk.getKeyType())) { KeyWrapper keyWrapper = new KeyWrapper(); keyWrapper.setKid(jwk.getKeyId()); @@ -66,10 +63,10 @@ public class JWKSUtils { keyWrapper.setType(jwk.getKeyType()); keyWrapper.setUse(getKeyUse(jwk.getPublicKeyUse())); keyWrapper.setPublicKey(parser.toPublicKey()); - result.put(keyWrapper.getKid(), keyWrapper); + result.add(keyWrapper); } } - return result; + return new PublicKeysWrapper(result); } private static KeyUse getKeyUse(String keyUse) { @@ -87,7 +84,7 @@ public class JWKSUtils { for (JWK jwk : keySet.getKeys()) { JWKParser parser = JWKParser.create(jwk); if (jwk.getPublicKeyUse() == null) { - logger.log(Level.FINE, "Ignoring JWK key '%s'. Missing required field 'use'.", jwk.getKeyId()); + logger.debugf("Ignoring JWK key '%s'. Missing required field 'use'.", jwk.getKeyId()); } else if (requestedUse.asString().equals(parser.getJwk().getPublicKeyUse()) && parser.isKeyTypeSupported(jwk.getKeyType())) { return jwk; } diff --git a/core/src/test/java/org/keycloak/util/JWKSUtilsTest.java b/core/src/test/java/org/keycloak/util/JWKSUtilsTest.java index 86785251a5e..983a391accd 100644 --- a/core/src/test/java/org/keycloak/util/JWKSUtilsTest.java +++ b/core/src/test/java/org/keycloak/util/JWKSUtilsTest.java @@ -21,12 +21,14 @@ import org.junit.ClassRule; import org.junit.Test; import org.keycloak.crypto.KeyUse; import org.keycloak.crypto.KeyWrapper; -import org.keycloak.jose.jwk.*; +import org.keycloak.crypto.PublicKeysWrapper; +import org.keycloak.jose.jwk.JSONWebKeySet; +import org.keycloak.jose.jwk.JWK; import org.keycloak.rule.CryptoInitRule; -import java.util.Map; - -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; public abstract class JWKSUtilsTest { @@ -61,6 +63,14 @@ public abstract class JWKSUtilsTest { " }," + " {" + " \"kty\": \"RSA\"," + + " \"alg\": \"RS512\"," + + " \"use\": \"sig\"," + + " \"kid\": \"" + kidRsa1 + "\"," + + " \"n\": \"soFDjoZ5mQ8XAA7reQAFg90inKAHk0DXMTizo4JuOsgzUbhcplIeZ7ks83hsEjm8mP8lUVaHMPMAHEIp3gu6Xxsg-s73ofx1dtt_Fo7aj8j383MFQGl8-FvixTVobNeGeC0XBBQjN8lEl-lIwOa4ZoERNAShplTej0ntDp7TQm0=\"," + + " \"e\": \"AQAB\"" + + " }," + + " {" + + " \"kty\": \"RSA\"," + " \"kid\": \"" + kidInvalidKey + "\"," + " \"n\": \"soFDjoZ5mQ8XAA7reQAFg90inKAHk0DXMTizo4JuOsgzUbhcplIeZ7ks83hsEjm8mP8lUVaHMPMAHEIp3gu6Xxsg-s73ofx1dtt_Fo7aj8j383MFQGl8-FvixTVobNeGeC0XBBQjN8lEl-lIwOa4ZoERNAShplTej0ntDp7TQm0=\"," + " \"e\": \"AQAB\"" + @@ -84,36 +94,61 @@ public abstract class JWKSUtilsTest { " }" + "] }"; JSONWebKeySet jsonWebKeySet = JsonSerialization.readValue(jwksJson, JSONWebKeySet.class); - Map keyWrappersForUse = JWKSUtils.getKeyWrappersForUse(jsonWebKeySet, JWK.Use.SIG); - assertEquals(4, keyWrappersForUse.size()); + PublicKeysWrapper keyWrappersForUse = JWKSUtils.getKeyWrappersForUse(jsonWebKeySet, JWK.Use.SIG); + assertEquals(5, keyWrappersForUse.getKeys().size()); - KeyWrapper key = keyWrappersForUse.get(kidRsa1); + // get by both kid and alg + KeyWrapper key = keyWrappersForUse.getKeyByKidAndAlg(kidRsa1, "RS256"); assertNotNull(key); assertEquals("RS256", key.getAlgorithmOrDefault()); assertEquals(KeyUse.SIG, key.getUse()); assertEquals(kidRsa1, key.getKid()); assertEquals("RSA", key.getType()); - key = keyWrappersForUse.get(kidRsa2); + // get by both kid and alg with RS512. It is same 'kid' as the previous, but should choose "RS512" key now + key = keyWrappersForUse.getKeyByKidAndAlg(kidRsa1, "RS512"); + assertNotNull(key); + assertEquals("RS512", key.getAlgorithmOrDefault()); + assertEquals(KeyUse.SIG, key.getUse()); + assertEquals(kidRsa1, key.getKid()); + assertEquals("RSA", key.getType()); + + // Get by kid only. Should choose default algorithm, so RS256 + key = keyWrappersForUse.getKeyByKidAndAlg(kidRsa1, null); + assertNotNull(key); + assertEquals("RS256", key.getAlgorithmOrDefault()); + assertEquals(KeyUse.SIG, key.getUse()); + assertEquals(kidRsa1, key.getKid()); + assertEquals("RSA", key.getType()); + + key = keyWrappersForUse.getKeyByKidAndAlg(kidRsa2, null); assertNotNull(key); assertEquals("RS256", key.getAlgorithmOrDefault()); assertEquals(KeyUse.SIG, key.getUse()); assertEquals(kidRsa2, key.getKid()); assertEquals("RSA", key.getType()); - key = keyWrappersForUse.get(kidEC1); + key = keyWrappersForUse.getKeyByKidAndAlg(kidEC1, null); assertNotNull(key); assertEquals("ES384", key.getAlgorithmOrDefault()); assertEquals(KeyUse.SIG, key.getUse()); assertEquals(kidEC1, key.getKid()); assertEquals("EC", key.getType()); - key = keyWrappersForUse.get(kidEC2); + key = keyWrappersForUse.getKeyByKidAndAlg(kidEC2, null); assertNotNull(key); assertNull(key.getAlgorithmOrDefault()); assertEquals(KeyUse.SIG, key.getUse()); assertEquals(kidEC2, key.getKid()); assertEquals("EC", key.getType()); + + // Search by alg only + key = keyWrappersForUse.getKeyByKidAndAlg(null, "ES384"); + assertNotNull(key); + assertEquals("ES384", key.getAlgorithmOrDefault()); + assertEquals(KeyUse.SIG, key.getUse()); + assertEquals(kidEC1, key.getKid()); + assertEquals("EC", key.getType()); } diff --git a/model/infinispan/src/main/java/org/keycloak/keys/infinispan/InfinispanPublicKeyStorageProvider.java b/model/infinispan/src/main/java/org/keycloak/keys/infinispan/InfinispanPublicKeyStorageProvider.java index d51e648ad02..6311dc58553 100644 --- a/model/infinispan/src/main/java/org/keycloak/keys/infinispan/InfinispanPublicKeyStorageProvider.java +++ b/model/infinispan/src/main/java/org/keycloak/keys/infinispan/InfinispanPublicKeyStorageProvider.java @@ -19,6 +19,7 @@ package org.keycloak.keys.infinispan; import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.Callable; @@ -30,6 +31,7 @@ import org.jboss.logging.Logger; import org.keycloak.cluster.ClusterProvider; import org.keycloak.common.util.Time; import org.keycloak.crypto.KeyWrapper; +import org.keycloak.crypto.PublicKeysWrapper; import org.keycloak.keys.PublicKeyLoader; import org.keycloak.keys.PublicKeyStorageProvider; import org.keycloak.models.KeycloakSession; @@ -115,22 +117,17 @@ public class InfinispanPublicKeyStorageProvider implements PublicKeyStorageProvi } } - - @Override - public KeyWrapper getPublicKey(String modelKey, String kid, PublicKeyLoader loader) { - return getPublicKey(modelKey, kid, null, loader); - } - @Override public KeyWrapper getFirstPublicKey(String modelKey, String algorithm, PublicKeyLoader loader) { return getPublicKey(modelKey, null, algorithm, loader); } - private KeyWrapper getPublicKey(String modelKey, String kid, String algorithm, PublicKeyLoader loader) { + @Override + public KeyWrapper getPublicKey(String modelKey, String kid, String algorithm, PublicKeyLoader loader) { // Check if key is in cache PublicKeysEntry entry = keys.get(modelKey); if (entry != null) { - KeyWrapper publicKey = algorithm != null ? getPublicKeyByAlg(entry.getCurrentKeys(), algorithm) : getPublicKey(entry.getCurrentKeys(), kid); + KeyWrapper publicKey = entry.getCurrentKeys().getKeyByKidAndAlg(kid, algorithm); if (publicKey != null) { // return a copy of the key to not modify the cached one return publicKey.cloneKey(); @@ -157,7 +154,7 @@ public class InfinispanPublicKeyStorageProvider implements PublicKeyStorageProvi entry = task.get(); // Computation finished. Let's see if key is available - KeyWrapper publicKey = algorithm != null ? getPublicKeyByAlg(entry.getCurrentKeys(), algorithm) : getPublicKey(entry.getCurrentKeys(), kid); + KeyWrapper publicKey = entry.getCurrentKeys().getKeyByKidAndAlg(kid, algorithm); if (publicKey != null) { // return a copy of the key to not modify the cached one return publicKey.cloneKey(); @@ -177,28 +174,12 @@ public class InfinispanPublicKeyStorageProvider implements PublicKeyStorageProvi log.warnf("Won't load the keys for model '%s' . Last request time was %d", modelKey, lastRequestTime); } - Set availableKids = entry==null ? Collections.emptySet() : entry.getCurrentKeys().keySet(); + List availableKids = entry==null ? Collections.emptyList() : entry.getCurrentKeys().getKids(); log.warnf("PublicKey wasn't found in the storage. Requested kid: '%s' . Available kids: '%s'", kid, availableKids); return null; } - private KeyWrapper getPublicKey(Map publicKeys, String kid) { - // Backwards compatibility - if (kid == null && !publicKeys.isEmpty()) { - return publicKeys.values().iterator().next(); - } else { - return publicKeys.get(kid); - } - } - - private KeyWrapper getPublicKeyByAlg(Map publicKeys, String algorithm) { - if (algorithm == null) return null; - for(KeyWrapper keyWrapper : publicKeys.values()) - if (algorithm.equals(keyWrapper.getAlgorithmOrDefault())) return keyWrapper; - return null; - } - @Override public void close() { @@ -224,10 +205,10 @@ public class InfinispanPublicKeyStorageProvider implements PublicKeyStorageProvi // Check again if we are allowed to send request. There is a chance other task was already finished and removed from tasksInProgress in the meantime. if (currentTime > lastRequestTime + minTimeBetweenRequests) { - Map publicKeys = delegate.loadKeys(); + PublicKeysWrapper publicKeys = delegate.loadKeys(); if (log.isDebugEnabled()) { - log.debugf("Public keys retrieved successfully for model %s. New kids: %s", modelKey, publicKeys.keySet().toString()); + log.debugf("Public keys retrieved successfully for model %s. New kids: %s", modelKey, publicKeys.getKids()); } entry = new PublicKeysEntry(currentTime, publicKeys); diff --git a/model/infinispan/src/main/java/org/keycloak/keys/infinispan/PublicKeysEntry.java b/model/infinispan/src/main/java/org/keycloak/keys/infinispan/PublicKeysEntry.java index 2f2d807de74..a26f4b46d11 100644 --- a/model/infinispan/src/main/java/org/keycloak/keys/infinispan/PublicKeysEntry.java +++ b/model/infinispan/src/main/java/org/keycloak/keys/infinispan/PublicKeysEntry.java @@ -18,9 +18,7 @@ package org.keycloak.keys.infinispan; import java.io.Serializable; -import java.util.Map; - -import org.keycloak.crypto.KeyWrapper; +import org.keycloak.crypto.PublicKeysWrapper; /** * @author Marek Posolda @@ -29,9 +27,9 @@ public class PublicKeysEntry implements Serializable { private final int lastRequestTime; - private final Map currentKeys; + private final PublicKeysWrapper currentKeys; - public PublicKeysEntry(int lastRequestTime, Map currentKeys) { + public PublicKeysEntry(int lastRequestTime, PublicKeysWrapper currentKeys) { this.lastRequestTime = lastRequestTime; this.currentKeys = currentKeys; } @@ -40,7 +38,7 @@ public class PublicKeysEntry implements Serializable { return lastRequestTime; } - public Map getCurrentKeys() { + public PublicKeysWrapper getCurrentKeys() { return currentKeys; } } diff --git a/model/infinispan/src/test/java/org/keycloak/keys/infinispan/InfinispanKeyStorageProviderTest.java b/model/infinispan/src/test/java/org/keycloak/keys/infinispan/InfinispanKeyStorageProviderTest.java index 4c0bca3b81a..efdecc8f80c 100644 --- a/model/infinispan/src/test/java/org/keycloak/keys/infinispan/InfinispanKeyStorageProviderTest.java +++ b/model/infinispan/src/test/java/org/keycloak/keys/infinispan/InfinispanKeyStorageProviderTest.java @@ -17,7 +17,6 @@ package org.keycloak.keys.infinispan; -import java.util.Collections; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -38,7 +37,7 @@ import org.junit.Before; import org.junit.Test; import org.keycloak.common.util.Time; import org.keycloak.connections.infinispan.InfinispanConnectionProvider; -import org.keycloak.crypto.KeyWrapper; +import org.keycloak.crypto.PublicKeysWrapper; import org.keycloak.keys.PublicKeyLoader; /** @@ -129,7 +128,7 @@ public class InfinispanKeyStorageProviderTest { @Override public void run() { InfinispanPublicKeyStorageProvider provider = new InfinispanPublicKeyStorageProvider(null, keys, tasksInProgress, minTimeBetweenRequests); - provider.getPublicKey(modelKey, "kid1", new SampleLoader(modelKey)); + provider.getPublicKey(modelKey, "kid1", null, new SampleLoader(modelKey)); } } @@ -144,12 +143,12 @@ public class InfinispanKeyStorageProviderTest { } @Override - public Map loadKeys() throws Exception { + public PublicKeysWrapper loadKeys() throws Exception { counters.putIfAbsent(modelKey, new AtomicInteger(0)); AtomicInteger currentCounter = counters.get(modelKey); currentCounter.incrementAndGet(); - return Collections.emptyMap(); + return PublicKeysWrapper.EMPTY; } } diff --git a/model/map/src/main/java/org/keycloak/models/map/keys/MapPublicKeyStorageProvider.java b/model/map/src/main/java/org/keycloak/models/map/keys/MapPublicKeyStorageProvider.java index 2926ab3b725..9f229f31f93 100644 --- a/model/map/src/main/java/org/keycloak/models/map/keys/MapPublicKeyStorageProvider.java +++ b/model/map/src/main/java/org/keycloak/models/map/keys/MapPublicKeyStorageProvider.java @@ -19,13 +19,14 @@ package org.keycloak.models.map.keys; import org.jboss.logging.Logger; import org.keycloak.crypto.KeyWrapper; +import org.keycloak.crypto.PublicKeysWrapper; import org.keycloak.keys.PublicKeyLoader; import org.keycloak.keys.PublicKeyStorageProvider; import org.keycloak.models.KeycloakSession; import java.util.Collections; +import java.util.List; import java.util.Map; -import java.util.Set; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.concurrent.FutureTask; @@ -36,28 +37,24 @@ public class MapPublicKeyStorageProvider implements PublicKeyStorageProvider { private final KeycloakSession session; - private final Map>> tasksInProgress; + private final Map> tasksInProgress; - public MapPublicKeyStorageProvider(KeycloakSession session, Map>> tasksInProgress) { + public MapPublicKeyStorageProvider(KeycloakSession session, Map> tasksInProgress) { this.session = session; this.tasksInProgress = tasksInProgress; } - @Override - public KeyWrapper getPublicKey(String modelKey, String kid, PublicKeyLoader loader) { - return getPublicKey(modelKey, kid, null, loader); - } - @Override public KeyWrapper getFirstPublicKey(String modelKey, String algorithm, PublicKeyLoader loader) { return getPublicKey(modelKey, null, algorithm, loader); } - private KeyWrapper getPublicKey(String modelKey, String kid, String algorithm, PublicKeyLoader loader) { + @Override + public KeyWrapper getPublicKey(String modelKey, String kid, String algorithm, PublicKeyLoader loader) { WrapperCallable wrapperCallable = new WrapperCallable(modelKey, loader); - FutureTask> task = new FutureTask<>(wrapperCallable); - FutureTask> existing = tasksInProgress.putIfAbsent(modelKey, task); - Map currentKeys; + FutureTask task = new FutureTask<>(wrapperCallable); + FutureTask existing = tasksInProgress.putIfAbsent(modelKey, task); + PublicKeysWrapper currentKeys; if (existing == null) { task.run(); @@ -69,7 +66,7 @@ public class MapPublicKeyStorageProvider implements PublicKeyStorageProvider { currentKeys = task.get(); // Computation finished. Let's see if key is available - KeyWrapper publicKey = algorithm != null ? getPublicKeyByAlg(currentKeys, algorithm) : getPublicKey(currentKeys, kid); + KeyWrapper publicKey = currentKeys.getKeyByKidAndAlg(kid, algorithm); if (publicKey != null) { return publicKey; } @@ -85,29 +82,13 @@ public class MapPublicKeyStorageProvider implements PublicKeyStorageProvider { } } - Set availableKids = currentKeys == null ? Collections.emptySet() : currentKeys.keySet(); + List availableKids = currentKeys == null ? Collections.emptyList() : currentKeys.getKids(); log.warnf("PublicKey wasn't found in the storage. Requested kid: '%s' . Available kids: '%s'", kid, availableKids); return null; } - private KeyWrapper getPublicKey(Map publicKeys, String kid) { - // Backwards compatibility - if (kid == null && !publicKeys.isEmpty()) { - return publicKeys.values().iterator().next(); - } else { - return publicKeys.get(kid); - } - } - - private KeyWrapper getPublicKeyByAlg(Map publicKeys, String algorithm) { - if (algorithm == null) return null; - for (KeyWrapper keyWrapper : publicKeys.values()) - if (algorithm.equals(keyWrapper.getAlgorithmOrDefault())) return keyWrapper; - return null; - } - - private class WrapperCallable implements Callable> { + private class WrapperCallable implements Callable { private final String modelKey; private final PublicKeyLoader delegate; @@ -118,11 +99,11 @@ public class MapPublicKeyStorageProvider implements PublicKeyStorageProvider { } @Override - public Map call() throws Exception { - Map publicKeys = delegate.loadKeys(); + public PublicKeysWrapper call() throws Exception { + PublicKeysWrapper publicKeys = delegate.loadKeys(); if (log.isDebugEnabled()) { - log.debugf("Public keys retrieved successfully for model %s. New kids: %s", modelKey, publicKeys.keySet().toString()); + log.debugf("Public keys retrieved successfully for model %s. New kids: %s", modelKey, publicKeys.getKids()); } return publicKeys; diff --git a/model/map/src/main/java/org/keycloak/models/map/keys/MapPublicKeyStorageProviderFactory.java b/model/map/src/main/java/org/keycloak/models/map/keys/MapPublicKeyStorageProviderFactory.java index a8b64910884..5979e1771d0 100644 --- a/model/map/src/main/java/org/keycloak/models/map/keys/MapPublicKeyStorageProviderFactory.java +++ b/model/map/src/main/java/org/keycloak/models/map/keys/MapPublicKeyStorageProviderFactory.java @@ -18,7 +18,7 @@ package org.keycloak.models.map.keys; import org.keycloak.common.Profile; -import org.keycloak.crypto.KeyWrapper; +import org.keycloak.crypto.PublicKeysWrapper; import org.keycloak.keys.PublicKeyStorageProviderFactory; import org.keycloak.models.KeycloakSession; import org.keycloak.models.map.common.AbstractEntity; @@ -32,7 +32,7 @@ import java.util.concurrent.FutureTask; public class MapPublicKeyStorageProviderFactory extends AbstractMapProviderFactory implements PublicKeyStorageProviderFactory, EnvironmentDependentProviderFactory { - private final Map>> tasksInProgress = new ConcurrentHashMap<>(); + private final Map> tasksInProgress = new ConcurrentHashMap<>(); public MapPublicKeyStorageProviderFactory() { super(Object.class, MapPublicKeyStorageProvider.class); diff --git a/server-spi-private/src/main/java/org/keycloak/keys/PublicKeyLoader.java b/server-spi-private/src/main/java/org/keycloak/keys/PublicKeyLoader.java index 3ec53f8bb77..caf8e65b840 100644 --- a/server-spi-private/src/main/java/org/keycloak/keys/PublicKeyLoader.java +++ b/server-spi-private/src/main/java/org/keycloak/keys/PublicKeyLoader.java @@ -17,15 +17,13 @@ package org.keycloak.keys; -import java.util.Map; - -import org.keycloak.crypto.KeyWrapper; +import org.keycloak.crypto.PublicKeysWrapper; /** * @author Marek Posolda */ public interface PublicKeyLoader { - Map loadKeys() throws Exception; + PublicKeysWrapper loadKeys() throws Exception; } diff --git a/server-spi-private/src/main/java/org/keycloak/keys/PublicKeyStorageProvider.java b/server-spi-private/src/main/java/org/keycloak/keys/PublicKeyStorageProvider.java index d96a0459293..fd079edf605 100644 --- a/server-spi-private/src/main/java/org/keycloak/keys/PublicKeyStorageProvider.java +++ b/server-spi-private/src/main/java/org/keycloak/keys/PublicKeyStorageProvider.java @@ -31,10 +31,11 @@ public interface PublicKeyStorageProvider extends Provider { * * @param modelKey * @param kid + * @param algorithm The returned key must match this algorithm (unless the algorithm is not set in the JWK) * @param loader * @return */ - KeyWrapper getPublicKey(String modelKey, String kid, PublicKeyLoader loader); + KeyWrapper getPublicKey(String modelKey, String kid, String algorithm, PublicKeyLoader loader); /** * Get first found public key to verify messages signed by particular client having several public keys. Used for example during JWT client authentication diff --git a/services/src/main/java/org/keycloak/keys/AbstractRsaKeyProvider.java b/services/src/main/java/org/keycloak/keys/AbstractRsaKeyProvider.java index 0ca522d2579..2ec55fc2721 100644 --- a/services/src/main/java/org/keycloak/keys/AbstractRsaKeyProvider.java +++ b/services/src/main/java/org/keycloak/keys/AbstractRsaKeyProvider.java @@ -75,7 +75,7 @@ public abstract class AbstractRsaKeyProvider implements KeyProvider { key.setProviderId(model.getId()); key.setProviderPriority(model.get("priority", 0l)); - key.setKid(KeyUtils.createKeyId(keyPair.getPublic())); + key.setKid(model.get(Attributes.KID_KEY) != null ? model.get(Attributes.KID_KEY) : KeyUtils.createKeyId(keyPair.getPublic())); key.setUse(keyUse == null ? KeyUse.SIG : keyUse); key.setType(KeyType.RSA); key.setAlgorithm(algorithm); diff --git a/services/src/main/java/org/keycloak/keys/loader/ClientPublicKeyLoader.java b/services/src/main/java/org/keycloak/keys/loader/ClientPublicKeyLoader.java index 7c236c7056f..4f48c6c56aa 100644 --- a/services/src/main/java/org/keycloak/keys/loader/ClientPublicKeyLoader.java +++ b/services/src/main/java/org/keycloak/keys/loader/ClientPublicKeyLoader.java @@ -22,6 +22,7 @@ import org.keycloak.authentication.authenticators.client.JWTClientAuthenticator; import org.keycloak.common.util.KeyUtils; import org.keycloak.crypto.KeyUse; import org.keycloak.crypto.KeyWrapper; +import org.keycloak.crypto.PublicKeysWrapper; import org.keycloak.jose.jwk.JSONWebKeySet; import org.keycloak.jose.jwk.JWK; import org.keycloak.keys.PublicKeyLoader; @@ -40,7 +41,6 @@ import org.keycloak.util.JsonSerialization; import java.security.PublicKey; import java.security.cert.X509Certificate; import java.util.Collections; -import java.util.Map; /** * @author Marek Posolda @@ -66,7 +66,7 @@ public class ClientPublicKeyLoader implements PublicKeyLoader { } @Override - public Map loadKeys() throws Exception { + public PublicKeysWrapper loadKeys() throws Exception { OIDCAdvancedConfigWrapper config = OIDCAdvancedConfigWrapper.fromClientModel(client); if (config.isUseJwksUrl()) { String jwksUrl = config.getJwksUrl(); @@ -80,14 +80,14 @@ public class ClientPublicKeyLoader implements PublicKeyLoader { try { CertificateRepresentation certInfo = CertificateInfoHelper.getCertificateFromClient(client, JWTClientAuthenticator.ATTR_PREFIX); KeyWrapper publicKey = getSignatureValidationKey(certInfo); - return Collections.singletonMap(publicKey.getKid(), publicKey); + return new PublicKeysWrapper(Collections.singletonList(publicKey)); } catch (ModelException me) { logger.warnf(me, "Unable to retrieve publicKey for verify signature of client '%s' . Error details: %s", client.getClientId(), me.getMessage()); - return Collections.emptyMap(); + return PublicKeysWrapper.EMPTY; } } else { logger.warnf("Unable to retrieve publicKey of client '%s' for the specified purpose other than verifying signature", client.getClientId()); - return Collections.emptyMap(); + return PublicKeysWrapper.EMPTY; } } diff --git a/services/src/main/java/org/keycloak/keys/loader/HardcodedPublicKeyLoader.java b/services/src/main/java/org/keycloak/keys/loader/HardcodedPublicKeyLoader.java index 7a2c1277c89..ccfdd16ac8e 100644 --- a/services/src/main/java/org/keycloak/keys/loader/HardcodedPublicKeyLoader.java +++ b/services/src/main/java/org/keycloak/keys/loader/HardcodedPublicKeyLoader.java @@ -19,15 +19,14 @@ package org.keycloak.keys.loader; import org.keycloak.common.util.Base64Url; import org.keycloak.common.util.KeyUtils; import org.keycloak.common.util.PemUtils; -import org.keycloak.crypto.Algorithm; import org.keycloak.crypto.JavaAlgorithm; import org.keycloak.crypto.KeyType; import org.keycloak.crypto.KeyUse; import org.keycloak.crypto.KeyWrapper; +import org.keycloak.crypto.PublicKeysWrapper; import org.keycloak.keys.PublicKeyLoader; import java.util.Collections; -import java.util.Map; /** * @@ -37,10 +36,6 @@ public class HardcodedPublicKeyLoader implements PublicKeyLoader { private final KeyWrapper keyWrapper; - public HardcodedPublicKeyLoader(String kid, String pem) { - this(kid, pem, Algorithm.RS256); - } - public HardcodedPublicKeyLoader(String kid, String encodedKey, String algorithm) { if (encodedKey != null && !encodedKey.trim().isEmpty()) { keyWrapper = new KeyWrapper(); @@ -63,10 +58,10 @@ public class HardcodedPublicKeyLoader implements PublicKeyLoader { } @Override - public Map loadKeys() throws Exception { + public PublicKeysWrapper loadKeys() throws Exception { return keyWrapper != null - ? Collections.unmodifiableMap(Collections.singletonMap(keyWrapper.getKid(), getSavedPublicKey())) - : Collections.emptyMap(); + ? new PublicKeysWrapper(Collections.singletonList(getSavedPublicKey())) + : PublicKeysWrapper.EMPTY; } protected KeyWrapper getSavedPublicKey() { diff --git a/services/src/main/java/org/keycloak/keys/loader/OIDCIdentityProviderPublicKeyLoader.java b/services/src/main/java/org/keycloak/keys/loader/OIDCIdentityProviderPublicKeyLoader.java index 45359187daa..83001b554e7 100644 --- a/services/src/main/java/org/keycloak/keys/loader/OIDCIdentityProviderPublicKeyLoader.java +++ b/services/src/main/java/org/keycloak/keys/loader/OIDCIdentityProviderPublicKeyLoader.java @@ -25,6 +25,7 @@ import org.keycloak.crypto.Algorithm; import org.keycloak.crypto.KeyType; import org.keycloak.crypto.KeyUse; import org.keycloak.crypto.KeyWrapper; +import org.keycloak.crypto.PublicKeysWrapper; import org.keycloak.jose.jwk.JSONWebKeySet; import org.keycloak.jose.jwk.JWK; import org.keycloak.keys.PublicKeyLoader; @@ -34,7 +35,6 @@ import org.keycloak.util.JWKSUtils; import java.security.PublicKey; import java.util.Collections; -import java.util.Map; /** * @author Marek Posolda @@ -52,7 +52,7 @@ public class OIDCIdentityProviderPublicKeyLoader implements PublicKeyLoader { } @Override - public Map loadKeys() throws Exception { + public PublicKeysWrapper loadKeys() throws Exception { if (config.isUseJwksUrl()) { String jwksUrl = config.getJwksUrl(); JSONWebKeySet jwks = JWKSHttpUtils.sendJwksRequest(session, jwksUrl); @@ -61,12 +61,12 @@ public class OIDCIdentityProviderPublicKeyLoader implements PublicKeyLoader { try { KeyWrapper publicKey = getSavedPublicKey(); if (publicKey == null) { - return Collections.emptyMap(); + return PublicKeysWrapper.EMPTY; } - return Collections.singletonMap(publicKey.getKid(), publicKey); + return new PublicKeysWrapper(Collections.singletonList(publicKey)); } catch (Exception e) { logger.warnf(e, "Unable to retrieve publicKey for verify signature of identityProvider '%s' . Error details: %s", config.getAlias(), e.getMessage()); - return Collections.emptyMap(); + return PublicKeysWrapper.EMPTY; } } } diff --git a/services/src/main/java/org/keycloak/keys/loader/PublicKeyStorageManager.java b/services/src/main/java/org/keycloak/keys/loader/PublicKeyStorageManager.java index b2fdba77c6a..d3ec0b03c03 100644 --- a/services/src/main/java/org/keycloak/keys/loader/PublicKeyStorageManager.java +++ b/services/src/main/java/org/keycloak/keys/loader/PublicKeyStorageManager.java @@ -49,10 +49,11 @@ public class PublicKeyStorageManager { public static KeyWrapper getClientPublicKeyWrapper(KeycloakSession session, ClientModel client, JWSInput input) { String kid = input.getHeader().getKeyId(); + String alg = input.getHeader().getRawAlgorithm(); PublicKeyStorageProvider keyStorage = session.getProvider(PublicKeyStorageProvider.class); String modelKey = PublicKeyStorageUtils.getClientModelCacheKey(client.getRealm().getId(), client.getId()); ClientPublicKeyLoader loader = new ClientPublicKeyLoader(session, client); - return keyStorage.getPublicKey(modelKey, kid, loader); + return keyStorage.getPublicKey(modelKey, kid, alg, loader); } public static KeyWrapper getClientPublicKeyWrapper(KeycloakSession session, ClientModel client, JWK.Use keyUse, String algAlgorithm) { @@ -89,11 +90,6 @@ public class PublicKeyStorageManager { : kid, pem, alg); } - return keyStorage.getPublicKey(modelKey, kid, loader); - } - - public static PublicKey getIdentityProviderPublicKey(KeycloakSession session, RealmModel realm, OIDCIdentityProviderConfig idpConfig, JWSInput input) { - KeyWrapper key = getIdentityProviderKeyWrapper(session, realm, idpConfig, input); - return key != null? (PublicKey) key.getPublicKey() : null; + return keyStorage.getPublicKey(modelKey, kid, alg, loader); } } diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/TestApplicationResourceProviderFactory.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/TestApplicationResourceProviderFactory.java index 5a44a2f9c18..9121e6b7855 100644 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/TestApplicationResourceProviderFactory.java +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/TestApplicationResourceProviderFactory.java @@ -34,6 +34,7 @@ import org.keycloak.services.resource.RealmResourceProviderFactory; import org.keycloak.testsuite.rest.representation.TestAuthenticationChannelRequest; import java.security.KeyPair; +import java.util.ArrayList; import java.util.List; import java.util.concurrent.BlockingQueue; import java.util.concurrent.ConcurrentHashMap; @@ -86,19 +87,24 @@ public class TestApplicationResourceProviderFactory implements RealmResourceProv public static class OIDCClientData { - private KeyPair keyPair; + private List keys = new ArrayList<>(); + private String oidcRequest; private List sectorIdentifierRedirectUris; - private String keyType = KeyType.RSA; - private String keyAlgorithm; - private KeyUse keyUse = KeyUse.SIG; - public KeyPair getSigningKeyPair() { - return keyPair; + public List getKeys() { + return keys; } - public void setSigningKeyPair(KeyPair signingKeyPair) { - this.keyPair = signingKeyPair; + public OIDCKeyData getFirstKey() { + return keys.isEmpty() ? null : keys.get(0); + } + + public void addKey(OIDCKeyData key, boolean keepExistingKeys) { + if (!keepExistingKeys) { + this.keys = new ArrayList<>(); + } + this.keys.add(0, key); } public String getOidcRequest() { @@ -117,6 +123,27 @@ public class TestApplicationResourceProviderFactory implements RealmResourceProv this.sectorIdentifierRedirectUris = sectorIdentifierRedirectUris; } + } + + public static class OIDCKeyData { + + private KeyPair keyPair; + + private String keyType = KeyType.RSA; + private String keyAlgorithm; + private KeyUse keyUse = KeyUse.SIG; + + // Kid will be randomly generated (based on the key hash) if not provided here + private String kid; + + public KeyPair getSigningKeyPair() { + return keyPair; + } + + public void setSigningKeyPair(KeyPair signingKeyPair) { + this.keyPair = signingKeyPair; + } + public String getSigningKeyType() { return keyType; } @@ -164,5 +191,13 @@ public class TestApplicationResourceProviderFactory implements RealmResourceProv public void setKeyUse(KeyUse keyUse) { this.keyUse = keyUse; } + + public String getKid() { + return kid; + } + + public void setKid(String kid) { + this.kid = kid; + } } } diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/resource/TestingOIDCEndpointsApplicationResource.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/resource/TestingOIDCEndpointsApplicationResource.java index 25d78e45d6b..6262a20c921 100644 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/resource/TestingOIDCEndpointsApplicationResource.java +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/resource/TestingOIDCEndpointsApplicationResource.java @@ -92,6 +92,7 @@ import java.util.Set; import java.util.concurrent.BlockingQueue; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.stream.Stream; /** * @author Marek Posolda @@ -120,7 +121,9 @@ public class TestingOIDCEndpointsApplicationResource { @Path("/generate-keys") @NoCache public Map generateKeys(@QueryParam("jwaAlgorithm") String jwaAlgorithm, - @QueryParam("advertiseJWKAlgorithm") Boolean advertiseJWKAlgorithm) { + @QueryParam("advertiseJWKAlgorithm") Boolean advertiseJWKAlgorithm, + @QueryParam("keepExistingKeys") Boolean keepExistingKeys, + @QueryParam("kid") String kid) { try { KeyPair keyPair = null; KeyUse keyUse = KeyUse.SIG; @@ -161,14 +164,17 @@ public class TestingOIDCEndpointsApplicationResource { throw new RuntimeException("Unsupported signature algorithm"); } - clientData.setKeyPair(keyPair); - clientData.setKeyType(keyType); + TestApplicationResourceProviderFactory.OIDCKeyData keyData = new TestApplicationResourceProviderFactory.OIDCKeyData(); + keyData.setKid(kid); // Can be null. It will be generated in that case + keyData.setKeyPair(keyPair); + keyData.setKeyType(keyType); if (advertiseJWKAlgorithm == null || Boolean.TRUE.equals(advertiseJWKAlgorithm)) { - clientData.setKeyAlgorithm(jwaAlgorithm); + keyData.setKeyAlgorithm(jwaAlgorithm); } else { - clientData.setKeyAlgorithm(null); + keyData.setKeyAlgorithm(null); } - clientData.setKeyUse(keyUse); + keyData.setKeyUse(keyUse); + clientData.addKey(keyData, keepExistingKeys != null && keepExistingKeys); } catch (Exception e) { throw new BadRequestException("Error generating signing keypair", e); } @@ -188,8 +194,9 @@ public class TestingOIDCEndpointsApplicationResource { @Produces(MediaType.APPLICATION_JSON) @Path("/get-keys-as-pem") public Map getKeysAsPem() { - String privateKeyPem = PemUtils.encodeKey(clientData.getSigningKeyPair().getPrivate()); - String publicKeyPem = PemUtils.encodeKey(clientData.getSigningKeyPair().getPublic()); + TestApplicationResourceProviderFactory.OIDCKeyData keyData = clientData.getFirstKey(); + String privateKeyPem = PemUtils.encodeKey(keyData.getSigningKeyPair().getPrivate()); + String publicKeyPem = PemUtils.encodeKey(keyData.getSigningKeyPair().getPublic()); Map res = new HashMap<>(); res.put(PRIVATE_KEY, privateKeyPem); @@ -202,8 +209,9 @@ public class TestingOIDCEndpointsApplicationResource { @Path("/get-keys-as-base64") public Map getKeysAsBase64() { // It seems that PemUtils.decodePrivateKey, decodePublicKey can only treat RSA type keys, not EC type keys. Therefore, these are not used. - String privateKeyPem = Base64.encodeBytes(clientData.getSigningKeyPair().getPrivate().getEncoded()); - String publicKeyPem = Base64.encodeBytes(clientData.getSigningKeyPair().getPublic().getEncoded()); + TestApplicationResourceProviderFactory.OIDCKeyData keyData = clientData.getFirstKey(); + String privateKeyPem = Base64.encodeBytes(keyData.getSigningKeyPair().getPrivate().getEncoded()); + String publicKeyPem = Base64.encodeBytes(keyData.getSigningKeyPair().getPublic().getEncoded()); Map res = new HashMap<>(); res.put(PRIVATE_KEY, privateKeyPem); @@ -216,22 +224,27 @@ public class TestingOIDCEndpointsApplicationResource { @Path("/get-jwks") @NoCache public JSONWebKeySet getJwks() { + Stream keysStream = clientData.getKeys().stream() + .map(keyData -> { + KeyPair keyPair = keyData.getKeyPair(); + String keyAlgorithm = keyData.getKeyAlgorithm(); + String keyType = keyData.getKeyType(); + KeyUse keyUse = keyData.getKeyUse(); + String kid = keyData.getKid(); + + JWKBuilder builder = JWKBuilder.create().algorithm(keyAlgorithm).kid(kid); + + if (KeyType.RSA.equals(keyType)) { + return builder.rsa(keyPair.getPublic(), keyUse); + } else if (KeyType.EC.equals(keyType)) { + return builder.ec(keyPair.getPublic()); + } else { + throw new IllegalArgumentException("Unknown keyType: " + keyType); + } + }); + JSONWebKeySet keySet = new JSONWebKeySet(); - KeyPair keyPair = clientData.getKeyPair(); - String keyAlgorithm = clientData.getKeyAlgorithm(); - String keyType = clientData.getKeyType(); - KeyUse keyUse = clientData.getKeyUse(); - - if (keyPair == null) { - keySet.setKeys(new JWK[] {}); - } else if (KeyType.RSA.equals(keyType)) { - keySet.setKeys(new JWK[] { JWKBuilder.create().algorithm(keyAlgorithm).rsa(keyPair.getPublic(), keyUse) }); - } else if (KeyType.EC.equals(keyType)) { - keySet.setKeys(new JWK[] { JWKBuilder.create().algorithm(keyAlgorithm).ec(keyPair.getPublic()) }); - } else { - keySet.setKeys(new JWK[] {}); - } - + keySet.setKeys(keysStream.toArray(JWK[]::new)); return keySet; } @@ -296,17 +309,18 @@ public class TestingOIDCEndpointsApplicationResource { if ("none".equals(jwaAlgorithm)) { clientData.setOidcRequest(new JWSBuilder().jsonContent(oidcRequest).none()); - } else if (clientData.getSigningKeyPair() == null) { + } else if (clientData.getFirstKey() == null) { throw new BadRequestException("signing key not set"); } else { - PrivateKey privateKey = clientData.getSigningKeyPair().getPrivate(); - String kid = KeyUtils.createKeyId(clientData.getSigningKeyPair().getPublic()); + TestApplicationResourceProviderFactory.OIDCKeyData keyData = clientData.getFirstKey(); + PrivateKey privateKey = keyData.getSigningKeyPair().getPrivate(); + String kid = keyData.getKid() != null ? keyData.getKid() : KeyUtils.createKeyId(keyData.getSigningKeyPair().getPublic()); KeyWrapper keyWrapper = new KeyWrapper(); - keyWrapper.setAlgorithm(clientData.getSigningKeyAlgorithm()); + keyWrapper.setAlgorithm(keyData.getSigningKeyAlgorithm()); keyWrapper.setKid(kid); keyWrapper.setPrivateKey(privateKey); SignatureSignerContext signer; - switch (clientData.getSigningKeyAlgorithm()) { + switch (keyData.getSigningKeyAlgorithm()) { case Algorithm.ES256: case Algorithm.ES384: case Algorithm.ES512: diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/client/resources/TestOIDCEndpointsApplicationResource.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/client/resources/TestOIDCEndpointsApplicationResource.java index 88d9084d42a..003f409e9ca 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/client/resources/TestOIDCEndpointsApplicationResource.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/client/resources/TestOIDCEndpointsApplicationResource.java @@ -46,11 +46,23 @@ public interface TestOIDCEndpointsApplicationResource { @Path("/generate-keys") Map generateKeys(@QueryParam("jwaAlgorithm") String jwaAlgorithm); + /** + * Generate single private/public keyPair + * + * @param jwaAlgorithm + * @param advertiseJWKAlgorithm whether algorithm should be adwertised in JWKS or not (Once the keys are returned by JWKS) + * @param keepExistingKeys Should be existing keys kept replaced with newly generated keyPair. If it is not kept, then resulting JWK will contain single key. It is false by default. + * The value 'true' is useful if we want to test with multiple client keys (For example mulitple keys set in the JWKS and test if correct key is picked) + * @param kid Explicitly set specified "kid" for newly generated keypair. If not specified, the kid will be generated + * @return + */ @GET @Produces(MediaType.APPLICATION_JSON) @Path("/generate-keys") Map generateKeys(@QueryParam("jwaAlgorithm") String jwaAlgorithm, - @QueryParam("advertiseJWKAlgorithm") Boolean advertiseJWKAlgorithm); + @QueryParam("advertiseJWKAlgorithm") Boolean advertiseJWKAlgorithm, + @QueryParam("keepExistingKeys") Boolean keepExistingKeys, + @QueryParam("kid") String kid); @GET @Produces(MediaType.APPLICATION_JSON) diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOIDCBrokerWithSignatureTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOIDCBrokerWithSignatureTest.java index a0ec9e1aa17..33a77e8c38d 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOIDCBrokerWithSignatureTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOIDCBrokerWithSignatureTest.java @@ -30,8 +30,10 @@ import org.keycloak.common.Profile; import org.keycloak.common.util.*; import org.keycloak.connections.infinispan.InfinispanConnectionProvider; import org.keycloak.crypto.Algorithm; +import org.keycloak.keys.Attributes; import org.keycloak.keys.KeyProvider; import org.keycloak.keys.PublicKeyStorageUtils; +import org.keycloak.models.Constants; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.protocol.oidc.OIDCConfigAttributes; import org.keycloak.protocol.oidc.OIDCLoginProtocolService; @@ -44,7 +46,9 @@ import org.keycloak.testsuite.ProfileAssume; import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.client.resources.TestingCacheResource; import org.keycloak.testsuite.updaters.ClientAttributeUpdater; +import org.keycloak.testsuite.updaters.RealmAttributeUpdater; import org.keycloak.testsuite.util.OAuthClient; +import org.keycloak.testsuite.util.WaitUtils; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; @@ -404,11 +408,51 @@ public class KcOIDCBrokerWithSignatureTest extends AbstractBaseBrokerTest { } } + // GH issue 14794 + @Test + public void testMultipleKeysWithSameKid() throws Exception { + updateIdentityProviderWithJwksUrl(); + String activeKid = providerRealm().keys().getKeyMetadata().getActive().get(Constants.DEFAULT_SIGNATURE_ALGORITHM); + + // Set the same "kid" of the default key and newly created key. + // Assumption is that used algorithm RS512 is NOT the realm default one. When the realm default is updated to RS512, this one will need to change + ComponentRepresentation newKeyRep = createComponentRep(Algorithm.RS512, "rsa-generated", providerRealm().toRepresentation().getId()); + newKeyRep.getConfig().putSingle(Attributes.KID_KEY, activeKid); + try (Response response = providerRealm().components().add(newKeyRep)) { + assertEquals(201, response.getStatus()); + } + + try (Closeable clientUpdater = ClientAttributeUpdater.forClient(adminClient, bc.providerRealmName(), bc.getIDPClientIdInProviderRealm()) + .setAttribute(OIDCConfigAttributes.ACCESS_TOKEN_SIGNED_RESPONSE_ALG, Algorithm.RS512) + .setAttribute(OIDCConfigAttributes.AUTHORIZATION_SIGNED_RESPONSE_ALG, Algorithm.RS512) + .setAttribute(OIDCConfigAttributes.ID_TOKEN_SIGNED_RESPONSE_ALG, Algorithm.RS512) + .update()) { + + // Check that user is able to login with ES256 + logInAsUserInIDPForFirstTime(); + assertLoggedInAccountManagement(); + logoutFromRealm(getConsumerRoot(), bc.consumerRealmName()); + + logInAsUserInIDP(); + logoutFromRealm(getConsumerRoot(), bc.consumerRealmName()); + } + } + private void rotateKeys(String algorithm, String providerId) { String activeKid = providerRealm().keys().getKeyMetadata().getActive().get(algorithm); // Rotate public keys on the parent broker String realmId = providerRealm().toRepresentation().getId(); + ComponentRepresentation keys = createComponentRep(algorithm, providerId, realmId); + try (Response response = providerRealm().components().add(keys)) { + assertEquals(201, response.getStatus()); + } + + String updatedActiveKid = providerRealm().keys().getKeyMetadata().getActive().get(algorithm); + assertNotEquals(activeKid, updatedActiveKid); + } + + private ComponentRepresentation createComponentRep(String algorithm, String providerId, String realmId) { ComponentRepresentation keys = new ComponentRepresentation(); keys.setName("generated"); keys.setProviderType(KeyProvider.class.getName()); @@ -417,12 +461,7 @@ public class KcOIDCBrokerWithSignatureTest extends AbstractBaseBrokerTest { keys.setConfig(new MultivaluedHashMap<>()); keys.getConfig().putSingle("priority", Long.toString(System.currentTimeMillis())); keys.getConfig().putSingle("algorithm", algorithm); - try (Response response = providerRealm().components().add(keys)) { - assertEquals(201, response.getStatus()); - } - - String updatedActiveKid = providerRealm().keys().getKeyMetadata().getActive().get(algorithm); - assertNotEquals(activeKid, updatedActiveKid); + return keys; } private void createHSKey(String algorithm, String size, String secret) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSignedJWTTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSignedJWTTest.java index 067ec4beceb..a5be82e75a2 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSignedJWTTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSignedJWTTest.java @@ -500,7 +500,7 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest { try { // setup Jwks String signingAlgorithm = Algorithm.PS256; - KeyPair keyPair = setupJwksUrl(signingAlgorithm, false, clientRepresentation, clientResource); + KeyPair keyPair = setupJwksUrl(signingAlgorithm, false, false, null, clientRepresentation, clientResource); PublicKey publicKey = keyPair.getPublic(); PrivateKey privateKey = keyPair.getPrivate(); @@ -523,7 +523,7 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest { try { // send a JWS using the default algorithm String signingAlgorithm = Algorithm.RS256; - KeyPair keyPair = setupJwksUrl(signingAlgorithm, false, clientRepresentation, clientResource); + KeyPair keyPair = setupJwksUrl(signingAlgorithm, false, false, null, clientRepresentation, clientResource); PublicKey publicKey = keyPair.getPublic(); PrivateKey privateKey = keyPair.getPrivate(); oauth.clientId("client2"); @@ -551,6 +551,38 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest { } } + // GH issue 14794 + @Test + public void testSuccessWhenMultipleKeysWithSameKid() throws Exception { + ClientRepresentation clientRepresentation = app2; + ClientResource clientResource = getClient(testRealm.getRealm(), clientRepresentation.getId()); + clientRepresentation = clientResource.toRepresentation(); + String origAccessTokenSignedResponseAlg = clientRepresentation.getAttributes().get(OIDCConfigAttributes.ACCESS_TOKEN_SIGNED_RESPONSE_ALG); + try { + clientRepresentation.getAttributes().put(OIDCConfigAttributes.ACCESS_TOKEN_SIGNED_RESPONSE_ALG, Algorithm.RS512); + // setup Jwks + String signingAlgorithm = Algorithm.RS256; + KeyPair keyPair = setupJwksUrl(signingAlgorithm, true, true, "my-kid", clientRepresentation, clientResource); + + signingAlgorithm = Algorithm.RS512; + keyPair = setupJwksUrl(signingAlgorithm, true, true, "my-kid", clientRepresentation, clientResource); + PublicKey publicKey = keyPair.getPublic(); + PrivateKey privateKey = keyPair.getPrivate(); + + // test + oauth.clientId("client2"); + JsonWebToken clientAuthJwt = createRequestToken("client2", getRealmInfoUrl()); + OAuthClient.AccessTokenResponse response = doGrantAccessTokenRequest("test-user@localhost", "password", + createSignledRequestToken(privateKey, publicKey, signingAlgorithm, "my-kid", clientAuthJwt)); + + assertEquals(200, response.getStatusCode()); + } finally { + // Revert jwks_url settings and signing algorithm + clientRepresentation.getAttributes().put(OIDCConfigAttributes.ACCESS_TOKEN_SIGNED_RESPONSE_ALG, origAccessTokenSignedResponseAlg); + revertJwksUriSettings(clientRepresentation, clientResource); + } + } + @Test public void testDirectGrantRequestSuccessES256() throws Exception { testDirectGrantRequestSuccess(Algorithm.ES256); @@ -964,7 +996,7 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest { parameters .add(new BasicNameValuePair(OAuth2Constants.CLIENT_ASSERTION_TYPE, OAuth2Constants.CLIENT_ASSERTION_TYPE_JWT)); parameters.add(new BasicNameValuePair(OAuth2Constants.CLIENT_ASSERTION, - createSignledRequestToken(privateKey, publicKey, Algorithm.PS256, assertion))); + createSignledRequestToken(privateKey, publicKey, Algorithm.PS256, null, assertion))); try (CloseableHttpResponse resp = sendRequest(oauth.getServiceAccountUrl(), parameters)) { OAuthClient.AccessTokenResponse response = new OAuthClient.AccessTokenResponse(resp); @@ -994,7 +1026,7 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest { parameters .add(new BasicNameValuePair(OAuth2Constants.CLIENT_ASSERTION_TYPE, OAuth2Constants.CLIENT_ASSERTION_TYPE_JWT)); parameters.add(new BasicNameValuePair(OAuth2Constants.CLIENT_ASSERTION, - createSignledRequestToken(privateKey, publicKey, Algorithm.PS256, assertion))); + createSignledRequestToken(privateKey, publicKey, Algorithm.PS256, null, assertion))); try (CloseableHttpResponse resp = sendRequest(oauth.getServiceAccountUrl(), parameters)) { OAuthClient.AccessTokenResponse response = new OAuthClient.AccessTokenResponse(resp); @@ -1431,13 +1463,14 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest { } private KeyPair setupJwksUrl(String algorithm, ClientRepresentation clientRepresentation, ClientResource clientResource) throws Exception { - return setupJwksUrl(algorithm, true, clientRepresentation, clientResource); + return setupJwksUrl(algorithm, true, false, null, clientRepresentation, clientResource); } - private KeyPair setupJwksUrl(String algorithm, boolean advertiseJWKAlgorithm, ClientRepresentation clientRepresentation, ClientResource clientResource) throws Exception { + private KeyPair setupJwksUrl(String algorithm, boolean advertiseJWKAlgorithm, boolean keepExistingKeys, String kid, + ClientRepresentation clientRepresentation, ClientResource clientResource) throws Exception { // generate and register client keypair TestOIDCEndpointsApplicationResource oidcClientEndpointsResource = testingClient.testApp().oidcClientEndpoints(); - oidcClientEndpointsResource.generateKeys(algorithm, advertiseJWKAlgorithm); + oidcClientEndpointsResource.generateKeys(algorithm, advertiseJWKAlgorithm, keepExistingKeys, kid); Map generatedKeys = oidcClientEndpointsResource.getKeysAsBase64(); KeyPair keyPair = getKeyPairFromGeneratedBase64(generatedKeys, algorithm); @@ -1510,11 +1543,13 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest { } private String createSignedRequestToken(String clientId, String realmInfoUrl, PrivateKey privateKey, PublicKey publicKey, String algorithm) { - return createSignledRequestToken(privateKey, publicKey, algorithm, createRequestToken(clientId, realmInfoUrl)); + return createSignledRequestToken(privateKey, publicKey, algorithm, null, createRequestToken(clientId, realmInfoUrl)); } - private String createSignledRequestToken(PrivateKey privateKey, PublicKey publicKey, String algorithm, JsonWebToken jwt) { - String kid = KeyUtils.createKeyId(publicKey); + private String createSignledRequestToken(PrivateKey privateKey, PublicKey publicKey, String algorithm, String kid, JsonWebToken jwt) { + if (kid == null) { + kid = KeyUtils.createKeyId(publicKey); + } SignatureSignerContext signer = oauth.createSigner(privateKey, kid, algorithm); String ret = new JWSBuilder().kid(kid).jsonContent(jwt).sign(signer); return ret;