From da38b94d5c4c31cf5d51ade0b63a4e8d75484a4b Mon Sep 17 00:00:00 2001 From: James Anderson Date: Wed, 25 Mar 2020 10:10:18 -0500 Subject: [PATCH 1/2] Check varargs null values in JWTVerifier --- .../main/java/com/auth0/jwt/JWTVerifier.java | 18 +++- .../java/com/auth0/jwt/JWTVerifierTest.java | 89 ++++++++++++++++++- 2 files changed, 102 insertions(+), 5 deletions(-) diff --git a/lib/src/main/java/com/auth0/jwt/JWTVerifier.java b/lib/src/main/java/com/auth0/jwt/JWTVerifier.java index 0f56b3da..f3b231e9 100644 --- a/lib/src/main/java/com/auth0/jwt/JWTVerifier.java +++ b/lib/src/main/java/com/auth0/jwt/JWTVerifier.java @@ -57,7 +57,7 @@ public static class BaseVerification implements Verification { @Override public Verification withIssuer(String... issuer) { - requireClaim(PublicClaims.ISSUER, issuer == null ? null : Arrays.asList(issuer)); + requireClaim(PublicClaims.ISSUER, checkVarargNull(issuer) ? null : Arrays.asList(issuer)); return this; } @@ -69,7 +69,7 @@ public Verification withSubject(String subject) { @Override public Verification withAudience(String... audience) { - requireClaim(PublicClaims.AUDIENCE, audience == null ? null : Arrays.asList(audience)); + requireClaim(PublicClaims.AUDIENCE, checkVarargNull(audience) ? null : Arrays.asList(audience)); return this; } @@ -230,6 +230,20 @@ private void requireClaim(String name, Object value) { } } + private static boolean checkVarargNull(String[] args) { + if (args == null || args.length == 0) { + return true; + } + boolean isAllNull = true; + for (String arg: args) { + if (arg != null) { + isAllNull = false; + break; + } + } + return isAllNull; + } + /** * Perform the verification against the given Token, using any previous configured options. diff --git a/lib/src/test/java/com/auth0/jwt/JWTVerifierTest.java b/lib/src/test/java/com/auth0/jwt/JWTVerifierTest.java index c60e31db..aec70c70 100644 --- a/lib/src/test/java/com/auth0/jwt/JWTVerifierTest.java +++ b/lib/src/test/java/com/auth0/jwt/JWTVerifierTest.java @@ -10,6 +10,7 @@ import org.junit.Test; import org.junit.rules.ExpectedException; +import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.Map; @@ -125,7 +126,7 @@ public void shouldValidateAudience() throws Exception { .verify(tokenArr); assertThat(jwtArr, is(notNullValue())); - } + } @Test public void shouldAcceptPartialAudience() throws Exception { @@ -150,12 +151,53 @@ public void shouldThrowOnInvalidAudience() throws Exception { .verify(token); } + @Test + public void shouldRemoveAudienceWhenPassingNullReference() throws Exception { + Algorithm algorithm = mock(Algorithm.class); + JWTVerifier verifier = JWTVerifier.init(algorithm) + .withAudience((String) null) + .build(); + + assertThat(verifier.claims, is(notNullValue())); + assertThat(verifier.claims, not(hasKey("aud"))); + + verifier = JWTVerifier.init(algorithm) + .withAudience((String[]) null) + .build(); + + assertThat(verifier.claims, is(notNullValue())); + assertThat(verifier.claims, not(hasKey("aud"))); + + verifier = JWTVerifier.init(algorithm) + .withAudience() + .build(); + + assertThat(verifier.claims, is(notNullValue())); + assertThat(verifier.claims, not(hasKey("aud"))); + + String emptyAud = " "; + verifier = JWTVerifier.init(algorithm) + .withAudience(emptyAud) + .build(); + + assertThat(verifier.claims, is(notNullValue())); + assertThat(verifier.claims, hasEntry("aud", Collections.singletonList(emptyAud))); + } + @Test public void shouldRemoveAudienceWhenPassingNull() throws Exception { Algorithm algorithm = mock(Algorithm.class); JWTVerifier verifier = JWTVerifier.init(algorithm) .withAudience("John") - .withAudience(null) + .withAudience((String) null) + .build(); + + assertThat(verifier.claims, is(notNullValue())); + assertThat(verifier.claims, not(hasKey("aud"))); + + verifier = JWTVerifier.init(algorithm) + .withAudience("John") + .withAudience((String[]) null) .build(); assertThat(verifier.claims, is(notNullValue())); @@ -674,11 +716,52 @@ public void shouldRemoveClaimWhenPassingNull() throws Exception { Algorithm algorithm = mock(Algorithm.class); JWTVerifier verifier = JWTVerifier.init(algorithm) .withIssuer("iss") - .withIssuer(null) + .withIssuer((String) null) .build(); assertThat(verifier.claims, is(notNullValue())); assertThat(verifier.claims, not(hasKey("iss"))); + + verifier = JWTVerifier.init(algorithm) + .withIssuer("iss") + .withIssuer((String[]) null) + .build(); + + assertThat(verifier.claims, is(notNullValue())); + assertThat(verifier.claims, not(hasKey("iss"))); + } + + @Test + public void shouldRemoveIssuerWhenPassingNullReference() throws Exception { + Algorithm algorithm = mock(Algorithm.class); + JWTVerifier verifier = JWTVerifier.init(algorithm) + .withIssuer((String) null) + .build(); + + assertThat(verifier.claims, is(notNullValue())); + assertThat(verifier.claims, not(hasKey("iss"))); + + verifier = JWTVerifier.init(algorithm) + .withIssuer((String[]) null) + .build(); + + assertThat(verifier.claims, is(notNullValue())); + assertThat(verifier.claims, not(hasKey("iss"))); + + verifier = JWTVerifier.init(algorithm) + .withIssuer() + .build(); + + assertThat(verifier.claims, is(notNullValue())); + assertThat(verifier.claims, not(hasKey("iss"))); + + String emptyIss = " "; + verifier = JWTVerifier.init(algorithm) + .withIssuer(emptyIss) + .build(); + + assertThat(verifier.claims, is(notNullValue())); + assertThat(verifier.claims, hasEntry("iss", Collections.singletonList(emptyIss))); } @Test From 3a8cebed06aec52bf8d57478d8fffd78cd6b2b10 Mon Sep 17 00:00:00 2001 From: James Anderson Date: Wed, 25 Mar 2020 10:31:42 -0500 Subject: [PATCH 2/2] Review feedback - Rename method for clarity --- lib/src/main/java/com/auth0/jwt/JWTVerifier.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/src/main/java/com/auth0/jwt/JWTVerifier.java b/lib/src/main/java/com/auth0/jwt/JWTVerifier.java index f3b231e9..24e109a4 100644 --- a/lib/src/main/java/com/auth0/jwt/JWTVerifier.java +++ b/lib/src/main/java/com/auth0/jwt/JWTVerifier.java @@ -57,7 +57,7 @@ public static class BaseVerification implements Verification { @Override public Verification withIssuer(String... issuer) { - requireClaim(PublicClaims.ISSUER, checkVarargNull(issuer) ? null : Arrays.asList(issuer)); + requireClaim(PublicClaims.ISSUER, isNullOrEmpty(issuer) ? null : Arrays.asList(issuer)); return this; } @@ -69,7 +69,7 @@ public Verification withSubject(String subject) { @Override public Verification withAudience(String... audience) { - requireClaim(PublicClaims.AUDIENCE, checkVarargNull(audience) ? null : Arrays.asList(audience)); + requireClaim(PublicClaims.AUDIENCE, isNullOrEmpty(audience) ? null : Arrays.asList(audience)); return this; } @@ -230,7 +230,7 @@ private void requireClaim(String name, Object value) { } } - private static boolean checkVarargNull(String[] args) { + private static boolean isNullOrEmpty(String[] args) { if (args == null || args.length == 0) { return true; }