address review comments

Author: richard-dennehy

x-pack/plugin/security/qa/jwt-realm/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/jwt/JwtRestIT.java

@@ -456,13 +456,13 @@ public void testFailureOnInvalidHMACSignature() throws Exception {
 
             {
                 // This is the correct HMAC passphrase (from build.gradle)
-                final SignedJWT jwt = signHmacJwt(claimsSet, HMAC_PASSPHRASE);
+                final SignedJWT jwt = signHmacJwt(claimsSet, HMAC_PASSPHRASE, false);
                 final TestSecurityClient client = getSecurityClient(jwt, Optional.of(VALID_SHARED_SECRET));
                 assertThat(client.authenticate(), hasEntry(User.Fields.USERNAME.getPreferredName(), username));
             }
             {
                 // This is not the correct HMAC passphrase
-                final SignedJWT invalidJwt = signHmacJwt(claimsSet, "invalid-HMAC-passphrase-" + randomAlphaOfLength(12));
+                final SignedJWT invalidJwt = signHmacJwt(claimsSet, "invalid-HMAC-passphrase-" + randomAlphaOfLength(12), false);
                 final TestSecurityClient client = getSecurityClient(invalidJwt, Optional.of(VALID_SHARED_SECRET));
                 // This fails because the HMAC is wrong
                 final ResponseException exception = expectThrows(ResponseException.class, client::authenticate);
@@ -487,7 +487,7 @@ public void testFailureOnRequiredClaims() throws JOSEException, IOException {
                 data.put("token_use", randomValueOtherThan("access", () -> randomAlphaOfLengthBetween(3, 10)));
             }
             final JWTClaimsSet claimsSet = buildJwt(data, Instant.now(), false, false);
-            final SignedJWT jwt = signHmacJwt(claimsSet, "test-HMAC/secret passphrase-value");
+            final SignedJWT jwt = signHmacJwt(claimsSet, "test-HMAC/secret passphrase-value", false);
             final TestSecurityClient client = getSecurityClient(jwt, Optional.of(VALID_SHARED_SECRET));
             final ResponseException exception = expectThrows(ResponseException.class, client::authenticate);
             assertThat(exception.getResponse(), hasStatusCode(RestStatus.UNAUTHORIZED));
@@ -747,18 +747,18 @@ private SignedJWT buildAndSignJwtForRealm3(String principal, Instant issueTime)
 
     private SignedJWT signJwtForRealm1(JWTClaimsSet claimsSet) throws IOException, JOSEException, ParseException {
         final RSASSASigner signer = loadRsaSigner();
-        return signJWT(signer, "RS256", claimsSet);
+        return signJWT(signer, "RS256", claimsSet, false);
     }
 
-    private SignedJWT signJwtForRealm2(JWTClaimsSet claimsSet) throws JOSEException, ParseException {
+    private SignedJWT signJwtForRealm2(JWTClaimsSet claimsSet) throws JOSEException {
         // Input string is configured in build.gradle
-        return signHmacJwt(claimsSet, "test-HMAC/secret passphrase-value");
+        return signHmacJwt(claimsSet, "test-HMAC/secret passphrase-value", true);
     }
 
     private SignedJWT signJwtForRealm3(JWTClaimsSet claimsSet) throws JOSEException, ParseException, IOException {
         final int bitSize = randomFrom(384, 512);
         final MACSigner signer = loadHmacSigner("test-hmac-" + bitSize);
-        return signJWT(signer, "HS" + bitSize, claimsSet);
+        return signJWT(signer, "HS" + bitSize, claimsSet, false);
     }
 
     private RSASSASigner loadRsaSigner() throws IOException, ParseException, JOSEException {
@@ -781,10 +781,10 @@ private MACSigner loadHmacSigner(String keyId) throws IOException, ParseExceptio
         }
     }
 
-    private SignedJWT signHmacJwt(JWTClaimsSet claimsSet, String hmacPassphrase) throws JOSEException {
+    private SignedJWT signHmacJwt(JWTClaimsSet claimsSet, String hmacPassphrase, boolean allowAtJwtType) throws JOSEException {
         final OctetSequenceKey hmac = JwkValidateUtil.buildHmacKeyFromString(hmacPassphrase);
         final JWSSigner signer = new MACSigner(hmac);
-        return signJWT(signer, "HS256", claimsSet);
+        return signJWT(signer, "HS256", claimsSet, allowAtJwtType);
     }
 
     // JWT construction
@@ -822,10 +822,14 @@ static JWTClaimsSet buildJwt(Map<String, Object> claims, Instant issueTime, bool
         return builder.build();
     }
 
-    static SignedJWT signJWT(JWSSigner signer, String algorithm, JWTClaimsSet claimsSet) throws JOSEException {
+    static SignedJWT signJWT(JWSSigner signer, String algorithm, JWTClaimsSet claimsSet, boolean allowAtJwtType) throws JOSEException {
         final JWSHeader.Builder builder = new JWSHeader.Builder(JWSAlgorithm.parse(algorithm));
         if (randomBoolean()) {
-            builder.type(JOSEObjectType.JWT);
+            if (allowAtJwtType && randomBoolean()) {
+                builder.type(new JOSEObjectType("at+jwt"));
+            } else {
+                builder.type(JOSEObjectType.JWT);
+            }
         }
         final JWSHeader jwtHeader = builder.build();
         final SignedJWT jwt = new SignedJWT(jwtHeader, claimsSet);

x-pack/plugin/security/qa/jwt-realm/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/jwt/JwtWithUnavailableSecurityIndexRestIT.java

@@ -279,7 +279,7 @@ private SignedJWT buildAndSignJwt(String principal, String dn, Instant issueTime
             issueTime
         );
         final RSASSASigner signer = loadRsaSigner();
-        return JwtRestIT.signJWT(signer, "RS256", claimsSet);
+        return JwtRestIT.signJWT(signer, "RS256", claimsSet, false);
     }
 
     private RSASSASigner loadRsaSigner() throws IOException, ParseException, JOSEException {

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/jwt/JwtTypeValidator.java

@@ -18,15 +18,12 @@
 public class JwtTypeValidator implements JwtFieldValidator {
 
     private final JOSEObjectTypeVerifier<SecurityContext> JWT_HEADER_TYPE_VERIFIER;
+    private static final JOSEObjectType AT_PLUS_JWT = new JOSEObjectType("at+jwt");
 
     public static final JwtTypeValidator ID_TOKEN_INSTANCE = new JwtTypeValidator(JOSEObjectType.JWT, null);
 
     // strictly speaking, this should only permit `at+jwt`, but removing the other two options is a breaking change
-    public static final JwtTypeValidator ACCESS_TOKEN_INSTANCE = new JwtTypeValidator(
-        JOSEObjectType.JWT,
-        new JOSEObjectType("at+jwt"),
-        null
-    );
+    public static final JwtTypeValidator ACCESS_TOKEN_INSTANCE = new JwtTypeValidator(JOSEObjectType.JWT, AT_PLUS_JWT, null);
 
     private JwtTypeValidator(JOSEObjectType... allowedTypes) {
         JWT_HEADER_TYPE_VERIFIER = new DefaultJOSEObjectTypeVerifier<>(allowedTypes);

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/jwt/JwtTypeValidatorTests.java

@@ -58,9 +58,10 @@ public void testValidAccessTokenType() throws ParseException {
 
     public void testInvalidType() throws ParseException {
         final JwtTypeValidator validator = randomFrom(JwtTypeValidator.ID_TOKEN_INSTANCE, JwtTypeValidator.ACCESS_TOKEN_INSTANCE);
+        final String type = randomBoolean() ? randomAlphaOfLengthBetween(4, 8) : "AT+JWT";
 
         final JWSHeader jwsHeader = JWSHeader.parse(
-            Map.of("typ", randomAlphaOfLengthBetween(4, 8), "alg", randomAlphaOfLengthBetween(3, 8))
+            Map.of("typ", type, "alg", randomAlphaOfLengthBetween(3, 8))
         );
 
         final IllegalArgumentException e = expectThrows(