From c63684f93b81b5c1f9e8d453349813deffc4ebfe Mon Sep 17 00:00:00 2001
From: Erik Hatcher <erik.hatcher@lucidworks.com>
Date: Wed, 16 Sep 2020 12:45:03 -0400
Subject: [PATCH] Revert "SOLR-14799: JWT authentication plugin only requires
 sub claim when principalClaim=sub"

This reverts commit bc0c9ffee31b47c2630126deea28d7ff3d829016.
---
 .../apache/solr/security/JWTAuthPlugin.java    |  8 +++++++-
 .../solr/security/JWTAuthPluginTest.java       | 18 +-----------------
 2 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/security/JWTAuthPlugin.java b/solr/core/src/java/org/apache/solr/security/JWTAuthPlugin.java
index 8b9271a130a..fb7b9db02c9 100644
--- a/solr/core/src/java/org/apache/solr/security/JWTAuthPlugin.java
+++ b/solr/core/src/java/org/apache/solr/security/JWTAuthPlugin.java
@@ -71,6 +71,7 @@ import org.slf4j.LoggerFactory;
 public class JWTAuthPlugin extends AuthenticationPlugin implements SpecProvider, ConfigEditablePlugin {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
   private static final String PARAM_BLOCK_UNKNOWN = "blockUnknown";
+  private static final String PARAM_REQUIRE_SUBJECT = "requireSub";
   private static final String PARAM_REQUIRE_ISSUER = "requireIss";
   private static final String PARAM_PRINCIPAL_CLAIM = "principalClaim";
   private static final String PARAM_ROLES_CLAIM = "rolesClaim";
@@ -91,7 +92,7 @@ public class JWTAuthPlugin extends AuthenticationPlugin implements SpecProvider,
   static final String PRIMARY_ISSUER = "PRIMARY";
 
   private static final Set<String> PROPS = ImmutableSet.of(PARAM_BLOCK_UNKNOWN,
-      PARAM_PRINCIPAL_CLAIM, PARAM_REQUIRE_EXPIRATIONTIME, PARAM_ALG_WHITELIST,
+      PARAM_REQUIRE_SUBJECT, PARAM_PRINCIPAL_CLAIM, PARAM_REQUIRE_EXPIRATIONTIME, PARAM_ALG_WHITELIST,
       PARAM_JWK_CACHE_DURATION, PARAM_CLAIMS_MATCH, PARAM_SCOPE, PARAM_REALM, PARAM_ROLES_CLAIM,
       PARAM_ADMINUI_SCOPE, PARAM_REDIRECT_URIS, PARAM_REQUIRE_ISSUER, PARAM_ISSUERS,
       // These keys are supported for now to enable PRIMARY issuer config through top-level keys
@@ -136,6 +137,10 @@ public class JWTAuthPlugin extends AuthenticationPlugin implements SpecProvider,
     blockUnknown = Boolean.parseBoolean(String.valueOf(pluginConfig.getOrDefault(PARAM_BLOCK_UNKNOWN, false)));
     requireIssuer = Boolean.parseBoolean(String.valueOf(pluginConfig.getOrDefault(PARAM_REQUIRE_ISSUER, "true")));
     requireExpirationTime = Boolean.parseBoolean(String.valueOf(pluginConfig.getOrDefault(PARAM_REQUIRE_EXPIRATIONTIME, "true")));
+    if (pluginConfig.get(PARAM_REQUIRE_SUBJECT) != null) {
+      log.warn("Parameter {} is no longer used and may generate error in a later version. A subject claim is now always required",
+          PARAM_REQUIRE_SUBJECT);
+    }
     principalClaim = (String) pluginConfig.getOrDefault(PARAM_PRINCIPAL_CLAIM, "sub");
 
     rolesClaim = (String) pluginConfig.get(PARAM_ROLES_CLAIM);
@@ -495,6 +500,7 @@ public class JWTAuthPlugin extends AuthenticationPlugin implements SpecProvider,
     } else {
       jwtConsumerBuilder.setSkipDefaultAudienceValidation();
     }
+    jwtConsumerBuilder.setRequireSubject();
     if (requireExpirationTime)
       jwtConsumerBuilder.setRequireExpirationTime();
     if (algWhitelist != null)
diff --git a/solr/core/src/test/org/apache/solr/security/JWTAuthPluginTest.java b/solr/core/src/test/org/apache/solr/security/JWTAuthPluginTest.java
index 90713417022..7b04c95daef 100644
--- a/solr/core/src/test/org/apache/solr/security/JWTAuthPluginTest.java
+++ b/solr/core/src/test/org/apache/solr/security/JWTAuthPluginTest.java
@@ -93,7 +93,6 @@ public class JWTAuthPluginTest extends SolrTestCaseJ4 {
     claims.unsetClaim("iss");
     claims.unsetClaim("aud");
     claims.unsetClaim("exp");
-    claims.setSubject(null);
     jws.setPayload(claims.toJson());
     String slimJwt = jws.getCompactSerialization();
     slimHeader = "Bearer" + " " + slimJwt;
@@ -128,7 +127,6 @@ public class JWTAuthPluginTest extends SolrTestCaseJ4 {
 
     testConfig = new HashMap<>();
     testConfig.put("class", "org.apache.solr.security.JWTAuthPlugin");
-    testConfig.put("principalClaim", "customPrincipal");
     testConfig.put("jwk", testJwk);
     plugin.init(testConfig);
     
@@ -218,25 +216,11 @@ public class JWTAuthPluginTest extends SolrTestCaseJ4 {
   public void authenticateOk() {
     JWTAuthPlugin.JWTAuthenticationResponse resp = plugin.authenticate(testHeader);
     assertTrue(resp.isAuthenticated());
-    assertEquals("custom", resp.getPrincipal().getName()); // principalClaim = customPrincipal, not sub here
+    assertEquals("solruser", resp.getPrincipal().getName());
   }
 
   @Test
   public void authFailedMissingSubject() {
-    minimalConfig.put("principalClaim","sub");  // minimalConfig has no subject specified
-    plugin.init(minimalConfig);
-    JWTAuthPlugin.JWTAuthenticationResponse resp = plugin.authenticate(testHeader);
-    assertFalse(resp.isAuthenticated());
-    assertEquals(JWTAuthPlugin.JWTAuthenticationResponse.AuthCode.JWT_VALIDATION_EXCEPTION, resp.getAuthCode());
-
-    testConfig.put("principalClaim","sub");  // testConfig has subject = solruser
-    plugin.init(testConfig);
-    resp = plugin.authenticate(testHeader);
-    assertTrue(resp.isAuthenticated());
-  }
-
-  @Test
-  public void authFailedMissingIssuer() {
     testConfig.put("iss", "NA");
     plugin.init(testConfig);
     JWTAuthPlugin.JWTAuthenticationResponse resp = plugin.authenticate(testHeader);