From 1d6b800d90f4d0187d3990843bc228748bb54a58 Mon Sep 17 00:00:00 2001 From: Lukas Pelzer Date: Thu, 25 Jan 2024 12:56:06 +0100 Subject: [PATCH 01/10] Implement JWKS --- pom.xml | 5 + .../microprofile/impl/jwtauth/jwt/JWK.java | 22 ++ .../microprofile/impl/jwtauth/jwt/JWKSet.java | 10 + .../impl/jwtauth/jwt/KidMapper.java | 214 +++++++++++------- .../impl/jwtauth/jwt/KidMapperTest.java | 92 ++++++++ src/test/resources/jwks.json | 16 ++ 6 files changed, 281 insertions(+), 78 deletions(-) create mode 100644 src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java create mode 100644 src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWKSet.java create mode 100644 src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapperTest.java create mode 100644 src/test/resources/jwks.json diff --git a/pom.xml b/pom.xml index 3f43904..94577c4 100644 --- a/pom.xml +++ b/pom.xml @@ -45,6 +45,11 @@ + + com.nimbusds + nimbus-jose-jwt + 9.22 + org.eclipse.microprofile.jwt microprofile-jwt-auth-api diff --git a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java new file mode 100644 index 0000000..3147ec5 --- /dev/null +++ b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java @@ -0,0 +1,22 @@ +package org.apache.geronimo.microprofile.impl.jwtauth.jwt; + +public class JWK { + + + + String kty; + String n; + String e; + + public String getKty() { + return kty; + } + + public String getN() { + return n; + } + + public String getE() { + return e; + } +} diff --git a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWKSet.java b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWKSet.java new file mode 100644 index 0000000..d08aad7 --- /dev/null +++ b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWKSet.java @@ -0,0 +1,10 @@ +package org.apache.geronimo.microprofile.impl.jwtauth.jwt; + +import java.util.HashSet; +import java.util.Set; + +public class JWKSet { + + Set keys = new HashSet<>(); + +} diff --git a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java index 83f5215..34bf327 100644 --- a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java +++ b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java @@ -24,7 +24,10 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.net.URL; import java.nio.file.Files; +import java.security.PublicKey; +import java.util.Base64; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; @@ -37,90 +40,145 @@ import javax.annotation.PostConstruct; import javax.enterprise.context.ApplicationScoped; +import javax.enterprise.inject.CreationException; import javax.inject.Inject; import org.apache.geronimo.microprofile.impl.jwtauth.config.GeronimoJwtAuthConfig; import org.apache.geronimo.microprofile.impl.jwtauth.io.PropertiesLoader; import org.eclipse.microprofile.jwt.config.Names; +import com.nimbusds.jose.JOSEException; +import com.nimbusds.jose.jwk.JWK; +import com.nimbusds.jose.jwk.JWKSet; +import com.nimbusds.jose.jwk.KeyType; + @ApplicationScoped public class KidMapper { - @Inject - private GeronimoJwtAuthConfig config; - - private final ConcurrentMap keyMapping = new ConcurrentHashMap<>(); - private final Map> issuerMapping = new HashMap<>(); - private String defaultKey; - private Set defaultIssuers; - - @PostConstruct - private void init() { - ofNullable(config.read("kids.key.mapping", null)) - .map(String::trim) - .filter(s -> !s.isEmpty()) - .map(PropertiesLoader::load) - .ifPresent(props -> props.stringPropertyNames() - .forEach(k -> keyMapping.put(k, loadKey(props.getProperty(k))))); - ofNullable(config.read("kids.issuer.mapping", null)) - .map(String::trim) - .filter(s -> !s.isEmpty()) - .map(PropertiesLoader::load) - .ifPresent(props -> props.stringPropertyNames() - .forEach(k -> { - issuerMapping.put(k, Stream.of(props.getProperty(k).split(",")) - .map(String::trim) - .filter(s -> !s.isEmpty()) - .collect(Collectors.toSet())); - })); - defaultIssuers = ofNullable(config.read("org.eclipse.microprofile.authentication.JWT.issuers", null)) - .map(s -> Stream.of(s.split(",")) - .map(String::trim) - .filter(it -> !it.isEmpty()) - .collect(Collectors.toSet())) - .orElseGet(HashSet::new); - ofNullable(config.read("issuer.default", config.read(Names.ISSUER, null))).ifPresent(defaultIssuers::add); - defaultKey = config.read("public-key.default", config.read(Names.VERIFIER_PUBLIC_KEY, null)); - } - - public String loadKey(final String property) { - String value = keyMapping.get(property); - if (value == null) { - value = tryLoad(property); - if (value != null && !property.equals(value) /* else we can leak easily*/) { - keyMapping.putIfAbsent(property, value); - } else if (defaultKey != null) { - value = defaultKey; - } - } - return value; - } - - public Collection loadIssuers(final String property) { - return issuerMapping.getOrDefault(property, defaultIssuers); - } - - private String tryLoad(final String value) { - // try external file - final File file = new File(value); - if (file.exists()) { - try { - return Files.readAllLines(file.toPath()).stream().collect(joining("\n")); - } catch (final IOException e) { - throw new IllegalArgumentException(e); - } - } - - // if not found try classpath resource - try (final InputStream stream = Thread.currentThread().getContextClassLoader() - .getResourceAsStream(value)) { - if (stream != null) { - return new BufferedReader(new InputStreamReader(stream)).lines().collect(joining("\n")); - } - } catch (final IOException e) { - throw new IllegalArgumentException(e); - } - - // else direct value - return value; - } + + @Inject + GeronimoJwtAuthConfig config; + + private final ConcurrentMap keyMapping = new ConcurrentHashMap<>(); + + private final Map> issuerMapping = new HashMap<>(); + + private static final String LINE_BREAK = "\n"; + + private String defaultKey; + + private String jwksUrl; + + private Set defaultIssuers; + + @PostConstruct + void init() { + ofNullable(config.read("kids.key.mapping", null)) + .map(String::trim) + .filter(s -> !s.isEmpty()) + .map(PropertiesLoader::load) + .ifPresent(props -> props.stringPropertyNames() + .forEach(k -> keyMapping.put(k, loadKey(props.getProperty(k))))); + ofNullable(config.read("kids.issuer.mapping", null)) + .map(String::trim) + .filter(s -> !s.isEmpty()) + .map(PropertiesLoader::load) + .ifPresent(props -> props.stringPropertyNames() + .forEach(k -> { + issuerMapping.put(k, Stream.of(props.getProperty(k).split(",")) + .map(String::trim) + .filter(s -> !s.isEmpty()) + .collect(Collectors.toSet())); + })); + defaultIssuers = ofNullable(config.read("org.eclipse.microprofile.authentication.JWT.issuers", null)) + .map(s -> Stream.of(s.split(",")) + .map(String::trim) + .filter(it -> !it.isEmpty()) + .collect(Collectors.toSet())) + .orElseGet(HashSet::new); + jwksUrl = config.read("verify.publickey.location", null); + ofNullable(config.read("issuer.default", config.read(Names.ISSUER, null))).ifPresent(defaultIssuers::add); + defaultKey = config.read("public-key.default", config.read(Names.VERIFIER_PUBLIC_KEY, null)); + } + + public String loadKey(final String property) { + String value = keyMapping.get(property); + if (value == null) { + value = tryLoad(property); + if (value != null && !property.equals(value) /* else we can leak easily*/) { + keyMapping.putIfAbsent(property, value); + } else if (defaultKey != null) { + value = defaultKey; + } + } + return value; + } + + public Collection loadIssuers(final String property) { + return issuerMapping.getOrDefault(property, defaultIssuers); + } + + private String tryLoad(final String value) { + // try external file + final File file = new File(value); + if (file.exists()) { + try { + return Files.readAllLines(file.toPath()).stream().collect(joining("\n")); + } catch (final IOException e) { + throw new IllegalArgumentException(e); + } + } + + // if not found try classpath resource + try (final InputStream stream = Thread.currentThread().getContextClassLoader() + .getResourceAsStream(value)) { + if (stream != null) { + return new BufferedReader(new InputStreamReader(stream)).lines().collect(joining("\n")); + } + } catch (final IOException e) { + throw new IllegalArgumentException(e); + } + + // load jwks via url + if (jwksUrl != null) { + JWKSet publicKeys = loadJwkSet(jwksUrl); + for (JWK jsonWebKey : publicKeys.getKeys()) { + String pemKey = convertJwkToPemKey(jsonWebKey); + String keyId = jsonWebKey.getKeyID(); + keyMapping.put(keyId, pemKey); + } + return keyMapping.get(value); + } + return value; + } + + private JWKSet loadJwkSet(String url) { + final int httpReadTimeoutMs = 5_000; + final int httpSizeLimitBytes = 100_000_000; + final int httpConnectTimeoutMs = 5_000; + try { + URL jwks = new URL(url); + return JWKSet.load(jwks, httpConnectTimeoutMs, httpReadTimeoutMs, httpSizeLimitBytes); + } catch (Exception e) { + throw new CreationException(e); + } + } + + private String convertJwkToPemKey(JWK jwk) { + if (isSupportedKeyType(jwk.getKeyType())) { + throw new IllegalArgumentException("Unsupported key type. Only RSA keys are allowed."); + } + PublicKey publicKey = null; + try { + publicKey = jwk.toRSAKey().toPublicKey(); + } catch (JOSEException e) { + throw new RuntimeException(e); + } + String base64PublicKey = Base64.getMimeEncoder(64, LINE_BREAK.getBytes()).encodeToString(publicKey.getEncoded()); + String result = "-----BEGIN PUBLIC KEY-----" + base64PublicKey + "-----END PUBLIC KEY-----"; + return result.replace(LINE_BREAK, ""); + } + + private boolean isSupportedKeyType(KeyType keyType) { + return keyType != KeyType.RSA; + } } diff --git a/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapperTest.java b/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapperTest.java new file mode 100644 index 0000000..5d5e208 --- /dev/null +++ b/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapperTest.java @@ -0,0 +1,92 @@ +package org.apache.geronimo.microprofile.impl.jwtauth.jwt; + +import static org.testng.Assert.assertEquals; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.PrintWriter; +import java.net.ServerSocket; +import java.net.Socket; + +import org.testng.annotations.AfterTest; +import org.testng.annotations.BeforeTest; +import org.testng.annotations.Test; + +@Test() +public class KidMapperTest { + + private Server jwksServer; + + @BeforeTest + void startJwksServer() throws IOException { + jwksServer = new Server(); + jwksServer.start(); + } + + @AfterTest + void stopJwksServer() throws IOException { + jwksServer.stop(); + } + + @Test + void convertJwksetToPem() { + KidMapper kidMapper = new KidMapper(); + String localJwksUrl = "http://localhost:" + jwksServer.getPort() + "/jwks.json"; + kidMapper.config = (key, defaultValue) -> "verify.publickey.location".equals(key) ? localJwksUrl : null; + kidMapper.init(); + + String key = kidMapper.loadKey("orange-1234"); + String expectedKey = "-----BEGIN PUBLIC KEY-----MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCyzNurU19lqnYhx5QI72sIX1lh8cTehTmboC+DLG7UuaUHqs096M754HtP2IiHFcIQqwYNzHgKmjmfGdbk9JBkz/DNeDVsA5nc7qTnsSgULXTxwHSF286IJdco5kasaJm4Xurlm3V+2oiTugraBsi1J0Ht0OtHgJIlIaGxK7mY/QIDAQAB-----END PUBLIC KEY-----"; + assertEquals(key, expectedKey); + } + + private static class Server { + + private static final String HEADER = "HTTP/1.0 200 OK\r\nConnection: close\r\n"; + + private ServerSocket serverSocket; + + Server() throws IOException { + serverSocket = new ServerSocket(0); + } + + int getPort() { + return serverSocket.getLocalPort(); + } + + void start() { + Thread server = new Thread(() -> { + while (!serverSocket.isClosed()) { + try (Socket client = serverSocket.accept(); + BufferedReader request = new BufferedReader( + new InputStreamReader(client.getInputStream())); + BufferedReader reader = new BufferedReader( + new InputStreamReader(getClass().getResourceAsStream(request.readLine().split("\\s")[1]))); + PrintWriter writer = new PrintWriter(client.getOutputStream())) { + + writer.println(HEADER); + writer.print(load(reader)); + } catch (IOException e) { + if (!serverSocket.isClosed()) { + e.printStackTrace(System.err); + } + } + } + }); + server.start(); + } + + void stop() throws IOException { + serverSocket.close(); + } + + private String load(BufferedReader reader) throws IOException { + StringBuilder content = new StringBuilder(); + for (String line = reader.readLine(); line != null; line = reader.readLine()) { + content.append(line).append("\r\n"); + } + return content.toString(); + } + } +} \ No newline at end of file diff --git a/src/test/resources/jwks.json b/src/test/resources/jwks.json new file mode 100644 index 0000000..f915815 --- /dev/null +++ b/src/test/resources/jwks.json @@ -0,0 +1,16 @@ +{ + "keys": [ + { + "kid": "orange-1234", + "kty": "RSA", + "n": "sszbq1NfZap2IceUCO9rCF9ZYfHE3oU5m6Avgyxu1LmlB6rNPejO-eB7T9iIhxXCEKsGDcx4Cpo5nxnW5PSQZM_wzXg1bAOZ3O6k57EoFC108cB0hdvOiCXXKOZGrGiZuF7q5Zt1ftqIk7oK2gbItSdB7dDrR4CSJSGhsSu5mP0", + "e": "AQAB" + }, + { + "kid": "orange-5678", + "kty": "RSA", + "n": "xC7RfPpTo7362rzATBu45Jv0updEZcr3IqymjbZRkpgTR8B19b_rS4dIficnyyU0plefkE2nJJyJbeW3Fon9BLe4_srfXtqiBKcyqINeg0GrzIqoztZBmmmdo13lELSrGP91oHL-UtCd1u5C1HoJc4bLpjUYxqOrJI4mmRC3Ksk5DV2OS1L5P4nBWIcR1oi6RQaFXy3zam3j1TbCD5urkE1CfUATFwfXfFSPTGo7shNqsgaWgy6B205l5Lq5UmMUBG0prK79ymjJemODwrB445z-lk3CTtlMN7bcQ3nC8xh-Mb2XmRB0uoU4K3kHTsofXG4dUHWJ8wGXEXgJNOPzOQ", + "e": "AQAB" + } + ] +} \ No newline at end of file From dadae7bc08adf4c1abc5955c7d4ef09ab796fca0 Mon Sep 17 00:00:00 2001 From: arne Date: Tue, 6 Feb 2024 16:03:33 +0100 Subject: [PATCH 02/10] Remove jose library --- pom.xml | 6 +- .../microprofile/impl/jwtauth/jwt/JWK.java | 70 ++++- .../microprofile/impl/jwtauth/jwt/JWKSet.java | 10 - .../impl/jwtauth/jwt/KidMapper.java | 250 ++++++++---------- .../impl/jwtauth/jwt/KidMapperTest.java | 192 ++++++++------ src/test/resources/geronimo.xml | 20 ++ 6 files changed, 315 insertions(+), 233 deletions(-) delete mode 100644 src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWKSet.java create mode 100644 src/test/resources/geronimo.xml diff --git a/pom.xml b/pom.xml index 94577c4..1e806e6 100644 --- a/pom.xml +++ b/pom.xml @@ -45,11 +45,6 @@ - - com.nimbusds - nimbus-jose-jwt - 9.22 - org.eclipse.microprofile.jwt microprofile-jwt-auth-api @@ -173,6 +168,7 @@ 2.21.0 + ${project.basedir}/src/test/resources/geronimo.xml ${project.basedir}/src/test/resources/tck.xml diff --git a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java index 3147ec5..a9927fd 100644 --- a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java +++ b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java @@ -1,14 +1,52 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.geronimo.microprofile.impl.jwtauth.jwt; +import java.math.BigInteger; +import java.security.KeyFactory; +import java.security.NoSuchAlgorithmException; +import java.security.PublicKey; +import java.security.interfaces.RSAPublicKey; +import java.security.spec.InvalidKeySpecException; +import java.security.spec.RSAPublicKeySpec; +import java.util.Base64; +import java.util.Base64.Decoder; + +import javax.json.JsonObject; + public class JWK { + private String kid; + private String kty; + private String n; + private String e; + public JWK(JsonObject jsonObject) { + kid = jsonObject.getString("kid"); + kty = jsonObject.getString("kty"); + n = jsonObject.getString("n"); + e = jsonObject.getString("e"); + } - String kty; - String n; - String e; + public String getKid() { + return kid; + } - public String getKty() { + public String getKty() { return kty; } @@ -19,4 +57,28 @@ public String getN() { public String getE() { return e; } + + public String toPemKey() { + PublicKey publicKey = toRSAPublicKey(); + String base64PublicKey = Base64.getMimeEncoder(64, "\n".getBytes()).encodeToString(publicKey.getEncoded()); + String result = "-----BEGIN PUBLIC KEY-----" + base64PublicKey + "-----END PUBLIC KEY-----"; + return result.replace("\n", ""); + } + + public RSAPublicKey toRSAPublicKey() { + if (!"RSA".equals(kty)) { + throw new UnsupportedOperationException("Unsupported key type. Only RSA keys are allowed."); + } + + Decoder decoder = Base64.getUrlDecoder(); + BigInteger modulus = new BigInteger(1, decoder.decode(n)); + BigInteger exponent = new BigInteger(1, decoder.decode(e)); + RSAPublicKeySpec spec = new RSAPublicKeySpec(modulus, exponent); + try { + KeyFactory factory = KeyFactory.getInstance("RSA"); + return (RSAPublicKey)factory.generatePublic(spec); + } catch (NoSuchAlgorithmException | InvalidKeySpecException e) { + throw new IllegalStateException(e); + } + } } diff --git a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWKSet.java b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWKSet.java deleted file mode 100644 index d08aad7..0000000 --- a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWKSet.java +++ /dev/null @@ -1,10 +0,0 @@ -package org.apache.geronimo.microprofile.impl.jwtauth.jwt; - -import java.util.HashSet; -import java.util.Set; - -public class JWKSet { - - Set keys = new HashSet<>(); - -} diff --git a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java index 34bf327..f77a84a 100644 --- a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java +++ b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java @@ -16,6 +16,7 @@ */ package org.apache.geronimo.microprofile.impl.jwtauth.jwt; +import static java.util.Collections.emptyMap; import static java.util.Optional.ofNullable; import static java.util.stream.Collectors.joining; @@ -24,13 +25,14 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.net.MalformedURLException; import java.net.URL; import java.nio.file.Files; -import java.security.PublicKey; -import java.util.Base64; +import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -40,145 +42,123 @@ import javax.annotation.PostConstruct; import javax.enterprise.context.ApplicationScoped; -import javax.enterprise.inject.CreationException; import javax.inject.Inject; +import javax.json.Json; +import javax.json.JsonArray; +import javax.json.JsonObject; +import javax.json.JsonReader; +import javax.json.JsonReaderFactory; import org.apache.geronimo.microprofile.impl.jwtauth.config.GeronimoJwtAuthConfig; import org.apache.geronimo.microprofile.impl.jwtauth.io.PropertiesLoader; import org.eclipse.microprofile.jwt.config.Names; -import com.nimbusds.jose.JOSEException; -import com.nimbusds.jose.jwk.JWK; -import com.nimbusds.jose.jwk.JWKSet; -import com.nimbusds.jose.jwk.KeyType; - @ApplicationScoped public class KidMapper { - - @Inject - GeronimoJwtAuthConfig config; - - private final ConcurrentMap keyMapping = new ConcurrentHashMap<>(); - - private final Map> issuerMapping = new HashMap<>(); - - private static final String LINE_BREAK = "\n"; - - private String defaultKey; - - private String jwksUrl; - - private Set defaultIssuers; - - @PostConstruct - void init() { - ofNullable(config.read("kids.key.mapping", null)) - .map(String::trim) - .filter(s -> !s.isEmpty()) - .map(PropertiesLoader::load) - .ifPresent(props -> props.stringPropertyNames() - .forEach(k -> keyMapping.put(k, loadKey(props.getProperty(k))))); - ofNullable(config.read("kids.issuer.mapping", null)) - .map(String::trim) - .filter(s -> !s.isEmpty()) - .map(PropertiesLoader::load) - .ifPresent(props -> props.stringPropertyNames() - .forEach(k -> { - issuerMapping.put(k, Stream.of(props.getProperty(k).split(",")) - .map(String::trim) - .filter(s -> !s.isEmpty()) - .collect(Collectors.toSet())); - })); - defaultIssuers = ofNullable(config.read("org.eclipse.microprofile.authentication.JWT.issuers", null)) - .map(s -> Stream.of(s.split(",")) - .map(String::trim) - .filter(it -> !it.isEmpty()) - .collect(Collectors.toSet())) - .orElseGet(HashSet::new); - jwksUrl = config.read("verify.publickey.location", null); - ofNullable(config.read("issuer.default", config.read(Names.ISSUER, null))).ifPresent(defaultIssuers::add); - defaultKey = config.read("public-key.default", config.read(Names.VERIFIER_PUBLIC_KEY, null)); - } - - public String loadKey(final String property) { - String value = keyMapping.get(property); - if (value == null) { - value = tryLoad(property); - if (value != null && !property.equals(value) /* else we can leak easily*/) { - keyMapping.putIfAbsent(property, value); - } else if (defaultKey != null) { - value = defaultKey; - } - } - return value; - } - - public Collection loadIssuers(final String property) { - return issuerMapping.getOrDefault(property, defaultIssuers); - } - - private String tryLoad(final String value) { - // try external file - final File file = new File(value); - if (file.exists()) { - try { - return Files.readAllLines(file.toPath()).stream().collect(joining("\n")); - } catch (final IOException e) { - throw new IllegalArgumentException(e); - } - } - - // if not found try classpath resource - try (final InputStream stream = Thread.currentThread().getContextClassLoader() - .getResourceAsStream(value)) { - if (stream != null) { - return new BufferedReader(new InputStreamReader(stream)).lines().collect(joining("\n")); - } - } catch (final IOException e) { - throw new IllegalArgumentException(e); - } - - // load jwks via url - if (jwksUrl != null) { - JWKSet publicKeys = loadJwkSet(jwksUrl); - for (JWK jsonWebKey : publicKeys.getKeys()) { - String pemKey = convertJwkToPemKey(jsonWebKey); - String keyId = jsonWebKey.getKeyID(); - keyMapping.put(keyId, pemKey); - } - return keyMapping.get(value); - } - return value; - } - - private JWKSet loadJwkSet(String url) { - final int httpReadTimeoutMs = 5_000; - final int httpSizeLimitBytes = 100_000_000; - final int httpConnectTimeoutMs = 5_000; - try { - URL jwks = new URL(url); - return JWKSet.load(jwks, httpConnectTimeoutMs, httpReadTimeoutMs, httpSizeLimitBytes); - } catch (Exception e) { - throw new CreationException(e); - } - } - - private String convertJwkToPemKey(JWK jwk) { - if (isSupportedKeyType(jwk.getKeyType())) { - throw new IllegalArgumentException("Unsupported key type. Only RSA keys are allowed."); - } - PublicKey publicKey = null; - try { - publicKey = jwk.toRSAKey().toPublicKey(); - } catch (JOSEException e) { - throw new RuntimeException(e); - } - String base64PublicKey = Base64.getMimeEncoder(64, LINE_BREAK.getBytes()).encodeToString(publicKey.getEncoded()); - String result = "-----BEGIN PUBLIC KEY-----" + base64PublicKey + "-----END PUBLIC KEY-----"; - return result.replace(LINE_BREAK, ""); - } - - private boolean isSupportedKeyType(KeyType keyType) { - return keyType != KeyType.RSA; - } + @Inject + private GeronimoJwtAuthConfig config; + + private final ConcurrentMap keyMapping = new ConcurrentHashMap<>(); + private final Map> issuerMapping = new HashMap<>(); + private String defaultKey; + private String jwksUrl; + private Set defaultIssuers; + private JsonReaderFactory readerFactory; + + @PostConstruct + private void init() { + ofNullable(config.read("kids.key.mapping", null)) + .map(String::trim) + .filter(s -> !s.isEmpty()) + .map(PropertiesLoader::load) + .ifPresent(props -> props.stringPropertyNames() + .forEach(k -> keyMapping.put(k, loadKey(props.getProperty(k))))); + ofNullable(config.read("kids.issuer.mapping", null)) + .map(String::trim) + .filter(s -> !s.isEmpty()) + .map(PropertiesLoader::load) + .ifPresent(props -> props.stringPropertyNames() + .forEach(k -> { + issuerMapping.put(k, Stream.of(props.getProperty(k).split(",")) + .map(String::trim) + .filter(s -> !s.isEmpty()) + .collect(Collectors.toSet())); + })); + defaultIssuers = ofNullable(config.read("org.eclipse.microprofile.authentication.JWT.issuers", null)) + .map(s -> Stream.of(s.split(",")) + .map(String::trim) + .filter(it -> !it.isEmpty()) + .collect(Collectors.toSet())) + .orElseGet(HashSet::new); + ofNullable(config.read("issuer.default", config.read(Names.ISSUER, null))).ifPresent(defaultIssuers::add); + jwksUrl = config.read("mp.jwt.verify.publickey.location", null); + defaultKey = config.read("public-key.default", config.read(Names.VERIFIER_PUBLIC_KEY, null)); + readerFactory = Json.createReaderFactory(emptyMap()); + } + + public String loadKey(final String property) { + String value = keyMapping.get(property); + if (value == null) { + value = tryLoad(property); + if (value != null && !property.equals(value) /* else we can leak easily*/) { + keyMapping.putIfAbsent(property, value); + } else if (defaultKey != null) { + value = defaultKey; + } + } + return value; + } + + public Collection loadIssuers(final String property) { + return issuerMapping.getOrDefault(property, defaultIssuers); + } + + private String tryLoad(final String value) { + // try external file + final File file = new File(value); + if (file.exists()) { + try { + return Files.readAllLines(file.toPath()).stream().collect(joining("\n")); + } catch (final IOException e) { + throw new IllegalArgumentException(e); + } + } + + // if not found try classpath resource + try (final InputStream stream = Thread.currentThread().getContextClassLoader() + .getResourceAsStream(value)) { + if (stream != null) { + return new BufferedReader(new InputStreamReader(stream)).lines().collect(joining("\n")); + } + } catch (final IOException e) { + throw new IllegalArgumentException(e); + } + + // load jwks via url + if (jwksUrl != null) { + loadJwkSet(jwksUrl).forEach(jwk -> keyMapping.put(jwk.getKid(), jwk.toPemKey())); + String key = keyMapping.get(value); + if (key != null) { + return key; + } + } + return value; + } + + private List loadJwkSet(String url) { + try { + URL jwks = new URL(url); + try (InputStream connection = jwks.openStream(); JsonReader jwksReader = readerFactory.createReader(connection)) { + JsonObject keySet = jwksReader.readObject(); + JsonArray keys = keySet.getJsonArray("keys"); + List parsedKeys = new ArrayList<>(keys.size()); + keys.forEach(key -> parsedKeys.add(new JWK((JsonObject)key))); + return parsedKeys; + } catch (IOException e) { + throw new IllegalStateException(e); + } + } catch (MalformedURLException e) { + throw new IllegalArgumentException(e); + } + } } diff --git a/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapperTest.java b/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapperTest.java index 5d5e208..3d0a528 100644 --- a/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapperTest.java +++ b/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapperTest.java @@ -1,5 +1,6 @@ package org.apache.geronimo.microprofile.impl.jwtauth.jwt; +import static javax.ws.rs.client.ClientBuilder.newClient; import static org.testng.Assert.assertEquals; import java.io.BufferedReader; @@ -8,85 +9,118 @@ import java.io.PrintWriter; import java.net.ServerSocket; import java.net.Socket; - -import org.testng.annotations.AfterTest; -import org.testng.annotations.BeforeTest; +import java.net.URISyntaxException; +import java.net.URL; + +import javax.enterprise.context.ApplicationScoped; +import javax.inject.Inject; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.PathParam; +import javax.ws.rs.Produces; + +import org.jboss.arquillian.container.test.api.Deployment; +import org.jboss.arquillian.container.test.api.RunAsClient; +import org.jboss.arquillian.test.api.ArquillianResource; +import org.jboss.arquillian.testng.Arquillian; +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.spec.WebArchive; +import org.testng.annotations.AfterClass; import org.testng.annotations.Test; -@Test() -public class KidMapperTest { - - private Server jwksServer; - - @BeforeTest - void startJwksServer() throws IOException { - jwksServer = new Server(); - jwksServer.start(); - } - - @AfterTest - void stopJwksServer() throws IOException { - jwksServer.stop(); - } - - @Test - void convertJwksetToPem() { - KidMapper kidMapper = new KidMapper(); - String localJwksUrl = "http://localhost:" + jwksServer.getPort() + "/jwks.json"; - kidMapper.config = (key, defaultValue) -> "verify.publickey.location".equals(key) ? localJwksUrl : null; - kidMapper.init(); - - String key = kidMapper.loadKey("orange-1234"); - String expectedKey = "-----BEGIN PUBLIC KEY-----MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCyzNurU19lqnYhx5QI72sIX1lh8cTehTmboC+DLG7UuaUHqs096M754HtP2IiHFcIQqwYNzHgKmjmfGdbk9JBkz/DNeDVsA5nc7qTnsSgULXTxwHSF286IJdco5kasaJm4Xurlm3V+2oiTugraBsi1J0Ht0OtHgJIlIaGxK7mY/QIDAQAB-----END PUBLIC KEY-----"; - assertEquals(key, expectedKey); - } - - private static class Server { - - private static final String HEADER = "HTTP/1.0 200 OK\r\nConnection: close\r\n"; - - private ServerSocket serverSocket; - - Server() throws IOException { - serverSocket = new ServerSocket(0); - } - - int getPort() { - return serverSocket.getLocalPort(); - } - - void start() { - Thread server = new Thread(() -> { - while (!serverSocket.isClosed()) { - try (Socket client = serverSocket.accept(); - BufferedReader request = new BufferedReader( - new InputStreamReader(client.getInputStream())); - BufferedReader reader = new BufferedReader( - new InputStreamReader(getClass().getResourceAsStream(request.readLine().split("\\s")[1]))); - PrintWriter writer = new PrintWriter(client.getOutputStream())) { - - writer.println(HEADER); - writer.print(load(reader)); - } catch (IOException e) { - if (!serverSocket.isClosed()) { - e.printStackTrace(System.err); - } - } - } - }); - server.start(); - } - - void stop() throws IOException { - serverSocket.close(); - } - - private String load(BufferedReader reader) throws IOException { - StringBuilder content = new StringBuilder(); - for (String line = reader.readLine(); line != null; line = reader.readLine()) { - content.append(line).append("\r\n"); - } - return content.toString(); - } - } +public class KidMapperTest extends Arquillian { + + private static Server jwksServer; + + @Deployment() + public static WebArchive createDeployment() throws Exception { + jwksServer = new Server(); + jwksServer.start(); + System.setProperty("mp.jwt.verify.publickey.location", "http://localhost:" + jwksServer.getPort() + "/jwks.json"); + return ShrinkWrap + .create(WebArchive.class) + .addAsWebInfResource("META-INF/beans.xml", "beans.xml") + .addClasses(JwtParser.class, KidMapper.class); + } + + @AfterClass + static void stopJwksServer() throws IOException { + jwksServer.stop(); + } + + @ArquillianResource + private URL serverUrl; + + @Test + @RunAsClient + void convertJwksetToPem() throws URISyntaxException { + String expectedKey = "-----BEGIN PUBLIC KEY-----MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCyzNurU19lqnYhx5QI72sIX1lh8cTehTmboC+DLG7UuaUHqs096M754HtP2IiHFcIQqwYNzHgKmjmfGdbk9JBkz/DNeDVsA5nc7qTnsSgULXTxwHSF286IJdco5kasaJm4Xurlm3V+2oiTugraBsi1J0Ht0OtHgJIlIaGxK7mY/QIDAQAB-----END PUBLIC KEY-----"; + + String key = newClient().target(serverUrl.toURI()).path("public-keys").path("orange-1234").request().get(String.class); + + assertEquals(key, expectedKey); + } + + @ApplicationScoped + @Path("public-keys") + public static class PublicKeyResource { + + @Inject + private KidMapper kidMapper; + + @GET + @Path("{kid}") + @Produces() + public String getPublicKey(@PathParam("kid") String kid) { + return kidMapper.loadKey(kid); + } + } + + private static class Server { + + private static final String HEADER = "HTTP/1.0 200 OK\r\nConnection: close\r\n"; + + private ServerSocket serverSocket; + + Server() throws IOException { + serverSocket = new ServerSocket(0); + } + + int getPort() { + return serverSocket.getLocalPort(); + } + + void start() { + Thread server = new Thread(() -> { + while (!serverSocket.isClosed()) { + try (Socket client = serverSocket.accept(); + BufferedReader request = new BufferedReader(new InputStreamReader(client.getInputStream())); + BufferedReader reader = new BufferedReader(new InputStreamReader( + getClass().getResourceAsStream(request.readLine().split("\\s")[1]))); + PrintWriter writer = new PrintWriter(client.getOutputStream())) { + + writer.println(HEADER); + writer.print(load(reader)); + } catch (IOException e) { + if (!serverSocket.isClosed()) { + e.printStackTrace(System.err); + } + } + } + }); + server.start(); + } + + void stop() throws IOException { + serverSocket.close(); + } + + private String load(BufferedReader reader) throws IOException { + StringBuilder content = new StringBuilder(); + for (String line = reader.readLine(); line != null; line = reader.readLine()) { + content.append(line).append("\r\n"); + } + return content.toString(); + } + } } \ No newline at end of file diff --git a/src/test/resources/geronimo.xml b/src/test/resources/geronimo.xml new file mode 100644 index 0000000..8b5ffb7 --- /dev/null +++ b/src/test/resources/geronimo.xml @@ -0,0 +1,20 @@ + + + + + + + + + From f388eab6fd69a605136d54dbf2c0c66cfd2f50a5 Mon Sep 17 00:00:00 2001 From: Lukas Pelzer Date: Thu, 8 Feb 2024 10:39:26 +0100 Subject: [PATCH 03/10] add filtering for sig use --- .../microprofile/impl/jwtauth/jwt/JWK.java | 17 ++++++++++---- .../impl/jwtauth/jwt/KidMapper.java | 23 +++++++++++++++++-- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java index a9927fd..1e8312d 100644 --- a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java +++ b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java @@ -25,6 +25,7 @@ import java.security.spec.RSAPublicKeySpec; import java.util.Base64; import java.util.Base64.Decoder; +import java.util.Optional; import javax.json.JsonObject; @@ -34,12 +35,14 @@ public class JWK { private String kty; private String n; private String e; + private String use; public JWK(JsonObject jsonObject) { kid = jsonObject.getString("kid"); kty = jsonObject.getString("kty"); n = jsonObject.getString("n"); e = jsonObject.getString("e"); + use = jsonObject.getString("use", null); } public String getKid() { @@ -50,13 +53,17 @@ public String getKty() { return kty; } - public String getN() { + public String getN() { return n; - } + } - public String getE() { - return e; - } + public String getE() { + return e; + } + + public Optional getUse() { + return Optional.ofNullable(use); + } public String toPemKey() { PublicKey publicKey = toRSAPublicKey(); diff --git a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java index f77a84a..0d54502 100644 --- a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java +++ b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java @@ -34,6 +34,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -151,8 +152,7 @@ private List loadJwkSet(String url) { try (InputStream connection = jwks.openStream(); JsonReader jwksReader = readerFactory.createReader(connection)) { JsonObject keySet = jwksReader.readObject(); JsonArray keys = keySet.getJsonArray("keys"); - List parsedKeys = new ArrayList<>(keys.size()); - keys.forEach(key -> parsedKeys.add(new JWK((JsonObject)key))); + List parsedKeys = parseKeys(keys); return parsedKeys; } catch (IOException e) { throw new IllegalStateException(e); @@ -161,4 +161,23 @@ private List loadJwkSet(String url) { throw new IllegalArgumentException(e); } } + + private List parseKeys(JsonArray keys) { + List parsedKeys = new ArrayList<>(keys.size()); + keys.forEach(key -> { + JWK jwk = new JWK((JsonObject) key); + if (isSignatureKey(jwk)) { + parsedKeys.add(jwk); + } + }); + return parsedKeys; + } + + private boolean isSignatureKey(JWK key) { + Optional use = key.getUse(); + if (use.isPresent()) { + return use.get().equals("sig"); + } + return true; + } } From ea6b0538403479a9dd032994950e22b5df95686d Mon Sep 17 00:00:00 2001 From: Lukas Pelzer Date: Fri, 9 Feb 2024 14:27:29 +0100 Subject: [PATCH 04/10] add EC kty support --- .../microprofile/impl/jwtauth/jwt/JWK.java | 77 +++++++++++++------ .../impl/jwtauth/jwt/KidMapper.java | 31 ++------ 2 files changed, 60 insertions(+), 48 deletions(-) diff --git a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java index 1e8312d..253bbda 100644 --- a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java +++ b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java @@ -17,29 +17,36 @@ package org.apache.geronimo.microprofile.impl.jwtauth.jwt; import java.math.BigInteger; +import java.security.AlgorithmParameters; import java.security.KeyFactory; import java.security.NoSuchAlgorithmException; import java.security.PublicKey; -import java.security.interfaces.RSAPublicKey; -import java.security.spec.InvalidKeySpecException; -import java.security.spec.RSAPublicKeySpec; +import java.security.spec.*; import java.util.Base64; import java.util.Base64.Decoder; -import java.util.Optional; import javax.json.JsonObject; +import static java.security.KeyFactory.getInstance; +import static java.util.Optional.*; + public class JWK { private String kid; private String kty; private String n; private String e; + private String x; + private String y; + private String crv; private String use; public JWK(JsonObject jsonObject) { kid = jsonObject.getString("kid"); kty = jsonObject.getString("kty"); + x = jsonObject.getString("x"); + y = jsonObject.getString("y"); + crv = jsonObject.getString("crv"); n = jsonObject.getString("n"); e = jsonObject.getString("e"); use = jsonObject.getString("use", null); @@ -49,43 +56,63 @@ public String getKid() { return kid; } - public String getKty() { - return kty; - } - - public String getN() { - return n; - } - - public String getE() { - return e; + public String getUse() { + return use; } - public Optional getUse() { - return Optional.ofNullable(use); - } public String toPemKey() { - PublicKey publicKey = toRSAPublicKey(); + PublicKey publicKey = toPublicKey(); String base64PublicKey = Base64.getMimeEncoder(64, "\n".getBytes()).encodeToString(publicKey.getEncoded()); String result = "-----BEGIN PUBLIC KEY-----" + base64PublicKey + "-----END PUBLIC KEY-----"; return result.replace("\n", ""); } - public RSAPublicKey toRSAPublicKey() { - if (!"RSA".equals(kty)) { - throw new UnsupportedOperationException("Unsupported key type. Only RSA keys are allowed."); + public PublicKey toPublicKey() { + if ("RSA".equals(kty)) { + return toRSAPublicKey(); + } else if("EC".equals(kty)) { + return toECPublicKey(); + } else { + throw new IllegalStateException("Unsupported kty. Only RSA and EC are supported."); } + } + private PublicKey toRSAPublicKey() { Decoder decoder = Base64.getUrlDecoder(); - BigInteger modulus = new BigInteger(1, decoder.decode(n)); - BigInteger exponent = new BigInteger(1, decoder.decode(e)); + BigInteger modulus = ofNullable(n).map(mod -> new BigInteger(1, decoder.decode(mod))).orElseThrow(() -> new IllegalStateException("n must be set for RSA keys.")); + BigInteger exponent = ofNullable(e).map(exp -> new BigInteger(1, decoder.decode(exp))).orElseThrow(() -> new IllegalStateException("e must be set for RSA keys.")); RSAPublicKeySpec spec = new RSAPublicKeySpec(modulus, exponent); try { - KeyFactory factory = KeyFactory.getInstance("RSA"); - return (RSAPublicKey)factory.generatePublic(spec); + KeyFactory factory = getInstance("RSA"); + return factory.generatePublic(spec); } catch (NoSuchAlgorithmException | InvalidKeySpecException e) { throw new IllegalStateException(e); } } + + private PublicKey toECPublicKey() { + Decoder decoder = Base64.getUrlDecoder(); + BigInteger pointX = ofNullable(x).map(x -> new BigInteger(1, decoder.decode(x))).orElseThrow(() -> new IllegalStateException("x must be set for EC keys.")); + BigInteger pointY = ofNullable(y).map(y -> new BigInteger(1, decoder.decode(y))).orElseThrow(() -> new IllegalStateException("y must be set for EC keys.")); + ECPoint pubPoint = new ECPoint(pointX, pointY); + try { + AlgorithmParameters parameters = AlgorithmParameters.getInstance("EC"); + parameters.init(ofNullable(crv).map(JWK::mapCrv).map(ECGenParameterSpec::new).orElseThrow(() -> new IllegalStateException("crv must be set for EC keys."))); + ECParameterSpec ecParameters = parameters.getParameterSpec(ECParameterSpec.class); + return getInstance("EC").generatePublic(new ECPublicKeySpec(pubPoint, ecParameters)); + } catch (NoSuchAlgorithmException | InvalidParameterSpecException | InvalidKeySpecException e) { + throw new IllegalStateException(e); + } + } + + private static String mapCrv(String crv) { + if (crv.endsWith("256")) { + return "secp256r1"; + } else if (crv.endsWith("384")) { + return "secp384r1"; + } else { + return "secp521r1"; + } + } } diff --git a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java index 0d54502..19fcc5d 100644 --- a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java +++ b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java @@ -19,6 +19,7 @@ import static java.util.Collections.emptyMap; import static java.util.Optional.ofNullable; import static java.util.stream.Collectors.joining; +import static java.util.stream.Collectors.toList; import java.io.BufferedReader; import java.io.File; @@ -28,14 +29,7 @@ import java.net.MalformedURLException; import java.net.URL; import java.nio.file.Files; -import java.util.ArrayList; -import java.util.Collection; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.Set; +import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.stream.Collectors; @@ -48,6 +42,7 @@ import javax.json.JsonArray; import javax.json.JsonObject; import javax.json.JsonReader; +import javax.json.JsonValue; import javax.json.JsonReaderFactory; import org.apache.geronimo.microprofile.impl.jwtauth.config.GeronimoJwtAuthConfig; @@ -163,21 +158,11 @@ private List loadJwkSet(String url) { } private List parseKeys(JsonArray keys) { - List parsedKeys = new ArrayList<>(keys.size()); - keys.forEach(key -> { - JWK jwk = new JWK((JsonObject) key); - if (isSignatureKey(jwk)) { - parsedKeys.add(jwk); - } - }); - return parsedKeys; + return keys.stream() + .map(JsonValue::asJsonObject) + .map(JWK::new) + .filter(it -> it.getUse() == null || "sig".equals(it.getUse())) + .collect(toList()); } - private boolean isSignatureKey(JWK key) { - Optional use = key.getUse(); - if (use.isPresent()) { - return use.get().equals("sig"); - } - return true; - } } From bbd162aa4c8352fffeb32a0f443dac2b24ececdc Mon Sep 17 00:00:00 2001 From: Lukas Pelzer Date: Fri, 9 Feb 2024 15:22:03 +0100 Subject: [PATCH 05/10] add non-blocking fetch of jwks --- .../impl/jwtauth/jwt/KidMapper.java | 82 ++++++++++--------- 1 file changed, 42 insertions(+), 40 deletions(-) diff --git a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java index 19fcc5d..cce4023 100644 --- a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java +++ b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java @@ -16,38 +16,42 @@ */ package org.apache.geronimo.microprofile.impl.jwtauth.jwt; -import static java.util.Collections.emptyMap; -import static java.util.Optional.ofNullable; -import static java.util.stream.Collectors.joining; -import static java.util.stream.Collectors.toList; +import org.apache.geronimo.microprofile.impl.jwtauth.config.GeronimoJwtAuthConfig; +import org.apache.geronimo.microprofile.impl.jwtauth.io.PropertiesLoader; +import org.eclipse.microprofile.jwt.config.Names; +import javax.annotation.PostConstruct; +import javax.enterprise.context.ApplicationScoped; +import javax.inject.Inject; +import javax.json.Json; +import javax.json.JsonArray; +import javax.json.JsonObject; +import javax.json.JsonReader; +import javax.json.JsonReaderFactory; +import javax.json.JsonValue; import java.io.BufferedReader; import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; -import java.net.MalformedURLException; -import java.net.URL; +import java.io.StringReader; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; import java.nio.file.Files; import java.util.*; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.ExecutionException; import java.util.stream.Collectors; import java.util.stream.Stream; -import javax.annotation.PostConstruct; -import javax.enterprise.context.ApplicationScoped; -import javax.inject.Inject; -import javax.json.Json; -import javax.json.JsonArray; -import javax.json.JsonObject; -import javax.json.JsonReader; -import javax.json.JsonValue; -import javax.json.JsonReaderFactory; - -import org.apache.geronimo.microprofile.impl.jwtauth.config.GeronimoJwtAuthConfig; -import org.apache.geronimo.microprofile.impl.jwtauth.io.PropertiesLoader; -import org.eclipse.microprofile.jwt.config.Names; +import static java.util.Collections.emptyMap; +import static java.util.Optional.ofNullable; +import static java.util.stream.Collectors.joining; +import static java.util.stream.Collectors.toList; @ApplicationScoped public class KidMapper { @@ -60,6 +64,7 @@ public class KidMapper { private String jwksUrl; private Set defaultIssuers; private JsonReaderFactory readerFactory; + private CompletableFuture> jwkSetRequest; @PostConstruct private void init() { @@ -88,6 +93,11 @@ private void init() { .orElseGet(HashSet::new); ofNullable(config.read("issuer.default", config.read(Names.ISSUER, null))).ifPresent(defaultIssuers::add); jwksUrl = config.read("mp.jwt.verify.publickey.location", null); + ofNullable(jwksUrl).ifPresent(url -> { + HttpClient httpClient = HttpClient.newBuilder().build(); + HttpRequest request = HttpRequest.newBuilder().GET().uri(URI.create(jwksUrl)).header("Accept", "application/json").build(); + jwkSetRequest = httpClient.sendAsync(request, HttpResponse.BodyHandlers.ofString()).thenApply(this::parseKeys); + }); defaultKey = config.read("public-key.default", config.read(Names.VERIFIER_PUBLIC_KEY, null)); readerFactory = Json.createReaderFactory(emptyMap()); } @@ -132,32 +142,24 @@ private String tryLoad(final String value) { // load jwks via url if (jwksUrl != null) { - loadJwkSet(jwksUrl).forEach(jwk -> keyMapping.put(jwk.getKid(), jwk.toPemKey())); - String key = keyMapping.get(value); - if (key != null) { - return key; + try { + List jwks = jwkSetRequest.get(); + jwks.forEach(jwk -> keyMapping.put(jwk.getKid(), jwk.toPemKey())); + String key = keyMapping.get(value); + if (key != null) { + return key; + } + } catch (InterruptedException | ExecutionException e) { + // loading of jwks failed } } return value; } - private List loadJwkSet(String url) { - try { - URL jwks = new URL(url); - try (InputStream connection = jwks.openStream(); JsonReader jwksReader = readerFactory.createReader(connection)) { - JsonObject keySet = jwksReader.readObject(); - JsonArray keys = keySet.getJsonArray("keys"); - List parsedKeys = parseKeys(keys); - return parsedKeys; - } catch (IOException e) { - throw new IllegalStateException(e); - } - } catch (MalformedURLException e) { - throw new IllegalArgumentException(e); - } - } - - private List parseKeys(JsonArray keys) { + private List parseKeys(HttpResponse keyResponse) { + JsonReader jwksReader = readerFactory.createReader(new StringReader(keyResponse.body())); + JsonObject keySet = jwksReader.readObject(); + JsonArray keys = keySet.getJsonArray("keys"); return keys.stream() .map(JsonValue::asJsonObject) .map(JWK::new) From f049a8eff6ae0cf5caaca7392cbfca9b3d81b0c9 Mon Sep 17 00:00:00 2001 From: Lukas Pelzer Date: Wed, 21 Feb 2024 10:20:45 +0100 Subject: [PATCH 06/10] add key invalidation interval --- README.adoc | 1 + .../microprofile/impl/jwtauth/jwt/JWK.java | 14 ++--- .../impl/jwtauth/jwt/KidMapper.java | 55 ++++++++++++++++--- 3 files changed, 54 insertions(+), 16 deletions(-) diff --git a/README.adoc b/README.adoc index 1741f82..eba7145 100644 --- a/README.adoc +++ b/README.adoc @@ -53,6 +53,7 @@ and if not system properties and `META-INF/geronimo/microprofile/jwt-auth.proper |geronimo.jwt-auth.jca.provider|The JCA provider (java security)|- (built-in one) |geronimo.jwt-auth.groups.mapping|The mapping for the groups|- |geronimo.jwt-auth.public-key.cache.active|Should public keys be cached|true +|geronimo.jwt-auth.jwks.invalidationInterval|Invalidation interval in seconds|600 |geronimo.jwt-auth.public-key.default|Default public key to verify JWT|- |=== diff --git a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java index 253bbda..dc55823 100644 --- a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java +++ b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java @@ -42,13 +42,13 @@ public class JWK { private String use; public JWK(JsonObject jsonObject) { - kid = jsonObject.getString("kid"); - kty = jsonObject.getString("kty"); - x = jsonObject.getString("x"); - y = jsonObject.getString("y"); - crv = jsonObject.getString("crv"); - n = jsonObject.getString("n"); - e = jsonObject.getString("e"); + kid = jsonObject.getString("kid",null); + kty = jsonObject.getString("kty",null); + x = jsonObject.getString("x",null); + y = jsonObject.getString("y",null); + crv = jsonObject.getString("crv",null); + n = jsonObject.getString("n",null); + e = jsonObject.getString("e",null); use = jsonObject.getString("use", null); } diff --git a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java index cce4023..311e144 100644 --- a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java +++ b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java @@ -21,6 +21,7 @@ import org.eclipse.microprofile.jwt.config.Names; import javax.annotation.PostConstruct; +import javax.annotation.PreDestroy; import javax.enterprise.context.ApplicationScoped; import javax.inject.Inject; import javax.json.Json; @@ -40,11 +41,20 @@ import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.nio.file.Files; -import java.util.*; +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -64,8 +74,9 @@ public class KidMapper { private String jwksUrl; private Set defaultIssuers; private JsonReaderFactory readerFactory; - private CompletableFuture> jwkSetRequest; - + private volatile CompletableFuture> jwkSetRequest; + HttpClient httpClient; + ScheduledExecutorService backgroundThread; @PostConstruct private void init() { ofNullable(config.read("kids.key.mapping", null)) @@ -94,14 +105,26 @@ private void init() { ofNullable(config.read("issuer.default", config.read(Names.ISSUER, null))).ifPresent(defaultIssuers::add); jwksUrl = config.read("mp.jwt.verify.publickey.location", null); ofNullable(jwksUrl).ifPresent(url -> { - HttpClient httpClient = HttpClient.newBuilder().build(); - HttpRequest request = HttpRequest.newBuilder().GET().uri(URI.create(jwksUrl)).header("Accept", "application/json").build(); - jwkSetRequest = httpClient.sendAsync(request, HttpResponse.BodyHandlers.ofString()).thenApply(this::parseKeys); + backgroundThread = Executors.newSingleThreadScheduledExecutor(); + httpClient = HttpClient.newBuilder().executor(backgroundThread).build(); + long SECONDS_REFRESH = getJwksRefreshInterval(); + backgroundThread.scheduleAtFixedRate(this::reloadRemoteKeys, 1,SECONDS_REFRESH, TimeUnit.SECONDS ); + reloadRemoteKeys(); }); defaultKey = config.read("public-key.default", config.read(Names.VERIFIER_PUBLIC_KEY, null)); readerFactory = Json.createReaderFactory(emptyMap()); } + private long getJwksRefreshInterval() { + String interval = config.read("jwks.invalidationInterval","600"); + return Integer.parseInt(interval); + } + + private void reloadRemoteKeys() { + HttpRequest request = HttpRequest.newBuilder().GET().uri(URI.create(jwksUrl)).header("Accept", "application/json").build(); + jwkSetRequest = httpClient.sendAsync(request, HttpResponse.BodyHandlers.ofString()); + } + public String loadKey(final String property) { String value = keyMapping.get(property); if (value == null) { @@ -143,14 +166,15 @@ private String tryLoad(final String value) { // load jwks via url if (jwksUrl != null) { try { - List jwks = jwkSetRequest.get(); + HttpResponse resposne = jwkSetRequest.get(1, TimeUnit.MINUTES); + List jwks = parseKeys(resposne); jwks.forEach(jwk -> keyMapping.put(jwk.getKid(), jwk.toPemKey())); String key = keyMapping.get(value); if (key != null) { return key; } - } catch (InterruptedException | ExecutionException e) { - // loading of jwks failed + } catch (InterruptedException | ExecutionException | TimeoutException e) { + throw new RuntimeException(e); } } return value; @@ -167,4 +191,17 @@ private List parseKeys(HttpResponse keyResponse) { .collect(toList()); } + @PreDestroy + private void destroy() { + if(backgroundThread != null) { + backgroundThread.shutdown(); + } + if(jwkSetRequest != null && !jwkSetRequest.isDone()){ + try { + jwkSetRequest.get(1, TimeUnit.MINUTES); + } catch (InterruptedException | ExecutionException | TimeoutException e) { + } + } + } + } From 6dc45b86d34de1ba6cc6188af0ec077500a68f2b Mon Sep 17 00:00:00 2001 From: Lukas Pelzer Date: Fri, 15 Mar 2024 15:13:38 +0100 Subject: [PATCH 07/10] add key invalidation interval --- .../impl/jwtauth/jwt/KidMapper.java | 35 +++++++++++-------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java index 311e144..57e51c0 100644 --- a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java +++ b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java @@ -68,13 +68,13 @@ public class KidMapper { @Inject private GeronimoJwtAuthConfig config; - private final ConcurrentMap keyMapping = new ConcurrentHashMap<>(); + private volatile ConcurrentMap keyMapping = new ConcurrentHashMap<>(); private final Map> issuerMapping = new HashMap<>(); private String defaultKey; private String jwksUrl; private Set defaultIssuers; private JsonReaderFactory readerFactory; - private volatile CompletableFuture> jwkSetRequest; + private volatile CompletableFuture> jwkSetRequest; HttpClient httpClient; ScheduledExecutorService backgroundThread; @PostConstruct @@ -104,15 +104,15 @@ private void init() { .orElseGet(HashSet::new); ofNullable(config.read("issuer.default", config.read(Names.ISSUER, null))).ifPresent(defaultIssuers::add); jwksUrl = config.read("mp.jwt.verify.publickey.location", null); + readerFactory = Json.createReaderFactory(emptyMap()); ofNullable(jwksUrl).ifPresent(url -> { backgroundThread = Executors.newSingleThreadScheduledExecutor(); httpClient = HttpClient.newBuilder().executor(backgroundThread).build(); long SECONDS_REFRESH = getJwksRefreshInterval(); - backgroundThread.scheduleAtFixedRate(this::reloadRemoteKeys, 1,SECONDS_REFRESH, TimeUnit.SECONDS ); - reloadRemoteKeys(); + backgroundThread.scheduleAtFixedRate(this::reloadRemoteKeys, getJwksRefreshInterval(),SECONDS_REFRESH, TimeUnit.SECONDS ); + reloadRemoteKeys(); // inital load, otherwise the background thread is too slow to start and serve }); defaultKey = config.read("public-key.default", config.read(Names.VERIFIER_PUBLIC_KEY, null)); - readerFactory = Json.createReaderFactory(emptyMap()); } private long getJwksRefreshInterval() { @@ -122,7 +122,12 @@ private long getJwksRefreshInterval() { private void reloadRemoteKeys() { HttpRequest request = HttpRequest.newBuilder().GET().uri(URI.create(jwksUrl)).header("Accept", "application/json").build(); - jwkSetRequest = httpClient.sendAsync(request, HttpResponse.BodyHandlers.ofString()); + CompletableFuture> httpResponseCompletableFuture = httpClient.sendAsync(request, HttpResponse.BodyHandlers.ofString()); + jwkSetRequest = httpResponseCompletableFuture.thenApply(result -> { + List jwks = parseKeys(result); + jwks.forEach(key -> keyMapping.put(key.getKid(),key.toPemKey())); + return jwks; + }); } public String loadKey(final String property) { @@ -165,23 +170,25 @@ private String tryLoad(final String value) { // load jwks via url if (jwksUrl != null) { - try { - HttpResponse resposne = jwkSetRequest.get(1, TimeUnit.MINUTES); - List jwks = parseKeys(resposne); - jwks.forEach(jwk -> keyMapping.put(jwk.getKid(), jwk.toPemKey())); + if(jwkSetRequest != null) { + try { + jwkSetRequest.get(1, TimeUnit.MINUTES); + } catch (InterruptedException | ExecutionException | TimeoutException e) { + throw new RuntimeException(e); + } + } String key = keyMapping.get(value); if (key != null) { return key; } - } catch (InterruptedException | ExecutionException | TimeoutException e) { - throw new RuntimeException(e); - } + } return value; } private List parseKeys(HttpResponse keyResponse) { - JsonReader jwksReader = readerFactory.createReader(new StringReader(keyResponse.body())); + StringReader stringReader = new StringReader(keyResponse.body()); + JsonReader jwksReader = readerFactory.createReader(stringReader); JsonObject keySet = jwksReader.readObject(); JsonArray keys = keySet.getJsonArray("keys"); return keys.stream() From fcaf6c7f5ec8c8a693c299f3a32154ad4c50627d Mon Sep 17 00:00:00 2001 From: Lukas Pelzer Date: Wed, 20 Mar 2024 09:19:47 +0100 Subject: [PATCH 08/10] refactor and format code --- .../microprofile/impl/jwtauth/jwt/JWK.java | 14 ++-- .../impl/jwtauth/jwt/KidMapper.java | 72 +++++++++---------- 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java index dc55823..caada2b 100644 --- a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java +++ b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JWK.java @@ -42,13 +42,13 @@ public class JWK { private String use; public JWK(JsonObject jsonObject) { - kid = jsonObject.getString("kid",null); - kty = jsonObject.getString("kty",null); - x = jsonObject.getString("x",null); - y = jsonObject.getString("y",null); - crv = jsonObject.getString("crv",null); - n = jsonObject.getString("n",null); - e = jsonObject.getString("e",null); + kid = jsonObject.getString("kid", null); + kty = jsonObject.getString("kty", null); + x = jsonObject.getString("x", null); + y = jsonObject.getString("y", null); + crv = jsonObject.getString("crv", null); + n = jsonObject.getString("n", null); + e = jsonObject.getString("e", null); use = jsonObject.getString("use", null); } diff --git a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java index 57e51c0..66d3c13 100644 --- a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java +++ b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java @@ -41,20 +41,8 @@ import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.nio.file.Files; -import java.util.Collection; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; +import java.util.*; +import java.util.concurrent.*; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -74,7 +62,7 @@ public class KidMapper { private String jwksUrl; private Set defaultIssuers; private JsonReaderFactory readerFactory; - private volatile CompletableFuture> jwkSetRequest; + private volatile CompletableFuture reloadJwksRequest; HttpClient httpClient; ScheduledExecutorService backgroundThread; @PostConstruct @@ -106,28 +94,43 @@ private void init() { jwksUrl = config.read("mp.jwt.verify.publickey.location", null); readerFactory = Json.createReaderFactory(emptyMap()); ofNullable(jwksUrl).ifPresent(url -> { - backgroundThread = Executors.newSingleThreadScheduledExecutor(); - httpClient = HttpClient.newBuilder().executor(backgroundThread).build(); - long SECONDS_REFRESH = getJwksRefreshInterval(); - backgroundThread.scheduleAtFixedRate(this::reloadRemoteKeys, getJwksRefreshInterval(),SECONDS_REFRESH, TimeUnit.SECONDS ); - reloadRemoteKeys(); // inital load, otherwise the background thread is too slow to start and serve + HttpClient.Builder builder = HttpClient.newBuilder(); + if(getJwksRefreshInterval() != null){ + builder.executor(backgroundThread); + long secondsRefresh = getJwksRefreshInterval(); + backgroundThread = Executors.newSingleThreadScheduledExecutor(); + backgroundThread.scheduleAtFixedRate(this::reloadRemoteKeys, getJwksRefreshInterval(), secondsRefresh, TimeUnit.SECONDS ); + } + httpClient = builder.build(); + reloadJwksRequest = reloadRemoteKeys();// inital load, otherwise the background thread is too slow to start and serve }); defaultKey = config.read("public-key.default", config.read(Names.VERIFIER_PUBLIC_KEY, null)); } - private long getJwksRefreshInterval() { - String interval = config.read("jwks.invalidationInterval","600"); - return Integer.parseInt(interval); + private Integer getJwksRefreshInterval() { + String interval = config.read("jwks.invalidationInterval",null); + if (interval != null) { + return Integer.parseInt(interval); + } else { + return null; + } } - private void reloadRemoteKeys() { + private CompletableFuture reloadRemoteKeys() { HttpRequest request = HttpRequest.newBuilder().GET().uri(URI.create(jwksUrl)).header("Accept", "application/json").build(); CompletableFuture> httpResponseCompletableFuture = httpClient.sendAsync(request, HttpResponse.BodyHandlers.ofString()); - jwkSetRequest = httpResponseCompletableFuture.thenApply(result -> { + CompletableFuture ongoingRequest = httpResponseCompletableFuture.thenApply(result -> { List jwks = parseKeys(result); - jwks.forEach(key -> keyMapping.put(key.getKid(),key.toPemKey())); - return jwks; + ConcurrentHashMap newKeys = new ConcurrentHashMap<>(); + jwks.forEach(key -> newKeys.put(key.getKid(), key.toPemKey())); + keyMapping = newKeys; + return null; + }); + + ongoingRequest.thenRun(() -> { + reloadJwksRequest = ongoingRequest; }); + return ongoingRequest; } public String loadKey(final String property) { @@ -170,10 +173,10 @@ private String tryLoad(final String value) { // load jwks via url if (jwksUrl != null) { - if(jwkSetRequest != null) { + if(reloadJwksRequest != null) { try { - jwkSetRequest.get(1, TimeUnit.MINUTES); - } catch (InterruptedException | ExecutionException | TimeoutException e) { + reloadJwksRequest.get(); + } catch (InterruptedException | ExecutionException e) { throw new RuntimeException(e); } } @@ -200,14 +203,11 @@ private List parseKeys(HttpResponse keyResponse) { @PreDestroy private void destroy() { - if(backgroundThread != null) { + if (backgroundThread != null) { backgroundThread.shutdown(); } - if(jwkSetRequest != null && !jwkSetRequest.isDone()){ - try { - jwkSetRequest.get(1, TimeUnit.MINUTES); - } catch (InterruptedException | ExecutionException | TimeoutException e) { - } + if (reloadJwksRequest != null) { + reloadJwksRequest.cancel(true); } } From 95671ccdbb34d14e5d3ea09a4cfa1262e0e7d8e4 Mon Sep 17 00:00:00 2001 From: Lukas Pelzer Date: Wed, 20 Mar 2024 09:20:54 +0100 Subject: [PATCH 09/10] fix documentation --- README.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.adoc b/README.adoc index eba7145..ea18760 100644 --- a/README.adoc +++ b/README.adoc @@ -53,7 +53,7 @@ and if not system properties and `META-INF/geronimo/microprofile/jwt-auth.proper |geronimo.jwt-auth.jca.provider|The JCA provider (java security)|- (built-in one) |geronimo.jwt-auth.groups.mapping|The mapping for the groups|- |geronimo.jwt-auth.public-key.cache.active|Should public keys be cached|true -|geronimo.jwt-auth.jwks.invalidationInterval|Invalidation interval in seconds|600 +|geronimo.jwt-auth.jwks.invalidationInterval|Invalidation interval in seconds|- |geronimo.jwt-auth.public-key.default|Default public key to verify JWT|- |=== From 8740a25439596e62d3628c797e4e8a4685434d20 Mon Sep 17 00:00:00 2001 From: Lukas Pelzer Date: Fri, 5 Apr 2024 14:25:03 +0200 Subject: [PATCH 10/10] fix NPE bug --- .../impl/jwtauth/jwt/KidMapper.java | 6 +- .../impl/jwtauth/jwt/JwksServer.java | 56 +++++++++++++++ .../impl/jwtauth/jwt/KidMapperTest.java | 69 ++----------------- .../impl/jwtauth/jwt/PublicKeyResource.java | 23 +++++++ .../impl/jwtauth/jwt/RefreshIntervalTest.java | 64 +++++++++++++++++ src/test/resources/geronimo.xml | 1 + 6 files changed, 151 insertions(+), 68 deletions(-) create mode 100644 src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JwksServer.java create mode 100644 src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/PublicKeyResource.java create mode 100644 src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/RefreshIntervalTest.java diff --git a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java index 66d3c13..f76ddb3 100644 --- a/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java +++ b/src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java @@ -95,10 +95,10 @@ private void init() { readerFactory = Json.createReaderFactory(emptyMap()); ofNullable(jwksUrl).ifPresent(url -> { HttpClient.Builder builder = HttpClient.newBuilder(); - if(getJwksRefreshInterval() != null){ - builder.executor(backgroundThread); + if (getJwksRefreshInterval() != null) { long secondsRefresh = getJwksRefreshInterval(); backgroundThread = Executors.newSingleThreadScheduledExecutor(); + builder.executor(backgroundThread); backgroundThread.scheduleAtFixedRate(this::reloadRemoteKeys, getJwksRefreshInterval(), secondsRefresh, TimeUnit.SECONDS ); } httpClient = builder.build(); @@ -108,7 +108,7 @@ private void init() { } private Integer getJwksRefreshInterval() { - String interval = config.read("jwks.invalidationInterval",null); + String interval = config.read("jwks.invalidation.interval",null); if (interval != null) { return Integer.parseInt(interval); } else { diff --git a/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JwksServer.java b/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JwksServer.java new file mode 100644 index 0000000..c3bd18d --- /dev/null +++ b/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JwksServer.java @@ -0,0 +1,56 @@ +package org.apache.geronimo.microprofile.impl.jwtauth.jwt; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.PrintWriter; +import java.net.ServerSocket; +import java.net.Socket; + +class JwksServer { + + private static final String HEADER = "HTTP/1.0 200 OK\r\nConnection: close\r\n"; + + private ServerSocket serverSocket; + + JwksServer() throws IOException { + serverSocket = new ServerSocket(0); + } + + int getPort() { + return serverSocket.getLocalPort(); + } + + void start() { + Thread server = new Thread(() -> { + while (!serverSocket.isClosed()) { + try (Socket client = serverSocket.accept(); + BufferedReader request = new BufferedReader(new InputStreamReader(client.getInputStream())); + BufferedReader reader = new BufferedReader(new InputStreamReader( + getClass().getResourceAsStream(request.readLine().split("\\s")[1]))); + PrintWriter writer = new PrintWriter(client.getOutputStream())) { + + writer.println(HEADER); + writer.print(load(reader)); + } catch (IOException e) { + if (!serverSocket.isClosed()) { + e.printStackTrace(System.err); + } + } + } + }); + server.start(); + } + + void stop() throws IOException { + serverSocket.close(); + } + + private String load(BufferedReader reader) throws IOException { + StringBuilder content = new StringBuilder(); + for (String line = reader.readLine(); line != null; line = reader.readLine()) { + content.append(line).append("\r\n"); + } + return content.toString(); + } +} \ No newline at end of file diff --git a/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapperTest.java b/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapperTest.java index 3d0a528..0ed7551 100644 --- a/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapperTest.java +++ b/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapperTest.java @@ -25,22 +25,23 @@ import org.jboss.arquillian.testng.Arquillian; import org.jboss.shrinkwrap.api.ShrinkWrap; import org.jboss.shrinkwrap.api.spec.WebArchive; +import org.omg.Security.Public; import org.testng.annotations.AfterClass; import org.testng.annotations.Test; public class KidMapperTest extends Arquillian { - private static Server jwksServer; + private static JwksServer jwksServer; @Deployment() public static WebArchive createDeployment() throws Exception { - jwksServer = new Server(); + jwksServer = new JwksServer(); jwksServer.start(); System.setProperty("mp.jwt.verify.publickey.location", "http://localhost:" + jwksServer.getPort() + "/jwks.json"); return ShrinkWrap .create(WebArchive.class) .addAsWebInfResource("META-INF/beans.xml", "beans.xml") - .addClasses(JwtParser.class, KidMapper.class); + .addClasses(JwtParser.class, KidMapper.class, PublicKeyResource.class); } @AfterClass @@ -61,66 +62,4 @@ void convertJwksetToPem() throws URISyntaxException { assertEquals(key, expectedKey); } - @ApplicationScoped - @Path("public-keys") - public static class PublicKeyResource { - - @Inject - private KidMapper kidMapper; - - @GET - @Path("{kid}") - @Produces() - public String getPublicKey(@PathParam("kid") String kid) { - return kidMapper.loadKey(kid); - } - } - - private static class Server { - - private static final String HEADER = "HTTP/1.0 200 OK\r\nConnection: close\r\n"; - - private ServerSocket serverSocket; - - Server() throws IOException { - serverSocket = new ServerSocket(0); - } - - int getPort() { - return serverSocket.getLocalPort(); - } - - void start() { - Thread server = new Thread(() -> { - while (!serverSocket.isClosed()) { - try (Socket client = serverSocket.accept(); - BufferedReader request = new BufferedReader(new InputStreamReader(client.getInputStream())); - BufferedReader reader = new BufferedReader(new InputStreamReader( - getClass().getResourceAsStream(request.readLine().split("\\s")[1]))); - PrintWriter writer = new PrintWriter(client.getOutputStream())) { - - writer.println(HEADER); - writer.print(load(reader)); - } catch (IOException e) { - if (!serverSocket.isClosed()) { - e.printStackTrace(System.err); - } - } - } - }); - server.start(); - } - - void stop() throws IOException { - serverSocket.close(); - } - - private String load(BufferedReader reader) throws IOException { - StringBuilder content = new StringBuilder(); - for (String line = reader.readLine(); line != null; line = reader.readLine()) { - content.append(line).append("\r\n"); - } - return content.toString(); - } - } } \ No newline at end of file diff --git a/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/PublicKeyResource.java b/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/PublicKeyResource.java new file mode 100644 index 0000000..7a89e6c --- /dev/null +++ b/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/PublicKeyResource.java @@ -0,0 +1,23 @@ +package org.apache.geronimo.microprofile.impl.jwtauth.jwt; + +import javax.enterprise.context.ApplicationScoped; +import javax.inject.Inject; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.PathParam; +import javax.ws.rs.Produces; + +@ApplicationScoped +@Path("public-keys") +public class PublicKeyResource { + + @Inject + private KidMapper kidMapper; + + @GET + @Path("{kid}") + @Produces() + public String getPublicKey(@PathParam("kid") String kid) { + return kidMapper.loadKey(kid); + } +} \ No newline at end of file diff --git a/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/RefreshIntervalTest.java b/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/RefreshIntervalTest.java new file mode 100644 index 0000000..655940c --- /dev/null +++ b/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/RefreshIntervalTest.java @@ -0,0 +1,64 @@ +package org.apache.geronimo.microprofile.impl.jwtauth.jwt; + +import org.jboss.arquillian.container.test.api.Deployment; +import org.jboss.arquillian.container.test.api.RunAsClient; +import org.jboss.arquillian.test.api.ArquillianResource; +import org.jboss.arquillian.testng.Arquillian; +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.spec.WebArchive; +import org.testng.annotations.AfterClass; +import org.testng.annotations.Test; + +import javax.enterprise.context.ApplicationScoped; +import javax.inject.Inject; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.PathParam; +import javax.ws.rs.Produces; +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.PrintWriter; +import java.net.ServerSocket; +import java.net.Socket; +import java.net.URISyntaxException; +import java.net.URL; + +import static javax.ws.rs.client.ClientBuilder.newClient; +import static org.testng.Assert.assertEquals; + +public class RefreshIntervalTest extends Arquillian { + + private static JwksServer jwksServer; + + @Deployment() + public static WebArchive createDeployment() throws Exception { + jwksServer = new JwksServer(); + jwksServer.start(); + System.setProperty("mp.jwt.verify.publickey.location", "http://localhost:" + jwksServer.getPort() + "/jwks.json"); + System.setProperty("geronimo.jwt-auth.jwks.invalidation.interval", "1"); + return ShrinkWrap + .create(WebArchive.class) + .addAsWebInfResource("META-INF/beans.xml", "beans.xml") + .addClasses(JwtParser.class, KidMapper.class, PublicKeyResource.class); + } + + @AfterClass + static void stopJwksServer() throws IOException { + jwksServer.stop(); + } + + @ArquillianResource + private URL serverUrl; + + @Test + @RunAsClient + void refreshIntervalTest() throws URISyntaxException { + String expectedKey = "-----BEGIN PUBLIC KEY-----MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCyzNurU19lqnYhx5QI72sIX1lh8cTehTmboC+DLG7UuaUHqs096M754HtP2IiHFcIQqwYNzHgKmjmfGdbk9JBkz/DNeDVsA5nc7qTnsSgULXTxwHSF286IJdco5kasaJm4Xurlm3V+2oiTugraBsi1J0Ht0OtHgJIlIaGxK7mY/QIDAQAB-----END PUBLIC KEY-----"; + + String key = newClient().target(serverUrl.toURI()).path("public-keys").path("orange-1234").request().get(String.class); + + assertEquals(key, expectedKey); + } + +} \ No newline at end of file diff --git a/src/test/resources/geronimo.xml b/src/test/resources/geronimo.xml index 8b5ffb7..d28e269 100644 --- a/src/test/resources/geronimo.xml +++ b/src/test/resources/geronimo.xml @@ -15,6 +15,7 @@ +