This is an automated email from the ASF dual-hosted git repository. dsmiley pushed a commit to branch branch_10x in repository https://gitbox.apache.org/repos/asf/solr.git
commit ca132d796949613e54a4beeb60261483bc4aa78b Author: Eric Pugh <[email protected]> AuthorDate: Mon Aug 31 18:04:13 2026 -0400 Review and tidy solr/modules/jwt-auth code (#4834) (cherry picked from commit 91f139a37ed9b249be97ea3080fa4cc23f0372e7) --- .../apache/solr/security/jwt/JWTAuthPlugin.java | 4 +- .../apache/solr/security/jwt/JWTIssuerConfig.java | 2 +- .../org/apache/solr/security/jwt/JWTPrincipal.java | 4 +- .../security/jwt/JWTAuthPluginIntegrationTest.java | 68 +++++++++++----------- .../solr/security/jwt/JWTAuthPluginTest.java | 8 ++- .../solr/security/jwt/JWTIssuerConfigTest.java | 21 +++---- .../jwt/JWTVerificationkeyResolverTest.java | 3 +- .../solr/security/jwt/KeystoreGenerator.java | 2 +- 8 files changed, 59 insertions(+), 53 deletions(-) diff --git a/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java b/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java index ebe1d01de35..bf046073783 100644 --- a/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java +++ b/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java @@ -73,7 +73,7 @@ import org.jose4j.lang.JoseException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -/** Authentication plugin that finds logged in user by validating the signature of a JWT token */ +/** Authentication plugin that finds logged-in user by validating the signature of a JWT token */ public class JWTAuthPlugin extends AuthenticationPlugin implements SpecProvider, ConfigEditablePlugin { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); @@ -689,7 +689,7 @@ public class JWTAuthPlugin extends AuthenticationPlugin } catch (InvalidJwtSignatureException ise) { return new JWTAuthenticationResponse(AuthCode.SIGNATURE_INVALID, ise); } catch (InvalidJwtException e) { - // Whether or not the JWT has expired being one common reason for invalidity + // Whether the JWT has expired being one common reason for invalidity if (e.hasExpired()) { return new JWTAuthenticationResponse( AuthCode.JWT_EXPIRED, diff --git a/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTIssuerConfig.java b/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTIssuerConfig.java index d8a6934317a..1bea5896a32 100644 --- a/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTIssuerConfig.java +++ b/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTIssuerConfig.java @@ -512,7 +512,7 @@ public class JWTIssuerConfig { * Fetch well-known config from a URL, with optional list of trusted certificates * * @param url the url to fetch - * @param trustedCerts optional list of trusted SSL certs. May be null to fall-back to Java's + * @param trustedCerts optional list of trusted SSL certs. May be null to fall back to Java's * defaults * @return an instance of WellKnownDiscoveryConfig object */ diff --git a/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTPrincipal.java b/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTPrincipal.java index b10d57d0bf0..25f092c4cd6 100644 --- a/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTPrincipal.java +++ b/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTPrincipal.java @@ -29,9 +29,9 @@ public class JWTPrincipal implements Principal { Map<String, Object> claims; /** - * User principal with user name as well as one or more roles that he/she belong to + * User principal with username as well as one or more roles that he/she belong to * - * @param username string with user name for user + * @param username string with username for user * @param token compact string representation of JWT token * @param claims list of verified JWT claims as a map */ diff --git a/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTAuthPluginIntegrationTest.java b/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTAuthPluginIntegrationTest.java index d480bd7ead6..cffee4cc0a4 100644 --- a/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTAuthPluginIntegrationTest.java +++ b/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTAuthPluginIntegrationTest.java @@ -95,7 +95,7 @@ public class JWTAuthPluginIntegrationTest extends SolrCloudAuthTestCase { @BeforeClass public static void beforeClass() throws Exception { - // Setup an OAuth2 mock server with SSL + // Set up an OAuth2 mock server with SSL Path p12Cert = JWT_TEST_PATH().resolve("security").resolve("jwt_plugin_idp_certs.p12"); pemFilePath = JWT_TEST_PATH().resolve("security").resolve("jwt_plugin_idp_cert.pem"); wrongPemFilePath = JWT_TEST_PATH().resolve("security").resolve("jwt_plugin_idp_wrongcert.pem"); @@ -165,13 +165,14 @@ public class JWTAuthPluginIntegrationTest extends SolrCloudAuthTestCase { assertEquals("Bearer realm=\"my-solr-jwt\"", headers.get("WWW-Authenticate")); String authData = new String(Base64.getDecoder().decode(headers.get("X-Solr-AuthData")), UTF_8); assertEquals( - "{\n" - + " \"tokenEndpoint\":\"http://acmepaymentscorp/oauth/oauth20/token\",\n" - + " \"authorization_flow\":\"code_pkce\",\n" - + " \"scope\":\"solr:admin\",\n" - + " \"redirect_uris\":[],\n" - + " \"authorizationEndpoint\":\"http://acmepaymentscorp/oauth/auz/authorize\",\n" - + " \"client_id\":\"solr-cluster\"}", + """ + { + "tokenEndpoint":"http://acmepaymentscorp/oauth/oauth20/token", + "authorization_flow":"code_pkce", + "scope":"solr:admin", + "redirect_uris":[], + "authorizationEndpoint":"http://acmepaymentscorp/oauth/auz/authorize", + "client_id":"solr-cluster"}""", authData); myCluster.shutdown(); } @@ -189,13 +190,14 @@ public class JWTAuthPluginIntegrationTest extends SolrCloudAuthTestCase { "Bearer realm=\"my-solr-jwt-blockunknown-false\"", headers.get("WWW-Authenticate")); String authData = new String(Base64.getDecoder().decode(headers.get("X-Solr-AuthData")), UTF_8); assertEquals( - "{\n" - + " \"tokenEndpoint\":\"http://acmepaymentscorp/oauth/oauth20/token\",\n" - + " \"authorization_flow\":\"code_pkce\",\n" - + " \"scope\":\"solr:admin\",\n" - + " \"redirect_uris\":[],\n" - + " \"authorizationEndpoint\":\"http://acmepaymentscorp/oauth/auz/authorize\",\n" - + " \"client_id\":\"solr-cluster\"}", + """ + { + "tokenEndpoint":"http://acmepaymentscorp/oauth/oauth20/token", + "authorization_flow":"code_pkce", + "scope":"solr:admin", + "redirect_uris":[], + "authorizationEndpoint":"http://acmepaymentscorp/oauth/auz/authorize", + "client_id":"solr-cluster"}""", authData); myCluster.shutdown(); } @@ -267,12 +269,12 @@ public class JWTAuthPluginIntegrationTest extends SolrCloudAuthTestCase { assertAuthMetricsMinimums(4, 4, 0, 0, 0, 0); assertPkiAuthMetricsMinimums(2, 2, 0, 0, 0, 0); - // First a non distributed query + // First a non-distributed query result = get(baseUrl + "/" + COLLECTION + "/query?q=*:*&distrib=false", jwtStaticTestToken); assertEquals(Integer.valueOf(200), result.second()); assertAuthMetricsMinimums(5, 5, 0, 0, 0, 0); - // Now do a distributed query, using JWTAuth for inter-node + // Now do a distributed query, using JWTAuth for internode result = get(baseUrl + "/" + COLLECTION + "/query?q=*:*", jwtStaticTestToken); assertEquals(Integer.valueOf(200), result.second()); assertAuthMetricsMinimums(10, 10, 0, 0, 0, 0); @@ -288,10 +290,10 @@ public class JWTAuthPluginIntegrationTest extends SolrCloudAuthTestCase { } /** - * Test if JWTPrincipal is passed correctly on internode communication. Setup a cluster with more - * nodes using jwtAuth for both authentication and authorization. Add a collection with restricted - * access and with less replicas and shards then the number of nodes. Test if we can query the - * collection on every node. + * Test if JWTPrincipal is passed correctly on inter-node communication. Set up a cluster with + * more nodes using jwtAuth for both authentication and authorization. Add a collection with + * restricted access and with fewer replicas and shards then the number of nodes. Test if we can + * query the collection on every node. */ @Test public void testInternodeAuthorization() throws Exception { @@ -345,8 +347,7 @@ public class JWTAuthPluginIntegrationTest extends SolrCloudAuthTestCase { int passThrough, int failWrongCredentials, int failMissingCredentials, - int errors) - throws InterruptedException { + int errors) { super.assertAuthMetricsMinimums( JWTAuthPlugin.class, requests, @@ -418,15 +419,16 @@ public class JWTAuthPluginIntegrationTest extends SolrCloudAuthTestCase { /** Initialize some static JWT keys */ private static void initStaticJwt() throws Exception { String jwkJSON = - "{\n" - + " \"kty\": \"RSA\",\n" - + " \"d\": \"i6pyv2z3o-MlYytWsOr3IE1olu2RXZBzjPRBNgWAP1TlLNaphHEvH5aHhe_CtBAastgFFMuP29CFhaL3_tGczkvWJkSveZQN2AHWHgRShKgoSVMspkhOt3Ghha4CvpnZ9BnQzVHnaBnHDTTTfVgXz7P1ZNBhQY4URG61DKIF-JSSClyh1xKuMoJX0lILXDYGGcjVTZL_hci4IXPPTpOJHV51-pxuO7WU5M9252UYoiYyCJ56ai8N49aKIMsqhdGuO4aWUwsGIW4oQpjtce5eEojCprYl-9rDhTwLAFoBtjy6LvkqlR2Ae5dKZYpStljBjK8PJrBvWZjXAEMDdQ8PuQ\",\n" - + " \"e\": \"AQAB\",\n" - + " \"use\": \"sig\",\n" - + " \"kid\": \"test\",\n" - + " \"alg\": \"RS256\",\n" - + " \"n\": \"jeyrvOaZrmKWjyNXt0myAc_pJ1hNt3aRupExJEx1ewPaL9J9HFgSCjMrYxCB1ETO1NDyZ3nSgjZis-jHHDqBxBjRdq_t1E2rkGFaYbxAyKt220Pwgme_SFTB9MXVrFQGkKyjmQeVmOmV6zM3KK8uMdKQJ4aoKmwBcF5Zg7EZdDcKOFgpgva1Jq-FlEsaJ2xrYDYo3KnGcOHIt9_0NQeLsqZbeWYLxYni7uROFncXYV5FhSJCeR4A_rrbwlaCydGxE0ToC_9HNYibUHlkJjqyUhAgORCbNS8JLCJH8NUi5sDdIawK9GTSyvsJXZ-QHqo4cMUuxWV5AJtaRGghuMUfqQ\"\n" - + "}"; + """ + { + "kty": "RSA", + "d": "i6pyv2z3o-MlYytWsOr3IE1olu2RXZBzjPRBNgWAP1TlLNaphHEvH5aHhe_CtBAastgFFMuP29CFhaL3_tGczkvWJkSveZQN2AHWHgRShKgoSVMspkhOt3Ghha4CvpnZ9BnQzVHnaBnHDTTTfVgXz7P1ZNBhQY4URG61DKIF-JSSClyh1xKuMoJX0lILXDYGGcjVTZL_hci4IXPPTpOJHV51-pxuO7WU5M9252UYoiYyCJ56ai8N49aKIMsqhdGuO4aWUwsGIW4oQpjtce5eEojCprYl-9rDhTwLAFoBtjy6LvkqlR2Ae5dKZYpStljBjK8PJrBvWZjXAEMDdQ8PuQ", + "e": "AQAB", + "use": "sig", + "kid": "test", + "alg": "RS256", + "n": "jeyrvOaZrmKWjyNXt0myAc_pJ1hNt3aRupExJEx1ewPaL9J9HFgSCjMrYxCB1ETO1NDyZ3nSgjZis-jHHDqBxBjRdq_t1E2rkGFaYbxAyKt220Pwgme_SFTB9MXVrFQGkKyjmQeVmOmV6zM3KK8uMdKQJ4aoKmwBcF5Zg7EZdDcKOFgpgva1Jq-FlEsaJ2xrYDYo3KnGcOHIt9_0NQeLsqZbeWYLxYni7uROFncXYV5FhSJCeR4A_rrbwlaCydGxE0ToC_9HNYibUHlkJjqyUhAgORCbNS8JLCJH8NUi5sDdIawK9GTSyvsJXZ-QHqo4cMUuxWV5AJtaRGghuMUfqQ" + }"""; PublicJsonWebKey jwk = RsaJsonWebKey.Factory.newPublicJwk(jwkJSON); JwtClaims claims = JWTAuthPluginTest.generateClaims(); @@ -558,7 +560,7 @@ public class JWTAuthPluginIntegrationTest extends SolrCloudAuthTestCase { } /** - * Creates a security.json string which points to the MockOAuth server using it's well-known URL + * Creates a security.json string which points to the MockOAuth server using its well-known URL * and trusting its SSL */ private static String createMockOAuthSecurityJson(Path pemFilePath) throws IOException { diff --git a/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTAuthPluginTest.java b/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTAuthPluginTest.java index ba7aec71d8a..80dcb8144da 100644 --- a/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTAuthPluginTest.java +++ b/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTAuthPluginTest.java @@ -146,7 +146,7 @@ public class JWTAuthPluginTest extends SolrTestCaseJ4 { claims.setClaim("admin", false); // another boolean claim List<String> roles = Arrays.asList("group-one", "other-group", "group-three"); claims.setStringListClaim( - "roles", roles); // multi-valued claims work too and will end up as a JSON array + "roles", roles); // multivalued claims work too and will end up as a JSON array // Keycloak Style resource_access roles HashMap<String, Object> solrMap = new HashMap<>(); @@ -701,7 +701,11 @@ public class JWTAuthPluginTest extends SolrTestCaseJ4 { () -> CryptoKeys.parseX509Certs( new ByteArrayInputStream( - ("-----BEGIN CERTIFICATE-----\n" + "foo\n" + "-----END CERTIFICATE-----\n") + (""" + -----BEGIN CERTIFICATE----- + foo + -----END CERTIFICATE----- + """) .getBytes(StandardCharsets.UTF_8)))); } diff --git a/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTIssuerConfigTest.java b/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTIssuerConfigTest.java index 13f50711429..a5aafd83261 100644 --- a/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTIssuerConfigTest.java +++ b/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTIssuerConfigTest.java @@ -61,16 +61,17 @@ public class JWTIssuerConfigTest extends SolrTestCase { testIssuerConfigMap = testIssuer.asConfig(); testIssuerJson = - "{\n" - + " \"aud\":\"audience\",\n" - + " \"tokenEndpoint\":\"https://issuer/token\",\n" - + " \"wellKnownUrl\":\"wellknown\",\n" - + " \"clientId\":\"clientid\",\n" - + " \"authorizationFlow\":\"code_pkce\",\n" - + " \"jwksUrl\":[\"https://issuer/path\"],\n" - + " \"name\":\"name\",\n" - + " \"iss\":\"issuer\",\n" - + " \"authorizationEndpoint\":\"https://issuer/authz\"}"; + """ + { + "aud":"audience", + "tokenEndpoint":"https://issuer/token", + "wellKnownUrl":"wellknown", + "clientId":"clientid", + "authorizationFlow":"code_pkce", + "jwksUrl":["https://issuer/path"], + "name":"name", + "iss":"issuer", + "authorizationEndpoint":"https://issuer/authz"}"""; } @Override diff --git a/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTVerificationkeyResolverTest.java b/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTVerificationkeyResolverTest.java index cc3d3ad253e..e4ac966557a 100644 --- a/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTVerificationkeyResolverTest.java +++ b/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTVerificationkeyResolverTest.java @@ -203,8 +203,7 @@ public class JWTVerificationkeyResolverTest extends SolrTestCaseJ4 { return claims; } - private static JsonWebSignature makeJws(KeyHolder keyHolder, JwtClaims claims) - throws JoseException { + private static JsonWebSignature makeJws(KeyHolder keyHolder, JwtClaims claims) { JsonWebSignature jws = new JsonWebSignature(); jws.setPayload(claims.toJson()); jws.setKey(keyHolder.getRsaKey().getPrivateKey()); diff --git a/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/KeystoreGenerator.java b/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/KeystoreGenerator.java index 6d2d6c9c23a..8124d86a48e 100644 --- a/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/KeystoreGenerator.java +++ b/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/KeystoreGenerator.java @@ -56,7 +56,7 @@ public class KeystoreGenerator { private static final String PASS_PHRASE = "secret"; public void generateKeystore(Path existingKeystore, Path newKeystore, String cn) { - KeyStore ks = null; + KeyStore ks; try (FileInputStream fis = new FileInputStream(existingKeystore.toFile())) { ks = KeyStore.getInstance(KeyStore.getDefaultType()); ks.load(fis, PASS_PHRASE.toCharArray());
